diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs index 47a41dd044..2f646f5eaf 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs @@ -14,11 +14,13 @@ namespace Umbraco.Core.Models.PublishedContent private readonly PublishedPropertyType[] _propertyTypes; // fast alias-to-index xref containing both the raw alias and its lowercase version + // fixme - benchmark this! private readonly Dictionary _indexes = new Dictionary(); // internal so it can be used by PublishedNoCache which does _not_ want to cache anything and so will never // use the static cache getter PublishedContentType.GetPublishedContentType(alias) below - anything else // should use it. + // fixme - not true anymore internal and all?! internal PublishedContentType(IContentType contentType) : this(PublishedItemType.Content, contentType) { } @@ -88,7 +90,7 @@ namespace Umbraco.Core.Models.PublishedContent // NOTE: code below defines and add custom, built-in, Umbraco properties for members // unless they are already user-defined in the content type, then they are skipped // not sure it's needed really - this is here for safety purposes - static readonly Dictionary> BuiltinMemberProperties = new Dictionary> + private static readonly Dictionary> BuiltinMemberProperties = new Dictionary> { // see also PublishedMember class - exposing special properties as properties { "Email", Tuple.Create(Constants.DataTypes.Textbox, Constants.PropertyEditors.TextboxAlias) }, @@ -115,7 +117,7 @@ namespace Umbraco.Core.Models.PublishedContent foreach (var propertyType in BuiltinMemberProperties .Where(kvp => aliases.Contains(kvp.Key) == false) - .Select(kvp => new PublishedPropertyType(kvp.Key, kvp.Value.Item1, kvp.Value.Item2, true))) + .Select(kvp => new PublishedPropertyType(kvp.Key, kvp.Value.Item1, kvp.Value.Item2, umbraco: true))) { if (contentType != null) propertyType.ContentType = contentType; yield return propertyType; diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyBase.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyBase.cs index e4cacb0972..2e00a89385 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyBase.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyBase.cs @@ -13,6 +13,23 @@ namespace Umbraco.Core.Models.PublishedContent { PropertyType = propertyType ?? throw new ArgumentNullException(nameof(propertyType)); ReferenceCacheLevel = referenceCacheLevel; + + ValidateCacheLevel(ReferenceCacheLevel); + ValidateCacheLevel(PropertyType.CacheLevel); + } + + private static void ValidateCacheLevel(PropertyCacheLevel cacheLevel) + { + switch (cacheLevel) + { + case PropertyCacheLevel.Content: + case PropertyCacheLevel.Snapshot: + case PropertyCacheLevel.Facade: + case PropertyCacheLevel.None: + break; + default: + throw new Exception("Invalid cache level."); + } } public PublishedPropertyType PropertyType { get; } diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyType.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyType.cs index aa81dbde7b..35eda44551 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyType.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedPropertyType.cs @@ -13,6 +13,15 @@ namespace Umbraco.Core.Models.PublishedContent /// if the property type changes, then a new class needs to be created. public class PublishedPropertyType { + private readonly PropertyValueConverterCollection _converters; + private readonly object _locker = new object(); + private volatile bool _initialized; + private IPropertyValueConverter _converter; + private PropertyCacheLevel _cacheLevel; + + private Type _modelClrType; + private Type _clrType; + #region Constructors /// @@ -26,6 +35,8 @@ namespace Umbraco.Core.Models.PublishedContent { // PropertyEditor [1:n] DataTypeDefinition [1:n] PropertyType + _converters = Current.PropertyValueConverters; // fixme really? + ContentType = contentType; PropertyTypeAlias = propertyType.Alias; @@ -46,8 +57,8 @@ namespace Umbraco.Core.Models.PublishedContent /// to make decisions, fetch prevalues, etc. /// The value of is assumed to be valid. /// - internal PublishedPropertyType(string propertyTypeAlias, string propertyEditorAlias) - : this(propertyTypeAlias, 0, propertyEditorAlias) + internal PublishedPropertyType(string propertyTypeAlias, string propertyEditorAlias, PropertyValueConverterCollection converters = null) + : this(propertyTypeAlias, 0, propertyEditorAlias, converters) { } /// @@ -63,11 +74,13 @@ namespace Umbraco.Core.Models.PublishedContent /// The values of and are /// assumed to be valid and consistent. /// - internal PublishedPropertyType(string propertyTypeAlias, int dataTypeDefinitionId, string propertyEditorAlias, bool umbraco = false) + internal PublishedPropertyType(string propertyTypeAlias, int dataTypeDefinitionId, string propertyEditorAlias, PropertyValueConverterCollection converters = null, bool umbraco = false) { // ContentType // - in unit tests, to be set by PublishedContentType when creating it + _converters = converters ?? Current.PropertyValueConverters; // fixme really? + PropertyTypeAlias = propertyTypeAlias; DataTypeId = dataTypeDefinitionId; @@ -109,32 +122,23 @@ namespace Umbraco.Core.Models.PublishedContent #region Converters - private readonly object _locker = new object(); - private volatile bool _initialized; - private IPropertyValueConverter _converter; - private PropertyCacheLevel _cacheLevel; - - private Type _modelClrType; - private Type _clrType; - - private void EnsureInitialized() + private void Initialize() { if (_initialized) return; lock (_locker) { if (_initialized) return; - InitializeConverters(); + InitializeLocked(); _initialized = true; } } - private void InitializeConverters() + private void InitializeLocked() { _converter = null; var isdefault = false; - var converterCollection = Current.PropertyValueConverters; - foreach (var converter in converterCollection) + foreach (var converter in _converters) { if (converter.IsConverter(this) == false) continue; @@ -142,20 +146,20 @@ namespace Umbraco.Core.Models.PublishedContent if (_converter == null) { _converter = converter; - isdefault = converterCollection.IsDefault(converter); + isdefault = _converters.IsDefault(converter); continue; } if (isdefault) { - if (converterCollection.IsDefault(converter)) + if (_converters.IsDefault(converter)) { // previous was default, and got another default - if (converterCollection.Shadows(_converter, converter)) + if (_converters.Shadows(_converter, converter)) { // previous shadows, ignore } - else if (converterCollection.Shadows(converter, _converter)) + else if (_converters.Shadows(converter, _converter)) { // shadows previous, replace _converter = converter; @@ -179,7 +183,7 @@ namespace Umbraco.Core.Models.PublishedContent } else { - if (converterCollection.IsDefault(converter)) + if (_converters.IsDefault(converter)) { // previous was non-default, ignore default } @@ -204,7 +208,7 @@ namespace Umbraco.Core.Models.PublishedContent { get { - EnsureInitialized(); + if (!_initialized) Initialize(); return _cacheLevel; } } @@ -215,7 +219,7 @@ namespace Umbraco.Core.Models.PublishedContent // preview: whether we are previewing or not public object ConvertSourceToInter(IPropertySet owner, object source, bool preview) { - EnsureInitialized(); + if (!_initialized) Initialize(); // use the converter if any, else just return the source value return _converter != null @@ -229,7 +233,7 @@ namespace Umbraco.Core.Models.PublishedContent // preview: whether we are previewing or not public object ConvertInterToObject(IPropertySet owner, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) { - EnsureInitialized(); + if (!_initialized) Initialize(); // use the converter if any, else just return the inter value return _converter != null @@ -244,7 +248,7 @@ namespace Umbraco.Core.Models.PublishedContent // preview: whether we are previewing or not public object ConvertInterToXPath(IPropertySet owner, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) { - EnsureInitialized(); + if (!_initialized) Initialize(); // use the converter if any if (_converter != null) @@ -264,7 +268,7 @@ namespace Umbraco.Core.Models.PublishedContent { get { - EnsureInitialized(); + if (!_initialized) Initialize(); return _modelClrType; } } @@ -276,7 +280,7 @@ namespace Umbraco.Core.Models.PublishedContent { get { - EnsureInitialized(); + if (!_initialized) Initialize(); return _clrType ?? (_clrType = ModelType.Map(_modelClrType, Current.PublishedContentModelFactory.ModelTypeMap)); } } diff --git a/src/Umbraco.Tests/PublishedContent/ModelsAndConvertersTests.cs b/src/Umbraco.Tests/PublishedContent/ModelsAndConvertersTests.cs index 466bb96b61..665ab2c6af 100644 --- a/src/Umbraco.Tests/PublishedContent/ModelsAndConvertersTests.cs +++ b/src/Umbraco.Tests/PublishedContent/ModelsAndConvertersTests.cs @@ -1,11 +1,10 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; -using System.Threading.Tasks; using LightInject; using Moq; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors; @@ -17,6 +16,17 @@ namespace Umbraco.Tests.PublishedContent [TestFixture] public class ModelsAndConvertersTests { + // fixme + // naming: IPublishedProperty is IPropertySetProperty or IFacadeProperty of some sort + // naming: general naming sucks at the moment + // caching: re-think how properties are cached + // - for true NuCache content it probably is OK (but needs explanation) + // - for true Xml cache content it probably is OK (but needs explanation) + // - for pure sets, I have no idea - should at least cache at content level? + // hold on - PropertySetPropertyBase probably handles it - need to check + + #region ModelType + [Test] public void ModelTypeEqualityTests() { @@ -61,16 +71,132 @@ namespace Umbraco.Tests.PublishedContent ModelType.Map(typeof(IEnumerable<>).MakeGenericType(ModelType.For("alias1").MakeArrayType()), map).ToString()); } + #endregion + + #region SimpleConverter1 + [Test] - public void ConverterTest1() + public void SimpleConverter1Test() + { + var converters = new PropertyValueConverterCollection(new IPropertyValueConverter[] + { + new SimpleConverter1(), + }); + + var setType1 = new PublishedContentType(1000, "set1", new[] + { + new PublishedPropertyType("prop1", "editor1", converters), + }); + + var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "1234" } }, false); + + Assert.AreEqual(1234, set1.Value("prop1")); + } + + private class SimpleConverter1 : IPropertyValueConverter + { + public bool IsConverter(PublishedPropertyType propertyType) + => propertyType.PropertyEditorAlias.InvariantEquals("editor1"); + + public Type GetPropertyValueType(PublishedPropertyType propertyType) + => typeof (int); + + public PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) + => PropertyCacheLevel.Content; + + public object ConvertSourceToInter(IPropertySet owner, PublishedPropertyType propertyType, object source, bool preview) + => int.TryParse(source as string, out int i) ? i : 0; + + public object ConvertInterToObject(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + => (int) inter; + + public object ConvertInterToXPath(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + => ((int) inter).ToString(); + } + + #endregion + + #region SimpleConverter2 + + [Test] + public void SimpleConverter2Test() + { + var cacheMock = new Mock(); + var cacheContent = new Dictionary(); + cacheMock.Setup(x => x.GetById(It.IsAny())).Returns(id => cacheContent.TryGetValue(id, out IPublishedContent content) ? content : null); + var facadeMock = new Mock(); + facadeMock.Setup(x => x.ContentCache).Returns(cacheMock.Object); + var facadeAccessorMock = new Mock(); + facadeAccessorMock.Setup(x => x.Facade).Returns(facadeMock.Object); + var facadeAccessor = facadeAccessorMock.Object; + + var converters = new PropertyValueConverterCollection(new IPropertyValueConverter[] + { + new SimpleConverter2(facadeAccessor), + }); + + var setType1 = new PublishedContentType(1000, "set1", new[] + { + new PublishedPropertyType("prop1", "editor2", converters), + }); + + var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "1234" } }, false); + + var cntType1 = new PublishedContentType(1001, "cnt1", Array.Empty()); + var cnt1 = new TestPublishedContent(cntType1, 1234, Guid.NewGuid(), new Dictionary(), false); + cacheContent[cnt1.Id] = cnt1; + + Assert.AreSame(cnt1, set1.Value("prop1")); + } + + private class SimpleConverter2 : IPropertyValueConverter + { + private readonly IFacadeAccessor _facadeAccessor; + private readonly PropertyCacheLevel _cacheLevel; + + public SimpleConverter2(IFacadeAccessor facadeAccessor, PropertyCacheLevel cacheLevel = PropertyCacheLevel.None) + { + _facadeAccessor = facadeAccessor; + _cacheLevel = cacheLevel; + } + + public bool IsConverter(PublishedPropertyType propertyType) + => propertyType.PropertyEditorAlias.InvariantEquals("editor2"); + + public Type GetPropertyValueType(PublishedPropertyType propertyType) + // the first version would be the "generic" version, but say we want to be more precise + // and return: whatever Clr type is generated for content type with alias "cnt1" -- which + // we cannot really typeof() at the moment because it has not been generated, hence ModelType. + // => typeof (IPublishedContent); + => ModelType.For("cnt1"); + + public PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) + => _cacheLevel; + + public object ConvertSourceToInter(IPropertySet owner, PublishedPropertyType propertyType, object source, bool preview) + => int.TryParse(source as string, out int i) ? i : -1; + + public object ConvertInterToObject(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + => _facadeAccessor.Facade.ContentCache.GetById((int) inter); + + public object ConvertInterToXPath(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + => ((int) inter).ToString(); + } + + #endregion + + #region SimpleConverter3 + + [Test] + public void SimpleConverter3Test() { Current.Reset(); var container = new ServiceContainer(); container.ConfigureUmbracoCore(); Current.Container.RegisterCollectionBuilder() - .Append() - .Append(); + .Append() + .Append(); IPublishedContentModelFactory factory = new PublishedContentModelFactory(new[] { @@ -79,6 +205,15 @@ namespace Umbraco.Tests.PublishedContent }); Current.Container.Register(f => factory); + var cacheMock = new Mock(); + var cacheContent = new Dictionary(); + cacheMock.Setup(x => x.GetById(It.IsAny())).Returns(id => cacheContent.TryGetValue(id, out IPublishedContent content) ? content : null); + var facadeMock = new Mock(); + facadeMock.Setup(x => x.ContentCache).Returns(cacheMock.Object); + var facadeAccessorMock = new Mock(); + facadeAccessorMock.Setup(x => x.Facade).Returns(facadeMock.Object); + Current.Container.Register(f => facadeAccessorMock.Object); + var setType1 = new PublishedContentType(1000, "set1", new[] { new PublishedPropertyType("prop1", "editor1"), @@ -101,41 +236,31 @@ namespace Umbraco.Tests.PublishedContent var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "val1" } }, false); var set2 = new PropertySet(setType2, Guid.NewGuid(), new Dictionary { { "prop2", "1003" } }, false); - var cnt1 = new TestPublishedContent(contentType1, Guid.NewGuid(), new Dictionary { { "prop1", "val1" } }, false); - var cnt2 = new TestPublishedContent(contentType2, Guid.NewGuid(), new Dictionary { { "prop2", "1003" } }, false); + var cnt1 = new TestPublishedContent(contentType1, 1003, Guid.NewGuid(), new Dictionary { { "prop1", "val1" } }, false); + var cnt2 = new TestPublishedContent(contentType2, 1004, Guid.NewGuid(), new Dictionary { { "prop2", "1003" } }, false); - var cache = new Dictionary - { - { 1003, cnt1.CreateModel() }, - { 1004, cnt2.CreateModel() }, - }; - - var facadeMock = new Mock(); - var cacheMock = new Mock(); - cacheMock.Setup(x => x.GetById(It.IsAny())).Returns(id => cache.TryGetValue(id, out IPublishedContent content) ? content : null); - facadeMock.Setup(x => x.ContentCache).Returns(cacheMock.Object); - var facade = facadeMock.Object; - Current.Container.Register(f => facade); + cacheContent[cnt1.Id] = cnt1.CreateModel(); + cacheContent[cnt2.Id] = cnt2.CreateModel(); // can get the actual property Clr type // ie ModelType gets properly mapped by IPublishedContentModelFactory // must test ModelClrType with special equals 'cos they are not ref-equals - Assert.IsTrue(ModelType.Equals(typeof (IEnumerable<>).MakeGenericType(ModelType.For("content1")), contentType2.GetPropertyType("prop2").ModelClrType)); - Assert.AreEqual(typeof (IEnumerable), contentType2.GetPropertyType("prop2").ClrType); + Assert.IsTrue(ModelType.Equals(typeof(IEnumerable<>).MakeGenericType(ModelType.For("content1")), contentType2.GetPropertyType("prop2").ModelClrType)); + Assert.AreEqual(typeof(IEnumerable), contentType2.GetPropertyType("prop2").ClrType); // can create a model for a property set var model1 = factory.CreateModel(set1); Assert.IsInstanceOf(model1); - Assert.AreEqual("val1", ((TestSetModel1) model1).Prop1); + Assert.AreEqual("val1", ((TestSetModel1)model1).Prop1); // can create a model for a published content var model2 = factory.CreateModel(set2); Assert.IsInstanceOf(model2); - var mmodel2 = (TestSetModel2) model2; + var mmodel2 = (TestSetModel2)model2; // and get direct property Assert.IsInstanceOf(model2.Value("prop2")); - Assert.AreEqual(1, ((TestContentModel1[]) model2.Value("prop2")).Length); + Assert.AreEqual(1, ((TestContentModel1[])model2.Value("prop2")).Length); // and get model property Assert.IsInstanceOf>(mmodel2.Prop2); @@ -143,42 +268,299 @@ namespace Umbraco.Tests.PublishedContent var mmodel1 = mmodel2.Prop2.First(); // and we get what we want - Assert.AreSame(cache[1003], mmodel1); + Assert.AreSame(cacheContent[mmodel1.Id], mmodel1); } - internal class TestPublishedContent : PropertySet, IPublishedContent + public class SimpleConverter3A : PropertyValueConverterBase { - public TestPublishedContent(PublishedContentType contentType, Guid key, Dictionary values, bool previewing) - : base(contentType, key, values, previewing) - { } + public override bool IsConverter(PublishedPropertyType propertyType) + => propertyType.PropertyEditorAlias == "editor1"; - public int Id { get; } - public int TemplateId { get; } - public int SortOrder { get; } - public string Name { get; } - public string UrlName { get; } - public string DocumentTypeAlias { get; } - public int DocumentTypeId { get; } - public string WriterName { get; } - public string CreatorName { get; } - public int WriterId { get; } - public int CreatorId { get; } - public string Path { get; } - public DateTime CreateDate { get; } - public DateTime UpdateDate { get; } - public Guid Version { get; } - public int Level { get; } - public string Url { get; } - public PublishedItemType ItemType { get; } - public bool IsDraft { get; } - public IPublishedContent Parent { get; } - public IEnumerable Children { get; } - public IPublishedProperty GetProperty(string alias, bool recurse) + public override Type GetPropertyValueType(PublishedPropertyType propertyType) + => typeof (string); + + public override PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) + => PropertyCacheLevel.Content; + } + + public class SimpleConverter3B : PropertyValueConverterBase + { + private readonly IFacadeAccessor _facadeAccessor; + + public SimpleConverter3B(IFacadeAccessor facadeAccessor) { - throw new NotImplementedException(); + _facadeAccessor = facadeAccessor; + } + + public override bool IsConverter(PublishedPropertyType propertyType) + => propertyType.PropertyEditorAlias == "editor2"; + + public override Type GetPropertyValueType(PublishedPropertyType propertyType) + => typeof (IEnumerable<>).MakeGenericType(ModelType.For("content1")); + + public override PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) + => PropertyCacheLevel.Snapshot; + + public override object ConvertSourceToInter(IPropertySet owner, PublishedPropertyType propertyType, object source, bool preview) + { + var s = source as string; + return s?.Split(',').Select(int.Parse).ToArray() ?? Array.Empty(); + } + + public override object ConvertInterToObject(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + { + return ((int[]) inter).Select(x => (TestContentModel1) _facadeAccessor.Facade.ContentCache.GetById(x)).ToArray(); } } + #endregion + + #region ConversionCache + + [TestCase(PropertyCacheLevel.None, 2)] + [TestCase(PropertyCacheLevel.Content, 1)] + [TestCase(PropertyCacheLevel.Snapshot, 1)] + [TestCase(PropertyCacheLevel.Facade, 1)] + public void CacheLevelTest(PropertyCacheLevel cacheLevel, int interConverts) + { + var converter = new CacheConverter1(cacheLevel); + + var converters = new PropertyValueConverterCollection(new IPropertyValueConverter[] + { + converter, + }); + + var setType1 = new PublishedContentType(1000, "set1", new[] + { + new PublishedPropertyType("prop1", "editor1", converters), + }); + + // PropertySetPropertyBase.GetCacheLevels: + // + // if property level is > reference level, or both are None + // use None for property & new reference + // else + // use Content for property, & keep reference + // + // PropertySet creates properties with reference being None + // if converter specifies None, keep using None + // anything else is not > None, use Content + // + // for standalone property sets, it's only None or Content + + var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "1234" } }, false); + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + Assert.AreEqual(1, converter.InterConverts); + + // source is always converted once and cached per content + // inter conversion depends on the specified cache level + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + Assert.AreEqual(interConverts, converter.InterConverts); + } + + // property is not cached, converted cached at Content, exept + // /None = not cached at all + [TestCase(PropertyCacheLevel.None, PropertyCacheLevel.None, 2, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.None, PropertyCacheLevel.Content, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.None, PropertyCacheLevel.Snapshot, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.None, PropertyCacheLevel.Facade, 1, 0, 0, 0, 0)] + + // property is cached at content level, converted cached at + // /None = not at all + // /Content = in content + // /Facade = in facade + // /Snapshot = in snapshot + [TestCase(PropertyCacheLevel.Content, PropertyCacheLevel.None, 2, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Content, PropertyCacheLevel.Content, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Content, PropertyCacheLevel.Snapshot, 1, 1, 0, 1, 0)] + [TestCase(PropertyCacheLevel.Content, PropertyCacheLevel.Facade, 1, 0, 1, 0, 1)] + + // property is cached at snapshot level, converted cached at Content, exept + // /None = not cached at all + // /Facade = cached in facade + [TestCase(PropertyCacheLevel.Snapshot, PropertyCacheLevel.None, 2, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Snapshot, PropertyCacheLevel.Content, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Snapshot, PropertyCacheLevel.Snapshot, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Snapshot, PropertyCacheLevel.Facade, 1, 0, 1, 0, 1)] + + // property is cached at facade level, converted cached at Content, exept + // /None = not cached at all + [TestCase(PropertyCacheLevel.Facade, PropertyCacheLevel.None, 2, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Facade, PropertyCacheLevel.Content, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Facade, PropertyCacheLevel.Snapshot, 1, 0, 0, 0, 0)] + [TestCase(PropertyCacheLevel.Facade, PropertyCacheLevel.Facade, 1, 0, 0, 0, 0)] + + public void CacheFacadeTest(PropertyCacheLevel referenceCacheLevel, PropertyCacheLevel converterCacheLevel, int interConverts, + int snapshotCount1, int facadeCount1, int snapshotCount2, int facadeCount2) + { + var converter = new CacheConverter1(converterCacheLevel); + + var converters = new PropertyValueConverterCollection(new IPropertyValueConverter[] + { + converter, + }); + + var setType1 = new PublishedContentType(1000, "set1", new[] + { + new PublishedPropertyType("prop1", "editor1", converters), + }); + + var snapshotCache = new Dictionary(); + var facadeCache = new Dictionary(); + + var facadeServiceMock = new Mock(); + facadeServiceMock + .Setup(x => x.CreateSetProperty(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((propertyType, set, previewing, refCacheLevel, value) => + { + return new TestPropertySetProperty(propertyType, set, previewing, refCacheLevel, value, () => snapshotCache, () => facadeCache); + }); + var facadeService = facadeServiceMock.Object; + + // pretend we're creating this set as a value for a property + // referenceCacheLevel is the cache level for this fictious property + // converterCacheLevel is the cache level specified by the converter + + var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "1234" } }, false, facadeService, referenceCacheLevel); + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + Assert.AreEqual(1, converter.InterConverts); + + Assert.AreEqual(snapshotCount1, snapshotCache.Count); + Assert.AreEqual(facadeCount1, facadeCache.Count); + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + Assert.AreEqual(interConverts, converter.InterConverts); + + Assert.AreEqual(snapshotCount2, snapshotCache.Count); + Assert.AreEqual(facadeCount2, facadeCache.Count); + + var oldFacadeCache = facadeCache; + facadeCache = new Dictionary(); + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + + Assert.AreEqual(snapshotCount2, snapshotCache.Count); + Assert.AreEqual(facadeCount2, facadeCache.Count); + + Assert.AreEqual((interConverts == 1 ? 1 : 3) + facadeCache.Count, converter.InterConverts); + + var oldSnapshotCache = snapshotCache; + snapshotCache = new Dictionary(); + + Assert.AreEqual(1234, set1.Value("prop1")); + Assert.AreEqual(1, converter.SourceConverts); + + Assert.AreEqual(snapshotCount2, snapshotCache.Count); + Assert.AreEqual(facadeCount2, facadeCache.Count); + + Assert.AreEqual((interConverts == 1 ? 1 : 4) + facadeCache.Count + snapshotCache.Count, converter.InterConverts); + } + + [Test] + public void CacheUnknownTest() + { + var converter = new CacheConverter1(PropertyCacheLevel.Unknown); + + var converters = new PropertyValueConverterCollection(new IPropertyValueConverter[] + { + converter, + }); + + var setType1 = new PublishedContentType(1000, "set1", new[] + { + new PublishedPropertyType("prop1", "editor1", converters), + }); + + Assert.Throws(() => + { + var set1 = new PropertySet(setType1, Guid.NewGuid(), new Dictionary { { "prop1", "1234" } }, false); + }); + } + + private class CacheConverter1 : IPropertyValueConverter + { + private readonly PropertyCacheLevel _cacheLevel; + + public CacheConverter1(PropertyCacheLevel cacheLevel) + { + _cacheLevel = cacheLevel; + } + + public int SourceConverts { get; private set; } + public int InterConverts { get; private set; } + + public bool IsConverter(PublishedPropertyType propertyType) + => propertyType.PropertyEditorAlias.InvariantEquals("editor1"); + + public Type GetPropertyValueType(PublishedPropertyType propertyType) + => typeof(int); + + public PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) + => _cacheLevel; + + public object ConvertSourceToInter(IPropertySet owner, PublishedPropertyType propertyType, object source, bool preview) + { + SourceConverts++; + return int.TryParse(source as string, out int i) ? i : 0; + } + + public object ConvertInterToObject(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + { + InterConverts++; + return (int) inter; + } + + public object ConvertInterToXPath(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) + => ((int) inter).ToString(); + } + + private class TestPropertySetProperty : PropertySetPropertyBase + { + private readonly Func> _getSnapshotCache; + private readonly Func> _getFacadeCache; + private string _valuesCacheKey; + + public TestPropertySetProperty(PublishedPropertyType propertyType, IPropertySet set, bool previewing, PropertyCacheLevel referenceCacheLevel, object sourceValue, + Func> getSnapshotCache, Func> getFacadeCache) + : base(propertyType, set, previewing, referenceCacheLevel, sourceValue) + { + _getSnapshotCache = getSnapshotCache; + _getFacadeCache = getFacadeCache; + } + + private string ValuesCacheKey => _valuesCacheKey ?? (_valuesCacheKey = $"CacheValues[{(IsPreviewing ? "D" : "P")}{Set.Key}:{PropertyType.PropertyTypeAlias}"); + + protected override CacheValues GetSnapshotCacheValues() + { + var snapshotCache = _getSnapshotCache(); + if (snapshotCache.TryGetValue(ValuesCacheKey, out object cacheValues)) + return (CacheValues) cacheValues; + snapshotCache[ValuesCacheKey] = cacheValues = new CacheValues(); + return (CacheValues) cacheValues; + } + + protected override CacheValues GetFacadeCacheValues() + { + var facadeCache = _getFacadeCache(); + if (facadeCache.TryGetValue(ValuesCacheKey, out object cacheValues)) + return (CacheValues)cacheValues; + facadeCache[ValuesCacheKey] = cacheValues = new CacheValues(); + return (CacheValues)cacheValues; + } + } + + #endregion + + #region Model classes + [PublishedContentModel("set1")] public class TestSetModel1 : PropertySetModel { @@ -219,47 +601,64 @@ namespace Umbraco.Tests.PublishedContent public IEnumerable Prop2 => this.Value>("prop2"); } - public class TestConverter1 : PropertyValueConverterBase + #endregion + + #region Support classes + + internal class TestPublishedContent : PropertySet, IPublishedContent { - public override bool IsConverter(PublishedPropertyType propertyType) - => propertyType.PropertyEditorAlias == "editor1"; + public TestPublishedContent(PublishedContentType contentType, int id, Guid key, Dictionary values, bool previewing) + : base(contentType, key, values, previewing) + { + Id = id; + } - public override Type GetPropertyValueType(PublishedPropertyType propertyType) - => typeof (string); + public int Id { get; } + public int TemplateId { get; set; } + public int SortOrder { get; set; } + public string Name { get; set; } + public string UrlName { get; set; } + public string DocumentTypeAlias => ContentType.Alias; + public int DocumentTypeId { get; set; } + public string WriterName { get; set; } + public string CreatorName { get; set; } + public int WriterId { get; set; } + public int CreatorId { get; set; } + public string Path { get; set; } + public DateTime CreateDate { get; set; } + public DateTime UpdateDate { get; set; } + public Guid Version { get; set; } + public int Level { get; set; } + public string Url { get; set; } + public PublishedItemType ItemType => ContentType.ItemType; + public bool IsDraft { get; set; } + public IPublishedContent Parent { get; set; } + public IEnumerable Children { get; set; } - public override PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) - => PropertyCacheLevel.Content; + // copied from PublishedContentBase + public IPublishedProperty GetProperty(string alias, bool recurse) + { + var property = GetProperty(alias); + if (recurse == false) return property; + + IPublishedContent content = this; + var firstNonNullProperty = property; + while (content != null && (property == null || property.HasValue == false)) + { + content = content.Parent; + property = content?.GetProperty(alias); + if (firstNonNullProperty == null && property != null) firstNonNullProperty = property; + } + + // if we find a content with the property with a value, return that property + // if we find no content with the property, return null + // if we find a content with the property without a value, return that property + // have to save that first property while we look further up, hence firstNonNullProperty + + return property != null && property.HasValue ? property : firstNonNullProperty; + } } - public class TestConverter2 : PropertyValueConverterBase - { - private readonly IFacade _facade; - - public TestConverter2(IFacade facade) - { - _facade = facade; - } - - public override bool IsConverter(PublishedPropertyType propertyType) - => propertyType.PropertyEditorAlias == "editor2"; - - // pretend ... when writing the converter, the model type for alias "set1" does not exist yet - public override Type GetPropertyValueType(PublishedPropertyType propertyType) - => typeof (IEnumerable<>).MakeGenericType(ModelType.For("content1")); - - public override PropertyCacheLevel GetPropertyCacheLevel(PublishedPropertyType propertyType) - => PropertyCacheLevel.Snapshot; - - public override object ConvertSourceToInter(IPropertySet owner, PublishedPropertyType propertyType, object source, bool preview) - { - var s = source as string; - return s?.Split(',').Select(int.Parse).ToArray() ?? Array.Empty(); - } - - public override object ConvertInterToObject(IPropertySet owner, PublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object inter, bool preview) - { - return ((int[]) inter).Select(x => (TestContentModel1) _facade.ContentCache.GetById(x)).ToArray(); - } - } + #endregion } } diff --git a/src/Umbraco.Web/Models/PublishedContentBase.cs b/src/Umbraco.Web/Models/PublishedContentBase.cs index 1dac635a0a..b39528fea1 100644 --- a/src/Umbraco.Web/Models/PublishedContentBase.cs +++ b/src/Umbraco.Web/Models/PublishedContentBase.cs @@ -179,7 +179,7 @@ namespace Umbraco.Web.Models while (content != null && (property == null || property.HasValue == false)) { content = content.Parent; - property = content == null ? null : content.GetProperty(alias); + property = content?.GetProperty(alias); if (firstNonNullProperty == null && property != null) firstNonNullProperty = property; } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Property.cs b/src/Umbraco.Web/PublishedCache/NuCache/Property.cs index 2e7ee9275d..2f0042e386 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/Property.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/Property.cs @@ -55,7 +55,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } public override bool HasValue => _sourceValue != null - && ((_sourceValue is string) == false || string.IsNullOrWhiteSpace((string)_sourceValue) == false); + && (!(_sourceValue is string) || string.IsNullOrWhiteSpace((string) _sourceValue) == false); private class CacheValues { diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PropertySetProperty.cs b/src/Umbraco.Web/PublishedCache/NuCache/PropertySetProperty.cs index eaad29a993..70be29721f 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PropertySetProperty.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PropertySetProperty.cs @@ -27,13 +27,13 @@ namespace Umbraco.Web.PublishedCache.NuCache // snapshot cache (if we don't want to pollute the snapshot cache with short-lived // data) depending on settings // for members, always cache in the facade cache - never pollute snapshot cache - var facade = (Facade)_facadeAccessor.Facade; + var facade = (Facade) _facadeAccessor.Facade; var cache = facade == null ? null : ((IsPreviewing == false || FacadeService.FullCacheWhenPreviewing) && (IsMember == false) ? facade.SnapshotCache : facade.FacadeCache); - return GetCacheValues(cache); + return GetCacheValues(cache); } protected override CacheValues GetFacadeCacheValues() diff --git a/src/Umbraco.Web/PublishedCache/PropertySetProperty.cs b/src/Umbraco.Web/PublishedCache/PropertySetProperty.cs index 784351115d..3bf0e29c10 100644 --- a/src/Umbraco.Web/PublishedCache/PropertySetProperty.cs +++ b/src/Umbraco.Web/PublishedCache/PropertySetProperty.cs @@ -4,7 +4,7 @@ using Umbraco.Core.PropertyEditors; namespace Umbraco.Web.PublishedCache { - class PropertySetProperty : PropertySetPropertyBase + internal class PropertySetProperty : PropertySetPropertyBase { public PropertySetProperty(PublishedPropertyType propertyType, IPropertySet set, bool previewing, PropertyCacheLevel cacheLevel, object sourceValue = null) : base(propertyType, set, previewing, cacheLevel, sourceValue) diff --git a/src/Umbraco.Web/PublishedCache/PropertySetPropertyBase.cs b/src/Umbraco.Web/PublishedCache/PropertySetPropertyBase.cs index f7d39c62fa..2036671efb 100644 --- a/src/Umbraco.Web/PublishedCache/PropertySetPropertyBase.cs +++ b/src/Umbraco.Web/PublishedCache/PropertySetPropertyBase.cs @@ -28,7 +28,7 @@ namespace Umbraco.Web.PublishedCache } public override bool HasValue => _sourceValue != null - && ((_sourceValue is string) == false || string.IsNullOrWhiteSpace((string)_sourceValue) == false); + && (!(_sourceValue is string) || string.IsNullOrWhiteSpace((string) _sourceValue) == false); protected class CacheValues { @@ -38,30 +38,12 @@ namespace Umbraco.Web.PublishedCache public object XPathValue; } - private static void ValidateCacheLevel(PropertyCacheLevel cacheLevel) - { - switch (cacheLevel) - { - case PropertyCacheLevel.Content: - case PropertyCacheLevel.Snapshot: - case PropertyCacheLevel.Facade: - case PropertyCacheLevel.None: - break; - default: - throw new Exception("Invalid cache level."); - } - } - private void GetCacheLevels(out PropertyCacheLevel cacheLevel, out PropertyCacheLevel referenceCacheLevel) { // based upon the current reference cache level (ReferenceCacheLevel) and this property // cache level (PropertyType.CacheLevel), determines both the actual cache level for the // property, and the new reference cache level. - // sanity checks - ValidateCacheLevel(ReferenceCacheLevel); - ValidateCacheLevel(PropertyType.CacheLevel); - // if the property cache level is 'shorter-termed' that the reference // then use it and it becomes the new reference, else use Content and // don't change the reference. @@ -71,7 +53,7 @@ namespace Umbraco.Web.PublishedCache // snapshot, ok to use content. OTOH, currently caching at snapshot, // property specifies facade, need to use facade. // - if (PropertyType.CacheLevel > ReferenceCacheLevel) + if (PropertyType.CacheLevel > ReferenceCacheLevel || PropertyType.CacheLevel == PropertyCacheLevel.None) { cacheLevel = PropertyType.CacheLevel; referenceCacheLevel = cacheLevel; @@ -130,8 +112,7 @@ namespace Umbraco.Web.PublishedCache { lock (_locko) { - PropertyCacheLevel cacheLevel, referenceCacheLevel; - GetCacheLevels(out cacheLevel, out referenceCacheLevel); + GetCacheLevels(out PropertyCacheLevel cacheLevel, out PropertyCacheLevel referenceCacheLevel); var cacheValues = GetCacheValues(cacheLevel); if (cacheValues.ObjectInitialized) return cacheValues.ObjectValue; @@ -149,8 +130,7 @@ namespace Umbraco.Web.PublishedCache { lock (_locko) { - PropertyCacheLevel cacheLevel, referenceCacheLevel; - GetCacheLevels(out cacheLevel, out referenceCacheLevel); + GetCacheLevels(out PropertyCacheLevel cacheLevel, out PropertyCacheLevel referenceCacheLevel); var cacheValues = GetCacheValues(cacheLevel); if (cacheValues.XPathInitialized) return cacheValues.XPathValue; diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/FacadeService.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/FacadeService.cs index e7f314d730..719a49bd1e 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/FacadeService.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/FacadeService.cs @@ -20,7 +20,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache /// /// Implements a facade service. /// - class FacadeService : FacadeServiceBase + internal class FacadeService : FacadeServiceBase { private readonly XmlStore _xmlStore; private readonly RoutesCache _routesCache; @@ -242,6 +242,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache public override IPublishedProperty CreateSetProperty(PublishedPropertyType propertyType, IPropertySet set, bool previewing, PropertyCacheLevel referenceCacheLevel, object sourceValue = null) { + // fixme - ouch? throw new NotImplementedException(); }