Elements level property cache should cache by variation (#18080)

This commit is contained in:
Kenn Jacobsen
2025-01-29 12:00:01 +01:00
committed by GitHub
parent 28019e47a9
commit 9c6e3ff928
14 changed files with 126 additions and 32 deletions

View File

@@ -1,3 +1,5 @@
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PropertyEditors;
@@ -16,9 +18,28 @@ public class PublishedElement : IPublishedElement
private readonly IPublishedProperty[] _propertiesArray;
[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing)
: this(contentType, key, values, previewing, PropertyCacheLevel.None, null)
{
}
[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, ICacheManager? cacheManager)
: this(
contentType,
key,
values,
previewing,
referenceCacheLevel,
StaticServiceProvider.Instance.GetRequiredService<IVariationContextAccessor>().VariationContext ?? new VariationContext(),
cacheManager)
{
}
// initializes a new instance of the PublishedElement class
// within the context of a published snapshot service (eg a published content property value)
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, ICacheManager? cacheManager)
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, VariationContext variationContext, ICacheManager? cacheManager)
{
if (key == Guid.Empty)
{
@@ -40,7 +61,7 @@ public class PublishedElement : IPublishedElement
.Select(propertyType =>
{
values.TryGetValue(propertyType.Alias, out var value);
return (IPublishedProperty)new PublishedElementPropertyBase(propertyType, this, previewing, referenceCacheLevel,cacheManager, value);
return (IPublishedProperty)new PublishedElementPropertyBase(propertyType, this, previewing, referenceCacheLevel, variationContext, cacheManager, value);
})
.ToArray()
?? new IPublishedProperty[0];
@@ -51,8 +72,8 @@ public class PublishedElement : IPublishedElement
// + using an initial reference cache level of .None ensures that everything will be
// cached at .Content level - and that reference cache level will propagate to all
// properties
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing)
: this(contentType, key, values, previewing, PropertyCacheLevel.None, null)
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing, VariationContext variationContext)
: this(contentType, key, values, previewing, PropertyCacheLevel.None, variationContext, null)
{
}

View File

@@ -1,4 +1,3 @@
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Extensions;
@@ -13,11 +12,11 @@ internal class PublishedElementPropertyBase : PublishedPropertyBase
// to store eg routes, property converted values, anything - caching
// means faster execution, but uses memory - not sure if we want it
// so making it configurable.
private const bool FullCacheWhenPreviewing = true;
private readonly Lock _locko = new();
private readonly object? _sourceValue;
protected readonly bool IsMember;
protected readonly bool IsPreviewing;
private readonly VariationContext _variationContext;
private readonly ICacheManager? _cacheManager;
private CacheValues? _cacheValues;
@@ -30,6 +29,7 @@ internal class PublishedElementPropertyBase : PublishedPropertyBase
IPublishedElement element,
bool previewing,
PropertyCacheLevel referenceCacheLevel,
VariationContext variationContext,
ICacheManager? cacheManager,
object? sourceValue = null)
: base(propertyType, referenceCacheLevel)
@@ -37,17 +37,22 @@ internal class PublishedElementPropertyBase : PublishedPropertyBase
_sourceValue = sourceValue;
Element = element;
IsPreviewing = previewing;
_variationContext = variationContext;
_cacheManager = cacheManager;
IsMember = propertyType.ContentType?.ItemType == PublishedItemType.Member;
}
// used to cache the CacheValues of this property
// ReSharper disable InconsistentlySynchronizedField
internal string ValuesCacheKey => _valuesCacheKey ??= PropertyCacheValues(Element.Key, Alias, IsPreviewing);
private string ValuesCacheKey => _valuesCacheKey ??= PropertyCacheValuesKey();
[Obsolete("Do not use this. Will be removed in V17.")]
public static string PropertyCacheValues(Guid contentUid, string typeAlias, bool previewing) =>
"PublishedSnapshot.Property.CacheValues[" + (previewing ? "D:" : "P:") + contentUid + ":" + typeAlias + "]";
private string PropertyCacheValuesKey() =>
$"PublishedSnapshot.Property.CacheValues[{(IsPreviewing ? "D:" : "P:")}{Element.Key}:{Alias}:{_variationContext.Culture.IfNullOrWhiteSpace("inv")}+{_variationContext.Segment.IfNullOrWhiteSpace("inv")}]";
// ReSharper restore InconsistentlySynchronizedField
public override bool HasValue(string? culture = null, string? segment = null)
{

View File

@@ -91,7 +91,7 @@ public sealed class BlockEditorConverter
return null;
}
IPublishedElement element = new PublishedElement(publishedContentType, key, propertyValues, preview, referenceCacheLevel, _cacheManager);
IPublishedElement element = new PublishedElement(publishedContentType, key, propertyValues, preview, referenceCacheLevel, variationContext, _cacheManager);
element = _publishedModelFactory.CreateModel(element);
return element;

View File

@@ -1869,4 +1869,67 @@ internal partial class BlockListElementLevelVariationTests
Assert.AreEqual("blocks", publishResult.InvalidProperties.First().Alias);
});
}
[Test]
public async Task Can_Handle_Elements_Level_Property_Cache()
{
var elementType = new ContentTypeBuilder()
.WithAlias("myElementType")
.WithName("My Element Type")
.WithIsElement(true)
.WithContentVariation(ContentVariation.Culture)
.AddPropertyType()
.WithAlias("contentPicker")
.WithName("Content Picker")
.WithDataTypeId(1046)
.WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.ContentPicker)
.WithValueStorageType(ValueStorageType.Nvarchar)
.WithVariations(ContentVariation.Culture)
.Done()
.Build();
ContentTypeService.Save(elementType);
var blockListDataType = await CreateBlockListDataType(elementType);
var contentType = CreateContentType(ContentVariation.Culture, blockListDataType);
var pickedContent1 = CreateContent(contentType, elementType, [], true);
var pickedContent2 = CreateContent(contentType, elementType, [], true);
var content = CreateContent(
contentType,
elementType,
[
new BlockProperty(
new List<BlockPropertyValue>
{
new() { Alias = "contentPicker", Value = pickedContent1.GetUdi().ToString(), Culture = "en-US" },
new() { Alias = "contentPicker", Value = pickedContent2.GetUdi().ToString(), Culture = "da-DK" },
},
[],
null,
null)
],
true);
AssertPropertyValues("en-US", pickedContent1);
AssertPropertyValues("da-DK", pickedContent2);
void AssertPropertyValues(string culture, IContent expectedPickedContent)
{
SetVariationContext(culture, null);
var publishedContent = GetPublishedContent(content.Key);
var value = publishedContent.Value<BlockListModel>("blocks");
Assert.IsNotNull(value);
Assert.AreEqual(1, value.Count);
var blockListItem = value.First();
Assert.AreEqual(1, blockListItem.Content.Properties.Count());
// need to ensure the property type cache level, otherwise this test has little value
Assert.AreEqual(PropertyCacheLevel.Elements, blockListItem.Content.Properties.First().PropertyType.CacheLevel);
var actualPickedPublishedContent = blockListItem.Content.Value<IPublishedContent>("contentPicker");
Assert.IsNotNull(actualPickedPublishedContent);
Assert.AreEqual(expectedPickedContent.Key, actualPickedPublishedContent.Key);
}
}
}

View File

@@ -36,7 +36,7 @@ public class CacheTests : DeliveryApiTests
var element = new Mock<IPublishedElement>();
var prop1 = new PublishedElementPropertyBase(propertyType, element.Object, false, cacheLevel, Mock.Of<ICacheManager>());
var prop1 = new PublishedElementPropertyBase(propertyType, element.Object, false, cacheLevel, new VariationContext(), Mock.Of<ICacheManager>());
var results = new List<string>
{

View File

@@ -17,8 +17,8 @@ public class ContentBuilderTests : DeliveryApiTests
{
var content = new Mock<IPublishedContent>();
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
var contentType = new Mock<IPublishedContentType>();
contentType.SetupGet(c => c.Alias).Returns("thePageType");

View File

@@ -72,8 +72,8 @@ public class ContentPickerValueConverterTests : PropertyValueConverterTests
{
var content = new Mock<IPublishedContent>();
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
var publishedPropertyType = new Mock<IPublishedPropertyType>();
publishedPropertyType.SetupGet(p => p.Alias).Returns("test");

View File

@@ -101,8 +101,8 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest
{
var content = new Mock<IPublishedContent>();
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), CacheManager);
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), CacheManager);
var key = Guid.NewGuid();
var urlSegment = "page-url-segment";

View File

@@ -46,8 +46,8 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
var apiContentBuilder = new ApiContentBuilder(new ApiContentNameProvider(), ApiContentRouteBuilder(), accessor);
var content = new Mock<IPublishedContent>();
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager);
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager);
var contentPickerContent = CreateSimplePickedContent(123, 456);
var contentPickerProperty = CreateContentPickerProperty(content.Object, contentPickerContent.Key, "contentPicker", apiContentBuilder);
@@ -303,7 +303,7 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
.Returns(expanding ? "Expanding" : "Not expanding");
var propertyType = SetupPublishedPropertyType(valueConverterMock.Object, "theAlias", Constants.PropertyEditors.Aliases.Label);
var property = new PublishedElementPropertyBase(propertyType, content.Object, false, PropertyCacheLevel.None, CacheManager, "The Value");
var property = new PublishedElementPropertyBase(propertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager, "The Value");
SetupContentMock(content, property);
@@ -378,7 +378,7 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
ContentPickerValueConverter contentPickerValueConverter = new ContentPickerValueConverter(PublishedContentCacheMock.Object, contentBuilder);
var contentPickerPropertyType = SetupPublishedPropertyType(contentPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.ContentPicker);
return new PublishedElementPropertyBase(contentPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, new GuidUdi(Constants.UdiEntityType.Document, pickedContentKey).ToString());
return new PublishedElementPropertyBase(contentPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, new GuidUdi(Constants.UdiEntityType.Document, pickedContentKey).ToString());
}
internal PublishedElementPropertyBase CreateMediaPickerProperty(IPublishedElement parent, Guid pickedMediaKey, string propertyTypeAlias, IApiMediaBuilder mediaBuilder)
@@ -389,7 +389,7 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
MediaPickerWithCropsValueConverter mediaPickerValueConverter = new MediaPickerWithCropsValueConverter(CacheManager.Media, PublishedUrlProvider, publishedValueFallback, new SystemTextJsonSerializer(), apiMediaWithCropsBuilder);
var mediaPickerPropertyType = SetupPublishedPropertyType(mediaPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.MediaPicker3, new MediaPicker3Configuration());
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, new GuidUdi(Constants.UdiEntityType.Media, pickedMediaKey).ToString());
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, new GuidUdi(Constants.UdiEntityType.Media, pickedMediaKey).ToString());
}
internal PublishedElementPropertyBase CreateMediaPicker3Property(IPublishedElement parent, Guid pickedMediaKey, string propertyTypeAlias, IApiMediaBuilder mediaBuilder)
@@ -409,13 +409,13 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
MediaPickerWithCropsValueConverter mediaPickerValueConverter = new MediaPickerWithCropsValueConverter(CacheManager.Media, PublishedUrlProvider, publishedValueFallback, new SystemTextJsonSerializer(), apiMediaWithCropsBuilder);
var mediaPickerPropertyType = SetupPublishedPropertyType(mediaPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.MediaPicker3, new MediaPicker3Configuration());
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, value);
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, value);
}
internal PublishedElementPropertyBase CreateNumberProperty(IPublishedElement parent, int propertyValue, string propertyTypeAlias)
{
var numberPropertyType = SetupPublishedPropertyType(new IntegerValueConverter(), propertyTypeAlias, Constants.PropertyEditors.Aliases.Label);
return new PublishedElementPropertyBase(numberPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, propertyValue);
return new PublishedElementPropertyBase(numberPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, propertyValue);
}
internal PublishedElementPropertyBase CreateElementProperty(
@@ -452,7 +452,7 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe
elementValueConverter.Setup(p => p.GetDeliveryApiPropertyCacheLevelForExpansion(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
var elementPropertyType = SetupPublishedPropertyType(elementValueConverter.Object, elementPropertyAlias, "My.Element.Property");
return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None, CacheManager);
return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager);
}
protected IApiContentRouteBuilder ApiContentRouteBuilder() => CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings());

View File

@@ -29,6 +29,8 @@ public class PropertyValueConverterTests : DeliveryApiTests
protected Mock<IPublishedUrlProvider> PublishedUrlProviderMock { get; private set; }
protected VariationContext VariationContext { get; } = new();
[SetUp]
public override void Setup()
{

View File

@@ -562,7 +562,7 @@ public class RichTextParserTests : PropertyValueConverterTests
element.SetupGet(c => c.ContentType).Returns(elementType.Object);
var numberPropertyType = SetupPublishedPropertyType(new IntegerValueConverter(), "number", Constants.PropertyEditors.Aliases.Label);
var property = new PublishedElementPropertyBase(numberPropertyType, element.Object, false, PropertyCacheLevel.None, CacheManager, propertyValue);
var property = new PublishedElementPropertyBase(numberPropertyType, element.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager, propertyValue);
element.SetupGet(c => c.Properties).Returns(new[] { property });
return element.Object;

View File

@@ -93,12 +93,14 @@ public class ConvertersTests
elementType1,
Guid.NewGuid(),
new Dictionary<string, object> { { "prop1", "val1" } },
false);
false,
new VariationContext());
var element2 = new PublishedElement(
elementType2,
Guid.NewGuid(),
new Dictionary<string, object> { { "prop2", "1003" } },
false);
false,
new VariationContext());
var cnt1 = new InternalPublishedContent(contentType1)
{
Id = 1003,

View File

@@ -43,16 +43,16 @@ public class ConvertersTests
var elementType1 = contentTypeFactory.CreateContentType(Guid.NewGuid(), 1000, "element1", CreatePropertyTypes);
var element1 = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false);
var element1 = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false, new VariationContext());
Assert.AreEqual(1234, element1.Value(Mock.Of<IPublishedValueFallback>(), "prop1"));
// 'null' would be considered a 'missing' value by the default, magic logic
var e = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", null } }, false);
var e = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", null } }, false, new VariationContext());
Assert.IsFalse(e.HasValue("prop1"));
// '0' would not - it's a valid integer - but the converter knows better
e = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "0" } }, false);
e = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "0" } }, false, new VariationContext());
Assert.IsFalse(e.HasValue("prop1"));
}
@@ -120,7 +120,7 @@ public class ConvertersTests
var elementType1 = contentTypeFactory.CreateContentType(Guid.NewGuid(), 1000, "element1", CreatePropertyTypes);
var element1 = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false);
var element1 = new PublishedElement(elementType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false, new VariationContext());
var cntType1 = contentTypeFactory.CreateContentType(Guid.NewGuid(), 1001, "cnt1", t => Enumerable.Empty<PublishedPropertyType>());
var cnt1 = new InternalPublishedContent(cntType1) { Id = 1234 };

View File

@@ -56,7 +56,7 @@ public class PropertyCacheLevelTests
// anything else is not > None, use Content
//
// for standalone elements, it's only None or Content
var set1 = new PublishedElement(setType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false);
var set1 = new PublishedElement(setType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false, new VariationContext());
Assert.AreEqual(1234, set1.Value(Mock.Of<IPublishedValueFallback>(), "prop1"));
Assert.AreEqual(1, converter.SourceConverts);
@@ -133,6 +133,7 @@ public class PropertyCacheLevelTests
},
false,
referenceCacheLevel,
new VariationContext(),
cacheManager.Object);
Assert.AreEqual(1234, set1.Value(Mock.Of<IPublishedValueFallback>(), "prop1"));
@@ -185,7 +186,7 @@ public class PropertyCacheLevelTests
Assert.Throws<Exception>(() =>
{
var unused = new PublishedElement(setType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false);
var unused = new PublishedElement(setType1, Guid.NewGuid(), new Dictionary<string, object> { { "prop1", "1234" } }, false, new VariationContext());
});
}