From 836740551167c10722e62ad9676f647632de6b89 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 21 Mar 2025 19:07:49 +0100 Subject: [PATCH 1/5] Clear roots before rebuilding navigation dictionary. --- .../Services/Navigation/ContentNavigationServiceBase.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs b/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs index 531b3952a790..d7562a76b95e 100644 --- a/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs +++ b/src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs @@ -293,12 +293,12 @@ public bool RestoreFromBin(Guid key, Guid? targetParentKey = null) /// The read lock value, should be -333 or -334 for content and media trees. /// The key of the object type to rebuild. /// Indicates whether the items are in the recycle bin. - protected async Task HandleRebuildAsync(int readLock, Guid objectTypeKey, bool trashed) + protected Task HandleRebuildAsync(int readLock, Guid objectTypeKey, bool trashed) { // This is only relevant for items in the content and media trees if (readLock != Constants.Locks.ContentTree && readLock != Constants.Locks.MediaTree) { - return; + return Task.CompletedTask; } using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true); @@ -307,14 +307,18 @@ protected async Task HandleRebuildAsync(int readLock, Guid objectTypeKey, bool t // Build the corresponding navigation structure if (trashed) { + _recycleBinRoots.Clear(); IEnumerable navigationModels = _navigationRepository.GetTrashedContentNodesByObjectType(objectTypeKey); BuildNavigationDictionary(_recycleBinNavigationStructure, _recycleBinRoots, navigationModels); } else { + _roots.Clear(); IEnumerable navigationModels = _navigationRepository.GetContentNodesByObjectType(objectTypeKey); BuildNavigationDictionary(_navigationStructure, _roots, navigationModels); } + + return Task.CompletedTask; } private bool TryGetParentKeyFromStructure(ConcurrentDictionary structure, Guid childKey, out Guid? parentKey) From 6e68af805d43178934dd313570fe424c27fc0634 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 22 Mar 2025 10:00:37 +0100 Subject: [PATCH 2/5] Added tests to verify fix. --- .../DocumentNavigationServiceTests.Rebuild.cs | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs index 52f9bb77bd07..b4bac19e8fe6 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs @@ -10,7 +10,18 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; public partial class DocumentNavigationServiceTests { [Test] - public async Task Structure_Can_Rebuild() + public async Task Structure_Can_Rebuild() => await Perform_Structure_Can_Rebuild(); + + [Test] + public async Task Structure_Can_Rebuild_Multiple_Times() + { + for (int i = 0; i < 2; i++) + { + await Perform_Structure_Can_Rebuild(); + } + } + + private async Task Perform_Structure_Can_Rebuild() { // Arrange Guid nodeKey = Root.Key; @@ -21,6 +32,7 @@ public async Task Structure_Can_Rebuild() DocumentNavigationQueryService.TryGetDescendantsKeys(nodeKey, out IEnumerable originalDescendantsKeys); DocumentNavigationQueryService.TryGetAncestorsKeys(nodeKey, out IEnumerable originalAncestorsKeys); DocumentNavigationQueryService.TryGetSiblingsKeys(nodeKey, out IEnumerable originalSiblingsKeys); + DocumentNavigationQueryService.TryGetRootKeys(out IEnumerable originalRouteKeys); // In-memory navigation structure is empty here var newDocumentNavigationService = new DocumentNavigationService( @@ -31,6 +43,7 @@ public async Task Structure_Can_Rebuild() // Act await newDocumentNavigationService.RebuildAsync(); + await newDocumentNavigationService.RebuildAsync(); // Capture rebuilt state var nodeExists = newDocumentNavigationService.TryGetParentKey(nodeKey, out Guid? parentKeyFromRebuild); @@ -38,6 +51,7 @@ public async Task Structure_Can_Rebuild() newDocumentNavigationService.TryGetDescendantsKeys(nodeKey, out IEnumerable descendantsKeysFromRebuild); newDocumentNavigationService.TryGetAncestorsKeys(nodeKey, out IEnumerable ancestorsKeysFromRebuild); newDocumentNavigationService.TryGetSiblingsKeys(nodeKey, out IEnumerable siblingsKeysFromRebuild); + newDocumentNavigationService.TryGetRootKeys(out IEnumerable routeKeysFromRebuild); // Assert Assert.Multiple(() => @@ -53,11 +67,23 @@ public async Task Structure_Can_Rebuild() CollectionAssert.AreEquivalent(originalDescendantsKeys, descendantsKeysFromRebuild); CollectionAssert.AreEquivalent(originalAncestorsKeys, ancestorsKeysFromRebuild); CollectionAssert.AreEquivalent(originalSiblingsKeys, siblingsKeysFromRebuild); + CollectionAssert.AreEquivalent(originalRouteKeys, routeKeysFromRebuild); }); } [Test] - public async Task Bin_Structure_Can_Rebuild() + public async Task Bin_Structure_Can_Rebuild() => await Perform_Bin_Structure_Can_Rebuild(); + + [Test] + public async Task Bin_Structure_Can_Rebuild_Multiple_Times() + { + for (int i = 0; i < 2; i++) + { + await Perform_Bin_Structure_Can_Rebuild(); + } + } + + private async Task Perform_Bin_Structure_Can_Rebuild() { // Arrange Guid nodeKey = Root.Key; @@ -69,6 +95,7 @@ public async Task Bin_Structure_Can_Rebuild() DocumentNavigationQueryService.TryGetDescendantsKeysInBin(nodeKey, out IEnumerable originalDescendantsKeys); DocumentNavigationQueryService.TryGetAncestorsKeysInBin(nodeKey, out IEnumerable originalAncestorsKeys); DocumentNavigationQueryService.TryGetSiblingsKeysInBin(nodeKey, out IEnumerable originalSiblingsKeys); + DocumentNavigationQueryService.TryGetRootKeys(out IEnumerable originalRouteKeys); // In-memory navigation structure is empty here var newDocumentNavigationService = new DocumentNavigationService( @@ -86,6 +113,7 @@ public async Task Bin_Structure_Can_Rebuild() newDocumentNavigationService.TryGetDescendantsKeysInBin(nodeKey, out IEnumerable descendantsKeysFromRebuild); newDocumentNavigationService.TryGetAncestorsKeysInBin(nodeKey, out IEnumerable ancestorsKeysFromRebuild); newDocumentNavigationService.TryGetSiblingsKeysInBin(nodeKey, out IEnumerable siblingsKeysFromRebuild); + newDocumentNavigationService.TryGetRootKeys(out IEnumerable routeKeysFromRebuild); // Assert Assert.Multiple(() => @@ -101,6 +129,7 @@ public async Task Bin_Structure_Can_Rebuild() CollectionAssert.AreEquivalent(originalDescendantsKeys, descendantsKeysFromRebuild); CollectionAssert.AreEquivalent(originalAncestorsKeys, ancestorsKeysFromRebuild); CollectionAssert.AreEquivalent(originalSiblingsKeys, siblingsKeysFromRebuild); + CollectionAssert.AreEquivalent(originalRouteKeys, routeKeysFromRebuild); }); } } From 3679385d1b6a7cf88c6e81f904e3ef9697e6fc45 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 22 Mar 2025 11:50:35 +0100 Subject: [PATCH 3/5] Correct test implementation. --- .../DocumentNavigationServiceTests.Rebuild.cs | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs index b4bac19e8fe6..5afc4badeb61 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs @@ -13,15 +13,9 @@ public partial class DocumentNavigationServiceTests public async Task Structure_Can_Rebuild() => await Perform_Structure_Can_Rebuild(); [Test] - public async Task Structure_Can_Rebuild_Multiple_Times() - { - for (int i = 0; i < 2; i++) - { - await Perform_Structure_Can_Rebuild(); - } - } + public async Task Structure_Can_Rebuild_Multiple_Times() => await Perform_Structure_Can_Rebuild(2); - private async Task Perform_Structure_Can_Rebuild() + private async Task Perform_Structure_Can_Rebuild(int numberOfRebuilds = 1) { // Arrange Guid nodeKey = Root.Key; @@ -42,8 +36,10 @@ private async Task Perform_Structure_Can_Rebuild() var initialNodeExists = newDocumentNavigationService.TryGetParentKey(nodeKey, out _); // Act - await newDocumentNavigationService.RebuildAsync(); - await newDocumentNavigationService.RebuildAsync(); + for (int i = 0; i < numberOfRebuilds; i++) + { + await newDocumentNavigationService.RebuildAsync(); + } // Capture rebuilt state var nodeExists = newDocumentNavigationService.TryGetParentKey(nodeKey, out Guid? parentKeyFromRebuild); @@ -75,15 +71,9 @@ private async Task Perform_Structure_Can_Rebuild() public async Task Bin_Structure_Can_Rebuild() => await Perform_Bin_Structure_Can_Rebuild(); [Test] - public async Task Bin_Structure_Can_Rebuild_Multiple_Times() - { - for (int i = 0; i < 2; i++) - { - await Perform_Bin_Structure_Can_Rebuild(); - } - } + public async Task Bin_Structure_Can_Rebuild_Multiple_Times() => await Perform_Bin_Structure_Can_Rebuild(2); - private async Task Perform_Bin_Structure_Can_Rebuild() + private async Task Perform_Bin_Structure_Can_Rebuild(int numberOfRebuilds = 1) { // Arrange Guid nodeKey = Root.Key; @@ -105,7 +95,10 @@ private async Task Perform_Bin_Structure_Can_Rebuild() var initialNodeExists = newDocumentNavigationService.TryGetParentKeyInBin(nodeKey, out _); // Act - await newDocumentNavigationService.RebuildBinAsync(); + for (int i = 0; i < numberOfRebuilds; i++) + { + await newDocumentNavigationService.RebuildBinAsync(); + } // Capture rebuilt state var nodeExists = newDocumentNavigationService.TryGetParentKeyInBin(nodeKey, out Guid? parentKeyFromRebuild); From 0f1149f8a4b03eb5a4572f13f2162a76c97c259a Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 4 Apr 2025 06:38:29 +0200 Subject: [PATCH 4/5] Convert integration tests with method overloads into test cases. --- .../DocumentNavigationServiceTests.Rebuild.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs index 5afc4badeb61..03891520748f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs @@ -9,13 +9,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; public partial class DocumentNavigationServiceTests { - [Test] - public async Task Structure_Can_Rebuild() => await Perform_Structure_Can_Rebuild(); - - [Test] - public async Task Structure_Can_Rebuild_Multiple_Times() => await Perform_Structure_Can_Rebuild(2); - - private async Task Perform_Structure_Can_Rebuild(int numberOfRebuilds = 1) + [TestCase(1, TestName = "Structure_Can_Rebuild")] + [TestCase(2, TestName = "Structure_Can_Rebuild_MultipleTimes")] + public async Task Structure_Can_Rebuild(int numberOfRebuilds) { // Arrange Guid nodeKey = Root.Key; @@ -67,13 +63,9 @@ private async Task Perform_Structure_Can_Rebuild(int numberOfRebuilds = 1) }); } - [Test] - public async Task Bin_Structure_Can_Rebuild() => await Perform_Bin_Structure_Can_Rebuild(); - - [Test] - public async Task Bin_Structure_Can_Rebuild_Multiple_Times() => await Perform_Bin_Structure_Can_Rebuild(2); - - private async Task Perform_Bin_Structure_Can_Rebuild(int numberOfRebuilds = 1) + [TestCase(1, TestName = "Bin_Structure_Can_Rebuild")] + [TestCase(2, TestName = "Bin_Structure_Can_Rebuild_MultipleTimes")] + public async Task Bin_Structure_Can_Rebuild(int numberOfRebuilds) { // Arrange Guid nodeKey = Root.Key; From 778502c5c929941dc2b9150d2cc9b792031b73ce Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 4 Apr 2025 07:52:45 +0200 Subject: [PATCH 5/5] Integration test compatibility supressions. --- .../CompatibilitySuppressions.xml | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml index 880504a1d43a..3b12394aaa35 100644 --- a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml +++ b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml @@ -78,6 +78,20 @@ lib/net9.0/Umbraco.Tests.Integration.dll true + + CP0002 + M:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.DocumentNavigationServiceTests.Bin_Structure_Can_Rebuild + lib/net9.0/Umbraco.Tests.Integration.dll + lib/net9.0/Umbraco.Tests.Integration.dll + true + + + CP0002 + M:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.DocumentNavigationServiceTests.Structure_Can_Rebuild + lib/net9.0/Umbraco.Tests.Integration.dll + lib/net9.0/Umbraco.Tests.Integration.dll + true + CP0002 M:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.UserServiceCrudTests.Cannot_Request_Disabled_If_Hidden(Umbraco.Cms.Core.Models.Membership.UserState) @@ -94,21 +108,21 @@ CP0002 - M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsApproved_Without_Access + M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.EntityServiceTests.CreateTestData lib/net9.0/Umbraco.Tests.Integration.dll lib/net9.0/Umbraco.Tests.Integration.dll true CP0002 - M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsLockedOut_Without_Access + M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsApproved_Without_Access lib/net9.0/Umbraco.Tests.Integration.dll lib/net9.0/Umbraco.Tests.Integration.dll true CP0002 - M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.EntityServiceTests.CreateTestData + M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.MemberEditingServiceTests.Cannot_Change_IsLockedOut_Without_Access lib/net9.0/Umbraco.Tests.Integration.dll lib/net9.0/Umbraco.Tests.Integration.dll true