[28.x] Bug 636342: External Storage - Document Attachments orphans blobs on delete#8434
Merged
Merged
Conversation
…obs on delete Backport of #8392 (AB#636342) to releases/28.x. The OnAfterDeleteEvent subscriber called DeleteFromExternalStorage(Rec), which started with Rec.Find() and exited as soon as that returned false because the row had already been deleted. The blob in Azure was never removed, telemetry tag 0000RNT never fired, and customers' Azure storage bills grew silently for every deleted attachment. Extract the blob-delete + telemetry logic into a path-based helper (DeleteExternalFile), keep DeleteFromExternalStorage for callers that still hold a live row (page action, sync report), and rewrite the OnAfterDelete subscriber to gate on Rec's field values (Stored Externally, External File Path, Skip Delete On Copy, Source Environment Hash) and call the helper directly, without Find() or Modify(). Also drop the dead post-call Modify() block that attempted to clear Skip Delete On Copy on the already-deleted row. Add regression tests covering the subscriber path, using a SingleInstance spy on Test File Storage Connector to record DeleteFile invocations (the mock's FileExists is a no-op and cannot be used to assert the contract). Tests cover: delete with the feature enabled removes the blob; Skip Delete On Copy preserves the shared blob from a copied attachment; "Delete from External Storage = false" preserves the blob; cross-environment files are not deleted by the local subscriber. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nikolakukrika
previously approved these changes
Jun 3, 2026
gggdttt
previously approved these changes
Jun 3, 2026
The variable is not a temporary record (no .Init/.Insert); it only holds the GetSpecificFileAccount out-parameter result, so the Temp prefix is prohibited by AA0237. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gggdttt
approved these changes
Jun 4, 2026
JesperSchulz
approved these changes
Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #8392 to
releases/28.x. Tracks AB#637539 (forward-port of AB#636342).What
The OnAfterDelete subscriber on
Table 1173 "Document Attachment"silently failed to delete the corresponding Azure blob, becauseDeleteFromExternalStoragestarted withRec.Find()on a row that the OnAfterDelete trigger had already removed. The opt-in setting"Delete from External Storage" = true(default) was non-functional on the automatic deletion path, and every deleted attachment orphaned its blob on the customer's Azure storage account.How
Extract the blob-delete + telemetry into a private path-based helper
DeleteExternalFile(ExternalFilePath; DocumentAttachmentForTelemetry). The two live-row callers (DocumentAttachmentExternal.Page.al,DAExternalStorageSync.Report.al) keep going throughDeleteFromExternalStorage, which now delegates to the helper and callsMarkAsNotUploadedToExternal()on success — observable behavior preserved. The subscriber now gates onRec's still-populated field values (Stored Externally,External File Path,Skip Delete On Copy,IsFileFromAnotherEnvironmentOrCompany) and calls the helper directly, with noFind(), noModify(). The dead post-callModify()block that tried to clearSkip Delete On Copyon the already-deleted row is removed.Tests
New regression tests in
DAExtStorageImplTests.Codeunit.al:RecordDeleteRemovesBlobFromExternalStorage— the direct regression.RecordDeleteKeepsBlobWhenSkipDeleteOnCopyIsSet— copied-attachment guard.RecordDeleteKeepsBlobWhenDeleteFromExternalStorageDisabled— opt-out guard.Tests assert
DeleteFilewas (or was not) called via aSingleInstancespy onTest File Storage Connector(GetLastDeletedPath()), because the mock'sFileExistsis a no-op and cannot be used to validate the contract. The connector spy cannotModify()a table from insideDeleteFile(TryFunction), hence the SingleInstance text field.Risk
Low. Same scope, same code paths as #8392 — backport-only. The two live-row entry points keep identical observable behavior. The subscriber's behavior goes from "silently does nothing" to "actually deletes the blob," which is the documented and intended behavior. Bounded to one app + one test library.
AB#637539