diff --git a/.changeset/river-fix-parallel-deploy-cache-key.md b/.changeset/river-fix-parallel-deploy-cache-key.md new file mode 100644 index 0000000000..4717e0d88a --- /dev/null +++ b/.changeset/river-fix-parallel-deploy-cache-key.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Fix `app deploy` reusing signed upload URLs across apps in the same organization. diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index 0ce583e51b..eb58a9f893 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -50,6 +50,7 @@ import { businessPlatformRequestDoc, } from '@shopify/cli-kit/node/api/business-platform' import {appManagementRequestDoc} from '@shopify/cli-kit/node/api/app-management' +import {setCurrentCommandId} from '@shopify/cli-kit/node/global-context' import {BugError} from '@shopify/cli-kit/node/error' import {randomUUID} from '@shopify/cli-kit/node/crypto' import {webhooksRequestDoc} from '@shopify/cli-kit/node/api/webhooks' @@ -61,8 +62,9 @@ vi.mock('@shopify/organizations') vi.mock('@shopify/cli-kit/node/api/webhooks') beforeEach(() => { - // Reset the singleton instance before each test AppManagementClient.resetInstance() + setCurrentCommandId('') + delete process.env.COMMAND_RUN_ID }) const extensionA = await testUIExtension({uid: 'extension-a-uuid'}) @@ -1212,7 +1214,7 @@ describe('deploy', () => { describe('AppManagementClient', () => { describe('generateSignedUploadUrl', () => { - test('passes Brotli format for uploads', async () => { + test('passes Brotli format for uploads and scopes the cache key to the app and command run', async () => { // Given const client = AppManagementClient.getInstance() const mockResponse = { @@ -1224,6 +1226,7 @@ describe('AppManagementClient', () => { // Mock the app management request vi.mocked(appManagementRequestDoc).mockResolvedValueOnce(mockResponse) + process.env.COMMAND_RUN_ID = 'test-run-id' const app: MinimalAppIdentifiers = { apiKey: 'test-api-key', @@ -1249,9 +1252,47 @@ describe('AppManagementClient', () => { }, cacheOptions: { cacheTTL: {minutes: 59}, + cacheExtraKey: 'test-api-key-test-run-id', }, }) }) + + test('produces different cache keys for different apps in the same organization', async () => { + // Given + const client = AppManagementClient.getInstance() + const mockResponse = { + appRequestSourceUploadUrl: { + sourceUploadUrl: 'https://example.com/upload-url', + userErrors: [], + }, + } + vi.mocked(appManagementRequestDoc).mockResolvedValueOnce(mockResponse).mockResolvedValueOnce(mockResponse) + process.env.COMMAND_RUN_ID = 'test-run-id' + + const appOne: MinimalAppIdentifiers = { + apiKey: 'app-one-api-key', + organizationId: '213141', + id: 'app-one-id', + } + const appTwo: MinimalAppIdentifiers = { + apiKey: 'app-two-api-key', + organizationId: '213141', + id: 'app-two-id', + } + + // When + client.token = () => Promise.resolve('token') + await client.generateSignedUploadUrl(appOne) + await client.generateSignedUploadUrl(appTwo) + + // Then + const calls = vi + .mocked(appManagementRequestDoc) + .mock.calls.map( + ([options]) => (options as {cacheOptions?: {cacheExtraKey?: string}}).cacheOptions?.cacheExtraKey, + ) + expect(calls).toEqual(['app-one-api-key-test-run-id', 'app-two-api-key-test-run-id']) + }) }) describe('bundleFormat', () => { diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 5d8ac4d1c8..5f48c7ea74 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -161,6 +161,7 @@ import { import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' import {versionSatisfies} from '@shopify/cli-kit/node/node-package-manager' import {outputDebug} from '@shopify/cli-kit/node/output' +import {getCurrentCommandId} from '@shopify/cli-kit/node/global-context' import {developerDashboardFqdn, normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {TokenItem} from '@shopify/cli-kit/node/ui' import {functionsRequestDoc, FunctionsRequestOptions} from '@shopify/cli-kit/node/api/functions' @@ -718,7 +719,7 @@ export class AppManagementClient implements DeveloperPlatformClient { } } - async generateSignedUploadUrl({organizationId}: MinimalAppIdentifiers): Promise { + async generateSignedUploadUrl({apiKey, organizationId}: MinimalAppIdentifiers): Promise { const variables = { sourceExtension: 'BR' as SourceExtension, organizationId: gidFromOrganizationIdForShopify(organizationId), @@ -726,7 +727,11 @@ export class AppManagementClient implements DeveloperPlatformClient { const result = await this.appManagementRequest({ query: CreateAssetUrl, variables, - cacheOptions: {cacheTTL: {minutes: 59}}, + cacheOptions: { + cacheTTL: {minutes: 59}, + // Avoid reusing signed upload URLs across apps or command runs. + cacheExtraKey: `${apiKey}-${process.env.COMMAND_RUN_ID ?? getCurrentCommandId()}`, + }, }) return { assetUrl: result.appRequestSourceUploadUrl.sourceUploadUrl,