Skip to content

fix(import): preserve env-locale pairing for asset publish (DX-9772) #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
naman-contentstack wants to merge 1 commit into
base: development
Choose a base branch
from
+158 −92
Open

fix(import): preserve env-locale pairing for asset publish (DX-9772) #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 55 additions & 33 deletions packages/contentstack-import/src/import/modules/assets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, any>[],
): { environments: string[]; locales: string[] }[] {
const localesByEnv = new Map<string, Set<string>>();
for (const { environment, locale } of publishDetails || []) {
if (!locale || !this.environments?.hasOwnProperty(environment)) continue;
let set = localesByEnv.get(environment);
if (!set) {
set = new Set<string>();
localesByEnv.set(environment, set);
}
set.add(locale);
}

// Coalesce environments with an identical locale set into a single payload.
const groups = new Map<string, { environments: string[]; locales: string[] }>();
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<void>} Promise<void>
Expand All @@ -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<string, any>) =>
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,
Expand Down
162 changes: 103 additions & 59 deletions packages/contentstack-import/test/unit/import/modules/assets.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});
});

Expand Down
Loading