From e44ed00c970b70bdb968ba6778b01c6131d8444b Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 3 Oct 2018 10:31:35 +0200 Subject: [PATCH] Another fallback iteration --- src/Umbraco.Core/Constants-Content.cs | 48 ------- .../Models/PublishedContent/Fallback.cs | 75 +++++++++++ .../IPublishedValueFallback.cs | 74 +++++------ .../NoopPublishedValueFallback.cs | 36 +++++- src/Umbraco.Core/Umbraco.Core.csproj | 2 +- .../Repositories/LanguageRepositoryTest.cs | 12 +- .../PublishedContentLanguageVariantTests.cs | 59 +++++++-- .../PublishedContent/PublishedContentTests.cs | 4 +- .../PublishedValueFallback.cs | 119 +++++++++++------- src/Umbraco.Web/PublishedContentExtensions.cs | 30 +++-- src/Umbraco.Web/PublishedElementExtensions.cs | 29 +++-- src/Umbraco.Web/PublishedPropertyExtension.cs | 63 +++++----- src/Umbraco.Web/umbraco.presentation/item.cs | 4 +- 13 files changed, 341 insertions(+), 214 deletions(-) delete mode 100644 src/Umbraco.Core/Constants-Content.cs create mode 100644 src/Umbraco.Core/Models/PublishedContent/Fallback.cs diff --git a/src/Umbraco.Core/Constants-Content.cs b/src/Umbraco.Core/Constants-Content.cs deleted file mode 100644 index 3f12ece6dc..0000000000 --- a/src/Umbraco.Core/Constants-Content.cs +++ /dev/null @@ -1,48 +0,0 @@ -namespace Umbraco.Core -{ - public static partial class Constants - { - /// - /// Defines content retrieval related constants - /// - public static class Content - { - /// - /// Defines core supported content fall-back options when retrieving content property values. - /// Defined as constants rather than enum to allow solution or package defined fall-back methods. - /// - public static class ValueFallback - { - /// - /// No fallback at all. - /// - public const int None = -1; - - /// - /// Default fallback. - /// - public const int Default = 0; - - /// - /// Recurse up the tree. - /// - public const int Recurse = 1; - - /// - /// Fallback to other languages. - /// - public const int Language = 2; - - /// - /// Recurse up the tree. If content not found, fallback to other languages. - /// - public const int RecurseThenLanguage = 3; - - /// - /// Fallback to other languages. If content not found, recurse up the tree. - /// - public const int LanguageThenRecurse = 4; - } - } - } -} diff --git a/src/Umbraco.Core/Models/PublishedContent/Fallback.cs b/src/Umbraco.Core/Models/PublishedContent/Fallback.cs new file mode 100644 index 0000000000..0434218555 --- /dev/null +++ b/src/Umbraco.Core/Models/PublishedContent/Fallback.cs @@ -0,0 +1,75 @@ +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Umbraco.Core.Models.PublishedContent +{ + /// + /// Manages the built-in fallback policies. + /// + public struct Fallback : IEnumerable + { + private readonly int[] _values; + + /// + /// Initializes a new instance of the struct with values. + /// + private Fallback(int[] values) + { + _values = values; + } + + /// + /// Gets an ordered set of fallback policies. + /// + /// + public static Fallback To(params int[] values) => new Fallback(values); + + /// + /// Do not fallback. + /// + public const int None = 0; + + /// + /// Fallback to default value. + /// + public const int DefaultValue = 1; + + /// + /// Gets the fallback to default value policy. + /// + public static Fallback ToDefaultValue => new Fallback(new[] { DefaultValue }); + + /// + /// Fallback to other languages. + /// + public const int Language = 2; + + /// + /// Gets the fallback to language policy. + /// + public static Fallback ToLanguage => new Fallback(new[] { Language }); + + /// + /// Fallback to tree ancestors. + /// + public const int Ancestors = 3; + + /// + /// Gets the fallback to tree ancestors policy. + /// + public static Fallback ToAncestors => new Fallback(new[] { Ancestors }); + + /// + public IEnumerator GetEnumerator() + { + return ((IEnumerable)_values ?? Array.Empty()).GetEnumerator(); + } + + /// + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + } +} diff --git a/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs b/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs index c67df36c4f..b03b0515cf 100644 --- a/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs +++ b/src/Umbraco.Core/Models/PublishedContent/IPublishedValueFallback.cs @@ -7,16 +7,6 @@ { // fixme discussions & challenges // - // - what's with visitedLanguage? should be internal to fallback implementation - // so that should be the case now, with latest changes - // - // - should be as simple as - // model.Value("price", fallback: ValueFallback.Language); - // model.Value("name", fallback: ValueFallback.Recurse); - // - // so chaining things through an array of ints is not... convenient - // it feels like ppl could have ValueFallback.LanguageAndRecurse or something? - // // - the fallback: parameter value must be open, so about anything can be passed to the IPublishedValueFallback // we have it now, it's an integer + constants, cool // @@ -24,14 +14,6 @@ // not! the default value of the fallback: parameter is 'default', not 'none', and if people // want to implement a different default behavior, they have to override the fallback provider // - // - currently, no policies on IPublishedProperty nor IPublishedElement, but some may apply (language) - // todo: implement - // - // - general defaultValue discussion: - // when HasValue is false, the converter may return something, eg an empty enumerable, even though - // defaultValue is null, so should we respect defaultValue only when it is not 'default'? - // todo: when defaultValue==default, and HasValue is false, still return GetValue to ensure this - // // - (and...) // ModelsBuilder model.Value(x => x.Price, ...) extensions need to be adjusted too // @@ -40,14 +22,15 @@ // OTOH we need to implement the readonly thing for languages /// - /// Gets a fallback value for a property. + /// Tries to get a fallback value for a property. /// /// The property. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever property.Value(culture, segment, defaultValue) is called, and /// property.HasValue(culture, segment) is false. @@ -55,18 +38,19 @@ /// At property level, property.GetValue() does *not* implement fallback, and one has to /// get property.Value() or property.Value{T}() to trigger fallback. /// - object GetValue(IPublishedProperty property, string culture, string segment, object defaultValue, int fallback); + bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, object defaultValue, out object value); /// - /// Gets a fallback value for a property. + /// Tries to get a fallback value for a property. /// /// The type of the value. /// The property. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever property.Value{T}(culture, segment, defaultValue) is called, and /// property.HasValue(culture, segment) is false. @@ -74,76 +58,78 @@ /// At property level, property.GetValue() does *not* implement fallback, and one has to /// get property.Value() or property.Value{T}() to trigger fallback. /// - T GetValue(IPublishedProperty property, string culture, string segment, T defaultValue, int fallback); + bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, T defaultValue, out T value); /// - /// Gets a fallback value for a published element property. + /// Tries to get a fallback value for a published element property. /// /// The published element. /// The property alias. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever getting the property value for the specified alias, culture and /// segment, either returned no property at all, or a property with HasValue(culture, segment) being false. /// It can only fallback at element level (no recurse). /// - object GetValue(IPublishedElement content, string alias, string culture, string segment, object defaultValue, int fallback); + bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value); /// - /// Gets a fallback value for a published element property. + /// Tries to get a fallback value for a published element property. /// /// The type of the value. /// The published element. /// The property alias. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever getting the property value for the specified alias, culture and /// segment, either returned no property at all, or a property with HasValue(culture, segment) being false. /// It can only fallback at element level (no recurse). /// - T GetValue(IPublishedElement content, string alias, string culture, string segment, T defaultValue, int fallback); + bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value); /// - /// Gets a fallback value for a published content property. + /// Tries to get a fallback value for a published content property. /// /// The published element. /// The property alias. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever getting the property value for the specified alias, culture and /// segment, either returned no property at all, or a property with HasValue(culture, segment) being false. - /// fixme explain & document priority + merge w/recurse? /// - object GetValue(IPublishedContent content, string alias, string culture, string segment, object defaultValue, int fallback); + bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value); /// - /// Gets a fallback value for a published content property. + /// Tries to get a fallback value for a published content property. /// /// The type of the value. /// The published element. /// The property alias. /// The requested culture. /// The requested segment. + /// A fallback strategy. /// An optional default value. - /// Integer value defining method to use for fallback when content not found - /// A fallback value, or null. + /// The fallback value. + /// A value indicating whether a fallback value could be provided. /// /// This method is called whenever getting the property value for the specified alias, culture and /// segment, either returned no property at all, or a property with HasValue(culture, segment) being false. - /// fixme explain & document priority + merge w/recurse? /// - T GetValue(IPublishedContent content, string alias, string culture, string segment, T defaultValue, int fallback); + bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value); } } diff --git a/src/Umbraco.Core/Models/PublishedContent/NoopPublishedValueFallback.cs b/src/Umbraco.Core/Models/PublishedContent/NoopPublishedValueFallback.cs index d920cefb24..cd7b063d44 100644 --- a/src/Umbraco.Core/Models/PublishedContent/NoopPublishedValueFallback.cs +++ b/src/Umbraco.Core/Models/PublishedContent/NoopPublishedValueFallback.cs @@ -9,21 +9,45 @@ public class NoopPublishedValueFallback : IPublishedValueFallback { /// - public object GetValue(IPublishedProperty property, string culture, string segment, object defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, object defaultValue, out object value) + { + value = default; + return false; + } /// - public T GetValue(IPublishedProperty property, string culture, string segment, T defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, T defaultValue, out T value) + { + value = default; + return false; + } /// - public object GetValue(IPublishedElement content, string alias, string culture, string segment, object defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value) + { + value = default; + return false; + } /// - public T GetValue(IPublishedElement content, string alias, string culture, string segment, T defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value) + { + value = default; + return false; + } /// - public object GetValue(IPublishedContent content, string alias, string culture, string segment, object defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value) + { + value = default; + return false; + } /// - public T GetValue(IPublishedContent content, string alias, string culture, string segment, T defaultValue, int fallback) => defaultValue; + public bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value) + { + value = default; + return false; + } } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 41c9ea1df9..15c057389f 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -299,7 +299,6 @@ - @@ -393,6 +392,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs index c62cf9c0db..a063d2e387 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs @@ -328,16 +328,16 @@ namespace Umbraco.Tests.Persistence.Repositories // Add language to delete as a fall-back language to another one var repository = CreateRepository(provider); var languageToFallbackFrom = repository.Get(5); - languageToFallbackFrom.FallbackLanguageId = 1; + languageToFallbackFrom.FallbackLanguageId = 2; // fall back to #2 (something we can delete) repository.Save(languageToFallbackFrom); - // Act - var languageToDelete = repository.Get(1); + // delete #2 + var languageToDelete = repository.Get(2); repository.Delete(languageToDelete); - var exists = repository.Exists(1); + var exists = repository.Exists(2); - // Assert + // has been deleted Assert.That(exists, Is.False); } } @@ -369,6 +369,8 @@ namespace Umbraco.Tests.Persistence.Repositories private void CreateTestData() { + //Id 1 is en-US - when Umbraco is installed + var languageDK = new Language("da-DK") { CultureName = "da-DK" }; ServiceContext.LocalizationService.Save(languageDK);//Id 2 diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentLanguageVariantTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentLanguageVariantTests.cs index d5e01fd424..4e98aea000 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentLanguageVariantTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentLanguageVariantTests.cs @@ -2,11 +2,13 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using System.Reflection; using Moq; using NUnit.Framework; using Umbraco.Core.Composing; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Tests.Testing; using Umbraco.Web; @@ -63,6 +65,8 @@ namespace Umbraco.Tests.PublishedContent var props = new[] { factory.CreatePropertyType("prop1", 1), + factory.CreatePropertyType("welcomeText", 1), + factory.CreatePropertyType("welcomeText2", 1), }; var contentType1 = factory.CreateContentType(1, "ContentType1", Enumerable.Empty(), props); @@ -168,7 +172,7 @@ namespace Umbraco.Tests.PublishedContent public void Can_Get_Content_For_Unpopulated_Requested_Language_With_Fallback() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First(); - var value = content.Value("welcomeText", "es", fallback: Core.Constants.Content.ValueFallback.Language); + var value = content.Value("welcomeText", "es", fallback: Fallback.ToLanguage); Assert.AreEqual("Welcome", value); } @@ -176,7 +180,7 @@ namespace Umbraco.Tests.PublishedContent public void Can_Get_Content_For_Unpopulated_Requested_Language_With_Fallback_Over_Two_Levels() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First(); - var value = content.Value("welcomeText", "it", fallback: Core.Constants.Content.ValueFallback.Language); + var value = content.Value("welcomeText", "it", fallback: Fallback.To(Fallback.Language, Fallback.Ancestors)); Assert.AreEqual("Welcome", value); } @@ -184,7 +188,7 @@ namespace Umbraco.Tests.PublishedContent public void Do_Not_GetContent_For_Unpopulated_Requested_Language_With_Fallback_Over_That_Loops() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First(); - var value = content.Value("welcomeText", "no", fallback: Core.Constants.Content.ValueFallback.Language); + var value = content.Value("welcomeText", "no", fallback: Fallback.ToLanguage); Assert.IsNull(value); } @@ -200,7 +204,7 @@ namespace Umbraco.Tests.PublishedContent public void Can_Get_Content_Recursively() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); - var value = content.Value("welcomeText2", fallback: Core.Constants.Content.ValueFallback.Recurse); + var value = content.Value("welcomeText2", fallback: Fallback.ToAncestors); Assert.AreEqual("Welcome", value); } @@ -208,7 +212,7 @@ namespace Umbraco.Tests.PublishedContent public void Can_Get_Content_With_Recursive_Priority() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); - var value = content.Value("welcomeText", "nl", fallback: Core.Constants.Content.ValueFallback.RecurseThenLanguage); + var value = content.Value("welcomeText", "nl", fallback: Fallback.To(Fallback.Ancestors, Fallback.Language)); // No Dutch value is directly assigned. Check has fallen back to Dutch value from parent. Assert.AreEqual("Welkom", value); @@ -218,7 +222,7 @@ namespace Umbraco.Tests.PublishedContent public void Can_Get_Content_With_Fallback_Language_Priority() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); - var value = content.Value("welcomeText", "nl", fallback: Core.Constants.Content.ValueFallback.LanguageThenRecurse); + var value = content.Value("welcomeText", "nl", fallback: Fallback.ToLanguage); // No Dutch value is directly assigned. Check has fallen back to English value from language variant. Assert.AreEqual("Welcome", value); @@ -228,7 +232,48 @@ namespace Umbraco.Tests.PublishedContent public void Throws_For_Non_Supported_Fallback() { var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); - Assert.Throws(() => content.Value("welcomeText", "nl", fallback: 999)); + Assert.Throws(() => content.Value("welcomeText", "nl", fallback: Fallback.To(999))); + } + + [Test] + public void Can_Fallback_To_Default_Value() + { + var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); + + // no Dutch value is assigned, so getting null + var value = content.Value("welcomeText", "nl"); + Assert.IsNull(value); + + // even if we 'just' provide a default value + value = content.Value("welcomeText", "nl", defaultValue: "woop"); + Assert.IsNull(value); + + // but it works with proper fallback settings + value = content.Value("welcomeText", "nl", fallback: Fallback.ToDefaultValue, defaultValue: "woop"); + Assert.AreEqual("woop", value); + } + + [Test] + public void Can_Have_Custom_Default_Value() + { + var content = UmbracoContext.Current.ContentCache.GetAtRoot().First().Children.First(); + + // hack the value, pretend the converter would return something + var prop = content.GetProperty("welcomeText") as SolidPublishedPropertyWithLanguageVariants; + Assert.IsNotNull(prop); + prop.SetValue("nl", "nope"); // HasValue false but getting value returns this + + // there is an EN value + var value = content.Value("welcomeText", "en-US"); + Assert.AreEqual("Welcome", value); + + // there is no NL value and we get the 'converted' value + value = content.Value("welcomeText", "nl"); + Assert.AreEqual("nope", value); + + // but it works with proper fallback settings + value = content.Value("welcomeText", "nl", fallback: Fallback.ToDefaultValue, defaultValue: "woop"); + Assert.AreEqual("woop", value); } } } diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs index e8ea3bc829..14dae46bcb 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs @@ -341,8 +341,8 @@ namespace Umbraco.Tests.PublishedContent public void Get_Property_Value_Recursive() { var doc = GetNode(1174); - var rVal = doc.Value("testRecursive", fallback: Constants.Content.ValueFallback.Recurse); - var nullVal = doc.Value("DoNotFindThis", fallback: Constants.Content.ValueFallback.Recurse); + var rVal = doc.Value("testRecursive", fallback: Fallback.ToAncestors); + var nullVal = doc.Value("DoNotFindThis", fallback: Fallback.ToAncestors); Assert.AreEqual("This is the recursive val", rVal); Assert.AreEqual(null, nullVal); } diff --git a/src/Umbraco.Web/Models/PublishedContent/PublishedValueFallback.cs b/src/Umbraco.Web/Models/PublishedContent/PublishedValueFallback.cs index bfcd339650..4e9cdaa8f9 100644 --- a/src/Umbraco.Web/Models/PublishedContent/PublishedValueFallback.cs +++ b/src/Umbraco.Web/Models/PublishedContent/PublishedValueFallback.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using Umbraco.Core; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Services; -using ValueFallback = Umbraco.Core.Constants.Content.ValueFallback; namespace Umbraco.Web.Models.PublishedContent { @@ -24,82 +23,110 @@ namespace Umbraco.Web.Models.PublishedContent } /// - public object GetValue(IPublishedProperty property, string culture, string segment, object defaultValue, int fallback) + public bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, object defaultValue, out object value) { - return GetValue(property, culture, segment, defaultValue, fallback); + return TryGetValue(property, culture, segment, fallback, defaultValue, out value); } /// - public T GetValue(IPublishedProperty property, string culture, string segment, T defaultValue, int fallback) + public bool TryGetValue(IPublishedProperty property, string culture, string segment, Fallback fallback, T defaultValue, out T value) { - switch (fallback) + foreach (var f in fallback) { - case ValueFallback.None: - case ValueFallback.Default: - return defaultValue; - case ValueFallback.Language: - return TryGetValueWithLanguageFallback(property, culture, segment, defaultValue, out var value2) ? value2 : defaultValue; - default: - throw NotSupportedFallbackMethod(fallback); + switch (f) + { + case Fallback.None: + continue; + case Fallback.DefaultValue: + value = defaultValue; + return true; + case Fallback.Language: + if (TryGetValueWithLanguageFallback(property, culture, segment, defaultValue, out value)) + return true; + break; + default: + throw NotSupportedFallbackMethod(f, "property"); + } } + + value = defaultValue; + return false; } /// - public object GetValue(IPublishedElement content, string alias, string culture, string segment, object defaultValue, int fallback) + public bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value) { - return GetValue(content, alias, culture, segment, defaultValue, fallback); + return TryGetValue(content, alias, culture, segment, fallback, defaultValue, out value); } /// - public T GetValue(IPublishedElement content, string alias, string culture, string segment, T defaultValue, int fallback) + public bool TryGetValue(IPublishedElement content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value) { - switch (fallback) + foreach (var f in fallback) { - case ValueFallback.None: - case ValueFallback.Default: - return defaultValue; - case ValueFallback.Language: - return TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out var value2) ? value2 : defaultValue; - default: - throw NotSupportedFallbackMethod(fallback); + switch (f) + { + case Fallback.None: + continue; + case Fallback.DefaultValue: + value = defaultValue; + return true; + case Fallback.Language: + if (TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out value)) + return true; + break; + default: + throw NotSupportedFallbackMethod(f, "element"); + } } + + value = defaultValue; + return false; } /// - public object GetValue(IPublishedContent content, string alias, string culture, string segment, object defaultValue, int fallback) + public bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, object defaultValue, out object value) { // is that ok? - return GetValue(content, alias, culture, segment, defaultValue, fallback); + return TryGetValue(content, alias, culture, segment, fallback, defaultValue, out value); } /// - public virtual T GetValue(IPublishedContent content, string alias, string culture, string segment, T defaultValue, int fallback) + public virtual bool TryGetValue(IPublishedContent content, string alias, string culture, string segment, Fallback fallback, T defaultValue, out T value) { - switch (fallback) + // note: we don't support "recurse & language" which would walk up the tree, + // looking at languages at each level - should someone need it... they'll have + // to implement it. + + foreach (var f in fallback) { - case ValueFallback.None: - case ValueFallback.Default: - return defaultValue; - case ValueFallback.Recurse: - return TryGetValueWithRecursiveFallback(content, alias, culture, segment, defaultValue, out var value1) ? value1 : defaultValue; - case ValueFallback.Language: - return TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out var value2) ? value2 : defaultValue; - case ValueFallback.RecurseThenLanguage: - return TryGetValueWithRecursiveFallback(content, alias, culture, segment, defaultValue, out var value3) - ? value3 - : TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out var value4) ? value4 : defaultValue; - case ValueFallback.LanguageThenRecurse: - return TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out var value5) - ? value5 - : TryGetValueWithRecursiveFallback(content, alias, culture, segment, defaultValue, out var value6) ? value6 : defaultValue; - default: - throw NotSupportedFallbackMethod(fallback); + switch (f) + { + case Fallback.None: + continue; + case Fallback.DefaultValue: + value = defaultValue; + return true; + case Fallback.Language: + if (TryGetValueWithLanguageFallback(content, alias, culture, segment, defaultValue, out value)) + return true; + break; + case Fallback.Ancestors: + if (TryGetValueWithRecursiveFallback(content, alias, culture, segment, defaultValue, out value)) + return true; + break; + default: + throw NotSupportedFallbackMethod(f, "content"); + } } + + value = defaultValue; + return false; } - private NotSupportedException NotSupportedFallbackMethod(int fallback) + private NotSupportedException NotSupportedFallbackMethod(int fallback, string level) { - return new NotSupportedException($"Fallback {GetType().Name} does not support policy code '{fallback}'."); + return new NotSupportedException($"Fallback {GetType().Name} does not support fallback code '{fallback}' at {level} level."); } // tries to get a value, recursing the tree diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index 88bb80f604..f0ddf62074 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -184,7 +184,7 @@ namespace Umbraco.Web #endregion #region Value - + /// /// Gets the value of a content's property identified by its alias, if it exists, otherwise a default value. /// @@ -192,23 +192,30 @@ namespace Umbraco.Web /// The property alias. /// The variation language. /// The variation segment. - /// The default value. /// Optional fallback strategy. + /// The default value. /// The value of the content's property identified by the alias, if it exists, otherwise a default value. - public static object Value(this IPublishedContent content, string alias, string culture = null, string segment = null, object defaultValue = default, int fallback = 0) + public static object Value(this IPublishedContent content, string alias, string culture = null, string segment = null, Fallback fallback = default, object defaultValue = default) { var property = content.GetProperty(alias); + // if we have a property, and it has a value, return that value if (property != null && property.HasValue(culture, segment)) return property.GetValue(culture, segment); - return PublishedValueFallback.GetValue(content, alias, culture, segment, defaultValue, fallback); + // else let fallback try to get a value + if (PublishedValueFallback.TryGetValue(content, alias, culture, segment, fallback, defaultValue, out var value)) + return value; + + // else... if we have a property, at least let the converter return its own + // vision of 'no value' (could be an empty enumerable) - otherwise, default + return property?.GetValue(culture, segment); } #endregion #region Value - + /// /// Gets the value of a content's property identified by its alias, converted to a specified type. /// @@ -217,17 +224,24 @@ namespace Umbraco.Web /// The property alias. /// The variation language. /// The variation segment. - /// The default value. /// Optional fallback strategy. + /// The default value. /// The value of the content's property identified by the alias, converted to the specified type. - public static T Value(this IPublishedContent content, string alias, string culture = null, string segment = null, T defaultValue = default, int fallback = 0) + public static T Value(this IPublishedContent content, string alias, string culture = null, string segment = null, Fallback fallback = default, T defaultValue = default) { var property = content.GetProperty(alias); + // if we have a property, and it has a value, return that value if (property != null && property.HasValue(culture, segment)) return property.Value(culture, segment); - return PublishedValueFallback.GetValue(content, alias, culture, segment, defaultValue, fallback); + // else let fallback try to get a value + if (PublishedValueFallback.TryGetValue(content, alias, culture, segment, fallback, defaultValue, out var value)) + return value; + + // else... if we have a property, at least let the converter return its own + // vision of 'no value' (could be an empty enumerable) - otherwise, default + return property == null ? default : property.Value(culture, segment); } // fixme - .Value() refactoring - in progress diff --git a/src/Umbraco.Web/PublishedElementExtensions.cs b/src/Umbraco.Web/PublishedElementExtensions.cs index eea92f4c6c..cd6ede9a7c 100644 --- a/src/Umbraco.Web/PublishedElementExtensions.cs +++ b/src/Umbraco.Web/PublishedElementExtensions.cs @@ -99,8 +99,8 @@ namespace Umbraco.Web /// The property alias. /// The variation language. /// The variation segment. - /// The default value. /// Optional fallback strategy. + /// The default value. /// The value of the content's property identified by the alias, if it exists, otherwise a default value. /// /// The value comes from IPublishedProperty field Value ie it is suitable for use when rendering content. @@ -108,19 +108,21 @@ namespace Umbraco.Web /// If eg a numeric property wants to default to 0 when value source is empty, this has to be done in the converter. /// The alias is case-insensitive. /// - public static object Value(this IPublishedElement content, string alias, string culture = null, string segment = null, object defaultValue = default, int fallback = 0) + public static object Value(this IPublishedElement content, string alias, string culture = null, string segment = null, Fallback fallback = default, object defaultValue = default) { var property = content.GetProperty(alias); + // if we have a property, and it has a value, return that value if (property != null && property.HasValue(culture, segment)) return property.GetValue(culture, segment); - // fixme defaultValue is a problem here - // assuming the value may return as an IEnumerable and no defaultValue is provided, then defaultValue is null - // and if HasValue is false, what we get is 'null' - but the converter may instead have been able to return an - // empty enumerable, which would be way nicer - so we need a way to tell that 'no defaultValue has been provided'? + // else let fallback try to get a value + if (PublishedValueFallback.TryGetValue(content, alias, culture, segment, fallback, defaultValue, out var value)) + return value; - return PublishedValueFallback.GetValue(content, alias, culture, segment, defaultValue, fallback); + // else... if we have a property, at least let the converter return its own + // vision of 'no value' (could be an empty enumerable) - otherwise, default + return property?.GetValue(culture, segment); } #endregion @@ -135,8 +137,8 @@ namespace Umbraco.Web /// The property alias. /// The variation language. /// The variation segment. - /// The default value. /// Optional fallback strategy. + /// The default value. /// The value of the content's property identified by the alias, converted to the specified type. /// /// The value comes from IPublishedProperty field Value ie it is suitable for use when rendering content. @@ -144,14 +146,21 @@ namespace Umbraco.Web /// If eg a numeric property wants to default to 0 when value source is empty, this has to be done in the converter. /// The alias is case-insensitive. /// - public static T Value(this IPublishedElement content, string alias, string culture = null, string segment = null, T defaultValue = default, int fallback = 0) + public static T Value(this IPublishedElement content, string alias, string culture = null, string segment = null, Fallback fallback = default, T defaultValue = default) { var property = content.GetProperty(alias); + // if we have a property, and it has a value, return that value if (property != null && property.HasValue(culture, segment)) return property.Value(culture, segment); - return PublishedValueFallback.GetValue(content, alias, culture, segment, defaultValue, fallback); + // else let fallback try to get a value + if (PublishedValueFallback.TryGetValue(content, alias, culture, segment, fallback, defaultValue, out var value)) + return value; + + // else... if we have a property, at least let the converter return its own + // vision of 'no value' (could be an empty enumerable) - otherwise, default + return property == null ? default : property.Value(culture, segment); } #endregion diff --git a/src/Umbraco.Web/PublishedPropertyExtension.cs b/src/Umbraco.Web/PublishedPropertyExtension.cs index 6632076022..b431f24828 100644 --- a/src/Umbraco.Web/PublishedPropertyExtension.cs +++ b/src/Umbraco.Web/PublishedPropertyExtension.cs @@ -16,56 +16,49 @@ namespace Umbraco.Web #region Value - public static object Value(this IPublishedProperty property, string culture = null, string segment = null, object defaultValue = default, int fallback = 0) + public static object Value(this IPublishedProperty property, string culture = null, string segment = null, Fallback fallback = default, object defaultValue = default) { if (property.HasValue(culture, segment)) return property.GetValue(culture, segment); - return PublishedValueFallback.GetValue(property, culture, segment, defaultValue, fallback); + return PublishedValueFallback.TryGetValue(property, culture, segment, fallback, defaultValue, out var value) + ? value + : property.GetValue(culture, segment); // give converter a chance to return it's own vision of "no value" } #endregion #region Value - public static T Value(this IPublishedProperty property, string culture = null, string segment = null, T defaultValue = default, int fallback = 0) + public static T Value(this IPublishedProperty property, string culture = null, string segment = null, Fallback fallback = default, T defaultValue = default) { - // for Value when defaultValue is not specified, and HasValue() is false, we still want to convert the result (see below) - // but we have no way to tell whether default value is specified or not - we could do it with overloads, but then defaultValue - // comes right after property and conflicts with culture when T is string - so we're just not doing it - if defaultValue is - // default, whether specified or not, we give a chance to the converter - // - //if (!property.HasValue(culture, segment) && 'defaultValue is explicitely specified') return defaultValue; + if (property.HasValue(culture, segment)) + { + // we have a value + // try to cast or convert it + var value = property.GetValue(culture, segment); + if (value is T valueAsT) return valueAsT; + var valueConverted = value.TryConvertTo(); + if (valueConverted) return valueConverted.Result; - // give the converter a chance to handle the default value differently - // eg for IEnumerable it may return Enumerable.Empty instead of null + // cannot cast nor convert the value, nothing we can return but 'default' + // note: we don't want to fallback in that case - would make little sense + return default; + } - var value = property.GetValue(culture, segment); + // we don't have a value, try fallback + if (PublishedValueFallback.TryGetValue(property, culture, segment, fallback, defaultValue, out var fallbackValue)) + return fallbackValue; - // if value is null (strange but why not) it still is OK to call TryConvertTo - // because it's an extension method (hence no NullRef) which will return a - // failed attempt. So, no need to care for value being null here. + // we don't have a value - neither direct nor fallback + // give a chance to the converter to return something (eg empty enumerable) + var noValue = property.GetValue(culture, segment); + if (noValue is T noValueAsT) return noValueAsT; + var noValueConverted = noValue.TryConvertTo(); + if (noValueConverted) return noValueConverted.Result; - // if already the requested type, return - if (value is T variable) return variable; - - // if can convert to requested type, return - var convert = value.TryConvertTo(); - if (convert.Success) return convert.Result; - - // at that point, the code tried with the raw value - // that makes no sense because it sort of is unpredictable, - // you wouldn't know when the converters run or don't run. - // so, it's commented out now. - - // try with the raw value - //var source = property.ValueSource; - //if (source is string) source = TextValueConverterHelper.ParseStringValueSource((string)source); - //if (source is T) return (T)source; - //convert = source.TryConvertTo(); - //if (convert.Success) return convert.Result; - - return PublishedValueFallback.GetValue(property, culture, segment, defaultValue, fallback); + // cannot cast noValue nor convert it, nothing we can return but 'default' + return default; } #endregion diff --git a/src/Umbraco.Web/umbraco.presentation/item.cs b/src/Umbraco.Web/umbraco.presentation/item.cs index cca6076391..d15d3d33c1 100644 --- a/src/Umbraco.Web/umbraco.presentation/item.cs +++ b/src/Umbraco.Web/umbraco.presentation/item.cs @@ -82,7 +82,7 @@ namespace umbraco //check for published content and get its value using that if (publishedContent != null && (publishedContent.HasProperty(_fieldName) || recursive)) { - var pval = publishedContent.Value(_fieldName, fallback: Constants.Content.ValueFallback.Recurse); + var pval = publishedContent.Value(_fieldName, fallback: Fallback.ToAncestors); var rval = pval == null ? string.Empty : pval.ToString(); _fieldContent = rval.IsNullOrWhiteSpace() ? _fieldContent : rval; } @@ -102,7 +102,7 @@ namespace umbraco { if (publishedContent != null && (publishedContent.HasProperty(altFieldName) || recursive)) { - var pval = publishedContent.Value(altFieldName, fallback: Constants.Content.ValueFallback.Recurse); + var pval = publishedContent.Value(altFieldName, fallback: Fallback.ToAncestors); var rval = pval == null ? string.Empty : pval.ToString(); _fieldContent = rval.IsNullOrWhiteSpace() ? _fieldContent : rval; }