Skip to content

Fixes #9144 - External File Attachment - Migrating files fails #9295

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
dorianwoolger wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixes #9144 - External File Attachment - Migrating files fails #9295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d978db
[Bug]: External File Attachment - Migrating files fails for Tenant Me…
dorianwoolger Jul 9, 2026
663e9ad
Updated Test codeunit to include new MediaReferenceCount
dorianwoolger Jul 9, 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
15 changes: 12 additions & 3 deletions ...pps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
var
DocumentAttachment: Record "Document Attachment";
DAExternalStorageImpl: Codeunit "DA External Storage Impl.";
MediaReferenceCounts: Dictionary of [Guid, Integer];
Result: Boolean;
begin
// [SCENARIO] Delete from internal should succeed after file is uploaded externally
Expand All @@ -135,8 +136,11 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
DocumentAttachment.SetRecFilter();
DocumentAttachment.FindFirst();

// [GIVEN] A reference count map reflecting the current single reference to the media
DAExternalStorageImpl.BuildMediaReferenceCounts(MediaReferenceCounts);

// [WHEN] Delete from internal is attempted
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment);
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment, MediaReferenceCounts);

// [THEN] Delete should succeed
Assert.IsTrue(Result, 'Delete from internal should succeed');
Expand Down Expand Up @@ -324,6 +328,7 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
var
DocumentAttachment: Record "Document Attachment";
DAExternalStorageImpl: Codeunit "DA External Storage Impl.";
MediaReferenceCounts: Dictionary of [Guid, Integer];
Result: Boolean;
begin
// [SCENARIO] Delete from internal storage should succeed for externally stored docs
Expand All @@ -334,8 +339,11 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
DocumentAttachment."Stored Externally" := true;
DocumentAttachment.Modify();

// [GIVEN] A reference count map reflecting the current single reference to the media
DAExternalStorageImpl.BuildMediaReferenceCounts(MediaReferenceCounts);

// [WHEN] Delete from internal is attempted
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment);
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment, MediaReferenceCounts);

// [THEN] Delete should succeed
Assert.IsTrue(Result, 'Delete from internal should succeed');
Expand All @@ -351,6 +359,7 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
var
DocumentAttachment: Record "Document Attachment";
DAExternalStorageImpl: Codeunit "DA External Storage Impl.";
MediaReferenceCounts: Dictionary of [Guid, Integer];
Result: Boolean;
begin
// [SCENARIO] Delete from internal should fail for non-external document
Expand All @@ -360,7 +369,7 @@ codeunit 136820 "DA Ext. Storage Impl. Tests"
CreateDocumentAttachmentWithContent(DocumentAttachment);

// [WHEN] Delete from internal is attempted
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment);
Result := DAExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment, MediaReferenceCounts);

// [THEN] Delete should fail
Assert.IsFalse(Result, 'Delete from internal should fail for non-external document');
Expand Down
11 changes: 10 additions & 1 deletion ...rnal Storage - Document Attachments/app/src/AutomaticSync/DAExternalStorageSync.Report.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ report 8752 "DA External Storage Sync"
ProcessedCount := 0;
FailedCount := 0;

// Multiple Document Attachment records can reference the same Tenant Media
// record (e.g. an original and its posted-document counterpart). Build a
// reference count once up front so the shared record is only physically
// deleted after the last referencing attachment has been migrated.
if SyncDirection = SyncDirection::"To External Storage" then
ExternalStorageImpl.BuildMediaReferenceCounts(MediaReferenceCounts);


if GuiAllowed() then
Dialog.Open(ProcessingMsg, TotalCount);
end;
Expand All @@ -61,7 +69,7 @@ report 8752 "DA External Storage Sync"
begin
SyncSuccess := ExternalStorageImpl.UploadToExternalStorage(DocumentAttachment);
if SyncSuccess and (Operation = Operation::Move) then begin
DeleteSuccess := ExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment);
DeleteSuccess := ExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment, MediaReferenceCounts);
if not DeleteSuccess then
FailedCount += 1;
end;
Expand Down Expand Up @@ -143,6 +151,7 @@ report 8752 "DA External Storage Sync"
var
ExternalStorageImpl: Codeunit "DA External Storage Impl.";
Dialog: Dialog;
MediaReferenceCounts: Dictionary of [Guid, Integer];
FailedCount: Integer;
MaxRecordsToProcess: Integer;
ProcessedCount: Integer;
Expand Down
63 changes: 56 additions & 7 deletions ...ument Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,11 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario"
/// </summary>
/// <param name="DocumentAttachment">The document attachment record to delete from internal storage.</param>
/// <returns>True if deletion was successful, false otherwise.</returns>
procedure DeleteFromInternalStorage(var DocumentAttachment: Record "Document Attachment"): Boolean
procedure DeleteFromInternalStorage(var DocumentAttachment: Record "Document Attachment"; var MediaReferenceCounts: Dictionary of [Guid, Integer]): Boolean
var
TenantMedia: Record "Tenant Media";
MediaId: Guid;
RemainingReferences: Integer;
begin
// Validate input parameters
if not DocumentAttachment."Document Reference ID".HasValue() then
Expand All @@ -432,16 +434,63 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario"
if not DocumentAttachment."Stored Externally" then
exit(false);

MediaId := DocumentAttachment."Document Reference ID".MediaId();
if not TenantMedia.Get(MediaId) then
exit(false);

// Multiple Document Attachment records can point to the same Tenant Media record
// (e.g. a posted document's attachment frequently shares its "Document Reference ID"
// with the attachment on the original document). Only remove the shared Tenant Media
// record once this is the last Document Attachment still referencing it - otherwise the
// other attachment(s) would lose access to their file content.
//
// Media fields cannot be filtered or passed as parameters (SetRange isn't supported on
// them), so reference counts are tracked in a Guid-keyed map built once up front by
// BuildMediaReferenceCounts, rather than re-scanning the table on every call.
if MediaReferenceCounts.Get(MediaId, RemainingReferences) then begin
RemainingReferences -= 1;
MediaReferenceCounts.Set(MediaId, RemainingReferences);
end else
// Not in the map (e.g. caller didn't pre-build it) - fall back to treating this as
// the only reference so behavior degrades to "delete immediately", same as before.
RemainingReferences := 0;

// Delete from Tenant Media
if TenantMedia.Get(DocumentAttachment."Document Reference ID".MediaId()) then begin
if RemainingReferences <= 0 then
TenantMedia.Delete();

// Mark Document Attachment as Not Stored Internally
DocumentAttachment.MarkAsDeletedInternally();
exit(true);
end;
// This attachment's own reference to internal storage is cleared either way.
DocumentAttachment.MarkAsDeletedInternally();
exit(true);
end;

exit(false);
/// <summary>
/// Scans Document Attachment once and counts, per Tenant Media (by MediaId), how many
/// Document Attachment records currently reference it. Call this once before a batch of
/// DeleteFromInternalStorage calls (e.g. from a report's OnPreDataItem) and reuse the same
/// dictionary instance for every call in that batch, since Media fields can't be filtered
/// directly and re-scanning per record would not scale.
/// </summary>
/// <param name="MediaReferenceCounts">The dictionary to populate with MediaId -&gt; reference count.</param>
procedure BuildMediaReferenceCounts(var MediaReferenceCounts: Dictionary of [Guid, Integer])
var
DocumentAttachment: Record "Document Attachment";
MediaId: Guid;
CurrentCount: Integer;
begin
Clear(MediaReferenceCounts);

DocumentAttachment.SetLoadFields("Document Reference ID");
if DocumentAttachment.FindSet() then
repeat
if DocumentAttachment."Document Reference ID".HasValue() then begin
MediaId := DocumentAttachment."Document Reference ID".MediaId();
if MediaReferenceCounts.Get(MediaId, CurrentCount) then
MediaReferenceCounts.Set(MediaId, CurrentCount + 1)
else
MediaReferenceCounts.Add(MediaId, 1);
end;
until DocumentAttachment.Next() = 0;
end;

/// <summary>
Expand Down
7 changes: 7 additions & 0 deletions ...ment Attachments/app/src/DocumentAttachmentIntegration/DocumentAttachmentExternal.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ page 8751 "Document Attachment - External"
var
DocumentAttachment: Record "Document Attachment";
ExternalStorageImpl: Codeunit "DA External Storage Impl.";
MediaReferenceCounts: Dictionary of [Guid, Integer];
SuccessCount: Integer;
FailedCount: Integer;
begin
Expand All @@ -231,6 +232,12 @@ page 8751 "Document Attachment - External"
DocumentAttachment.SetRange("Stored Internally", true);
SuccessCount := 0;
FailedCount := 0;

// Multiple Document Attachment records can share the same Tenant Media
// record; build a reference count once so it's only physically deleted
// after the last referencing attachment in the selection is processed.
ExternalStorageImpl.BuildMediaReferenceCounts(MediaReferenceCounts, MediaReferenceCounts);

if DocumentAttachment.FindSet() then
repeat
if ExternalStorageImpl.DeleteFromInternalStorage(DocumentAttachment) then
Expand Down
Loading