Skip to content

Commit 9c6e3ff

Browse files
kjackjac
authored
Elements level property cache should cache by variation (#18080)
1 parent 28019e4 commit 9c6e3ff

File tree

14 files changed

+126
-32
lines changed

14 files changed

+126
-32
lines changed

‎src/Umbraco.Core/PublishedCache/PublishedElement.cs‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using Microsoft.Extensions.DependencyInjection;
2+
using Umbraco.Cms.Core.DependencyInjection;
13
using Umbraco.Cms.Core.Models.PublishedContent;
24
using Umbraco.Cms.Core.PropertyEditors;
35

@@ -16,9 +18,28 @@ public class PublishedElement : IPublishedElement
1618

1719
private readonly IPublishedProperty[] _propertiesArray;
1820

21+
[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
22+
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing)
23+
: this(contentType, key, values, previewing, PropertyCacheLevel.None, null)
24+
{
25+
}
26+
27+
[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
28+
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, ICacheManager? cacheManager)
29+
: this(
30+
contentType,
31+
key,
32+
values,
33+
previewing,
34+
referenceCacheLevel,
35+
StaticServiceProvider.Instance.GetRequiredService<IVariationContextAccessor>().VariationContext ?? new VariationContext(),
36+
cacheManager)
37+
{
38+
}
39+
1940
// initializes a new instance of the PublishedElement class
2041
// within the context of a published snapshot service (eg a published content property value)
21-
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, ICacheManager? cacheManager)
42+
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?>? values, bool previewing, PropertyCacheLevel referenceCacheLevel, VariationContext variationContext, ICacheManager? cacheManager)
2243
{
2344
if (key == Guid.Empty)
2445
{
@@ -40,7 +61,7 @@ public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<
4061
.Select(propertyType =>
4162
{
4263
values.TryGetValue(propertyType.Alias, out var value);
43-
return (IPublishedProperty)new PublishedElementPropertyBase(propertyType, this, previewing, referenceCacheLevel,cacheManager, value);
64+
return (IPublishedProperty)new PublishedElementPropertyBase(propertyType, this, previewing, referenceCacheLevel, variationContext, cacheManager, value);
4465
})
4566
.ToArray()
4667
?? new IPublishedProperty[0];
@@ -51,8 +72,8 @@ public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<
5172
// + using an initial reference cache level of .None ensures that everything will be
5273
// cached at .Content level - and that reference cache level will propagate to all
5374
// properties
54-
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing)
55-
: this(contentType, key, values, previewing, PropertyCacheLevel.None, null)
75+
public PublishedElement(IPublishedContentType contentType, Guid key, Dictionary<string, object?> values, bool previewing, VariationContext variationContext)
76+
: this(contentType, key, values, previewing, PropertyCacheLevel.None, variationContext, null)
5677
{
5778
}
5879

‎src/Umbraco.Core/PublishedCache/PublishedElementPropertyBase.cs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using Umbraco.Cms.Core.Cache;
21
using Umbraco.Cms.Core.Models.PublishedContent;
32
using Umbraco.Cms.Core.PropertyEditors;
43
using Umbraco.Extensions;
@@ -13,11 +12,11 @@ internal class PublishedElementPropertyBase : PublishedPropertyBase
1312
// to store eg routes, property converted values, anything - caching
1413
// means faster execution, but uses memory - not sure if we want it
1514
// so making it configurable.
16-
private const bool FullCacheWhenPreviewing = true;
1715
private readonly Lock _locko = new();
1816
private readonly object? _sourceValue;
1917
protected readonly bool IsMember;
2018
protected readonly bool IsPreviewing;
19+
private readonly VariationContext _variationContext;
2120
private readonly ICacheManager? _cacheManager;
2221
private CacheValues? _cacheValues;
2322

@@ -30,24 +29,30 @@ public PublishedElementPropertyBase(
3029
IPublishedElement element,
3130
bool previewing,
3231
PropertyCacheLevel referenceCacheLevel,
32+
VariationContext variationContext,
3333
ICacheManager? cacheManager,
3434
object? sourceValue = null)
3535
: base(propertyType, referenceCacheLevel)
3636
{
3737
_sourceValue = sourceValue;
3838
Element = element;
3939
IsPreviewing = previewing;
40+
_variationContext = variationContext;
4041
_cacheManager = cacheManager;
4142
IsMember = propertyType.ContentType?.ItemType == PublishedItemType.Member;
4243
}
4344

4445
// used to cache the CacheValues of this property
4546
// ReSharper disable InconsistentlySynchronizedField
46-
internal string ValuesCacheKey => _valuesCacheKey ??= PropertyCacheValues(Element.Key, Alias, IsPreviewing);
47+
private string ValuesCacheKey => _valuesCacheKey ??= PropertyCacheValuesKey();
4748

49+
[Obsolete("Do not use this. Will be removed in V17.")]
4850
public static string PropertyCacheValues(Guid contentUid, string typeAlias, bool previewing) =>
4951
"PublishedSnapshot.Property.CacheValues[" + (previewing ? "D:" : "P:") + contentUid + ":" + typeAlias + "]";
5052

53+
private string PropertyCacheValuesKey() =>
54+
$"PublishedSnapshot.Property.CacheValues[{(IsPreviewing ? "D:" : "P:")}{Element.Key}:{Alias}:{_variationContext.Culture.IfNullOrWhiteSpace("inv")}+{_variationContext.Segment.IfNullOrWhiteSpace("inv")}]";
55+
5156
// ReSharper restore InconsistentlySynchronizedField
5257
public override bool HasValue(string? culture = null, string? segment = null)
5358
{

‎src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/BlockEditorConverter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public BlockEditorConverter(
9191
return null;
9292
}
9393

94-
IPublishedElement element = new PublishedElement(publishedContentType, key, propertyValues, preview, referenceCacheLevel, _cacheManager);
94+
IPublishedElement element = new PublishedElement(publishedContentType, key, propertyValues, preview, referenceCacheLevel, variationContext, _cacheManager);
9595
element = _publishedModelFactory.CreateModel(element);
9696

9797
return element;

‎tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Publishing.cs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,4 +1869,67 @@ public async Task Cannot_Publish_Missing_Variant_Properties()
18691869
Assert.AreEqual("blocks", publishResult.InvalidProperties.First().Alias);
18701870
});
18711871
}
1872+
1873+
[Test]
1874+
public async Task Can_Handle_Elements_Level_Property_Cache()
1875+
{
1876+
var elementType = new ContentTypeBuilder()
1877+
.WithAlias("myElementType")
1878+
.WithName("My Element Type")
1879+
.WithIsElement(true)
1880+
.WithContentVariation(ContentVariation.Culture)
1881+
.AddPropertyType()
1882+
.WithAlias("contentPicker")
1883+
.WithName("Content Picker")
1884+
.WithDataTypeId(1046)
1885+
.WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.ContentPicker)
1886+
.WithValueStorageType(ValueStorageType.Nvarchar)
1887+
.WithVariations(ContentVariation.Culture)
1888+
.Done()
1889+
.Build();
1890+
ContentTypeService.Save(elementType);
1891+
var blockListDataType = await CreateBlockListDataType(elementType);
1892+
var contentType = CreateContentType(ContentVariation.Culture, blockListDataType);
1893+
1894+
var pickedContent1 = CreateContent(contentType, elementType, [], true);
1895+
var pickedContent2 = CreateContent(contentType, elementType, [], true);
1896+
1897+
var content = CreateContent(
1898+
contentType,
1899+
elementType,
1900+
[
1901+
new BlockProperty(
1902+
new List<BlockPropertyValue>
1903+
{
1904+
new() { Alias = "contentPicker", Value = pickedContent1.GetUdi().ToString(), Culture = "en-US" },
1905+
new() { Alias = "contentPicker", Value = pickedContent2.GetUdi().ToString(), Culture = "da-DK" },
1906+
},
1907+
[],
1908+
null,
1909+
null)
1910+
],
1911+
true);
1912+
1913+
AssertPropertyValues("en-US", pickedContent1);
1914+
1915+
AssertPropertyValues("da-DK", pickedContent2);
1916+
1917+
void AssertPropertyValues(string culture, IContent expectedPickedContent)
1918+
{
1919+
SetVariationContext(culture, null);
1920+
var publishedContent = GetPublishedContent(content.Key);
1921+
1922+
var value = publishedContent.Value<BlockListModel>("blocks");
1923+
Assert.IsNotNull(value);
1924+
Assert.AreEqual(1, value.Count);
1925+
1926+
var blockListItem = value.First();
1927+
Assert.AreEqual(1, blockListItem.Content.Properties.Count());
1928+
// need to ensure the property type cache level, otherwise this test has little value
1929+
Assert.AreEqual(PropertyCacheLevel.Elements, blockListItem.Content.Properties.First().PropertyType.CacheLevel);
1930+
var actualPickedPublishedContent = blockListItem.Content.Value<IPublishedContent>("contentPicker");
1931+
Assert.IsNotNull(actualPickedPublishedContent);
1932+
Assert.AreEqual(expectedPickedContent.Key, actualPickedPublishedContent.Key);
1933+
}
1934+
}
18721935
}

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/CacheTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public void PublishedElementProperty_CachesDeliveryApiValueConversion(PropertyCa
3636

3737
var element = new Mock<IPublishedElement>();
3838

39-
var prop1 = new PublishedElementPropertyBase(propertyType, element.Object, false, cacheLevel, Mock.Of<ICacheManager>());
39+
var prop1 = new PublishedElementPropertyBase(propertyType, element.Object, false, cacheLevel, new VariationContext(), Mock.Of<ICacheManager>());
4040

4141
var results = new List<string>
4242
{

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ public void ContentBuilder_MapsContentDataAndPropertiesCorrectly()
1717
{
1818
var content = new Mock<IPublishedContent>();
1919

20-
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
21-
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
20+
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
21+
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
2222

2323
var contentType = new Mock<IPublishedContentType>();
2424
contentType.SetupGet(c => c.Alias).Returns("thePageType");

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ public void ContentPickerValueConverter_RendersContentProperties()
7272
{
7373
var content = new Mock<IPublishedContent>();
7474

75-
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
76-
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, Mock.Of<ICacheManager>());
75+
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
76+
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
7777

7878
var publishedPropertyType = new Mock<IPublishedPropertyType>();
7979
publishedPropertyType.SetupGet(p => p.Alias).Returns("test");

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ public void MultiNodeTreePickerValueConverter_RendersContentProperties(string en
101101
{
102102
var content = new Mock<IPublishedContent>();
103103

104-
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
105-
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
104+
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), CacheManager);
105+
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), CacheManager);
106106

107107
var key = Guid.NewGuid();
108108
var urlSegment = "page-url-segment";

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public void OutputExpansionStrategy_ExpandsNothingByDefault()
4646
var apiContentBuilder = new ApiContentBuilder(new ApiContentNameProvider(), ApiContentRouteBuilder(), accessor);
4747

4848
var content = new Mock<IPublishedContent>();
49-
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
50-
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, CacheManager);
49+
var prop1 = new PublishedElementPropertyBase(DeliveryApiPropertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager);
50+
var prop2 = new PublishedElementPropertyBase(DefaultPropertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager);
5151

5252
var contentPickerContent = CreateSimplePickedContent(123, 456);
5353
var contentPickerProperty = CreateContentPickerProperty(content.Object, contentPickerContent.Key, "contentPicker", apiContentBuilder);
@@ -303,7 +303,7 @@ public void OutputExpansionStrategy_ForwardsExpansionStateToPropertyValueConvert
303303
.Returns(expanding ? "Expanding" : "Not expanding");
304304

305305
var propertyType = SetupPublishedPropertyType(valueConverterMock.Object, "theAlias", Constants.PropertyEditors.Aliases.Label);
306-
var property = new PublishedElementPropertyBase(propertyType, content.Object, false, PropertyCacheLevel.None, CacheManager, "The Value");
306+
var property = new PublishedElementPropertyBase(propertyType, content.Object, false, PropertyCacheLevel.None, VariationContext, CacheManager, "The Value");
307307

308308
SetupContentMock(content, property);
309309

@@ -378,7 +378,7 @@ internal PublishedElementPropertyBase CreateContentPickerProperty(IPublishedElem
378378
ContentPickerValueConverter contentPickerValueConverter = new ContentPickerValueConverter(PublishedContentCacheMock.Object, contentBuilder);
379379
var contentPickerPropertyType = SetupPublishedPropertyType(contentPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.ContentPicker);
380380

381-
return new PublishedElementPropertyBase(contentPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, new GuidUdi(Constants.UdiEntityType.Document, pickedContentKey).ToString());
381+
return new PublishedElementPropertyBase(contentPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, new GuidUdi(Constants.UdiEntityType.Document, pickedContentKey).ToString());
382382
}
383383

384384
internal PublishedElementPropertyBase CreateMediaPickerProperty(IPublishedElement parent, Guid pickedMediaKey, string propertyTypeAlias, IApiMediaBuilder mediaBuilder)
@@ -389,7 +389,7 @@ internal PublishedElementPropertyBase CreateMediaPickerProperty(IPublishedElemen
389389
MediaPickerWithCropsValueConverter mediaPickerValueConverter = new MediaPickerWithCropsValueConverter(CacheManager.Media, PublishedUrlProvider, publishedValueFallback, new SystemTextJsonSerializer(), apiMediaWithCropsBuilder);
390390
var mediaPickerPropertyType = SetupPublishedPropertyType(mediaPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.MediaPicker3, new MediaPicker3Configuration());
391391

392-
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, new GuidUdi(Constants.UdiEntityType.Media, pickedMediaKey).ToString());
392+
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, new GuidUdi(Constants.UdiEntityType.Media, pickedMediaKey).ToString());
393393
}
394394

395395
internal PublishedElementPropertyBase CreateMediaPicker3Property(IPublishedElement parent, Guid pickedMediaKey, string propertyTypeAlias, IApiMediaBuilder mediaBuilder)
@@ -409,13 +409,13 @@ internal PublishedElementPropertyBase CreateMediaPicker3Property(IPublishedEleme
409409
MediaPickerWithCropsValueConverter mediaPickerValueConverter = new MediaPickerWithCropsValueConverter(CacheManager.Media, PublishedUrlProvider, publishedValueFallback, new SystemTextJsonSerializer(), apiMediaWithCropsBuilder);
410410
var mediaPickerPropertyType = SetupPublishedPropertyType(mediaPickerValueConverter, propertyTypeAlias, Constants.PropertyEditors.Aliases.MediaPicker3, new MediaPicker3Configuration());
411411

412-
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, value);
412+
return new PublishedElementPropertyBase(mediaPickerPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, value);
413413
}
414414

415415
internal PublishedElementPropertyBase CreateNumberProperty(IPublishedElement parent, int propertyValue, string propertyTypeAlias)
416416
{
417417
var numberPropertyType = SetupPublishedPropertyType(new IntegerValueConverter(), propertyTypeAlias, Constants.PropertyEditors.Aliases.Label);
418-
return new PublishedElementPropertyBase(numberPropertyType, parent, false, PropertyCacheLevel.None, CacheManager, propertyValue);
418+
return new PublishedElementPropertyBase(numberPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager, propertyValue);
419419
}
420420

421421
internal PublishedElementPropertyBase CreateElementProperty(
@@ -452,7 +452,7 @@ internal PublishedElementPropertyBase CreateElementProperty(
452452
elementValueConverter.Setup(p => p.GetDeliveryApiPropertyCacheLevelForExpansion(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
453453

454454
var elementPropertyType = SetupPublishedPropertyType(elementValueConverter.Object, elementPropertyAlias, "My.Element.Property");
455-
return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None, CacheManager);
455+
return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None, VariationContext, CacheManager);
456456
}
457457

458458
protected IApiContentRouteBuilder ApiContentRouteBuilder() => CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings());

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public class PropertyValueConverterTests : DeliveryApiTests
2929

3030
protected Mock<IPublishedUrlProvider> PublishedUrlProviderMock { get; private set; }
3131

32+
protected VariationContext VariationContext { get; } = new();
33+
3234
[SetUp]
3335
public override void Setup()
3436
{

0 commit comments

Comments
 (0)