Skip to content

Commit

Permalink
Merge pull request #4111 from Shopify/duplicate-subscription-error
Browse files Browse the repository at this point in the history 
duplicate subscripition error
  • Loading branch information
@gonzaloriestra
gonzaloriestra authored Jun 21, 2024
2 parents dd645db + a012c8a commit 4de11c5
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 38 deletions.
104 changes: 74 additions & 30 deletions packages/app/src/cli/models/app/loader.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ import {
import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath, dirname, cwd, normalizePath} from '@shopify/cli-kit/node/path'
import {platformAndArch} from '@shopify/cli-kit/node/os'
import {outputContent} from '@shopify/cli-kit/node/output'
import {outputContent, outputToken} from '@shopify/cli-kit/node/output'
import {zod} from '@shopify/cli-kit/node/schema'
import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
import {currentProcessIsGlobal} from '@shopify/cli-kit/node/is-global'
import colors from '@shopify/cli-kit/node/colors'
// eslint-disable-next-line no-restricted-imports
import {resolve} from 'path'

Expand Down Expand Up @@ -417,7 +418,7 @@ wrong = "property"
})

// When
await expect(loadTestingApp()).rejects.toThrow(/Validation errors in/)
await expect(loadTestingApp()).rejects.toThrow(/Validation errors/)
})

test('throws an error if the extension type is invalid', async () => {
Expand Down Expand Up @@ -2483,9 +2484,9 @@ describe('parseConfigurationObject', () => {
message: 'Boolean is required',
},
]
const expectedFormatted = outputContent`App configuration is not valid\nValidation errors in tmp:\n\n${parseHumanReadableError(
errorObject,
)}`
const expectedFormatted = outputContent`\n${outputToken.errorText(
'Validation errors',
)} in tmp:\n\n${parseHumanReadableError(errorObject)}`

const abortOrReport = vi.fn()

Expand All @@ -2510,9 +2511,9 @@ describe('parseConfigurationObject', () => {
message: 'Expected string, received array',
},
]
const expectedFormatted = outputContent`App configuration is not valid\nValidation errors in tmp:\n\n${parseHumanReadableError(
errorObject,
)}`
const expectedFormatted = outputContent`\n${outputToken.errorText(
'Validation errors',
)} in tmp:\n\n${parseHumanReadableError(errorObject)}`
const abortOrReport = vi.fn()
await parseConfigurationObject(LegacyAppSchema, 'tmp', configurationObject, abortOrReport)

Expand Down Expand Up @@ -2559,9 +2560,9 @@ describe('parseConfigurationObject', () => {
message: 'Invalid input',
},
]
const expectedFormatted = outputContent`App configuration is not valid\nValidation errors in tmp:\n\n${parseHumanReadableError(
errorObject,
)}`
const expectedFormatted = outputContent`\n${outputToken.errorText(
'Validation errors',
)} in tmp:\n\n${parseHumanReadableError(errorObject)}`
const abortOrReport = vi.fn()
await parseConfigurationObject(WebConfigurationSchema, 'tmp', configurationObject, abortOrReport)

Expand Down Expand Up @@ -2712,16 +2713,14 @@ describe('WebhooksSchema', () => {
test('throws an error if we have duplicate subscriptions in same topics array', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
subscriptions: [
{uri: 'https://example.com', topics: ['products/create', 'products/create']},
{uri: 'https://example.com', topics: ['products/create']},
],
subscriptions: [{uri: 'https://example.com', topics: ['products/create', 'products/create']}],
}
const webhookFields = colors.dim(`\n\ntopic: products/create\nuri: https://example.com`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand All @@ -2736,11 +2735,12 @@ describe('WebhooksSchema', () => {
{uri: 'https://example.com', topics: ['products/create', 'products/update']},
],
}
const webhookFields = colors.dim(`\n\ntopic: products/create\nuri: https://example.com`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand Down Expand Up @@ -2814,11 +2814,12 @@ describe('WebhooksSchema', () => {
},
],
}
const webhookFields = colors.dim(`\n\ntopic: products/create\nuri: https://example.com`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand All @@ -2839,11 +2840,12 @@ describe('WebhooksSchema', () => {
},
],
}
const webhookFields = colors.dim(`\n\ntopic: products/create\nuri: pubsub://my-project-123:my-topic`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand All @@ -2864,11 +2866,14 @@ describe('WebhooksSchema', () => {
},
],
}
const webhookFields = colors.dim(
`\n\ntopic: products/create\nuri: arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/123/my_webhook_path`,
)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand All @@ -2891,11 +2896,50 @@ describe('WebhooksSchema', () => {
},
],
}
const webhookFields = colors.dim(`\n\ntopic: products/update\nuri: https://example.com\nfilter: title:shoes`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/update'],
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp')
})

test('shows multiple duplicate subscriptions in error message', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
subscriptions: [
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shoes',
},
{
topics: ['products/create'],
uri: 'https://example.com',
},
{
topics: ['products/create'],
uri: 'https://example.com',
},
],
}
const webhookFields =
colors.dim(`\n\ntopic: products/update\nuri: https://example.com\nfilter: title:shoes`) +
colors.dim(`\n\ntopic: products/create\nuri: https://example.com`)
const errorObj = {
code: zod.ZodIssueCode.custom,
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${webhookFields}`,
fatal: true,
path: ['webhooks', 'subscriptions'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
Expand Down Expand Up @@ -2993,9 +3037,9 @@ describe('WebhooksSchema', () => {

async function setupParsing(errorObj: zod.ZodIssue | {}, webhookConfigOverrides: WebhooksConfig) {
const err = Array.isArray(errorObj) ? errorObj : [errorObj]
const expectedFormatted = outputContent`App configuration is not valid\nValidation errors in tmp:\n\n${parseHumanReadableError(
err,
)}`
const expectedFormatted = outputContent`\n${outputToken.errorText(
'Validation errors',
)} in tmp:\n\n${parseHumanReadableError(err)}`
const abortOrReport = vi.fn()

const {path, ...toParse} = getWebhookConfig(webhookConfigOverrides)
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/models/app/loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export function parseConfigurationObject<TSchema extends zod.ZodType>(
const parseResult = schema.safeParse(configurationObject)
if (!parseResult.success) {
return abortOrReport(
outputContent`App configuration is not valid\nValidation errors in ${outputToken.path(
outputContent`\n${outputToken.errorText('Validation errors')} in ${outputToken.path(
filepath,
)}:\n\n${parseHumanReadableError(parseResult.error.issues)}`,
fallbackOutput,
Expand Down
25 changes: 18 additions & 7 deletions packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {zod} from '@shopify/cli-kit/node/schema'
import {uniq} from '@shopify/cli-kit/common/array'
import colors from '@shopify/cli-kit/node/colors'
import type {WebhooksConfig} from '../types/app_config_webhook.js'

export function webhookValidator(schema: object, ctx: zod.RefinementCtx) {
Expand All @@ -14,6 +15,7 @@ export function webhookValidator(schema: object, ctx: zod.RefinementCtx) {
function validateSubscriptions(webhookConfig: WebhooksConfig) {
const {subscriptions = []} = webhookConfig
const uniqueSubscriptionSet = new Set()
const duplicatedSubscriptionsFields: string[] = []

if (!subscriptions.length) return

Expand Down Expand Up @@ -49,19 +51,28 @@ function validateSubscriptions(webhookConfig: WebhooksConfig) {
}
}

for (const [j, topic] of topics.entries()) {
topics.forEach((topic) => {
const key = `${topic}::${uri}::${filter}`

if (uniqueSubscriptionSet.has(key)) {
return {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: [...path, 'topics', j, topic],
}
const subscriptionFieldsString = filter
? colors.dim(`\n\ntopic: ${topic}\nuri: ${uri}\nfilter: ${filter}`)
: colors.dim(`\n\ntopic: ${topic}\nuri: ${uri}`)

duplicatedSubscriptionsFields.push(subscriptionFieldsString)
}

uniqueSubscriptionSet.add(key)
})
}

if (duplicatedSubscriptionsFields.length > 0) {
const fieldsArrToString = duplicatedSubscriptionsFields.join('')

return {
code: zod.ZodIssueCode.custom,
message: `Multiple subscriptions with the exact same topic, uri, and filter. To resolve, remove or edit the duplicates ${fieldsArrToString}`,
path: ['subscriptions'],
}
}
}

0 comments on commit 4de11c5

Please sign in to comment.