yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cleanup and deal with FIXMEs

Browse Source
This commit is contained in:
Stephan
2018-12-20 14:31:46 +01:00
parent 1ffd457985
commit e959605b39
3 changed files with 18 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
src/Umbraco.Web/Routing/DomainHelper.cs
View File

@@ -30,10 +30,10 @@ namespace Umbraco.Web.Routing
/// <param name="culture">The culture, or null.</param>
/// <returns>The domain and its uri, if any, that best matches the specified uri and culture, else null.</returns>
/// <remarks>
/// <para>f at least a domain is set on the node then the method returns the domain that
/// <para>If at least a domain is set on the node then the method returns the domain that
/// best matches the specified uri and culture, else it returns null.</para>
/// <para>If culture is null, uses the default culture for the installation instead.</para>
/// fixme not exactly - if culture is !null, we MUST have a value for THAT culture, else we use default as hint
/// <para>If culture is null, uses the default culture for the installation instead. Otherwise,
/// will try with the specified culture, else return null.</para>
/// </remarks>
internal DomainAndUri DomainForNode(int nodeId, Uri current, string culture = null)
{
@@ -49,13 +49,9 @@ namespace Umbraco.Web.Routing
return null;
// else filter
var domainAndUri = SelectDomain(domains, current, culture, _domainCache.DefaultCulture,
// it could be that none apply (due to culture)
return SelectDomain(domains, current, culture, _domainCache.DefaultCulture,
(cdomainAndUris, ccurrent, cculture, cdefaultCulture) => _siteDomainHelper.MapDomain(cdomainAndUris, ccurrent, cculture, cdefaultCulture));
if (domainAndUri == null)
throw new Exception("DomainForUri returned null.");
return domainAndUri;
}
/// <summary>
@@ -202,19 +198,12 @@ namespace Umbraco.Web.Routing
private static IReadOnlyCollection<DomainAndUri> SelectByCulture(IReadOnlyCollection<DomainAndUri> domainsAndUris, string culture, string defaultCulture)
{
// we try our best to match cultures, but may end with a bogus domain
if (culture != null) // try the supplied culture
{
var cultureDomains = domainsAndUris.Where(x => x.Culture.Name.InvariantEquals(culture)).ToList();
if (cultureDomains.Count > 0) return cultureDomains;
// if a culture is supplied, we *want* a url for that culture, else fail
//throw new InvalidOperationException($"Could not find a domain for culture \"{culture}\".");
//fixme: Review - throwing here causes a problem because the UrlProviderExtensions.GetContentUrls iterates through
// ALL cultures even if those cultures are not assigned for use within a branch. Returning null
// here fixes that problem and the URLs resolve correctly, however i don't know if this is causing other
// residual problems. It would also suggest that below in GetByCulture we don't throw either but instead return null??
return null;
}
if (defaultCulture != null) // try the defaultCulture culture
@@ -230,13 +219,12 @@ namespace Umbraco.Web.Routing
{
DomainAndUri domainAndUri;
// we try our best to match cultures, but may end with a bogus domain
if (culture != null) // try the supplied culture
{
domainAndUri = domainsAndUris.FirstOrDefault(x => x.Culture.Name.InvariantEquals(culture));
if (domainAndUri != null) return domainAndUri;
// if a culture is supplied, we *want* a url for that culture, else fail
throw new InvalidOperationException($"Could not find a domain for culture \"{culture}\".");
}
if (defaultCulture != null) // try the defaultCulture culture

5
src/Umbraco.Web/Routing/UrlProvider.cs
View File

@@ -245,10 +245,7 @@ namespace Umbraco.Web.Routing
/// </remarks>
public IEnumerable<UrlInfo> GetOtherUrls(int id, Uri current)
{
// providers can return null or an empty list or a non-empty list, be prepared
var urls = _urlProviders.SelectMany(provider => provider.GetOtherUrls(_umbracoContext, id, current));
return urls;
return _urlProviders.SelectMany(provider => provider.GetOtherUrls(_umbracoContext, id, current));
}
#endregion

16
src/Umbraco.Web/Routing/UrlProviderExtensions.cs
View File

@@ -5,7 +5,6 @@ using Umbraco.Core.Models;
using Umbraco.Core.Services;
using Umbraco.Core;
using Umbraco.Core.Logging;
using LightInject;
namespace Umbraco.Web.Routing
{
@@ -38,24 +37,25 @@ namespace Umbraco.Web.Routing
{
yield return UrlInfo.Message(textService.Localize("content/itemNotPublished"));
yield break;
}
var urls = new HashSet<UrlInfo>();
}
// build a list of urls, for the back-office
// which will contain
// - the 'main' urls, which is what .Url would return, for each culture
// - the 'other' urls we know (based upon domains, etc)
//
// need to work through each installed culture.
// fixme: Why not just work with each culture assigned to domains in the branch?
// on invariant nodes, each culture returns the same url segment
// but, we don't know if the branch to this content is invariant so we need to ask
// need to work through each installed culture:
// on invariant nodes, each culture returns the same url segment but,
// we don't know if the branch to this content is invariant, so we need to ask
// for URLs for all cultures.
// and, not only for those assigned to domains in the branch, because we want
// to show what GetUrl() would return, for every culture.
var urls = new HashSet<UrlInfo>();
var cultures = localizationService.GetAllLanguages().Select(x => x.IsoCode).ToList();
//get all URLs for all cultures
//in a HashSet, so de-duplicates too
foreach (var cultureUrl in GetContentUrlsByCulture(content, cultures, publishedRouter, umbracoContext, contentService, textService, logger))
{
urls.Add(cultureUrl);