-
Notifications
You must be signed in to change notification settings - Fork 90
start to implement update entity use case into layered architecture #8776
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.
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
base: production
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new UpdateEntity application use case and wires it into the layered architecture, moving entity updates (and, conditionally, creates) off the legacy saveEntity path and onto v2 controllers, use cases, and listeners.
Changes:
- Add a new
UpdateEntityUseCasewith rich tests, backed byMongoMultiLanguageEntityDataSource.getById/update, and expose it viaUpdateEntityUseCaseFactoryandUpdateEntityController. - Extend the entity domain model (
Entity,EntityTranslation, newEntityUpdatedEvent,EntityDoesNotExistError) to support update semantics (icon, generated TOC, template migration, editDate refresh) and emit events consumed by new listeners. - Wire the
/api/entitiesroute and server startup to use the new controller and listeners, registering them in the queue system for denormalization and relationship processing.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| app/server.js | Adds a side-effect import of api/core/infrastructure/listeners/Listeners so event listeners are registered when the server starts, enabling the new entity update event flow. |
| app/queueRegistry.ts | Registers DenormalizeEntityUpdatedListener and ProcessRelationshipAfterEntityUpdatedListener as queue jobs so EntityUpdatedEvent emissions are processed asynchronously. |
| app/api/entities/routes.js | Replaces the legacy saveEntity/withTransaction path for non-v2-create requests with UpdateEntityController, while keeping the v2 create path for feature-flagged new entities; also still handles upload middleware selection. |
| app/api/entities.v2/database/MongoMultiLanguageEntityDataSource.ts | Adds getById and update methods to work with the new use case, updates the aggregation pipeline to group by sharedId before template lookup, and continues to integrate with search indexing via modifiedSharedIds. |
| app/api/entities.v2/contracts/MultiLanguageEntitiesDataSource.ts | Extends the datasource contract with update(entity) and getById(sharedId) returning ResultType<Entity, EntityDoesNotExistError>, so use cases can depend on these operations. |
| app/api/core/libs/eventEmitter/Event.ts | Changes EventPayload to only require userId, affecting how payloads for event-based queue jobs are typed and constructed. |
| app/api/core/libs/DependenciesContext.ts | Adds an attachContext helper that wraps a use case’s execute method in an AsyncLocalStorage context, wiring transactionManager, jobsDispatcher, eventEmitter, and idGenerator access for events and jobs. |
| app/api/core/infrastructure/listeners/DenormalizeEntityUpdatedListener.ts | New listener for EntityUpdatedEvent that recreates before/after entities, maps them to DB objects, and calls denormalizeRelated for the target language, with basic error handling for missing translations/templates. |
| app/api/core/infrastructure/listeners/ProcessRelationshipAfterEntityUpdatedListener.ts | New listener that, on EntityUpdatedEvent, recomputes relationship-based references via relationships.saveEntityBasedReferences for the updated entity/language. |
| app/api/core/infrastructure/listeners/Listeners.ts | Centralizes side-effect imports so all relevant listeners (including the new entity-updated ones) are registered when Listeners is imported. |
| app/api/core/infrastructure/factories/UpdateEntityUseCaseFactory.ts | Factory that builds UpdateEntityUseCase with the appropriate datasources, file service, property assignment strategy, and emitter, pulling transactionManager from DependenciesContext. |
| app/api/core/infrastructure/express/entity/UpdateEntityController.ts | New express controller that parses the update DTO (including multipart form payloads), maps it to UpdateEntityUseCaseInput, executes the use case, and returns the updated language-specific entity DBO while emitting documentProcessed. |
| app/api/core/infrastructure/express/entity/Schemas.ts | Refactors entity request validation into a shared MutateEntitySchema, defines CreateEntitySchema and a new UpdateEntitySchema (with _id, sharedId, and language), and exports corresponding DTO types. |
| app/api/core/infrastructure/express/entity/ExpressEntityMapper.ts | Extends the mapper to support toEntityUpdateInput, translating the update DTO (title, metadata, template, icon, uploaded files and URL attachments) into UpdateEntityUseCaseInput. |
| app/api/core/domain/entity/EntityTranslation.ts | Adds mergeMetadata for template changes and refreshEditDate to centralize editDate handling when metadata is modified. |
| app/api/core/domain/entity/Entity.ts | Enhances the entity domain model with generatedToc, an update method, template-change logic that migrates metadata via EntityTranslation.mergeMetadata, an editDate refresh helper, and exposes prevState/getProps for event emission. |
| app/api/core/domain/entity/errors.ts | Introduces EntityDoesNotExistError, a domain error used when a requested entity sharedId is not found. |
| app/api/core/domain/entity/EntityUpdatedEvent.ts | Defines EntityUpdatedEvent with before/after payloads and factory method create, intended to snapshot entity state around updates for listeners. |
| app/api/core/domain/entity/specs/Entity.spec.ts | Adds tests covering the new update behavior (icon, generatedToc, editDate) and a detailed scenario for template changes and metadata migration across languages. |
| app/api/core/application/UpdateEntity.ts | Implements UpdateEntityUseCase, orchestrating entity lookup, optional template change, property assignment updates, file storage, and emission of EntityUpdatedEvent inside a transaction. |
| app/api/core/application/specs/UpdateEntity.spec.ts | Comprehensive test suite for UpdateEntityUseCase, covering basic field updates, metadata clearing and mutation, file upload/linking behavior, event emission, and template changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const dbos = MongoEntityMapper.toDBO(entity); | ||
|
|
||
| await this.getCollection().bulkWrite( | ||
| dbos.map(dbo => ({ updateOne: { filter: { _id: dbo._id }, update: { $set: dbo } } })), |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In update, you're using $set: dbo where dbo includes the _id field (lines 47-51). MongoDB does not allow updating _id, so this bulkWrite will fail with a "Mod on _id not allowed" error when this path is exercised. To avoid this, either switch to a replaceOne operation like bulkUpdate does, or build the $set object without the _id field (and any other immutable fields).
| dbos.map(dbo => ({ updateOne: { filter: { _id: dbo._id }, update: { $set: dbo } } })), | |
| dbos.map(dbo => ({ | |
| replaceOne: { | |
| filter: { _id: dbo._id }, | |
| replacement: dbo, | |
| }, | |
| })), |
| const sanitized = filename | ||
| .replace(/\.\.[\\\/]/g, '') |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
../
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 13 hours ago
General approach: ensure that any occurrence or emergent occurrence of directory traversal or path separators is fully removed. We can do this either by (a) repeatedly applying the same replacement until no further change occurs, or (b) strengthening the regexes so that any number of dots before a separator are removed in one go, or (c) both, for defense in depth. We also want to keep the rest of the logic (trimming, length checks, etc.) unchanged.
Best concrete fix here without changing external behavior is to harden sanitizeFilename:
- First, remove any sequence of 2 or more dots immediately followed by a path separator: use
/\.{2,}[\\\/]/g. This removes../,..../,.../,..\\, etc., in one pass without reintroducing traversal patterns. - Then remove all path separators everywhere with
/[\\\/]/g, and null bytes with/\0/gas before. - Optionally add a simple loop to reapply the
\.{2,}[\\\/]removal until the string stabilizes, to be fully robust against crafted corner cases.
This change is entirely local to sanitizeFilename in app/api/core/domain/files/BaseFile.ts. No new imports are required; we stay within standard JavaScript/TypeScript capabilities and keep the same function signature and uses (the zod schema transformations).
-
Copy modified line R26 -
Copy modified lines R28-R42
| @@ -23,11 +23,23 @@ | ||
|
|
||
| const sanitizeFilename = (filename: string) => { | ||
| // Remove path traversal attempts, path separators, and null bytes | ||
| const sanitized = filename | ||
| .replace(/\.\.[\\\/]/g, '') | ||
| .replace(/[\\\/]/g, '') | ||
| .replace(/\0/g, ''); | ||
| let sanitized = filename; | ||
|
|
||
| // Repeatedly strip any sequence of two or more dots followed by a path separator, | ||
| // to ensure patterns like ".../", "..../", "../", and "..\\" are fully removed | ||
| // without leaving behind new "../" segments. | ||
| let previous: string; | ||
| do { | ||
| previous = sanitized; | ||
| sanitized = sanitized.replace(/\.{2,}[\\\/]/g, ''); | ||
| } while (sanitized !== previous); | ||
|
|
||
| // Remove any remaining path separators | ||
| sanitized = sanitized.replace(/[\\\/]/g, ''); | ||
|
|
||
| // Remove null bytes | ||
| sanitized = sanitized.replace(/\0/g, ''); | ||
|
|
||
| return sanitized; | ||
| }; | ||
|
|
…update-entity-layered-architecture
fixes #
PR checklist:
QA checklist: