[releases/25.x] Re-enable Retention Policy tests (#2050)

This pull request backports #1973 to releases/25.x Fixes [AB#548787](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/548787) Co-authored-by: Maria Zhelezova <[email protected]> Co-authored-by: Alexander Holstrup <[email protected]>
microsoft · Sep 18, 2024 · 4dc31ce · 4dc31ce
1 parent eea5c47
commit 4dc31ce
Show file tree

Hide file tree

Showing 22 changed files with 435 additions and 220 deletions.
diff --git a/build/projects/Business Foundation/.AL-Go/CompileAppInBcContainer.ps1 b/build/projects/Business Foundation/.AL-Go/CompileAppInBcContainer.ps1
@@ -3,6 +3,4 @@ Param(
 )
 
 $scriptPath = Join-Path $PSScriptRoot "../../../scripts/CompileAppInBcContainer.ps1" -Resolve
-$projectFolder = Join-Path $PSScriptRoot "../../Business Foundation"
-
-. $scriptPath -parameters $parameters -currentProjectFolder $projectFolder
+. $scriptPath -parameters $parameters
diff --git a/build/projects/Performance Toolkit/.AL-Go/CompileAppInBcContainer.ps1 b/build/projects/Performance Toolkit/.AL-Go/CompileAppInBcContainer.ps1
@@ -3,6 +3,4 @@ Param(
 )
 
 $scriptPath = Join-Path $PSScriptRoot "../../../scripts/CompileAppInBcContainer.ps1" -Resolve
-$projectFolder = Join-Path $PSScriptRoot "../../Performance Toolkit" -Resolve
-
-. $scriptPath -parameters $parameters -currentProjectFolder $projectFolder
+. $scriptPath -parameters $parameters
diff --git a/build/projects/System Application Tests (No Isolation)/.AL-Go/CompileAppInBcContainer.ps1 b/build/projects/System Application Tests (No Isolation)/.AL-Go/CompileAppInBcContainer.ps1
@@ -0,0 +1,6 @@
+Param(
+    [Hashtable] $parameters
+)
+
+$scriptPath = Join-Path $PSScriptRoot "../../../scripts/CompileAppInBcContainer.ps1" -Resolve
+. $scriptPath -parameters $parameters
diff --git a/...rojects/System Application Tests (No Isolation)/.AL-Go/ImportTestToolkitToBcContainer.ps1 b/...rojects/System Application Tests (No Isolation)/.AL-Go/ImportTestToolkitToBcContainer.ps1
@@ -0,0 +1,6 @@
+[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'parameters', Justification = 'The parameter is not used, but it''s script needs to match this format')]
+Param(
+    [hashtable] $parameters
+)
+
+Write-Host "Skipping Test Toolkit import"
diff --git a/build/projects/System Application Tests (No Isolation)/.AL-Go/NewBcContainer.ps1 b/build/projects/System Application Tests (No Isolation)/.AL-Go/NewBcContainer.ps1
@@ -0,0 +1,6 @@
+Param(
+    [Hashtable]$parameters
+)
+
+$script = Join-Path $PSScriptRoot "../../../scripts/NewBcContainer.ps1" -Resolve
+. $script -parameters $parameters
diff --git a/build/projects/System Application Tests (No Isolation)/.AL-Go/RunTestsInBcContainer.ps1 b/build/projects/System Application Tests (No Isolation)/.AL-Go/RunTestsInBcContainer.ps1
@@ -0,0 +1,6 @@
+Param(
+    [Hashtable]$parameters
+)
+
+$script = Join-Path $PSScriptRoot "../../../scripts/RunTestsInBcContainer.ps1" -Resolve
+. $script -parameters $parameters -DisableTestIsolation
diff --git a/build/projects/System Application Tests (No Isolation)/.AL-Go/settings.json b/build/projects/System Application Tests (No Isolation)/.AL-Go/settings.json
@@ -0,0 +1,7 @@
+{
+    "projectName": "System Application Tests (No Isolation)",
+    "testFolders":  [
+        "../../../src/System Application/Test",
+        "../../../src/System Application/Test Library"
+    ]
+}
diff --git a/build/projects/System Application Tests/.AL-Go/CompileAppInBcContainer.ps1 b/build/projects/System Application Tests/.AL-Go/CompileAppInBcContainer.ps1
@@ -3,6 +3,4 @@ Param(
 )
 
 $scriptPath = Join-Path $PSScriptRoot "../../../scripts/CompileAppInBcContainer.ps1" -Resolve
-$projectFolder = Join-Path $PSScriptRoot "../../System Application Tests"
-
-. $scriptPath -parameters $parameters -currentProjectFolder $projectFolder
+. $scriptPath -parameters $parameters
diff --git a/build/scripts/RunTestsInBcContainer.ps1 b/build/scripts/RunTestsInBcContainer.ps1
@@ -1,5 +1,6 @@
 Param(
-    [Hashtable]$parameters
+    [Hashtable] $parameters,
+    [switch] $DisableTestIsolation
 )
 
 Import-Module $PSScriptRoot\EnlistmentHelperFunctions.psm1
@@ -31,7 +32,45 @@ function Get-DisabledTests
     return @($disabledTests)
 }
 
-$disabledTests = Get-DisabledTests
+function Get-TestsInGroup {
+    param(
+        [Parameter(Mandatory = $true)]
+        [string] $groupName
+    )
+
+    $baseFolder = Get-BaseFolder
+
+    $groupFiles = Get-ChildItem -Path $baseFolder -Filter 'TestGroups.json' -Recurse -File
+
+    $testsInGroup = @()
+    foreach($groupFile in $groupFiles)
+    {
+        $testsInGroup += Get-Content -Raw -Path $groupFile.FullName | ConvertFrom-Json | Where-Object { $_.group -eq $groupName }
+    }
+
+    return $testsInGroup
+}
+
+$disabledTests = @(Get-DisabledTests)
+$noIsolationTests = Get-TestsInGroup -groupName "No Test Isolation"
+
+if ($DisableTestIsolation)
+{
+    $parameters["testRunnerCodeunitId"] = "130451" # Test Runner with disabled test isolation
+
+    # When test isolation is disabled, only tests from the "No Test Isolation" group should be run
+    $parameters["testCodeunitRange"] = @($noIsolationTests | ForEach-Object { $_.codeunitId }) -join "|"
+}
+else { # Test isolation is enabled
+    # Manually disable the test codeunits, as they need to be run without test isolation
+    $noIsolationTests | ForEach-Object {
+        $disabledTests += @{
+            "codeunitId" = $_.codeunitId
+            "codeunitName" = $_.codeunitName
+            "method" = "*"
+        }
+    }
+}
 
 if ($disabledTests)
 {

diff --git a/src/System Application/App/Retention Policy/app.json b/src/System Application/App/Retention Policy/app.json
@@ -74,8 +74,8 @@
   ],
   "internalsVisibleTo": [
     {
-      "id": "489a0bcb-0619-4bd1-b626-9f30dbe8af4d",
-      "name": "Retention Policy Test",
+      "id": "33003b8a-f6d8-4efe-af22-1a8cb8fbacbe",
+      "name": "Retention Policy Test Library",
       "publisher": "Microsoft"
     }
   ],

diff --git a/...tion/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al b/...tion/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al
@@ -162,9 +162,9 @@ codeunit 3904 "Apply Retention Policy Impl."
         RetentionPolicyLog.LogInfo(LogCategory(), StrSubstNo(StartRetentionPolicyRecordCountLbl, RetentionPolicySetup."Table Id", RetentionPolicySetup."Table Caption"));
 
         if GetExpiredRecords(RetentionPolicySetup, RecordRef, ExpiredRecordExpirationDate) then begin
-            ExpiredRecordCount := Count(RecordRef);
+            ExpiredRecordCount := this.Count(RecordRef);
             RecordRef.Reset();
-            TotalRecordCount := Count(RecordRef);
+            TotalRecordCount := this.Count(RecordRef);
         end;
         RetentionPolicyLog.LogInfo(LogCategory(), StrSubstNo(EndRetentionPolicyRecordCountLbl, RetentionPolicySetup."Table Id", RetentionPolicySetup."Table Caption"));
         RetentionPolicyLog.LogInfo(LogCategory(), StrSubstNo(NumberOfExpiredRecordsLbl, ExpiredRecordCount, TotalRecordCount, RetentionPolicySetup."Table Id", RetentionPolicySetup."Table Caption"));
@@ -226,10 +226,10 @@ codeunit 3904 "Apply Retention Policy Impl."
         RecordCountBefore: Integer;
         RecordCountAfter: Integer;
     begin
-        RecordCountBefore := Count(RecordRef);
         RecordRefDuplicate := RecordRef.Duplicate();
 
         FillTempRetenPolDeletingParamTable(TempRetenPolDeletingParam, RecordRef);
+        RecordCountBefore := TempRetenPolDeletingParam."Record Count Before Delete";
 
         RetenPolDeleting := RetenPolAllowedTables.GetRetenPolDeleting(RecordRef.Number);
         RetenPolDeleting.DeleteRecords(RecordRef, TempRetenPolDeletingParam);
@@ -249,7 +249,7 @@ codeunit 3904 "Apply Retention Policy Impl."
         RetenPolicyTelemetryImpl.SendTelemetryOnRecordsDeleted(RecordRefDuplicate.Number, RecordRefDuplicate.Name, NumberOfRecordsDeleted, IsUserInvokedRun);
 
         if not TempRetenPolDeletingParam."Skip Event Rec. Limit Exceeded" then
-            RaiseRecordLimitExceededEvent(RecordRefDuplicate);
+            RaiseRecordLimitExceededEvent(RecordRefDuplicate, RecordCountAfter);
     end;
 
     local procedure CheckAndContinueWithRerun(var RetentionPolicySetup: Record "Retention Policy Setup")
@@ -265,23 +265,24 @@ codeunit 3904 "Apply Retention Policy Impl."
 
     local procedure FillTempRetenPolDeletingParamTable(var TempRetenPolDeletingParam: Record "Reten. Pol. Deleting Param" temporary; var RecordRef: RecordRef)
     begin
+        TempRetenPolDeletingParam."Record Count Before Delete" := this.Count(RecordRef);
         TempRetenPolDeletingParam."Indirect Permission Required" := VerifyIndirectDeletePermission(RecordRef.Number);
         TempRetenPolDeletingParam."Skip Event Indirect Perm. Req." := not TempRetenPolDeletingParam."Indirect Permission Required";
         TempRetenPolDeletingParam."Max. Number of Rec. to Delete" := MaxNumberOfRecordsToDelete() - TotalNumberOfRecordsDeleted;
-        TempRetenPolDeletingParam."Skip Event Rec. Limit Exceeded" := TempRetenPolDeletingParam."Max. Number of Rec. to Delete" > Count(RecordRef);
+        TempRetenPolDeletingParam."Skip Event Rec. Limit Exceeded" := TempRetenPolDeletingParam."Max. Number of Rec. to Delete" > TempRetenPolDeletingParam."Record Count Before Delete";
         TempRetenPolDeletingParam."Total Max. Nr. of Rec. to Del." := MaxNumberOfRecordsToDelete();
         TempRetenPolDeletingParam."User Invoked Run" := IsUserInvokedRun;
     end;
 
-    local procedure RaiseRecordLimitExceededEvent(var RecordRefDuplicate: RecordRef)
+    local procedure RaiseRecordLimitExceededEvent(var RecordRefDuplicate: RecordRef; RecordCountAfter: Integer)
     var
         RetentionPolicyLog: Codeunit "Retention Policy Log";
         ApplyRetentionPolicyFacade: Codeunit "Apply Retention Policy";
         Handled: Boolean;
     begin
         EndCurrentRun := true;
         RetentionPolicyLog.LogWarning(LogCategory(), EndCurrentRunLbl);
-        ApplyRetentionPolicyFacade.OnApplyRetentionPolicyRecordLimitExceeded(RecordRefDuplicate.Number, Count(RecordRefDuplicate), ApplyAllRetentionPolicies, IsUserInvokedRun, Handled);
+        ApplyRetentionPolicyFacade.OnApplyRetentionPolicyRecordLimitExceeded(RecordRefDuplicate.Number, RecordCountAfter, ApplyAllRetentionPolicies, IsUserInvokedRun, Handled);
         if IsUserInvokedRun and (not Handled) and GuiAllowed() then begin
             Commit();
             if Confirm(ConfirmRerunMsg, true, MaxNumberOfRecordsToDelete()) then
@@ -431,6 +432,11 @@ codeunit 3904 "Apply Retention Policy Impl."
         exit(10000)
     end;
 
+    internal procedure NumberOfRecordsToDeleteBuffer(): Integer
+    begin
+        exit(0)
+    end;
+
     local procedure Count(RecordRef: RecordRef): Integer;
     begin
         if RecordRef.ReadPermission() then

diff --git a/...pplication/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al b/...pplication/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al
@@ -14,6 +14,8 @@ codeunit 3916 "Reten. Pol. Delete. Impl." implements "Reten. Pol. Deleting"
     InherentPermissions = X;
 
     var
+        TooManyRecordsToDeleteLbl: Label 'Reached the maximum number of records that can be deleted at the same time. The maximum number allowed is %1.', Comment = '%1 = integer';
+        LimitNumberOfRecordsLbl: Label 'Limited the number of records to delete for table %1, %2 to %3 records. The maximum number of records that can be deleted at the same time is %4, and %5 records were previously deleted in one or more tables.', Comment = '%1 = a id of a table (integer), %2 = the caption of the table, %3, %4, %5 = integer';
         MissingReadPermissionLbl: Label 'Unable to check number of records to delete due to missing read permission for table %1, %2', Comment = '%1 = table number, %2 = table caption';
         MaxNumberofRecToDeleteNegLbl: Label 'Max. Number of Rec. To Delete is less than 0.';
 
@@ -31,14 +33,54 @@ codeunit 3916 "Reten. Pol. Delete. Impl." implements "Reten. Pol. Deleting"
         RecordReference.Initialize(RecordRef, RecordReferenceIndirectPermission);
 
         if not RecordReferenceIndirectPermission.ReadPermission(RecordRef) then
-            RetentionPolicyLog.LogWarning(LogCategory(), StrSubstNo(MissingReadPermissionLbl, RecordRef.Number, RecordRef.Caption));
+            RetentionPolicyLog.LogWarning(LogCategory(), StrSubstNo(MissingReadPermissionLbl, RecordRef.Number, RecordRef.Caption))
+        else
+            if not RecordReferenceIndirectPermission.IsEmpty(RecordRef) then
+                if (RetenPolDeletingParam."Record Count Before Delete" > (RetenPolDeletingParam."Max. Number of Rec. To Delete" + NumberOfRecordsToDeleteBuffer())) then begin
+                    RetentionPolicyLog.LogWarning(LogCategory(), StrSubstNo(TooManyRecordsToDeleteLbl, RetenPolDeletingParam."Total Max. Nr. of Rec. to Del."));
+                    LimitRecordsToBeDeleted(RecordRef, RecordReferenceIndirectPermission, RetenPolDeletingParam."Skip Event Rec. Limit Exceeded", RetenPolDeletingParam."Max. Number of Rec. To Delete", RetenPolDeletingParam."Total Max. Nr. of Rec. to Del.");
+                end;
 
         if not RetenPolDeletingParam."Indirect Permission Required" then
             RecordReferenceIndirectPermission.DeleteAll(RecordRef, true);
         RetenPolDeletingParam."Skip Event Indirect Perm. Req." := not RetenPolDeletingParam."Indirect Permission Required";
         RecordRef.Close();
     end;
 
+    local procedure LimitRecordsToBeDeleted(var RecordRef: RecordRef; RecordReferenceIndirectPermission: Interface "Record Reference"; var SkipOnApplyRetentionPolicyRecordLimitExceeded: Boolean; MaxNumberOfRecordsToDelete: Integer; TotalMaxNumberOfRecordsToDelete: Integer)
+    var
+        RetentionPolicyLog: Codeunit "Retention Policy Log";
+    begin
+        RetentionPolicyLog.LogInfo(LogCategory(), StrSubstNo(LimitNumberOfRecordsLbl, RecordRef.Number, RecordRef.Caption, MaxNumberOfRecordsToDelete, TotalMaxNumberOfRecordsToDelete, TotalMaxNumberOfRecordsToDelete - MaxNumberOfRecordsToDelete));
+        FilterRecordsToLimit(RecordRef, RecordReferenceIndirectPermission, MaxNumberOfRecordsToDelete);
+        SkipOnApplyRetentionPolicyRecordLimitExceeded := false;
+    end;
+
+    local procedure FilterRecordsToLimit(var RecordRef: RecordRef; RecordReferenceIndirectPermission: Interface "Record Reference"; StartRecordIndex: Integer)
+    var
+        FieldRef: FieldRef;
+        KeyRef: KeyRef;
+        i: integer;
+    begin
+        RecordReferenceIndirectPermission.FindSet(RecordRef);
+        RecordReferenceIndirectPermission.Next(RecordRef, StartRecordIndex);
+
+        RecordRef.FilterGroup := 15;
+        KeyRef := RecordRef.KeyIndex(RecordRef.CurrentKeyIndex());
+        for i := 1 to KeyRef.FieldCount() do begin
+            FieldRef := KeyRef.FieldIndex(i);
+            FieldRef.SetFilter('<%1', FieldRef.Value);
+        end;
+        RecordRef.FilterGroup := 0;
+    end;
+
+    local procedure NumberOfRecordsToDeleteBuffer(): Integer
+    var
+        ApplyRetentionPolicyImpl: Codeunit "Apply Retention Policy Impl.";
+    begin
+        exit(ApplyRetentionPolicyImpl.NumberOfRecordsToDeleteBuffer())
+    end;
+
     local procedure LogCategory(): Enum "Retention Policy Log Category"
     var
         RetentionPolicyLogCategory: Enum "Retention Policy Log Category";

diff --git a/...pplication/App/Retention Policy/src/Apply Retention Policy/RetenPolDeletingParam.Table.al b/...pplication/App/Retention Policy/src/Apply Retention Policy/RetenPolDeletingParam.Table.al
@@ -84,6 +84,14 @@ table 3907 "Reten. Pol. Deleting Param"
         {
             DataClassification = SystemMetadata;
         }
+        /// <summary>
+        /// The number of records in the table before the deletion.
+        /// The count is calculated once before passing this table to the deletion implementation and is used to limit the number of records to be deleted as well as record the number of records actually deleted.
+        /// </summary>
+        field(8; "Record Count Before Delete"; Integer)
+        {
+            DataClassification = SystemMetadata;
+        }
     }
 
     keys

diff --git a/...ication/App/Retention Policy/src/Apply Retention Policy/RetenPolFilteringImpl.Codeunit.al b/...ication/App/Retention Policy/src/Apply Retention Policy/RetenPolFilteringImpl.Codeunit.al
@@ -275,13 +275,18 @@ codeunit 3915 "Reten. Pol. Filtering Impl." implements "Reten. Pol. Filtering"
 
     // Keep track of the result set of expired records in a hash set (in addition to marks on the RecordRef) for performance reasons (i. e. to have efficient Count() calls).
     local procedure SetSelectedSystemId(SelectedSystemId: Guid; IsSelected: Boolean; var SelectedSystemIds: Dictionary of [Guid, Boolean])
+    var
+        ContainsKey: Boolean;
     begin
-        if IsSelected then
-            if not SelectedSystemIds.ContainsKey(SelectedSystemId) then
-                SelectedSystemIds.Add(SelectedSystemId, true)
-            else
-                if SelectedSystemIds.ContainsKey(SelectedSystemId) then
-                    SelectedSystemIds.Remove(SelectedSystemId)
+        ContainsKey := SelectedSystemIds.ContainsKey(SelectedSystemId);
+        case true of
+            // Selected, not yet added -> add
+            IsSelected and (not ContainsKey):
+                SelectedSystemIds.Add(SelectedSystemId, true);
+            // Not selected, already added -> remove
+            (not IsSelected) and ContainsKey:
+                SelectedSystemIds.Remove(SelectedSystemId);
+        end;
     end;
 
     local procedure SetRetentionPolicyLineTableFilter(TableFilterView: Text; var RecordRef: RecordRef; FilterGroup: Integer);