From f537d43fc944abf661733d273514af47d461a1f1 Mon Sep 17 00:00:00 2001 From: naman-contentstack Date: Fri, 24 Jul 2026 17:22:53 +0530 Subject: [PATCH] fix(import): preserve env-locale pairing for asset publish (DX-9772) publish() flattened each asset's publish_details into independent environments[] and locales[] arrays, so the CMA republished the cartesian product. For a ragged publish state (different locales on different environments) this over-published to env-locale pairs that never existed on the source stack. Add buildPublishGroups: group publish_details by environment, coalesce environments with an identical locale set, and emit one publish call per group so each call is a single rectangle the CMA reproduces exactly. A rectangular asset still collapses to one call (unchanged behavior). The DX-1656 invalid-environment guard is preserved (envs absent from the destination are still skipped). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/import/modules/assets.ts | 88 ++++++---- .../test/unit/import/modules/assets.test.ts | 162 +++++++++++------- 2 files changed, 158 insertions(+), 92 deletions(-) diff --git a/packages/contentstack-import/src/import/modules/assets.ts b/packages/contentstack-import/src/import/modules/assets.ts index e8b792f0b..38481d5eb 100644 --- a/packages/contentstack-import/src/import/modules/assets.ts +++ b/packages/contentstack-import/src/import/modules/assets.ts @@ -4,7 +4,6 @@ import filter from 'lodash/filter'; import unionBy from 'lodash/unionBy'; import orderBy from 'lodash/orderBy'; import isEmpty from 'lodash/isEmpty'; -import uniq from 'lodash/uniq'; import { existsSync } from 'node:fs'; import includes from 'lodash/includes'; import { resolve as pResolve, join } from 'node:path'; @@ -301,6 +300,47 @@ export default class ImportAssets extends BaseClass { return apiOptions; } + /** + * Groups already-filtered `publish_details` into publish payloads that preserve the source + * env↔locale pairing. Entries are grouped by environment, then environments sharing an + * identical locale set are coalesced into one payload. Each returned group is a rectangle + * (its environments × its locales), so the CMA env×locale cross-product reproduces exactly + * the source pairs — never a phantom combination (the DX-9772 over-publish). + * + * A fully-rectangular input (every env published to the same locales) collapses to a single + * group, so behavior is unchanged for the common case; a ragged input fans out into one group + * per distinct locale set. Only environments present in `this.environments` are kept, so the + * env-name lookup is always safe — this preserves the DX-1656 invalid-environment guard. + */ + private buildPublishGroups( + publishDetails: Record[], + ): { environments: string[]; locales: string[] }[] { + const localesByEnv = new Map>(); + for (const { environment, locale } of publishDetails || []) { + if (!locale || !this.environments?.hasOwnProperty(environment)) continue; + let set = localesByEnv.get(environment); + if (!set) { + set = new Set(); + localesByEnv.set(environment, set); + } + set.add(locale); + } + + // Coalesce environments with an identical locale set into a single payload. + const groups = new Map(); + for (const [envUid, localeSet] of localesByEnv) { + const locales = [...localeSet].sort(); + const signature = locales.join(' '); + const existing = groups.get(signature); + if (existing) { + existing.environments.push(this.environments[envUid].name); + } else { + groups.set(signature, { environments: [this.environments[envUid].name], locales }); + } + } + return [...groups.values()]; + } + /** * @method publish * @returns {Promise} Promise @@ -325,49 +365,31 @@ export default class ImportAssets extends BaseClass { handleAndLogError(error, { ...this.importConfig.context, uid, title }); }; + // apiData is a pre-expanded sub-item ({ uid, title, publishDetails }); one per env-locale-set + // group (see below). Pairing is already preserved, so this only resolves the destination UID. const serializeData = (apiOptions: ApiOptions) => { - const { apiData: asset } = apiOptions; - const publishDetails = filter(asset.publish_details, ({ environment }) => { - return this.environments?.hasOwnProperty(environment); - }); - - if (publishDetails.length) { - const environments = uniq(map(publishDetails, ({ environment }) => this.environments[environment].name)); - const locales = uniq(map(publishDetails, 'locale')); - - if (environments.length === 0 || locales.length === 0) { - log.debug( - `Skipping publish for asset ${asset.uid}: no valid environments/locales`, - this.importConfig.context, - ); - apiOptions.entity = undefined; - return apiOptions; - } - - asset.locales = locales; - asset.environments = environments; - apiOptions.apiData.publishDetails = { locales, environments }; - log.debug(`Prepared publish details for asset ${asset.uid}`, this.importConfig.context); - } - - apiOptions.uid = this.assetsUidMap[asset.uid] as string; - + const { apiData } = apiOptions; + apiOptions.uid = this.assetsUidMap[apiData.uid] as string; if (!apiOptions.uid) { - log.debug(`Skipping publish for asset ${asset.uid}: no UID mapping found.`, this.importConfig.context); + log.debug(`Skipping publish for asset ${apiData.uid}: no UID mapping found.`, this.importConfig.context); apiOptions.entity = undefined; } - return apiOptions; }; for (const index in indexer) { log.debug(`Processing publish chunk ${index} of ${indexerCount}`, this.importConfig.context); - const apiContent = filter( - values(await fs.readChunkFiles.next()), - ({ publish_details }) => !isEmpty(publish_details), + // Expand each asset into one sub-item per env-locale-set group, so each makeConcurrentCall + // item is a single-rectangle publish (preserves env↔locale pairing). + const apiContent = values(await fs.readChunkFiles.next()).flatMap((asset: Record) => + this.buildPublishGroups(asset.publish_details).map((publishDetails) => ({ + uid: asset.uid, + title: asset.title, + publishDetails, + })), ); - log.debug(`Found ${apiContent.length} publishable assets in chunk`, this.importConfig.context); + log.debug(`Found ${apiContent.length} asset publish calls in chunk`, this.importConfig.context); await this.makeConcurrentCall({ apiContent, diff --git a/packages/contentstack-import/test/unit/import/modules/assets.test.ts b/packages/contentstack-import/test/unit/import/modules/assets.test.ts index 9cb43df50..c171af5b3 100644 --- a/packages/contentstack-import/test/unit/import/modules/assets.test.ts +++ b/packages/contentstack-import/test/unit/import/modules/assets.test.ts @@ -651,101 +651,145 @@ describe('ImportAssets', () => { expect(makeConcurrentCallStub.called).to.be.true; }); - it('should filter publish_details by valid environments', async () => { - importAssets['environments'] = { 'env-1': { name: 'production' } }; + it('should skip publish when no UID mapping found', async () => { + importAssets['assetsUidMap'] = {}; makeConcurrentCallStub.callsFake(async (options: any) => { const serializeData = options.apiParams.serializeData; + // apiData is now a pre-grouped sub-item. const result = serializeData({ apiData: { - uid: 'asset-1', - publish_details: [ - { environment: 'env-1', locale: 'en-us' }, - { environment: 'env-invalid', locale: 'en-us' } - ] + uid: 'asset-unknown', + publishDetails: { environments: ['production'], locales: ['en-us'] } } }); - - expect(result.apiData.environments).to.deep.equal(['production']); - expect(result.apiData.locales).to.deep.equal(['en-us']); + + expect(result.entity).to.be.undefined; }); await (importAssets as any).publish(); }); - it('should skip publish when no valid environments', async () => { + it('should set correct UID from mapping and preserve the grouped payload', async () => { + importAssets['assetsUidMap'] = { 'asset-1': 'mapped-asset-1' }; + makeConcurrentCallStub.callsFake(async (options: any) => { const serializeData = options.apiParams.serializeData; const result = serializeData({ apiData: { uid: 'asset-1', - publish_details: [ - { environment: 'env-invalid', locale: 'en-us' } - ] + publishDetails: { environments: ['production'], locales: ['en-us'] } } }); - - expect(result.entity).to.be.undefined; + + expect(result.uid).to.equal('mapped-asset-1'); + // serializeData no longer flattens — it leaves the pre-grouped payload untouched. + expect(result.apiData.publishDetails).to.deep.equal({ environments: ['production'], locales: ['en-us'] }); }); await (importAssets as any).publish(); }); - it('should skip publish when no UID mapping found', async () => { - importAssets['assetsUidMap'] = {}; + it('preserves env-locale pairing for a RAGGED asset (DX-9772) — no phantom pairs', async () => { + // production published only in en-us; preview published only in fr-fr. + importAssets['assetsUidMap'] = { 'asset-1': 'new-asset-1' }; + importAssets['environments'] = { 'env-1': { name: 'production' }, 'env-2': { name: 'preview' } }; + Object.defineProperty(FsUtility.prototype, 'readChunkFiles', { + get: sinon.stub().returns({ + next: sinon.stub().resolves([ + { + uid: 'asset-1', + title: 'ragged.jpg', + publish_details: [ + { environment: 'env-1', locale: 'en-us' }, + { environment: 'env-2', locale: 'fr-fr' } + ] + } + ]) + }), + configurable: true + }); + let apiContent: any[] = []; makeConcurrentCallStub.callsFake(async (options: any) => { - const serializeData = options.apiParams.serializeData; - const result = serializeData({ - apiData: { - uid: 'asset-unknown', - publish_details: [{ environment: 'env-1', locale: 'en-us' }] - } - }); - - expect(result.entity).to.be.undefined; + apiContent = options.apiContent; }); await (importAssets as any).publish(); + + // Two separate publish calls, each a single-rectangle payload — never a cartesian. + expect(apiContent).to.have.lengthOf(2); + const payloads = apiContent.map((i) => i.publishDetails); + expect(payloads).to.deep.include.members([ + { environments: ['production'], locales: ['en-us'] }, + { environments: ['preview'], locales: ['fr-fr'] } + ]); + for (const p of payloads) { + const hasPhantom = + (p.environments.includes('production') && p.locales.includes('fr-fr')) || + (p.environments.includes('preview') && p.locales.includes('en-us')); + expect(hasPhantom, `phantom pair in ${JSON.stringify(p)}`).to.be.false; + } }); + }); - it('should set correct UID from mapping', async () => { - importAssets['assetsUidMap'] = { 'asset-1': 'mapped-asset-1' }; + describe('buildPublishGroups() method', () => { + it('collapses a RECTANGULAR asset to a single publish call (behavior-preserving)', () => { + importAssets['environments'] = { + 'e1': { name: 'production' }, + 'e2': { name: 'preview' }, + 'e3': { name: 'development' } + }; + const pd = ['e1', 'e2', 'e3'].flatMap((environment) => [ + { environment, locale: 'en-us' }, + { environment, locale: 'fr-fr' } + ]); - makeConcurrentCallStub.callsFake(async (options: any) => { - const serializeData = options.apiParams.serializeData; - const result = serializeData({ - apiData: { - uid: 'asset-1', - publish_details: [{ environment: 'env-1', locale: 'en-us' }] - } - }); - - expect(result.uid).to.equal('mapped-asset-1'); - }); + const groups = (importAssets as any).buildPublishGroups(pd); - await (importAssets as any).publish(); + expect(groups).to.have.lengthOf(1); + expect(groups[0].locales).to.deep.equal(['en-us', 'fr-fr']); + expect(groups[0].environments).to.have.members(['production', 'preview', 'development']); }); - it('should extract unique locales from publish_details', async () => { - makeConcurrentCallStub.callsFake(async (options: any) => { - const serializeData = options.apiParams.serializeData; - const result = serializeData({ - apiData: { - uid: 'asset-1', - publish_details: [ - { environment: 'env-1', locale: 'en-us' }, - { environment: 'env-1', locale: 'en-us' }, - { environment: 'env-1', locale: 'fr-fr' } - ] - } - }); - - expect(result.apiData.locales).to.have.lengthOf(2); - expect(result.apiData.locales).to.include.members(['en-us', 'fr-fr']); - }); + it('fans a RAGGED asset out into one group per distinct locale set (DX-9772)', () => { + importAssets['environments'] = { 'e1': { name: 'new' }, 'e2': { name: 'blt5795' } }; + const pd = [ + { environment: 'e2', locale: 'en-us' }, + { environment: 'e1', locale: 'en-us' }, + { environment: 'e1', locale: 'ar' } + ]; - await (importAssets as any).publish(); + const groups = (importAssets as any).buildPublishGroups(pd); + + expect(groups).to.have.lengthOf(2); + expect(groups).to.deep.include.members([ + { environments: ['new'], locales: ['ar', 'en-us'] }, + { environments: ['blt5795'], locales: ['en-us'] } + ]); + const blt5795 = groups.find((g: any) => g.environments.includes('blt5795')); + expect(blt5795.locales).to.not.include('ar'); + }); + + it('handles a single env-locale pair (1×1) as one group', () => { + importAssets['environments'] = { 'e1': { name: 'production' } }; + const groups = (importAssets as any).buildPublishGroups([{ environment: 'e1', locale: 'en-us' }]); + expect(groups).to.deep.equal([{ environments: ['production'], locales: ['en-us'] }]); + }); + + it('drops environments absent from the destination (preserves DX-1656 guard)', () => { + importAssets['environments'] = { 'e1': { name: 'production' } }; + const groups = (importAssets as any).buildPublishGroups([ + { environment: 'e1', locale: 'en-us' }, + { environment: 'gone', locale: 'en-us' } + ]); + expect(groups).to.deep.equal([{ environments: ['production'], locales: ['en-us'] }]); + }); + + it('returns [] for empty publish_details', () => { + importAssets['environments'] = { 'e1': { name: 'production' } }; + expect((importAssets as any).buildPublishGroups([])).to.deep.equal([]); + expect((importAssets as any).buildPublishGroups(undefined)).to.deep.equal([]); }); });