Fixes routing issues (#17572)

* Child of second root should also hide root path and backoffice needs to show all domains for each language.

* Fixes routing issues based on findings #17572 (comment)

* Revert "Fixes routing issues based on findings #17572 (comment)"

This reverts commit ba7fb5c.

* Fix urls of descendants of non-first roots do not show the root urlsegment when HideTopLevel is true

Author: bergmania

src/Umbraco.Core/Services/DocumentUrlService.cs

@@ -393,7 +393,19 @@ public async Task DeleteUrlsFromCacheAsync(IEnumerable<Guid> documentKeysEnumera
             return GetTopMostRootKey(isDraft, culture);
         }
 
-        // Otherwise we have to find the root items (or child of the first root when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key
+        // Special case for all top level nodes except the first (that will have /)
+        if (runnerKey is null && urlSegments.Length == 1 && hideTopLevelNodeFromPath is true)
+        {
+            IEnumerable<Guid> rootKeys = GetKeysInRoot(false, isDraft, culture);
+            Guid? rootKeyWithUrlSegment = GetChildWithUrlSegment(rootKeys, urlSegments.First(), culture, isDraft);
+
+            if (rootKeyWithUrlSegment is not null)
+            {
+                return rootKeyWithUrlSegment;
+            }
+        }
+
+        // Otherwise we have to find the root items (or child of the roots when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key
         for (var index = 0; index < urlSegments.Length; index++)
         {
             var urlSegment = urlSegments[index];
@@ -447,7 +459,7 @@ public string GetLegacyRouteFormat(Guid documentKey, string? culture, bool isDra
         var cultureOrDefault = string.IsNullOrWhiteSpace(culture) is false ? culture : _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult();
 
         Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray();
-        IDictionary<Guid, Domain?> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, ancestorKey =>
+        ILookup<Guid, Domain?> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToLookup(x => x, ancestorKey =>
         {
             Attempt<int> idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document);
 
@@ -466,13 +478,11 @@ public string GetLegacyRouteFormat(Guid documentKey, string? culture, bool isDra
 
         foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
         {
-            if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Domain? domain))
+            var domains = ancestorOrSelfKeyToDomains[ancestorOrSelfKey].WhereNotNull();
+            if (domains.Any())
             {
-                if (domain is not null)
-                {
-                    foundDomain = domain;
-                    break;
-                }
+                foundDomain = domains.First();// What todo here that is better?
+                break;
             }
 
             if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, cultureOrDefault, isDraft), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
@@ -528,7 +538,7 @@ public async Task<IEnumerable<UrlInfo>> ListUrlsAsync(Guid contentKey)
         var cultures = languages.ToDictionary(x=>x.IsoCode);
 
         Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray();
-        Dictionary<Guid, Task<Dictionary<string, Domain>>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey =>
+        Dictionary<Guid, Task<ILookup<string, Domain>>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey =>
         {
             Attempt<int> idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document);
 
@@ -537,70 +547,119 @@ public async Task<IEnumerable<UrlInfo>> ListUrlsAsync(Guid contentKey)
                 return null;
             }
             IEnumerable<Domain> domains = _domainCacheService.GetAssigned(idAttempt.Result, false);
-            return domains.ToDictionary(x => x.Culture!);
+            return domains.ToLookup(x => x.Culture!);
         })!;
 
         foreach ((string culture, ILanguage language) in cultures)
         {
-           var urlSegments = new List<string>();
-           Domain? foundDomain = null;
+            var urlSegments = new List<string>();
+            List<Domain?> foundDomains = new List<Domain?>();
 
-           var hasUrlInCulture = true;
-           foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
-           {
-                if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Task<Dictionary<string, Domain>>? domainDictionaryTask))
+            var hasUrlInCulture = true;
+            foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
+            {
+                var domainLookup = await ancestorOrSelfKeyToDomains[ancestorOrSelfKey];
+                if (domainLookup.Any())
                 {
-                    Dictionary<string, Domain> domainDictionary = await domainDictionaryTask;
-                    if (domainDictionary.TryGetValue(culture, out Domain? domain))
+                    var domains = domainLookup[culture];
+                    foreach (Domain domain in domains)
                     {
-                        Attempt<Guid> domainKeyAttempt = _idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document);
+                        Attempt<Guid> domainKeyAttempt =
+                            _idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document);
                         if (domainKeyAttempt.Success)
                         {
                             if (_publishStatusQueryService.IsDocumentPublished(domainKeyAttempt.Result, culture))
                             {
-                                foundDomain = domain;
-                                break;
+                                foundDomains.Add(domain);
                             }
                         }
                     }
+
+                    if (foundDomains.Any())
+                    {
+                        break;
+                    }
                 }
 
-                if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, culture, false), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
+                if (_cache.TryGetValue(
+                        CreateCacheKey(ancestorOrSelfKey, culture, false),
+                        out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
                 {
                     urlSegments.Add(publishedDocumentUrlSegment.UrlSegment);
                 }
                 else
                 {
                     hasUrlInCulture = false;
                 }
-           }
+            }
+
+            //If we did not find a domain and this is not the default language, then the content is not routable
+            if (foundDomains.Any() is false && language.IsDefault is false)
+            {
+                continue;
+            }
 
-           //If we did not find a domain and this is not the default language, then the content is not routable
-           if (foundDomain is null && language.IsDefault is false)
-           {
-               continue;
-           }
 
-           var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last();
+            var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last();
+
+            var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight
+                              || CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false;
+            if (leftToRight)
+            {
+                urlSegments.Reverse();
+            }
+
+            // If no domain was found, we need to add a null domain to the list to make sure we check for no domains.
+            if (foundDomains.Any() is false)
+            {
+                foundDomains.Add(null);
+            }
+
+            foreach (Domain? foundDomain in foundDomains)
+            {
+                var foundUrl = GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight);
+
+                if (foundDomain is not null)
+                {
+                    // if we found a domain, it should be safe to show url
+                    result.Add(new UrlInfo(
+                        text: foundUrl,
+                        isUrl: hasUrlInCulture,
+                        culture: culture
+                    ));
+                }
+                else
+                {
+                    // otherwise we need to ensure that no other page has the same url
+                    // e.g. a site with two roots that both have a child with the same name
+                    Guid? documentKeyByRoute = GetDocumentKeyByRoute(foundUrl, culture, foundDomain?.ContentId, false);
+                    if (contentKey.Equals(documentKeyByRoute))
+                    {
+                        result.Add(new UrlInfo(
+                            text: foundUrl,
+                            isUrl: hasUrlInCulture,
+                            culture: culture
+                        ));
+                    }
+                    else
+                    {
+                        result.Add(new UrlInfo(
+                            text: "Conflict: Other page has the same url",
+                            isUrl: false,
+                            culture: culture
+                        ));
+                    }
+                }
 
-           var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight
-                             || CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false;
-           if (leftToRight)
-           {
-               urlSegments.Reverse();
-           }
 
-           result.Add(new UrlInfo(
-               text: GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight),
-               isUrl: hasUrlInCulture,
-               culture: culture
-           ));
 
+            }
         }
 
         return result;
     }
 
+
     private string GetFullUrl(bool isRootFirstItem, List<string> segments, Domain? foundDomain, bool leftToRight)
     {
         var urlSegments = new List<string>(segments);
@@ -610,7 +669,7 @@ private string GetFullUrl(bool isRootFirstItem, List<string> segments, Domain? f
             return foundDomain.Name.EnsureEndsWith("/") + string.Join('/', urlSegments);
         }
 
-        var hideTopLevel = _globalSettings.HideTopLevelNodeFromPath && isRootFirstItem;
+        var hideTopLevel = HideTopLevel(_globalSettings.HideTopLevelNodeFromPath, isRootFirstItem, urlSegments);
         if (leftToRight)
         {
             return '/' + string.Join('/', urlSegments.Skip(hideTopLevel ? 1 : 0));
@@ -624,6 +683,21 @@ private string GetFullUrl(bool isRootFirstItem, List<string> segments, Domain? f
         return '/' + string.Join('/', urlSegments);
     }
 
+    private bool HideTopLevel(bool hideTopLevelNodeFromPath, bool isRootFirstItem, List<string> urlSegments)
+    {
+        if (hideTopLevelNodeFromPath is false)
+        {
+            return false;
+        }
+
+        if(isRootFirstItem is false && urlSegments.Count == 1)
+        {
+            return false;
+        }
+
+        return true;
+    }
+
     public async Task CreateOrUpdateUrlSegmentsWithDescendantsAsync(Guid key)
     {
         var id = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Document).Result;
@@ -646,7 +720,7 @@ public async Task CreateOrUpdateUrlSegmentsAsync(Guid key)
         }
     }
 
-    private IEnumerable<Guid> GetKeysInRoot(bool addFirstLevelChildren, bool isDraft, string culture)
+    private IEnumerable<Guid> GetKeysInRoot(bool considerFirstLevelAsRoot, bool isDraft, string culture)
     {
         if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeysEnumerable) is false)
         {
@@ -655,13 +729,10 @@ private IEnumerable<Guid> GetKeysInRoot(bool addFirstLevelChildren, bool isDraft
 
         IEnumerable<Guid> rootKeys = rootKeysEnumerable as Guid[] ?? rootKeysEnumerable.ToArray();
 
-        foreach (Guid rootKey in rootKeys)
+        if (considerFirstLevelAsRoot)
         {
-            yield return rootKey;
-        }
+            yield return rootKeys.First();
 
-        if (addFirstLevelChildren)
-        {
             foreach (Guid rootKey in rootKeys)
             {
                 if (isDraft is false && IsContentPublished(rootKey, culture) is false)
@@ -677,6 +748,13 @@ private IEnumerable<Guid> GetKeysInRoot(bool addFirstLevelChildren, bool isDraft
                 }
             }
         }
+        else
+        {
+            foreach (Guid rootKey in rootKeys)
+            {
+                yield return rootKey;
+            }
+        }
 
     }
 
@@ -710,23 +788,28 @@ private IEnumerable<Guid> GetChildKeys(Guid documentKey)
         return Enumerable.Empty<Guid>();
     }
 
-    /// <summary>
-    /// Gets the top most root key.
-    /// </summary>
-    /// <returns>The top most root key.</returns>
-    private Guid? GetTopMostRootKey(bool isDraft, string culture)
+    private IEnumerable<Guid> GetRootKeys(bool isDraft, string culture)
     {
         if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys))
         {
             foreach (Guid rootKey in rootKeys)
             {
                 if (isDraft || IsContentPublished(rootKey, culture))
                 {
-                    return rootKey;
+                    yield return rootKey;
                 }
             }
         }
-        return null;
+    }
+
+
+    /// <summary>
+    /// Gets the top most root key.
+    /// </summary>
+    /// <returns>The top most root key.</returns>
+    private Guid? GetTopMostRootKey(bool isDraft, string culture)
+    {
+        return GetRootKeys(isDraft, culture).Cast<Guid?>().FirstOrDefault();
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]

src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts

@@ -22,7 +22,7 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
 	private _variantOptions?: Array<UmbDocumentVariantOptionModel>;
 
 	@state()
-	private _lookup: Record<string, string> = {};
+	private _lookup: Record<string, string[]> = {};
 
 	constructor() {
 		super();
@@ -56,7 +56,10 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
 		if (data?.length) {
 			data[0].urls.forEach((item) => {
 				if (item.culture && item.url) {
-					this._lookup[item.culture] = item.url;
+					if(this._lookup[item.culture] == null){
+						this._lookup[item.culture] = [];
+					}
+					this._lookup[item.culture].push(item.url);
 				}
 			});
 			this.requestUpdate('_lookup');
@@ -108,18 +111,20 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
 	#renderUrl(variantOption: UmbDocumentVariantOptionModel) {
 		const varies = !!variantOption.culture;
 		const culture = varies ? variantOption.culture! : variantOption.language.unique;
-		const url = this._lookup[culture];
+		const urls = this._lookup[culture];
 		return when(
-			url,
+			urls && urls.length >= 1,
 			() => html`
-				<a class="link-item" href=${url} target="_blank">
-					<span>
-						<span class="culture">${varies ? culture : nothing}</span>
-						<span>${url}</span>
-					</span>
-					<uui-icon name="icon-out"></uui-icon>
-				</a>
-			`,
+				${urls.map((url) =>
+					html`
+						<a class="link-item" href=${url} target="_blank">
+							<span>
+								<span class="culture">${varies ? culture : nothing}</span>
+								<span>${url}</span>
+							</span>
+							<uui-icon name="icon-out"></uui-icon>
+						</a>`
+				)}`,
 			() => html`
 				<div class="link-item">
 					<span>