Skip to content

Commit

Permalink
support relative paths to application_url for declarative webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderMontague committed Feb 13, 2024
1 parent 4ec1461 commit 5e2db99
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 27 deletions.
35 changes: 26 additions & 9 deletions packages/app/src/cli/models/app/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2350,9 +2350,8 @@ describe('WebhooksSchema', () => {
],
}
const errorObj = {
validation: 'regex' as zod.ZodInvalidStringIssue['validation'],
code: zod.ZodIssueCode.invalid_string,
message: 'Invalid',
code: zod.ZodIssueCode.custom,
message: 'Must be a valid https, pubsub, or ARN URI',
path: ['webhooks', 'subscriptions', 0, 'uri'],
}

Expand All @@ -2378,9 +2377,8 @@ describe('WebhooksSchema', () => {
subscriptions: [{uri: 'my::URI-thing::Shopify::123', topics: ['products/create']}],
}
const errorObj = {
validation: 'regex' as zod.ZodInvalidStringIssue['validation'],
code: zod.ZodIssueCode.invalid_string,
message: 'Invalid',
code: zod.ZodIssueCode.custom,
message: 'Must be a valid https, pubsub, or ARN URI',
path: ['webhooks', 'subscriptions', 0, 'uri'],
}

Expand Down Expand Up @@ -2426,6 +2424,22 @@ describe('WebhooksSchema', () => {
expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig)
})

test('accepts a relative path uri', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
subscriptions: [
{
uri: '/webhooks',
topics: ['products/create'],
},
],
}

const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig)
expect(abortOrReport).not.toHaveBeenCalled()
expect(parsedConfiguration.webhooks).toMatchObject(webhookConfig)
})

test('accepts combination of uris', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
Expand All @@ -2442,6 +2456,10 @@ describe('WebhooksSchema', () => {
uri: 'pubsub://my-project-123:my-topic',
topics: ['products/create', 'products/update'],
},
{
uri: '/webhooks',
topics: ['products/create', 'products/update'],
},
],
}

Expand Down Expand Up @@ -2531,9 +2549,8 @@ describe('WebhooksSchema', () => {
],
}
const errorObj = {
validation: 'regex' as zod.ZodInvalidStringIssue['validation'],
code: zod.ZodIssueCode.invalid_string,
message: 'Invalid',
code: zod.ZodIssueCode.custom,
message: 'Must be a valid https, pubsub, or ARN URI',
path: ['webhooks', 'subscriptions', 0, 'uri'],
}

Expand Down
3 changes: 3 additions & 0 deletions packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class AppLoader {
private errors: AppErrors = new AppErrors()
private specifications: ExtensionSpecification[]
private remoteBetas: BetaFlag[]
private fullAppConfiguration?: AppConfiguration

constructor({directory, configName, mode, specifications, remoteBetas}: AppLoaderConstructorArgs) {
this.mode = mode ?? 'strict'
Expand Down Expand Up @@ -220,6 +221,7 @@ class AppLoader {
specifications: this.specifications,
})
const {directory, configuration, configurationLoadResultMetadata, configSchema} = await configurationLoader.loaded()
this.fullAppConfiguration = configuration
await logMetadataFromAppLoadingProcess(configurationLoadResultMetadata)

const dotenv = await loadDotEnv(directory, configuration.path)
Expand Down Expand Up @@ -334,6 +336,7 @@ class AppLoader {
entryPath,
directory,
specification,
fullAppConfig: this.fullAppConfiguration,
})

const validateResult = await extensionInstance.validate()
Expand Down
8 changes: 7 additions & 1 deletion packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import {bundleThemeExtension} from '../../services/extensions/bundle.js'
import {Identifiers} from '../app/identifiers.js'
import {uploadWasmBlob} from '../../services/deploy/upload.js'
import {AppConfiguration} from '../app/app.js'
import {ok} from '@shopify/cli-kit/node/result'
import {constantize, slugify} from '@shopify/cli-kit/common/string'
import {randomUUID} from '@shopify/cli-kit/node/crypto'
Expand Down Expand Up @@ -42,6 +43,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
outputPath: string
handle: string
specification: ExtensionSpecification
fullAppConfiguration?: AppConfiguration

get graphQLType() {
return (this.specification.graphQLType ?? this.specification.identifier).toUpperCase()
Expand Down Expand Up @@ -109,6 +111,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
entryPath?: string
directory: string
specification: ExtensionSpecification
fullAppConfig?: AppConfiguration
}) {
this.configuration = options.configuration
this.configurationPath = options.configurationPath
Expand All @@ -123,6 +126,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
this.localIdentifier = this.handle
this.idEnvironmentVariableName = `SHOPIFY_${constantize(this.localIdentifier)}_ID`
this.outputPath = this.directory
this.fullAppConfiguration = options.fullAppConfig

if (this.features.includes('esbuild') || this.type === 'tax_calculation') {
this.outputPath = joinPath(this.directory, 'dist', `${this.outputFileName}`)
Expand Down Expand Up @@ -175,7 +179,9 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi

async commonDeployConfig(apiKey: string): Promise<{[key: string]: unknown} | undefined> {
const deployConfig = await this.specification.deployConfig?.(this.configuration, this.directory, apiKey, undefined)
const transformedConfig = this.specification.transform?.(this.configuration) as {[key: string]: unknown} | undefined
const transformedConfig = this.specification.transform?.(this.configuration, this.fullAppConfiguration) as
| {[key: string]: unknown}
| undefined
const resultDeployConfig = deployConfig ?? transformedConfig ?? undefined
return resultDeployConfig && Object.keys(resultDeployConfig).length > 0 ? resultDeployConfig : undefined
}
Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/cli/models/extensions/specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface TransformationConfig {
}

export interface CustomTransformationConfig {
forward?: (obj: object) => object
forward?: (obj: object, fullAppConfiguration?: object) => object
reverse?: (obj: object) => object
}

Expand Down Expand Up @@ -55,8 +55,8 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
buildValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
hasExtensionPointTarget?(config: TConfiguration, target: string): boolean
appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[]
transform?: (content: object) => object
reverseTransform?: (content: object) => object
transform?: (content: object, fullAppConfiguration?: object) => object
reverseTransform?: (content: object, fullAppConfiguration?: object) => object
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,39 @@ describe('webhooks', () => {
api_version: '2021-01',
})
})
test('when a relative URI is used, it inherits the application_url', () => {
// Given
const object = {
webhooks: {
api_version: '2021-01',
subscriptions: [
{
topics: ['products/update', 'products/delete'],
uri: '/products',
},
],
},
}
const webhookSpec = spec

// When
const result = webhookSpec.transform!(object, {application_url: 'https://my-app-url.com'})

// Then
expect(result).toEqual({
api_version: '2021-01',
subscriptions: [
{
topic: 'products/update',
uri: 'https://my-app-url.com/products',
},
{
topic: 'products/delete',
uri: 'https://my-app-url.com/products',
},
],
})
})
})
describe('reverseTransform', () => {
test('should return the reversed transformed object', () => {
Expand Down Expand Up @@ -246,5 +279,62 @@ describe('webhooks', () => {
},
})
})
test('when subscriptions share the application_url base, simplify with a relative path', () => {
// Given
const object = {
api_version: '2021-01',
subscriptions: [
{
topic: 'products/update',
uri: 'https://my-app-url.com/products',
},
{
topic: 'products/delete',
uri: 'https://my-app-url.com/products',
},
{
topic: 'orders/update',
uri: 'https://my-app-url.com/orders',
},
{
topic: 'customers/create',
uri: 'https://customers-url.com',
},
{
topic: 'customers/delete',
uri: 'pubsub://absolute-feat-test:pub-sub-topic',
},
],
}
const webhookSpec = spec

// When
const result = webhookSpec.reverseTransform!(object, {application_url: 'https://my-app-url.com'})

// Then
expect(result).toMatchObject({
webhooks: {
api_version: '2021-01',
subscriptions: [
{
topics: ['products/update', 'products/delete'],
uri: '/products',
},
{
topics: ['orders/update'],
uri: '/orders',
},
{
topics: ['customers/create'],
uri: 'https://customers-url.com',
},
{
topics: ['customers/delete'],
uri: 'pubsub://absolute-feat-test:pub-sub-topic',
},
],
},
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const WebhooksSchema = zod.object({
subscriptions: zod.array(WebhookSubscriptionSchema).optional(),
})

export const WebhooksSchemaWithDeclarative = WebhooksSchema.superRefine(webhookValidator)
const WebhooksSchemaWithDeclarative = WebhooksSchema.superRefine(webhookValidator)

export const WebhookSchema = zod.object({
webhooks: WebhooksSchemaWithDeclarative,
Expand All @@ -34,8 +34,8 @@ export const WebhookSchema = zod.object({
export const WebhooksSpecIdentifier = 'webhooks'

const WebhookTransformConfig: CustomTransformationConfig = {
forward: (content: object) => transformWebhookConfig(content),
reverse: (content: object) => transformToWebhookConfig(content),
forward: transformWebhookConfig,
reverse: transformToWebhookConfig,
}

const spec = createConfigExtensionSpecification({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
import {SpecsAppConfiguration} from '../types/app_config.js'
import {WebhooksConfig, NormalizedWebhookSubscription} from '../types/app_config_webhook.js'
import {deepMergeObjects, getPathValue} from '@shopify/cli-kit/common/object'

export function transformWebhookConfig(content: object) {
export function transformWebhookConfig(content: object, fullAppConfig?: object) {
const webhooks = getPathValue(content, 'webhooks') as WebhooksConfig
if (!webhooks) return content

const appConfig = fullAppConfig as SpecsAppConfiguration
const applicationUrl = appConfig?.application_url

const webhookSubscriptions = []
// eslint-disable-next-line @typescript-eslint/naming-convention
const {api_version, subscriptions = []} = webhooks

// eslint-disable-next-line no-warning-comments
// TODO: pass along compliance_topics once we're ready to store them in its own module
for (const {uri, topics, compliance_topics: _, ...optionalFields} of subscriptions) {
webhookSubscriptions.push(topics.map((topic) => ({uri, topic, ...optionalFields})))
const uriWithRelativePath = uri.startsWith('/') && applicationUrl ? `${applicationUrl}${uri}` : uri
webhookSubscriptions.push(topics.map((topic) => ({uri: uriWithRelativePath, topic, ...optionalFields})))
}

return webhookSubscriptions.length > 0 ? {subscriptions: webhookSubscriptions.flat(), api_version} : {api_version}
}

export function transformToWebhookConfig(content: object) {
export function transformToWebhookConfig(content: object, fullAppConfig?: object) {
let webhooks = {}
const apiVersion = getPathValue(content, 'api_version') as string
webhooks = {...(apiVersion ? {webhooks: {api_version: apiVersion}} : {})}
Expand All @@ -27,15 +32,22 @@ export function transformToWebhookConfig(content: object) {

const webhooksSubscriptions: WebhooksConfig['subscriptions'] = []

const appConfig = fullAppConfig as SpecsAppConfiguration
const applicationUrl = appConfig?.application_url

// eslint-disable-next-line @typescript-eslint/naming-convention
for (const {uri, topic, sub_topic, ...optionalFields} of serverWebhooks) {
const currSubscription = webhooksSubscriptions.find((sub) => sub.uri === uri && sub.sub_topic === sub_topic)
const uriWithRelativePath = uri.includes(applicationUrl) ? uri.replace(applicationUrl, '') : uri

const currSubscription = webhooksSubscriptions.find(
(sub) => sub.uri === uriWithRelativePath && sub.sub_topic === sub_topic,
)
if (currSubscription) {
currSubscription.topics.push(topic)
} else {
webhooksSubscriptions.push({
topics: [topic],
uri,
uri: uriWithRelativePath,
...(sub_topic ? {sub_topic} : {}),
...optionalFields,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ export const ensureHttpsOnlyUrl = validateUrl(zod.string(), {
message: 'Only https urls are allowed',
}).refine((url) => !url.endsWith('/'), {message: 'URL can’t end with a forward slash'})

export const UriValidation = zod.union([
zod.string().regex(httpsRegex),
zod.string().regex(pubSubRegex),
zod.string().regex(arnRegex),
])
export const UriValidation = zod.string().refine(
(uri) => {
if (uri.startsWith('/')) return true

return httpsRegex.test(uri) || pubSubRegex.test(uri) || arnRegex.test(uri)
},
{
message: 'Must be a valid https, pubsub, or ARN URI',
},
)
5 changes: 4 additions & 1 deletion packages/app/src/cli/services/app/select-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export function remoteAppConfigurationExtensionContent(
if (!configString) return
const config = configString ? JSON.parse(configString) : {}

remoteAppConfig = deepMergeObjects(remoteAppConfig, configSpec.reverseTransform?.(config) ?? config)
remoteAppConfig = deepMergeObjects(
remoteAppConfig,
configSpec.reverseTransform?.(config, remoteAppConfig) ?? config,
)
})
return {...remoteAppConfig}
}

0 comments on commit 5e2db99

Please sign in to comment.