Skip to content

fix(attachments): prevent orphaned DMS files by moving DMS operations to on phase (opt-in) #303

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
akgarg06 wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

fix(attachments): prevent orphaned DMS files by moving DMS operations to on phase (opt-in) #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3339c51
change srv.before to srv,on
akgarg06 Aug 5, 2026
03624f6
change more handlers to on
akgarg06 Aug 6, 2026
28ed7e9
flag based approach
akgarg06 Aug 11, 2026
2881873
Merge branch 'develop' into RBSDMS-orphan-file-issue-fix-feature
akgarg06 Aug 14, 2026
51a072a
Merge branch 'develop' into RBSDMS-orphan-file-issue-fix-feature
akgarg06 Aug 14, 2026
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
136 changes: 79 additions & 57 deletions lib/sdm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,15 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
return attachmentCompositions;
}

async draftEntityRenameHandler(req) {
async draftEntityRenameHandler(req, next) {
const { repositoryId } = getConfigurations();
const attachmentCompositions = this.getAttachmentCompositions(req.target);
LOG.debug(`[DEBUG] [draftEntityRenameHandler] entity=${req.target.name} compositions=${attachmentCompositions.length}`);

for (const composition of attachmentCompositions) {
await this.processCompositionRename(req, composition, repositoryId);
}
return next?.();
}

async processCompositionRename(req, compositionName, repositoryId) {
Expand Down Expand Up @@ -572,9 +573,13 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
}
}

async draftAttachmentUploadHandler(req) {
async draftAttachmentUploadHandler(req, next) {

if (req?.data?.content) {
// Not a content upload (e.g. metadata-only draft edit) -> pass control down the
// on-handler chain so the base attachments/default handler can run.
if (!req?.data?.content) return next?.();

{
// Read actual file size from HTTP Content-Length header so the chunked
// upload path can be selected before the stream is consumed.
const rawContentLength = req.req?.headers?.['content-length'] || req.headers?.['content-length'];
Expand Down Expand Up @@ -1029,7 +1034,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
req.parentId.push(folderId);
}

async attachURLsToDeleteFromAttachmentsDraft(req) {
async attachURLsToDeleteFromAttachmentsDraft(req, next) {
let draftAttachments = cds.model.definitions[req.target.name];
if(draftAttachments) {
const attachmentsToDeleteFromDraft = await getURLToDeleteFromDraftAttachments(req.data.ID, draftAttachments);
Expand All @@ -1041,6 +1046,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
await this.deleteAttachmentsWithKeys(req.attachmentsToDelete, req);
}
}
return next?.();
}

async deleteAttachmentsWithKeys(records, req) {
Expand Down Expand Up @@ -1574,12 +1580,12 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
}
}

async handleDraftDiscardForLinks(req) {
async handleDraftDiscardForLinks(req, next) {
let parentId = req.data.ID;
const baseEntityName = req.target.name.replace(/\.drafts$/, "");
const baseEntity = cds.model.definitions[baseEntityName];
if (!baseEntity) {
return;
return next?.();
}
LOG.debug(`[DEBUG] [handleDraftDiscardForLinks] parentId=${parentId} entity=${baseEntityName}`);
const attachmentCompositions = this.getAttachmentCompositions({ name: baseEntityName });
Expand Down Expand Up @@ -1614,6 +1620,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
}
}
}
return next?.();
}

async revertLinkInSDM(draftAttachment, originalLinkUrl, req, attachmentsEntity) {
Expand Down Expand Up @@ -1676,9 +1683,10 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
*
* @param {import('@sap/cds').Request} req - The request object
*/
async nonDraftAttachmentCreateHandler(req) {
// Skip if no content or if this is a draft entity (handled by draftPutHandler)
if (!req.data.content || req.target.isDraft) return;
async nonDraftAttachmentCreateHandler(req, next) {
// Skip if no content or if this is a draft entity (handled by draftPutHandler).
// Chain to the next on-handler / default persistence.
if (!req.data.content || req.target.isDraft) return next?.();

LOG.info(`[INFO] [nonDraftAttachmentCreateHandler] event=${req.event} target=${req.target.name}`);
const rawContentLength = req.req?.headers?.['content-length'] || req.headers?.['content-length'];
Expand Down Expand Up @@ -1741,27 +1749,29 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
}

req.data.content = null;
// Upload to DMS done. Chain to default handler to persist metadata to the DB.
return next?.();
}

/**
* Handler for direct UPDATE operations on non-draft attachment entities
* Called on PATCH/UPDATE of attachment entity directly (e.g., Projects.references)
* @param {import('@sap/cds').Request} req - The request object
*/
async nonDraftAttachmentUpdateHandler(req) {
async nonDraftAttachmentUpdateHandler(req, next) {
// Skip if this is a draft entity
if (req.target.isDraft) {
return;
return next?.();
}

// Skip if this is a PUT /content operation (handled by nonDraftAttachmentCreateHandler)
if (req.data.content) {
return;
return next?.();
}

// Skip if filename is not being changed and no custom properties
if (!('filename' in req.data) && Object.keys(req.data).length <= 1) {
return;
return next?.();
}

LOG.info(`[INFO] [nonDraftAttachmentUpdateHandler] Updating attachment metadata target=${req.target.name} ID=${req.data.ID}`);
Expand All @@ -1775,7 +1785,6 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
if (!currentAttachment) {
return req.reject(404, 'Attachment not found');
}

// Merge request data with current attachment for validation
const attachmentToUpdate = { ...currentAttachment, ...req.data };

Expand All @@ -1801,35 +1810,39 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
// Use the existing _updateAttachments method which handles all validation and SDM updates
const failedReq = await this._updateAttachments(req, context);

// Handle errors using req.reject (direct operation pattern)
// Handle errors using req.reject (direct operation pattern). Explicit return after
// reject so the trailing next() is not reached on a rejected request.
if (failedReq && failedReq.length > 0) {
const error = failedReq[0];

if (error.typeOfError === 'restricted characters') {
req.reject(409, nameConstrainErr([error.name], "Update"));
return req.reject(409, nameConstrainErr([error.name], "Update"));
} else if (error.typeOfError === 'empty name') {
req.reject(400, emptyFileNameErr);
return req.reject(400, emptyFileNameErr);
} else if (error.typeOfError === 'duplicate') {
req.reject(409, duplicateFileErr([error.name]));
return req.reject(409, duplicateFileErr([error.name]));
} else if (error.typeOfError === 'no sdm roles') {
req.reject(403, userNotAuthorisedError);
return req.reject(403, userNotAuthorisedError);
} else if (error.typeOfError === 'not found') {
req.reject(404, renameFileErr([error.name], getStatusCondition(404)));
return req.reject(404, renameFileErr([error.name], getStatusCondition(404)));
} else if (error.typeOfError === 'unsupported properties') {
// Parse CMIS property IDs from error.details (comma-separated string)
const cmisPropertyIds = error.details.split(',').map(name => name.trim());

// For unsupported properties, we warn but don't reject (matches draft behavior)
// The properties couldn't be updated, but the operation should still succeed
const warningMessage = unsupportedPropertiesErrorMessage(cmisPropertyIds);
req.warn(warningMessage);
// Continue - don't reject, just warn
// Continue - don't reject, just warn (falls through to next() below)
} else if (error.typeOfError === 'bad request') {
req.reject(500, error.message || renameOtherFilesErr([error.name], ['Update failed']));
return req.reject(500, error.message || renameOtherFilesErr([error.name], ['Update failed']));
} else {
req.reject(500, 'Update failed');
return req.reject(500, 'Update failed');
}
}

// Metadata/rename update in DMS done. Chain to default handler to persist to DB.
return next?.();
}

/**
Expand All @@ -1841,20 +1854,21 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
*
* @param {import('@sap/cds').Request} req - The request object
*/
async nonDraftEntityRenameHandler(req) {
async nonDraftEntityRenameHandler(req, next) {
const { repositoryId } = getConfigurations();
const attachmentsEntity = cds.model.definitions[req.target.name + ".attachments"];
if (!attachmentsEntity) return;

if (!attachmentsEntity) return next?.();

LOG.debug(`[DEBUG] [nonDraftEntityRenameHandler] entity=${req.target.name} repositoryId=${repositoryId}`);
const updatedAttachments = await this._getUpdatedAttachments(req);
if (!updatedAttachments || updatedAttachments.length === 0) return;
if (!updatedAttachments || updatedAttachments.length === 0) return next?.();

const validationContext = this._prepareValidationContext(attachmentsEntity, updatedAttachments[0]);
const allErrors = await this._processAttachmentUpdates(req, updatedAttachments, attachmentsEntity, validationContext);

this._handleUpdateResults(req, repositoryId, allErrors, validationContext.propertyTitles);
return next?.();
}

/**
Expand Down Expand Up @@ -2140,6 +2154,27 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;


registerSDMHandlers(srv, entity, target) {
// Feature flag: when settings.uploadInOnPhase (or env SDM_UPLOAD_IN_ON_PHASE) is
// true, the DMS-mutating handlers are registered in the `on` phase (prepended) so a
// customer validation reject in `before` prevents the DMS operation -> no orphaned
// files. Default is false, which keeps the original `before`-phase behavior for full
// backward compatibility. Opt-in only.
const useOnPhase =
process.env.SDM_UPLOAD_IN_ON_PHASE === "true" ||
cds.env?.requires?.["sdm"]?.settings?.uploadInOnPhase === true;
LOG.info(`[INFO] [registerSDMHandlers] DMS handler phase = ${useOnPhase ? "on (opt-in)" : "before (default)"}`);

// Helper: register a DMS-mutating handler in `on` (prepended) when the flag is set,
// otherwise in `before`. The handlers accept (req, next) and use next?.() so they run
// correctly in either phase.
const registerDmsHandler = (events, ent, handler) => {
if (useOnPhase) {
srv.prepend(() => srv.on(events, ent, handler));
} else {
srv.before(events, ent, handler);
}
};

// When @SDM.useClientCredential is set, override createdBy/modifiedBy with
// the SDM technical user clientid so the plugin DB matches DMS/DI. Run
// these first so they win against later managed-aspect defaults.
Expand Down Expand Up @@ -2182,17 +2217,17 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;

// Draft-specific handlers
if (entity.drafts) {
srv.before("DELETE", entity.drafts, this.handleDraftDiscardForLinks.bind(this));
registerDmsHandler("DELETE", entity.drafts, this.handleDraftDiscardForLinks.bind(this));
// Snapshot existing attachment IDs BEFORE activation so the after-SAVE
// stamp can target only freshly activated rows. Registered before the
// rename handler so req._sdmSaveSnapshot is populated for everything
// downstream that runs in the SAVE flow.
srv.before("SAVE", entity, this.captureSaveSnapshot.bind(this));
srv.after("SAVE", entity, this.handleDraftSaveForLinks.bind(this));
srv.before("SAVE", entity, this.draftEntityRenameHandler.bind(this));
registerDmsHandler("SAVE", entity, this.draftEntityRenameHandler.bind(this));
} else {
// Non-draft rename/update handler
srv.before("UPDATE", entity, this.nonDraftEntityRenameHandler.bind(this));
registerDmsHandler("UPDATE", entity, this.nonDraftEntityRenameHandler.bind(this));
}

srv.after(
Expand All @@ -2208,7 +2243,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;

// Handle DELETE on attachment entity (draft and non-draft)
if (target.drafts) {
srv.before(["DELETE"], target.drafts, this.attachURLsToDeleteFromAttachmentsDraft.bind(this));
registerDmsHandler(["DELETE"], target.drafts, this.attachURLsToDeleteFromAttachmentsDraft.bind(this));
}

// Handle direct DELETE on attachment entity
Expand All @@ -2223,35 +2258,22 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn;
srv.before("READ", targets, this.setRepository.bind(this));
srv.before("READ", targets, this.filterAttachments.bind(this));

// Handle PUT for draft attachments
// Handle PUT for draft/non-draft attachment content upload.
// When the opt-in flag is set, these run in the `on` phase (prepended, so they
// run before the base @cap-js/attachments on-handler); CAP only enters `on` if no
// req.errors occurred in `before`, so a customer validation reject prevents the DMS
// upload -> no orphaned files. Default (flag off) keeps the original `before` phase.
if (target.drafts) {
srv.before(
"PUT",
target.drafts,
this.draftAttachmentUploadHandler.bind(this)
);
registerDmsHandler("PUT", target.drafts, this.draftAttachmentUploadHandler.bind(this));
} else {
// Handle PUT for non-draft attachments (content upload)
srv.before(
"PUT",
target,
this.nonDraftAttachmentCreateHandler.bind(this)
);
registerDmsHandler("PUT", target, this.nonDraftAttachmentCreateHandler.bind(this));
}

// Handle CREATE for non-draft attachments
srv.before(
"CREATE",
target,
this.nonDraftAttachmentCreateHandler.bind(this)
);
// Handle CREATE for non-draft attachments (same phase rationale)
registerDmsHandler("CREATE", target, this.nonDraftAttachmentCreateHandler.bind(this));

// Handle direct UPDATE/PATCH on non-draft attachment entity
srv.before(
"UPDATE",
target,
this.nonDraftAttachmentUpdateHandler.bind(this)
);
// Handle direct UPDATE/PATCH on non-draft attachment entity (same phase rationale)
registerDmsHandler("UPDATE", target, this.nonDraftAttachmentUpdateHandler.bind(this));

srv.after(
"DELETE",
Expand Down
Loading
Loading