From 66153fb509500b2cf2c22680b72e65b646d264c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Hartvig=20Gr=C3=B8nbech?= Date: Mon, 1 Jun 2026 15:02:22 +0200 Subject: [PATCH 1/5] Bug 636342: External Storage - Document Attachments orphans blobs on delete 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, 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: a 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. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/DAExtStorageImplTests.Codeunit.al | 91 +++++++++++++++++++ .../DAExternalStorageImpl.Codeunit.al | 58 +++++++----- 2 files changed, 124 insertions(+), 25 deletions(-) diff --git a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al index 8eb6669c20..0c70f2b4dc 100644 --- a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al +++ b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al @@ -368,6 +368,97 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" #endregion + #region OnAfterDelete Subscriber Tests + + [Test] + [HandlerFunctions('ConfirmYesHandler')] + procedure RecordDeleteRemovesBlobFromExternalStorage() + var + DocumentAttachment: Record "Document Attachment"; + DAExternalStorageImpl: Codeunit "DA External Storage Impl."; + ExternalFilePath: Text; + begin + // [SCENARIO] Deleting a Document Attachment row must delete its blob via the OnAfterDelete subscriber. + // Regression test for the bug where the subscriber called DeleteFromExternalStorage(Rec), which + // started with Rec.Find() and exited because the row was already gone, leaving the blob orphaned. + Initialize(); + SetupFileScenarioWithTestConnector(); + EnableFeatureWithDelete(); + + // [GIVEN] A document attachment that has been uploaded to external storage + CreateDocumentAttachmentWithContent(DocumentAttachment); + DAExternalStorageImpl.UploadToExternalStorage(DocumentAttachment); + DocumentAttachment.SetRecFilter(); + DocumentAttachment.FindFirst(); + ExternalFilePath := DocumentAttachment."External File Path"; + Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Precondition: blob should exist after upload'); + + // [WHEN] The Document Attachment row is deleted (fires OnAfterDeleteEvent) + DocumentAttachment.Delete(true); + + // [THEN] The external blob is removed + Assert.IsFalse(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be deleted from external storage when the attachment row is deleted'); + end; + + [Test] + [HandlerFunctions('ConfirmYesHandler')] + procedure RecordDeleteKeepsBlobWhenSkipDeleteOnCopyIsSet() + var + DocumentAttachment: Record "Document Attachment"; + DAExternalStorageImpl: Codeunit "DA External Storage Impl."; + ExternalFilePath: Text; + begin + // [SCENARIO] When the row was created by an attachment copy, the blob is shared with the source + // and must NOT be deleted when the copy is removed. + Initialize(); + SetupFileScenarioWithTestConnector(); + EnableFeatureWithDelete(); + + // [GIVEN] An externally-stored attachment flagged as a copy + CreateDocumentAttachmentWithContent(DocumentAttachment); + DAExternalStorageImpl.UploadToExternalStorage(DocumentAttachment); + DocumentAttachment.SetRecFilter(); + DocumentAttachment.FindFirst(); + DocumentAttachment."Skip Delete On Copy" := true; + DocumentAttachment.Modify(); + ExternalFilePath := DocumentAttachment."External File Path"; + + // [WHEN] The row is deleted + DocumentAttachment.Delete(true); + + // [THEN] The shared blob is still present + Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when Skip Delete On Copy is set'); + end; + + [Test] + [HandlerFunctions('ConfirmYesHandler')] + procedure RecordDeleteKeepsBlobWhenDeleteFromExternalStorageDisabled() + var + DocumentAttachment: Record "Document Attachment"; + DAExternalStorageImpl: Codeunit "DA External Storage Impl."; + ExternalFilePath: Text; + begin + // [SCENARIO] When the user opted out of automatic deletion, the blob must stay even if the row is deleted. + Initialize(); + SetupFileScenarioWithTestConnector(); + EnableFeature(); // "Delete from External Storage" = false + + // [GIVEN] An uploaded externally-stored attachment + CreateDocumentAttachmentWithContent(DocumentAttachment); + DAExternalStorageImpl.UploadToExternalStorage(DocumentAttachment); + DocumentAttachment.SetRecFilter(); + DocumentAttachment.FindFirst(); + ExternalFilePath := DocumentAttachment."External File Path"; + + // [WHEN] The row is deleted + DocumentAttachment.Delete(true); + + // [THEN] The blob remains + Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when Delete from External Storage is disabled'); + end; + + #endregion + #region MIME Type Tests [Test] diff --git a/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al b/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al index d8ef9a0151..ca981c7c4b 100644 --- a/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al +++ b/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al @@ -364,13 +364,6 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" /// The document attachment record to delete from external storage. /// True if deletion was successful, false otherwise. procedure DeleteFromExternalStorage(var DocumentAttachment: Record "Document Attachment"): Boolean - var - TempFileAccount: Record "File Account"; - ExternalFileStorage: Codeunit "External File Storage"; - FileScenarioCU: Codeunit "File Scenario"; - DAFeatureTelemetry: Codeunit "DA Feature Telemetry"; - FileScenario: Enum "File Scenario"; - ExternalFilePath: Text; begin // Check if feature is enabled if not IsFeatureEnabled() then @@ -395,23 +388,31 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" exit(true); end; - // Use the stored external file path - ExternalFilePath := DocumentAttachment."External File Path"; + if not TryDeleteExternalFile(DocumentAttachment."External File Path", DocumentAttachment) then + exit(false); - // Search for External Storage assigned File Scenario + DocumentAttachment.MarkAsNotUploadedToExternal(); + exit(true); + end; + + local procedure TryDeleteExternalFile(ExternalFilePath: Text; DocumentAttachmentForTelemetry: Record "Document Attachment"): Boolean + var + TempFileAccount: Record "File Account"; + ExternalFileStorage: Codeunit "External File Storage"; + FileScenarioCU: Codeunit "File Scenario"; + DAFeatureTelemetry: Codeunit "DA Feature Telemetry"; + FileScenario: Enum "File Scenario"; + begin FileScenario := FileScenario::"Doc. Attach. - External Storage"; if not FileScenarioCU.GetSpecificFileAccount(FileScenario, TempFileAccount) then exit(false); - // Delete the file with connector using the File Account framework ExternalFileStorage.Initialize(FileScenario); - if ExternalFileStorage.DeleteFile(ExternalFilePath) then begin - DocumentAttachment.MarkAsNotUploadedToExternal(); - DAFeatureTelemetry.LogFileDeleted(DocumentAttachment); - exit(true); - end; + if not ExternalFileStorage.DeleteFile(ExternalFilePath) then + exit(false); - exit(false); + DAFeatureTelemetry.LogFileDeleted(DocumentAttachmentForTelemetry); + exit(true); end; /// @@ -800,7 +801,6 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" local procedure OnAfterDeleteDocumentAttachment(var Rec: Record "Document Attachment"; RunTrigger: Boolean) var ExternalStorageSetup: Record "DA External Storage Setup"; - ExternalStorageImpl: Codeunit "DA External Storage Impl."; begin // Exit early if trigger is not running if not RunTrigger then @@ -810,7 +810,7 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" if Rec.IsTemporary() then exit; - // Check if auto upload is enabled + // Check if auto delete is enabled if not ExternalStorageSetup.Get() then exit; @@ -821,13 +821,21 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" if not Rec."Stored Externally" then exit; - // Delete from external storage - ExternalStorageImpl.DeleteFromExternalStorage(Rec); + if Rec."External File Path" = '' then + exit; - if Rec."Skip Delete On Copy" then begin - Rec."Skip Delete On Copy" := false; - Rec.Modify(); - end; + // Copied attachments share the source file - never delete the shared blob + if Rec."Skip Delete On Copy" then + exit; + + // Files from another environment/company are managed by their owning environment + if IsFileFromAnotherEnvironmentOrCompany(Rec) then + exit; + + // The Document Attachment row is already deleted at this point, so we cannot + // Find() or Modify() it. Delete the blob using the field values still carried + // on Rec, bypassing the record-based DeleteFromExternalStorage entry point. + TryDeleteExternalFile(Rec."External File Path", Rec); end; /// From fa7d0790b57a38162ff88b307142a9d75e35bb51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Hartvig=20Gr=C3=B8nbech?= Date: Mon, 1 Jun 2026 15:12:06 +0200 Subject: [PATCH 2/5] Bug 636342: add cross-environment regression test for subscriber Covers the IsFileFromAnotherEnvironmentOrCompany early-exit branch in OnAfterDeleteDocumentAttachment - blobs owned by another environment must not be deleted by the local environment's row-delete trigger. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/DAExtStorageImplTests.Codeunit.al | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al index 0c70f2b4dc..a0eb07e674 100644 --- a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al +++ b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al @@ -430,6 +430,37 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when Skip Delete On Copy is set'); end; + [Test] + [HandlerFunctions('ConfirmYesHandler')] + procedure RecordDeleteKeepsBlobWhenFileIsFromAnotherEnvironment() + var + DocumentAttachment: Record "Document Attachment"; + DAExternalStorageImpl: Codeunit "DA External Storage Impl."; + ExternalFilePath: Text; + begin + // [SCENARIO] Files owned by another environment or company must not be deleted + // when the local attachment row is removed - the owning environment is responsible + // for the blob's lifecycle. + Initialize(); + SetupFileScenarioWithTestConnector(); + EnableFeatureWithDelete(); + + // [GIVEN] An externally-stored attachment carrying a foreign source environment hash + CreateDocumentAttachmentWithContent(DocumentAttachment); + DAExternalStorageImpl.UploadToExternalStorage(DocumentAttachment); + DocumentAttachment.SetRecFilter(); + DocumentAttachment.FindFirst(); + DocumentAttachment."Source Environment Hash" := 'DIFFERENTHASH123'; + DocumentAttachment.Modify(); + ExternalFilePath := DocumentAttachment."External File Path"; + + // [WHEN] The row is deleted + DocumentAttachment.Delete(true); + + // [THEN] The blob is preserved + Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when the file belongs to another environment or company'); + end; + [Test] [HandlerFunctions('ConfirmYesHandler')] procedure RecordDeleteKeepsBlobWhenDeleteFromExternalStorageDisabled() From 1b0ac099c125f4fedb45dd771c5116eab55e8e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Hartvig=20Gr=C3=B8nbech?= Date: Tue, 2 Jun 2026 08:43:05 +0200 Subject: [PATCH 3/5] Bug 636342: assert subscriber via DeleteFile spy, not no-op FileExists The OnAfterDelete regression tests asserted blob presence via CheckIfFileExistInExternalStorage, which routes to the Test File Storage Connector mock. The mock's CreateFile, DeleteFile and FileExists are empty by design, so FileExists always returned false, failing the precondition and the post-delete asserts. Record DeleteFile invocations on Test File Connector Setup (Last Deleted File Path) and expose them through FileConnectorMock.GetLastDeletedPath. The four subscriber tests now assert DeleteFile was (or was not) called with the stored External File Path, which is the actual contract under test for the subscriber path. --- .../src/DAExtStorageImplTests.Codeunit.al | 22 +++++++++++-------- .../src/Mock/FileConnectorMock.Codeunit.al | 9 ++++++++ .../src/TestFileConnectorSetup.Table.al | 4 ++++ .../src/TestFileStorageConnector.Codeunit.al | 6 +++++ 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al index a0eb07e674..8cb55920e1 100644 --- a/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al +++ b/src/Apps/W1/External Storage - Document Attachments/Test/src/DAExtStorageImplTests.Codeunit.al @@ -391,13 +391,14 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" DocumentAttachment.SetRecFilter(); DocumentAttachment.FindFirst(); ExternalFilePath := DocumentAttachment."External File Path"; - Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Precondition: blob should exist after upload'); + Assert.AreNotEqual('', ExternalFilePath, 'Precondition: upload should set the External File Path'); // [WHEN] The Document Attachment row is deleted (fires OnAfterDeleteEvent) DocumentAttachment.Delete(true); - // [THEN] The external blob is removed - Assert.IsFalse(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be deleted from external storage when the attachment row is deleted'); + // [THEN] The subscriber invoked DeleteFile against the external connector with the stored path + Assert.AreEqual(ExternalFilePath, FileConnectorMock.GetLastDeletedPath(), + 'External connector DeleteFile should be invoked with the stored External File Path when the attachment row is deleted'); end; [Test] @@ -426,8 +427,9 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" // [WHEN] The row is deleted DocumentAttachment.Delete(true); - // [THEN] The shared blob is still present - Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when Skip Delete On Copy is set'); + // [THEN] The subscriber must NOT invoke DeleteFile for this path + Assert.AreNotEqual(ExternalFilePath, FileConnectorMock.GetLastDeletedPath(), + 'External connector DeleteFile should not be invoked when Skip Delete On Copy is set'); end; [Test] @@ -457,8 +459,9 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" // [WHEN] The row is deleted DocumentAttachment.Delete(true); - // [THEN] The blob is preserved - Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when the file belongs to another environment or company'); + // [THEN] The subscriber must NOT invoke DeleteFile for this path + Assert.AreNotEqual(ExternalFilePath, FileConnectorMock.GetLastDeletedPath(), + 'External connector DeleteFile should not be invoked when the file belongs to another environment or company'); end; [Test] @@ -484,8 +487,9 @@ codeunit 136820 "DA Ext. Storage Impl. Tests" // [WHEN] The row is deleted DocumentAttachment.Delete(true); - // [THEN] The blob remains - Assert.IsTrue(DAExternalStorageImpl.CheckIfFileExistInExternalStorage(ExternalFilePath), 'Blob should be preserved when Delete from External Storage is disabled'); + // [THEN] The subscriber must NOT invoke DeleteFile when the feature setting opts out + Assert.AreNotEqual(ExternalFilePath, FileConnectorMock.GetLastDeletedPath(), + 'External connector DeleteFile should not be invoked when Delete from External Storage is disabled'); end; #endregion diff --git a/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al b/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al index 9a9b580e0c..b23a714f2d 100644 --- a/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al +++ b/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al @@ -116,4 +116,13 @@ codeunit 135810 "File Connector Mock" TestFileConnectorSetup."Unsuccessful Register" := Fail; TestFileConnectorSetup.Modify(); end; + + procedure GetLastDeletedPath(): Text + var + TestFileConnectorSetup: Record "Test File Connector Setup"; + begin + if not TestFileConnectorSetup.FindFirst() then + exit(''); + exit(TestFileConnectorSetup."Last Deleted File Path"); + end; } \ No newline at end of file diff --git a/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al b/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al index 5c5ca4fb6d..46db3ae088 100644 --- a/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al +++ b/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al @@ -28,6 +28,10 @@ table 135811 "Test File Connector Setup" { Caption = 'Unsuccessful Register'; } + field(5; "Last Deleted File Path"; Text[2048]) + { + Caption = 'Last Deleted File Path'; + } } keys diff --git a/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al b/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al index dd52da94dc..e6f6687fdd 100644 --- a/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al +++ b/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al @@ -43,7 +43,13 @@ codeunit 135814 "Test File Storage Connector" implements "External File Storage end; procedure DeleteFile(AccountId: Guid; Path: Text); + var + TestFileConnectorSetup: Record "Test File Connector Setup"; begin + if TestFileConnectorSetup.FindFirst() then begin + TestFileConnectorSetup."Last Deleted File Path" := CopyStr(Path, 1, MaxStrLen(TestFileConnectorSetup."Last Deleted File Path")); + TestFileConnectorSetup.Modify(); + end; end; procedure ListDirectories(AccountId: Guid; Path: Text; FilePaginationData: Codeunit "File Pagination Data"; var TempFileAccountContent: Record "File Account Content" temporary); From 5261de201703eb500a47e8f29a23050cf3397395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Hartvig=20Gr=C3=B8nbech?= Date: Tue, 2 Jun 2026 14:00:32 +0200 Subject: [PATCH 4/5] Bug 636342: stash deleted path in SingleInstance, not via table Modify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform's External File Storage wraps connector callbacks in a TryFunction so a connector error can't leave the caller in an unrecoverable state. The mock connector therefore cannot Modify() a table from inside DeleteFile() — doing so failed both new subscriber regression tests with "Call to the function 'MODIFY' is not allowed inside the call to '...' when it is used as a TryFunction." Make `Test File Storage Connector` SingleInstance and remember the last deleted path in a Text global on the codeunit instance, exposed as GetLastDeletedPath()/ResetLastDeletedPath(). FileConnectorMock now delegates to it and resets it during Initialize(). Drop the now-unused "Last Deleted File Path" field from `Test File Connector Setup`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/Mock/FileConnectorMock.Codeunit.al | 9 ++++---- .../src/TestFileConnectorSetup.Table.al | 4 ---- .../src/TestFileStorageConnector.Codeunit.al | 22 ++++++++++++++----- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al b/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al index b23a714f2d..aed2edee61 100644 --- a/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al +++ b/src/System Application/Test Library/External File Storage/src/Mock/FileConnectorMock.Codeunit.al @@ -17,6 +17,7 @@ codeunit 135810 "File Connector Mock" var TestFileAccount: Record "Test File Account"; TestFileConnectorSetup: Record "Test File Connector Setup"; + TestFileStorageConnector: Codeunit "Test File Storage Connector"; begin TestFileConnectorSetup.DeleteAll(); TestFileConnectorSetup.Init(); @@ -27,6 +28,8 @@ codeunit 135810 "File Connector Mock" TestFileConnectorSetup.Insert(); TestFileAccount.DeleteAll(); + + TestFileStorageConnector.ResetLastDeletedPath(); end; procedure GetAccounts(var FileAccount: Record "File Account") @@ -119,10 +122,8 @@ codeunit 135810 "File Connector Mock" procedure GetLastDeletedPath(): Text var - TestFileConnectorSetup: Record "Test File Connector Setup"; + TestFileStorageConnector: Codeunit "Test File Storage Connector"; begin - if not TestFileConnectorSetup.FindFirst() then - exit(''); - exit(TestFileConnectorSetup."Last Deleted File Path"); + exit(TestFileStorageConnector.GetLastDeletedPath()); end; } \ No newline at end of file diff --git a/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al b/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al index 46db3ae088..5c5ca4fb6d 100644 --- a/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al +++ b/src/System Application/Test Library/External File Storage/src/TestFileConnectorSetup.Table.al @@ -28,10 +28,6 @@ table 135811 "Test File Connector Setup" { Caption = 'Unsuccessful Register'; } - field(5; "Last Deleted File Path"; Text[2048]) - { - Caption = 'Last Deleted File Path'; - } } keys diff --git a/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al b/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al index e6f6687fdd..9dfe8b9806 100644 --- a/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al +++ b/src/System Application/Test Library/External File Storage/src/TestFileStorageConnector.Codeunit.al @@ -9,6 +9,8 @@ using System.ExternalFileStorage; codeunit 135814 "Test File Storage Connector" implements "External File Storage Connector" { + SingleInstance = true; + procedure ListFiles(AccountId: Guid; Path: Text; FilePaginationData: Codeunit "File Pagination Data"; var TempFileAccountContent: Record "File Account Content" temporary); begin TempFileAccountContent.Init(); @@ -43,13 +45,20 @@ codeunit 135814 "Test File Storage Connector" implements "External File Storage end; procedure DeleteFile(AccountId: Guid; Path: Text); - var - TestFileConnectorSetup: Record "Test File Connector Setup"; begin - if TestFileConnectorSetup.FindFirst() then begin - TestFileConnectorSetup."Last Deleted File Path" := CopyStr(Path, 1, MaxStrLen(TestFileConnectorSetup."Last Deleted File Path")); - TestFileConnectorSetup.Modify(); - end; + // The platform invokes connector callbacks inside a TryFunction, so we cannot + // Modify() a table from here. Stash the path in a SingleInstance global instead. + LastDeletedFilePath := Path; + end; + + internal procedure GetLastDeletedPath(): Text + begin + exit(LastDeletedFilePath); + end; + + internal procedure ResetLastDeletedPath() + begin + Clear(LastDeletedFilePath); end; procedure ListDirectories(AccountId: Guid; Path: Text; FilePaginationData: Codeunit "File Pagination Data"; var TempFileAccountContent: Record "File Account Content" temporary); @@ -113,4 +122,5 @@ codeunit 135814 "Test File Storage Connector" implements "External File Storage var FileConnectorMock: Codeunit "File Connector Mock"; + LastDeletedFilePath: Text; } \ No newline at end of file From 846d9a5388539beaec27ed0747d5b87c187732aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Hartvig=20Gr=C3=B8nbech?= Date: Tue, 2 Jun 2026 16:32:32 +0200 Subject: [PATCH 5/5] Bug 636342: drop Try prefix from DeleteExternalFile darjoo: the function doesn't carry the [TryFunction] attribute and any unhandled error from ExternalFileStorage.DeleteFile() or the file scenario lookup still propagates, so the Try prefix mis-signalled the contract. Renamed to DeleteExternalFile to match what it actually does. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../DAExternalStorageImpl.Codeunit.al | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al b/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al index ca981c7c4b..67118fb58c 100644 --- a/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al +++ b/src/Apps/W1/External Storage - Document Attachments/app/src/DocumentAttachmentIntegration/DAExternalStorageImpl.Codeunit.al @@ -388,14 +388,14 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" exit(true); end; - if not TryDeleteExternalFile(DocumentAttachment."External File Path", DocumentAttachment) then + if not DeleteExternalFile(DocumentAttachment."External File Path", DocumentAttachment) then exit(false); DocumentAttachment.MarkAsNotUploadedToExternal(); exit(true); end; - local procedure TryDeleteExternalFile(ExternalFilePath: Text; DocumentAttachmentForTelemetry: Record "Document Attachment"): Boolean + local procedure DeleteExternalFile(ExternalFilePath: Text; DocumentAttachmentForTelemetry: Record "Document Attachment"): Boolean var TempFileAccount: Record "File Account"; ExternalFileStorage: Codeunit "External File Storage"; @@ -835,7 +835,7 @@ codeunit 8751 "DA External Storage Impl." implements "File Scenario" // The Document Attachment row is already deleted at this point, so we cannot // Find() or Modify() it. Delete the blob using the field values still carried // on Rec, bypassing the record-based DeleteFromExternalStorage entry point. - TryDeleteExternalFile(Rec."External File Path", Rec); + DeleteExternalFile(Rec."External File Path", Rec); end; ///