diff --git a/src/Umbraco.Core/Collections/CompositeStringStringKey.cs b/src/Umbraco.Core/Collections/CompositeStringStringKey.cs index e18f91707a..78ee9b0d2a 100644 --- a/src/Umbraco.Core/Collections/CompositeStringStringKey.cs +++ b/src/Umbraco.Core/Collections/CompositeStringStringKey.cs @@ -19,12 +19,12 @@ namespace Umbraco.Core.Collections /// public CompositeStringStringKey(string key1, string key2) { - _key1 = key1?.ToLowerInvariant() ?? "NULL"; + // fixme temp - debugging + if (key1 == null) throw new Exception("Getting null culture in CompositeStringStringKey constructor, fix!"); + if (key2 == null) throw new Exception("Getting null segment in CompositeStringStringKey constructor, fix!"); - //fixme - we are changing this to null if it is an empty string, this is because if we don't do this than this key will not match - // anything see comments http://issues.umbraco.org/issue/U4-11227#comment=67-46399 - // since we're not dealing with segments right now and I just need to get something working, this is the 'fix' - _key2 = !key2.IsNullOrWhiteSpace() ? key2.ToLowerInvariant() : "NULL"; + _key1 = key1?.ToLowerInvariant() ?? "NULL"; + _key2 = key2?.ToLowerInvariant() ?? "NULL"; } public bool Equals(CompositeStringStringKey other) diff --git a/src/Umbraco.Core/Models/PublishedContent/ThreadStaticVariationContextAccessor.cs b/src/Umbraco.Core/Models/PublishedContent/ThreadStaticVariationContextAccessor.cs deleted file mode 100644 index 70e54bb4d6..0000000000 --- a/src/Umbraco.Core/Models/PublishedContent/ThreadStaticVariationContextAccessor.cs +++ /dev/null @@ -1,23 +0,0 @@ -//using System; - -//namespace Umbraco.Core.Models.PublishedContent -//{ -// /// -// /// Provides a ThreadStatic-based implementation of . -// /// -// /// -// /// Something must set the current context. -// /// -// public class ThreadStaticVariationContextAccessor : IVariationContextAccessor -// { -// [ThreadStatic] -// private static VariationContext _context; - -// /// -// public VariationContext VariationContext -// { -// get => _context; -// set => _context = value; -// } -// } -//} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c476838302..384f1defa6 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -361,7 +361,6 @@ - diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentCache.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentCache.cs index 576677d6ac..0ad7c83a08 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentCache.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentCache.cs @@ -104,8 +104,8 @@ namespace Umbraco.Web.PublishedCache.NuCache // hideTopLevelNode = support legacy stuff, look for /*/path/to/node // else normal, look for /path/to/node content = hideTopLevelNode.Value - ? GetAtRoot(preview).SelectMany(x => x.Children).FirstOrDefault(x => x.GetCulture(culture).UrlSegment == parts[0]) - : GetAtRoot(preview).FirstOrDefault(x => x.GetCulture(culture).UrlSegment == parts[0]); + ? GetAtRoot(preview).SelectMany(x => x.Children).FirstOrDefault(x => x.GetUrlSegment(culture) == parts[0]) + : GetAtRoot(preview).FirstOrDefault(x => x.GetUrlSegment(culture) == parts[0]); content = FollowRoute(content, parts, 1, culture); } @@ -114,7 +114,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // have to look for /foo (see note in ApplyHideTopLevelNodeFromPath). if (content == null && hideTopLevelNode.Value && parts.Length == 1) { - content = GetAtRoot(preview).FirstOrDefault(x => x.GetCulture(culture).UrlSegment == parts[0]); + content = GetAtRoot(preview).FirstOrDefault(x => x.GetUrlSegment(culture) == parts[0]); } return content; @@ -147,18 +147,11 @@ namespace Umbraco.Web.PublishedCache.NuCache var hasDomains = _domainHelper.NodeHasDomains(n.Id); while (hasDomains == false && n != null) // n is null at root { - var varies = n.ContentType.Variations.Has(ContentVariation.CultureNeutral); - var urlSegment = varies ? n.GetCulture(culture)?.UrlSegment : n.UrlSegment; + var urlSegment = n.GetUrlSegment(culture); + + // without a segment, we cannot continue, really if (urlSegment.IsNullOrWhiteSpace()) - { - //we cannot continue, it will be null if the item is not published return null; - } - - //// at that point we should have an urlSegment, unless something weird is happening - //// at content level, such as n.GetCulture() returning null for some (weird) reason, - //// and then what? fallback to the invariant segment... far from perfect but eh... - //if (string.IsNullOrWhiteSpace(urlSegment)) urlSegment = n.UrlSegment; pathParts.Add(urlSegment); @@ -189,7 +182,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var part = parts[i++]; content = content.Children.FirstOrDefault(x => { - var urlSegment = x.GetCulture(culture).UrlSegment; + var urlSegment = x.GetUrlSegment(culture); return urlSegment == part; }); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.DictionaryOfPropertyDataSerializer.cs b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.DictionaryOfPropertyDataSerializer.cs index 7d8b869218..19c8beedb5 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.DictionaryOfPropertyDataSerializer.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.DictionaryOfPropertyDataSerializer.cs @@ -33,8 +33,12 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource pdatas.Add(pdata); // everything that can be null is read/written as object - pdata.Culture = ReadStringObject(stream); - pdata.Segment = ReadStringObject(stream); + // even though - culture and segment should never be null here, as 'null' represents + // the 'current' value, and string.Empty should be used to represent the invariant or + // neutral values - PropertyData throws when getting nulls, so falling back to + // string.Empty here - what else? + pdata.Culture = ReadStringObject(stream) ?? string.Empty; + pdata.Segment = ReadStringObject(stream) ?? string.Empty; pdata.Value = ReadObject(stream); } @@ -61,8 +65,10 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource foreach (var pdata in values) { // everything that can be null is read/written as object - WriteObject(pdata.Culture, stream); - WriteObject(pdata.Segment, stream); + // even though - culture and segment should never be null here, + // see note in ReadFrom() method above + WriteObject(pdata.Culture ?? string.Empty, stream); + WriteObject(pdata.Segment ?? string.Empty, stream); WriteObject(pdata.Value, stream); } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/PropertyData.cs b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/PropertyData.cs index 29cbed08b6..4317a9b1ee 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/PropertyData.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/PropertyData.cs @@ -5,11 +5,22 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource { internal class PropertyData { + private string _culture; + private string _segment; + [JsonProperty("culture")] - public string Culture { get; set; } + public string Culture + { + get => _culture; + set => _culture = value ?? throw new ArgumentNullException(nameof(value)); // fixme or fallback to string.Empty? CANNOT be null + } [JsonProperty("seg")] - public string Segment { get; set; } + public string Segment + { + get => _segment; + set => _segment = value ?? throw new ArgumentNullException(nameof(value)); // fixme or fallback to string.Empty? CANNOT be null + } [JsonProperty("val")] public object Value { get; set; } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Property.cs b/src/Umbraco.Web/PublishedCache/NuCache/Property.cs index 6d161dc3b9..31573ac81a 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/Property.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/Property.cs @@ -87,8 +87,13 @@ namespace Umbraco.Web.PublishedCache.NuCache _variations = origin._variations; } - public override bool HasValue(string culture = null, string segment = null) => _sourceValue != null - && (!(_sourceValue is string) || string.IsNullOrWhiteSpace((string) _sourceValue) == false); + public override bool HasValue(string culture = null, string segment = null) + { + ContextualizeVariation(ref culture, ref segment); + + return _sourceValue != null && + (!(_sourceValue is string) || string.IsNullOrWhiteSpace((string) _sourceValue) == false); + } // used to cache the CacheValues of this property internal string ValuesCacheKey => _valuesCacheKey diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs index 0b8e6a4fe9..567201ef6f 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedContent.cs @@ -266,7 +266,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (_contentData.CultureInfos == null) throw new Exception("oops: _contentDate.CultureInfos is null."); return _cultureInfos = _contentData.CultureInfos - .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value.Name, x.Value.Date), StringComparer.InvariantCultureIgnoreCase); + .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value.Name, x.Value.Date), StringComparer.OrdinalIgnoreCase); } } diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index 7708abc57b..f3973a65e2 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -70,6 +70,26 @@ namespace Umbraco.Web throw new ArgumentOutOfRangeException(); } } + + /// + /// Gets the Url segment. + /// + /// + /// Gets the url segment for the document, taking its content type and a specified + /// culture in account. For invariant content types, the culture is ignored, else it is + /// used to try and find the segment corresponding to the culture. May return null. + /// + public static string GetUrlSegment(this IPublishedContent content, string culture = null) + { + // for invariant content, return the invariant url segment + if (!content.ContentType.Variations.Has(ContentVariation.CultureNeutral)) + return content.UrlSegment; + + // else try and get the culture info + // return the corresponding url segment, or null if none (ie the culture is not published) + var cultureInfo = content.GetCulture(culture); + return cultureInfo?.UrlSegment; + } #endregion diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 78290401c0..3d712f3abb 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -267,26 +267,29 @@ namespace Umbraco.Web.Routing var domainsCache = request.UmbracoContext.PublishedSnapshot.Domains; var domains = domainsCache.GetAll(includeWildcards: false).ToList(); - var contentForDomains = new Dictionary(); - //filter the domains to ensure that any referenced content is actually published - domains = domains.Where(x => - { - //get or look up the content assigned - if (!contentForDomains.TryGetValue(x.ContentId, out var contentForDomain)) - { - contentForDomain = request.UmbracoContext.PublishedSnapshot.Content.GetById(x.ContentId); - contentForDomains[x.ContentId] = contentForDomain; - } - //definitely not published - if (contentForDomain == null) + // determines whether a domain corresponds to a published document, since some + // domains may exist but on a document that has been unpublished - as a whole - or + // that is not published for the domain's culture - in which case the domain does + // not apply + bool IsPublishedContentDomain(Domain domain) + { + // just get it from content cache - optimize there, not here + var domainDocument = request.UmbracoContext.PublishedSnapshot.Content.GetById(domain.ContentId); + + // not published - at all + if (domainDocument == null) return false; - //invariant so no need to check variations - if (!contentForDomain.ContentType.Variations.Has(ContentVariation.CultureNeutral)) + + // invariant - always published + if (!domainDocument.ContentType.Variations.Has(ContentVariation.CultureNeutral)) return true; - //variant so ensure the culture name exists - return contentForDomain.Cultures.ContainsKey(x.Culture.Name); - }).ToList(); + + // variant, ensure that the culture corresponding to the domain's language is published + return domainDocument.Cultures.ContainsKey(domain.Culture.Name); + } + + domains = domains.Where(IsPublishedContentDomain).ToList(); var defaultCulture = domainsCache.DefaultCulture;