Feat/delta aem#1092
Conversation
…nd mapping integration
# Conflicts: # ui/src/components/ContentMapper/index.tsx
- api: add unit tests for the delta Asset Mapper decision branches (isUpdate reuse vs in-place update), saveAssetMetadata/ loadPreviousAssetMetadata, ensureUpdateConfigFile/ enrichConfigWithAssetUpdates, and the entry-update-script.cjs helpers (isAssetField, resolveAssetField 3-way, mergeFlatPayloadIntoEntry, task runner). Expose the cjs helpers for testing. - api: fix stale contentMapper.routes mock (add getAssetMapping, getSingleContentTypes, getSingleGlobalField, updateAssetStatus) and uid-mapper assertion to match the current always-write behavior. - upload-api: add extractEntries/extractAssets to the migration-aem mock in aem.controller.test so createAemMapper makes both API calls.
…fix EntryMapper wiring - Extract pure logic from assetMapper.tsx and entryMapper.tsx into assetMapper.utils.ts / entryMapper.utils.ts (row mapping, selection build/apply, changed-uid diff, content-type status filtering) and add unit tests for both. - Fix two pre-existing bugs in the delta EntryMapper render in index.tsx: reference the existing isDeltaIteration flag instead of an undefined 'iteration' variable, and pass the required handleStepChange prop instead of tableHeight/selectedContentTypeId (which EntryMapper ignores).
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
Add a removeExistingAssets test that drives the entry-JSON reference rewrite path (entriesDir present -> content-type/locale/chunk walk -> replaceAssetRefsInObject). This covers the four previously-uncovered functions in asset-update.utils.ts, raising api function coverage from 79.9% to 80.82% (above the 80% CI threshold).
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
…-23) The delta update step built database/<projectId>/<iteration>/ config paths from the raw, HTTP-derived projectId and later read them with fs.readFileSync (enrichConfigWithAssetMapping/enrichConfigWithAssetUpdates), allowing path traversal. Reuse the existing sanitizeProjectId result (safePid) — which rebuilds the value from a char allowlist and rejects '..', '/', '\' — for saveAssetMetadata, removeExistingAssets, removeEntriesFromDatabase, ensureUpdateConfigFile and enrichConfigWithAssetMapping, and bail out when the id is invalid.
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
There was a problem hiding this comment.
⚠️ Not ready to approve
There are confirmed correctness issues in the new asset-mapper API/UI (including a guaranteed-200 toggle response and crash-prone search filtering) plus a committed machine-specific config path that should be fixed before merge.
Pull request overview
Adds delta-migration support for AEM by generating stable asset/entry mapping inputs at upload time, exposing delta decision UIs (asset reuse vs update-in-place; entry update selection), and updating backend delta utilities + CLI update script to carry mappings forward and apply in-place asset replacements.
Changes:
- Upload-api AEM mapper generation now extracts entry + asset mapping and sends it to the API for delta decision storage.
- UI adds Asset Mapper + pure utility modules with unit tests; Content Mapper can switch between Entry/Asset delta decisions.
- API adds asset-mapper storage/endpoints, improves delta asset handling + update script, and merges uid-maps across iterations; includes AEM importer robustness fixes and a projectId sanitization guard in delta flow.
File summaries
| File | Description |
|---|---|
| upload-api/tests/unit/controllers/aem.controller.test.ts | Updates mocks for new migration-aem exports used by the controller. |
| upload-api/src/controllers/aem/index.ts | Calls new extractEntries/extractAssets during AEM mapper creation. |
| upload-api/src/config/index.ts | Adds a TS config module (in addition to existing JSON config). |
| upload-api/src/config/index.json | Adjusts default config values for upload-api. |
| upload-api/migration-aem/libs/entries/index.ts | New entry mapping extraction for delta decisions. |
| upload-api/migration-aem/libs/contentType/createContentTypes.ts | Adds JSON fallback fields and hardens schema generation logic. |
| upload-api/migration-aem/libs/assets/index.ts | New asset mapping extraction for delta decisions. |
| upload-api/migration-aem/index.ts | Exports new extraction helpers from migration-aem package. |
| ui/src/services/api/migration.service.ts | Adds UI API calls for asset mapping list + status updates. |
| ui/src/components/ContentMapper/index.tsx | Integrates AssetMapper/EntryMapper views into ContentMapper. |
| ui/src/components/ContentMapper/index.scss | Adds styling for the mapper view toggle. |
| ui/src/components/ContentMapper/entryMapper.utils.ts | Extracts pure entry-mapper logic for unit testing. |
| ui/src/components/ContentMapper/entryMapper.tsx | Refactors to use extracted pure helpers/shared selection helpers. |
| ui/src/components/ContentMapper/contentMapper.interface.ts | Adds AssetMapperType interface. |
| ui/src/components/ContentMapper/assetMapper.utils.ts | New pure helper functions for AssetMapper (formatting/selection). |
| ui/src/components/ContentMapper/assetMapper.tsx | New AssetMapper component (infinite table + save decisions). |
| ui/src/components/ContentMapper/tests/entryMapper.utils.test.ts | Adds unit tests for EntryMapper pure helpers. |
| ui/src/components/ContentMapper/tests/assetMapper.utils.test.ts | Adds unit tests for AssetMapper pure helpers. |
| api/tests/unit/utils/uid-mapper.utils.test.ts | Updates test expectations for carry-forward uid-mapper behavior. |
| api/tests/unit/utils/entry-update.enrich.test.ts | Adds tests for new config enrichment helpers. |
| api/tests/unit/utils/entry-update-script.test.ts | Adds tests for entry-update script helpers and asset-update task. |
| api/tests/unit/utils/asset-update.decisions.test.ts | Adds tests covering delta asset decision branches + metadata helpers. |
| api/tests/unit/routes/contentMapper.routes.test.ts | Extends route mocks for new asset mapping endpoints. |
| api/src/utils/uid-mapper.utils.ts | Carries uid mappings forward by merging previous iteration maps. |
| api/src/utils/entry-update.utils.ts | Adds helpers to ensure/update config for asset updates and mapping injection. |
| api/src/utils/entry-update-script.cjs | Adds optional “Update Assets” task and exposes helpers for tests. |
| api/src/utils/content-type-creator.utils.ts | Treats AEM component fallback JSON as JSON schema in creator utils. |
| api/src/utils/asset-update.utils.ts | Implements asset decision processing + in-place replace queue + ref rewriting. |
| api/src/services/migration.service.ts | Wires delta asset updates into update-config and runs update CLI; adds projectId sanitization guard in delta step. |
| api/src/services/contentMapper.service.ts | Stores/serves/toggles asset mapper rows; enriches asset rows with uid-mapper and change detection. |
| api/src/services/aem.service.ts | Stabilizes asset/entry UIDs across runs, improves locale mapping, and hardens JSON-RTE fallback behavior. |
| api/src/routes/contentMapper.routes.ts | Adds asset mapping list + status update routes. |
| api/src/models/assetMapper.ts | New lowdb model for asset-mapper persistence. |
| api/src/controllers/projects.contentMapper.controller.ts | Exposes new asset mapping controller methods. |
| api/src/constants/index.ts | Adds ASSET_MAPPER database file constant. |
Copilot's findings
- Files reviewed: 35/35 changed files
- Comments generated: 10
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- contentMapper.service: updateAssetStatus now returns 404 when no rows match instead of always returning 200 (foundAssets array was truthy) - assetMapper model: sanitize HTTP-derived projectId before using it as a path segment to prevent path traversal (CWE-23) - assetMapper UI: use API total count for counts/pagination and merge each fetched page at its offset so loadMoreItems no longer drops loaded rows - upload-api config: replace machine-specific localPath and AEM-specific cmsType with neutral defaults (overridable via CMS_LOCAL_PATH/CMS_TYPE)
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
1 similar comment
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
🔗 Jira Ticket
MIGRATION-1001
📋 PR Type
📝 Description
What changed?
api): per-iteration asset snapshot (saveAssetMetadata), reuse/update dedup (removeExistingAssets), entry subtraction + update config (removeEntriesFromDatabase,enrichConfig*), and theentry-update-script.cjsupdate path (cm:stacks:migration).ui): per-asset reuse vs update-in-place and per-entry update decisions, with content-type status reflection; pure logic extracted intoassetMapper.utils.ts/entryMapper.utils.ts.aem.service.ts):jsonfields are always created as JSON-RTE, so the importer walksvalue.children; the AEM-component fallback now emits a valid RTE doc ornullinstead of a childless object (which crashedcm:stacks:importwith "Cannot read properties of undefined (reading 'forEach')").migration.service.ts): the delta step builtdatabase/<projectId>/…config paths from the raw HTTPprojectIdand laterreadFileSync'd them; now sanitized viasanitizeProjectIdwith an early bail on invalid input.index.tsx): use the existingisDeltaIterationflag instead of an undefinediteration, and pass the requiredhandleStepChangeprop.entry-update-script.cjs; ui mapper utils; fixed stale mocks/assertions incontentMapper.routes,uid-mapper.utils, andaem.controller.Why?
Enables re-running an AEM migration without duplicating already-migrated content, and hardens the delta path (crash + security) that the feature introduced.
🧩 Affected Areas
api— Node.js backendui— React frontendupload-api— Upload API serverdocker/docker-compose🧪 How to Test
Expected result: No duplicate content on re-run; the delta Entry Mapper screen renders and navigates; no
forEachcrash during entryimport.
📸 Screenshots / Recordings
🔗 Related PRs / Dependencies
devinto the branch (no external PR d✅ Author Checklist
feature/,bugfix/, orhotfix/+ 5–30 lowercase chars.env/example.envupdated if new environment variables were added (none added)npm test) — api 655, ui 352, upload-api 300README.md/ docs updated if behaviour changed n Confluence -->👀 Reviewer Notes
index.tsxwiring fix.uid-mapper.utilstest assertion was updatedwrite behavior — confirm that behavior is intended.migration.service.tsnow sanitizesprojectIdbefore anydatabase/<projectId>/…file path (CWE-23 fix).