Skip to content

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

Open
akgarg06 wants to merge 5 commits into
developfrom
RBSDMS-orphan-file-issue-fix-feature
Open

akgarg06 wants to merge 5 commits into
developfrom
RBSDMS-orphan-file-issue-fix-feature

Conversation

@akgarg06

@akgarg06 akgarg06 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Problem

DMS operations (upload, rename, delete, link-revert) ran in CAP before handlers.
Since before-handlers execute concurrently and the error gate is checked only after
they all complete, a validation reject (or any rollback) could abort the CAP
transaction after the DMS call already executed — leaving an orphaned file in DMS
with no DB record. srv.prepend does not fix this (ordering ≠ prevention).

Fix

Move all DMS-mutating attachment handlers from before to the on phase
(prepended, with next() chaining). CAP only enters on if no before errors
occurred, so a validation reject now prevents the DMS operation entirely.

Handlers moved: draft upload, non-draft create/upload, non-draft update/rename,
draft rename (SAVE), non-draft rename, draft discard, draft-attachment delete.
Prep/collect/read handlers stay in before; the DMS delete stays in after.

Opt-in flag (default OFF — no behavior change unless enabled)

  • cds.requires.sdm.settings.uploadInOnPhase: true, or
  • env var SDM_UPLOAD_IN_ON_PHASE=true

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist before requesting a review

  • I have tested the functionality on my cloud environment.
  • I have provided sufficient automated/ unit tests for the code.
  • I have increased or maintained the test coverage.
  • I have ran integration tests on my cloud environment.
  • I have validated blackduck portal for any vulnerability after my commit.

Upload Screenshots/lists of the scenarios tested

  • I have Uploaded Screenshots or added lists of the scenarios tested in description

Multi tenant Integration test : https://github.com/cap-js/sdm/actions/runs/31776642709
Single Tenant Integration test : https://github.com/cap-js/sdm/actions/runs/31776340192

This branch was previously deployed

2 inactive (1 outdated) deployments
pr-analysis 51a072a0 Deployed Aug 14, 2026 by akgarg06 via build #809
dev 2881873a Deployed Aug 14, 2026 by akgarg06 via virusscan-repo-cleanup #90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant