Skip to content

Commit 5d7b789

Browse files
authored
Fix media cleanup runtime error (#3277)
<!-- Thank you for submitting a Pull Request. If you're new to contributing to BCApps please read our pull request guideline below * https://github.com/microsoft/BCApps/Contributing.md --> #### Summary <!-- Provide a general summary of your changes --> Fix media cleanup runtime error Update SplitListIntoSubLists procedure to get consistent result Tests for edge cases for SplitListIntoSubLists and Media Cleanup #### Work Item(s) <!-- Add the issue number here after the #. The issue needs to be open and approved. Submitting PRs with no linked issues or unapproved issues is highly discouraged. --> Fixes #3271 Fixes [AB#571724](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/571724)
1 parent a876522 commit 5d7b789

File tree

3 files changed

+248
-17
lines changed

3 files changed

+248
-17
lines changed

src/System Application/App/Data Administration/app.json

+4-5
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@
1212
"logo": "",
1313
"screenshots": [],
1414
"platform": "27.0.0.0",
15-
"dependencies": [
15+
"internalsVisibleTo": [
1616
{
17-
"id": "fd3d0b8e-61fa-4f87-b2fc-c4a1f3e53a63",
18-
"name": "Math",
19-
"publisher": "Microsoft",
20-
"version": "27.0.0.0"
17+
"id": "6d4dab82-dc5c-4c77-8e1e-d9aaeb7d0420",
18+
"name": "Data Administration Test",
19+
"publisher": "Microsoft"
2120
}
2221
],
2322
"idRanges": [

src/System Application/App/Data Administration/src/MediaCleanupImpl.Codeunit.al

+15-12
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
namespace System.DataAdministration;
77

8-
using System.Utilities;
98
using System.Environment;
109

1110
codeunit 1928 "Media Cleanup Impl."
@@ -245,20 +244,24 @@ codeunit 1928 "Media Cleanup Impl."
245244
end;
246245

247246
// 322, 100 will result in [[1, 100], [101, 200], [201, 300], [301, 322]]
248-
local procedure SplitListIntoSubLists(var InputList: List of [Guid]; SubListCount: Integer; var SplitList: List of [List of [Guid]])
247+
procedure SplitListIntoSubLists(var InputList: List of [Guid]; SubListCount: Integer; var SplitList: List of [List of [Guid]])
249248
var
250-
Math: Codeunit Math;
251249
ListNumber: Integer;
252-
SubList: List of [Guid];
253-
From: Integer;
254-
ToInt: Integer;
250+
FromIndex: Integer;
251+
SubSize: Integer;
252+
NumberOfBatches: Integer;
253+
TempSubList: List of [Guid];
255254
begin
256-
for ListNumber := 0 to Round(InputList.Count() / SubListCount, 1) do begin
257-
Clear(SubList);
258-
From := ListNumber * SubListCount + 1;
259-
ToInt := Math.Min(SubListCount, InputList.Count() - ListNumber * SubListCount);
260-
SubList := InputList.GetRange(From, ToInt);
261-
SplitList.Add(SubList);
255+
NumberOfBatches := (InputList.Count() + SubListCount - 1) div SubListCount;
256+
257+
for ListNumber := 1 to NumberOfBatches do begin
258+
Clear(TempSubList);
259+
FromIndex := (ListNumber - 1) * SubListCount + 1;
260+
SubSize := InputList.Count() - (FromIndex - 1);
261+
if SubSize > SubListCount then
262+
SubSize := SubListCount;
263+
TempSubList := InputList.GetRange(FromIndex, SubSize);
264+
SplitList.Add(TempSubList);
262265
end;
263266
end;
264267
}

src/System Application/Test/Data Administration/src/MediaCleanupTest.Codeunit.al

+229
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ codeunit 135018 "Media Cleanup Test"
2020
var
2121
LibraryAssert: Codeunit "Library Assert";
2222
PermissionsMock: Codeunit "Permissions Mock";
23+
MediaCleanupImpl: Codeunit "Media Cleanup Impl.";
2324
Any: Codeunit Any;
2425

2526
[Test]
@@ -119,6 +120,94 @@ codeunit 135018 "Media Cleanup Test"
119120
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
120121
end;
121122

123+
[Test]
124+
procedure EnsureDetachedMediaAndMediaSetAreCleanedUpSingleFullSubListPortion()
125+
var
126+
TempTenantMedia: Record "Tenant Media" temporary;
127+
MediaCleanup: Codeunit "Media Cleanup";
128+
begin
129+
PermissionsMock.ClearAssignments();
130+
CreateDetachedMediaThroughMediaSet(10, 100 * 1024); // 10 media of 100 kb
131+
CreateDetachedMedia(10, 100 * 1024); // 10 media of 100 kb
132+
PermissionsMock.Set('Data Cleanup - Admin');
133+
GetDetachedTenantMedia(TempTenantMedia);
134+
LibraryAssert.AreEqual(20, TempTenantMedia.Count(), 'Tenant media does not contain all detached media.');
135+
136+
MediaCleanup.DeleteDetachedTenantMediaSet();
137+
MediaCleanup.DeleteDetachedTenantMedia();
138+
GetDetachedTenantMedia(TempTenantMedia);
139+
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
140+
end;
141+
142+
[Test]
143+
procedure EnsureDetachedMediaAndMediaSetAreCleanedUpNotDivisibleSubListBigger()
144+
var
145+
TempTenantMedia: Record "Tenant Media" temporary;
146+
MediaCleanup: Codeunit "Media Cleanup";
147+
begin
148+
PermissionsMock.ClearAssignments();
149+
CreateDetachedMediaThroughMediaSet(101, 100 * 1024); // 101 media of 100 kb
150+
CreateDetachedMedia(101, 100 * 1024); // 101 media of 100 kb
151+
PermissionsMock.Set('Data Cleanup - Admin');
152+
GetDetachedTenantMedia(TempTenantMedia);
153+
LibraryAssert.AreEqual(202, TempTenantMedia.Count(), 'Tenant media does not contain all detached media.');
154+
155+
MediaCleanup.DeleteDetachedTenantMediaSet();
156+
MediaCleanup.DeleteDetachedTenantMedia();
157+
GetDetachedTenantMedia(TempTenantMedia);
158+
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
159+
end;
160+
161+
[Test]
162+
procedure EnsureDetachedMediaAndMediaSetAreCleanedUpNotDivisibleSubListLess()
163+
var
164+
TempTenantMedia: Record "Tenant Media" temporary;
165+
MediaCleanup: Codeunit "Media Cleanup";
166+
begin
167+
PermissionsMock.ClearAssignments();
168+
CreateDetachedMediaThroughMediaSet(99, 100 * 1024); // 99 media of 100 kb
169+
CreateDetachedMedia(99, 100 * 1024); // 99 media of 100 kb
170+
PermissionsMock.Set('Data Cleanup - Admin');
171+
GetDetachedTenantMedia(TempTenantMedia);
172+
LibraryAssert.AreEqual(198, TempTenantMedia.Count(), 'Tenant media does not contain all detached media.');
173+
174+
MediaCleanup.DeleteDetachedTenantMediaSet();
175+
MediaCleanup.DeleteDetachedTenantMedia();
176+
GetDetachedTenantMedia(TempTenantMedia);
177+
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
178+
end;
179+
180+
[Test]
181+
procedure EnsureDetachedMediaAndMediaSetAreCleanedUpLessThanBatch()
182+
var
183+
TempTenantMedia: Record "Tenant Media" temporary;
184+
MediaCleanup: Codeunit "Media Cleanup";
185+
begin
186+
PermissionsMock.ClearAssignments();
187+
CreateDetachedMediaThroughMediaSet(7, 100 * 1024); // 7 media of 100 kb
188+
CreateDetachedMedia(7, 100 * 1024); // 7 media of 100 kb
189+
PermissionsMock.Set('Data Cleanup - Admin');
190+
GetDetachedTenantMedia(TempTenantMedia);
191+
LibraryAssert.AreEqual(14, TempTenantMedia.Count(), 'Tenant media does not contain all detached media.');
192+
193+
MediaCleanup.DeleteDetachedTenantMediaSet();
194+
MediaCleanup.DeleteDetachedTenantMedia();
195+
GetDetachedTenantMedia(TempTenantMedia);
196+
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
197+
end;
198+
199+
[Test]
200+
procedure EnsureDetachedMediaAndMediaSetAreSuccessWithEmptyData()
201+
var
202+
TempTenantMedia: Record "Tenant Media" temporary;
203+
MediaCleanup: Codeunit "Media Cleanup";
204+
begin
205+
MediaCleanup.DeleteDetachedTenantMediaSet();
206+
MediaCleanup.DeleteDetachedTenantMedia();
207+
GetDetachedTenantMedia(TempTenantMedia);
208+
LibraryAssert.IsTrue(TempTenantMedia.IsEmpty(), 'Detached Tenant media was not cleaned up properly.');
209+
end;
210+
122211
[Test]
123212
procedure EnsureDetachedMediaAndMediaSetAreCleanedUpThroughCodeunit()
124213
var
@@ -199,6 +288,146 @@ codeunit 135018 "Media Cleanup Test"
199288
LibraryAssert.IsFalse(TenantMediaSetup.IsEmpty(), 'Normal tenant media set is also affected.');
200289
end;
201290

291+
[Test]
292+
procedure SplitListIntoSubLists322Into100CountSublists()
293+
var
294+
InputList: List of [Guid];
295+
ResultSublists: List of [List of [Guid]];
296+
i: Integer;
297+
begin
298+
// Basic Test: 322 items with SubListCount = 100
299+
// Expected outcome: 4 sublists (100, 100, 100, and 22 items respectively)
300+
for i := 1 to 322 do
301+
InputList.Add(Any.GuidValue());
302+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 100, ResultSublists);
303+
304+
// Verify the total number of sublists is 4
305+
LibraryAssert.AreEqual(4, ResultSublists.Count(), '322 items with batch size 100 should produce 4 sublists.');
306+
307+
// Verify the size of each sublist
308+
LibraryAssert.AreEqual(100, ResultSublists.Get(1).Count(), 'First sublist should contain 100 items.');
309+
LibraryAssert.AreEqual(100, ResultSublists.Get(2).Count(), 'Second sublist should contain 100 items.');
310+
LibraryAssert.AreEqual(100, ResultSublists.Get(3).Count(), 'Third sublist should contain 100 items.');
311+
LibraryAssert.AreEqual(22, ResultSublists.Get(4).Count(), 'Fourth sublist (remainder) should contain 22 items.');
312+
end;
313+
314+
[Test]
315+
procedure SplitListIntoSubLists350Into100CountSublists()
316+
var
317+
InputList: List of [Guid];
318+
ResultSublists: List of [List of [Guid]];
319+
i: Integer;
320+
begin
321+
// Basic Test: 350 items with SubListCount = 100
322+
// Expected outcome: 4 sublists (100, 100, 100, and 50 items respectively)
323+
for i := 1 to 350 do
324+
InputList.Add(Any.GuidValue());
325+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 100, ResultSublists);
326+
327+
// Verify the total number of sublists is 4
328+
LibraryAssert.AreEqual(4, ResultSublists.Count(), '350 items with batch size 100 should produce 4 sublists.');
329+
330+
// Verify the size of each sublist
331+
LibraryAssert.AreEqual(100, ResultSublists.Get(1).Count(), 'First sublist should contain 100 items.');
332+
LibraryAssert.AreEqual(100, ResultSublists.Get(2).Count(), 'Second sublist should contain 100 items.');
333+
LibraryAssert.AreEqual(100, ResultSublists.Get(3).Count(), 'Third sublist should contain 100 items.');
334+
LibraryAssert.AreEqual(50, ResultSublists.Get(4).Count(), 'Fourth sublist (remainder) should contain 50 items.');
335+
end;
336+
337+
[Test]
338+
procedure SplitListIntoSubListsReturnsEmptyResult()
339+
var
340+
InputList: List of [Guid];
341+
ResultSublists: List of [List of [Guid]];
342+
begin
343+
// Edge Case: Empty input list, SubListCount = 100
344+
// Expected outcome: result is an empty list of sublists (no sublists)
345+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 100, ResultSublists);
346+
347+
// Verify that result is empty (no sublists)
348+
LibraryAssert.AreEqual(0, ResultSublists.Count(), 'Empty input list should produce an empty result (0 sublists).');
349+
end;
350+
351+
[Test]
352+
procedure SplitListIntoSubListsFewerItemsThanBatchReturnsSingleSublist()
353+
var
354+
InputList: List of [Guid];
355+
ResultSublists: List of [List of [Guid]];
356+
i: Integer;
357+
begin
358+
// Edge Case: Input list has fewer items than SubListCount
359+
// Example: 50 items, SubListCount = 100
360+
// Expected outcome: a single sublist containing all 50 items (since no split needed)
361+
for i := 1 to 50 do
362+
InputList.Add(Any.GuidValue());
363+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 100, ResultSublists);
364+
365+
// Verify that there is exactly 1 sublist
366+
LibraryAssert.AreEqual(1, ResultSublists.Count(), '50 items with batch size 100 should produce 1 sublist (all items in one batch).');
367+
368+
// Verify that the single sublist contains all 50 items
369+
LibraryAssert.AreEqual(50, ResultSublists.Get(1).Count(), 'The single sublist should contain all 50 items.');
370+
end;
371+
372+
[Test]
373+
procedure SplitListIntoSubListsCreatesIndividualItemSublists()
374+
var
375+
InputList: List of [Guid];
376+
ResultSublists: List of [List of [Guid]];
377+
i: Integer;
378+
idx: Integer;
379+
begin
380+
// Edge Case: SubListCount = 1 (each item should be its own sublist)
381+
// Example: 5 items, SubListCount = 1 -> expect 5 sublists, each with 1 item
382+
for i := 1 to 5 do
383+
InputList.Add(Any.GuidValue());
384+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 1, ResultSublists);
385+
386+
// Verify that the number of sublists equals the number of items (5 sublists for 5 items)
387+
LibraryAssert.AreEqual(5, ResultSublists.Count(), '5 items with batch size 1 should produce 5 sublists.');
388+
389+
// Verify each sublist contains exactly one item
390+
for idx := 1 to ResultSublists.Count() do
391+
LibraryAssert.AreEqual(1, ResultSublists.Get(idx).Count(), 'Sublist should contain exactly 1 item.');
392+
end;
393+
394+
[Test]
395+
procedure SplitListIntoSubListsReturnsSingleFullSublist()
396+
var
397+
InputList: List of [Guid];
398+
ResultSublists: List of [List of [Guid]];
399+
i: Integer;
400+
begin
401+
// Edge Case: SubListCount equals the input list count
402+
// Example: 8 items, SubListCount = 8 -> expect a single sublist with all 8 items
403+
for i := 1 to 8 do
404+
InputList.Add(Any.GuidValue());
405+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 8, ResultSublists);
406+
407+
// Verify that there is exactly 1 sublist
408+
LibraryAssert.AreEqual(1, ResultSublists.Count(), '8 items with batch size 8 should produce 1 sublist.');
409+
410+
// Verify that the single sublist contains all 8 items
411+
LibraryAssert.AreEqual(8, ResultSublists.Get(1).Count(), 'The single sublist should contain all 8 items.');
412+
end;
413+
414+
[Test]
415+
procedure SplitListIntoSubListsOneItemBatchOfOne()
416+
var
417+
InputList: List of [Guid];
418+
ResultSublists: List of [List of [Guid]];
419+
begin
420+
// Boundary Case: Minimum input values (1 item, SubListCount = 1)
421+
// Expected outcome: 1 sublist containing the single item
422+
InputList.Add(Any.GuidValue());
423+
MediaCleanupImpl.SplitListIntoSubLists(InputList, 1, ResultSublists);
424+
425+
// Verify one sublist is created
426+
LibraryAssert.AreEqual(1, ResultSublists.Count(), '1 item with batch size 1 should result in 1 sublist.');
427+
428+
// Verify the sublist contains that one item
429+
LibraryAssert.AreEqual(1, ResultSublists.Get(1).Count(), 'The single sublist should contain the one item.');
430+
end;
202431

203432
local procedure GetDetachedTenantMedia(var TempTenantMedia: Record "Tenant Media" temporary)
204433
var

0 commit comments

Comments
 (0)