Skip to content

Commit 93d61d0

Browse files
AndyButlandkjac
AndyButland
and
kjac
authored
Return 404 on delivery API requests for segments that are invalid or not created (#19718)
* Return 404 on delivery API requests for segments that are invalid or not created. * Handled case with no segmented properties. * Let the property decide if it has a value or not --------- Co-authored-by: kjac <[email protected]>
1 parent 55cc415 commit 93d61d0

File tree

5 files changed

+132
-21
lines changed

5 files changed

+132
-21
lines changed

‎src/Umbraco.Core/DeliveryApi/ApiContentBuilder.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ namespace Umbraco.Cms.Core.DeliveryApi;
77

88
public sealed class ApiContentBuilder : ApiContentBuilderBase<IApiContent>, IApiContentBuilder
99
{
10-
private readonly IVariationContextAccessor _variationContextAccessor;
11-
1210
[Obsolete("Please use the constructor that takes an IVariationContextAccessor instead. Scheduled for removal in V17.")]
1311
public ApiContentBuilder(
1412
IApiContentNameProvider apiContentNameProvider,
@@ -27,9 +25,10 @@ public ApiContentBuilder(
2725
IApiContentRouteBuilder apiContentRouteBuilder,
2826
IOutputExpansionStrategyAccessor outputExpansionStrategyAccessor,
2927
IVariationContextAccessor variationContextAccessor)
30-
: base(apiContentNameProvider, apiContentRouteBuilder, outputExpansionStrategyAccessor)
31-
=> _variationContextAccessor = variationContextAccessor;
28+
: base(apiContentNameProvider, apiContentRouteBuilder, outputExpansionStrategyAccessor, variationContextAccessor)
29+
{
30+
}
3231

3332
protected override IApiContent Create(IPublishedContent content, string name, IApiContentRoute route, IDictionary<string, object?> properties)
34-
=> new ApiContent(content.Key, name, content.ContentType.Alias, content.CreateDate, content.CultureDate(_variationContextAccessor), route, properties);
33+
=> new ApiContent(content.Key, name, content.ContentType.Alias, content.CreateDate, content.CultureDate(VariationContextAccessor), route, properties);
3534
}

‎src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,72 @@
1-
using Umbraco.Cms.Core.Models.DeliveryApi;
1+
using Microsoft.Extensions.DependencyInjection;
2+
using Umbraco.Cms.Core.DependencyInjection;
3+
using Umbraco.Cms.Core.Models.DeliveryApi;
24
using Umbraco.Cms.Core.Models.PublishedContent;
5+
using Umbraco.Extensions;
36

47
namespace Umbraco.Cms.Core.DeliveryApi;
58

69
public abstract class ApiContentBuilderBase<T>
710
where T : IApiContent
811
{
912
private readonly IApiContentNameProvider _apiContentNameProvider;
10-
private readonly IApiContentRouteBuilder _apiContentRouteBuilder;
1113
private readonly IOutputExpansionStrategyAccessor _outputExpansionStrategyAccessor;
1214

13-
protected ApiContentBuilderBase(IApiContentNameProvider apiContentNameProvider, IApiContentRouteBuilder apiContentRouteBuilder, IOutputExpansionStrategyAccessor outputExpansionStrategyAccessor)
15+
[Obsolete("Please use the constructor that takes all parameters. Scheduled for removal in Umbraco 17.")]
16+
protected ApiContentBuilderBase(
17+
IApiContentNameProvider apiContentNameProvider,
18+
IApiContentRouteBuilder apiContentRouteBuilder,
19+
IOutputExpansionStrategyAccessor outputExpansionStrategyAccessor)
20+
: this(
21+
apiContentNameProvider,
22+
apiContentRouteBuilder,
23+
outputExpansionStrategyAccessor,
24+
StaticServiceProvider.Instance.GetRequiredService<IVariationContextAccessor>())
25+
{
26+
}
27+
28+
protected ApiContentBuilderBase(
29+
IApiContentNameProvider apiContentNameProvider,
30+
IApiContentRouteBuilder apiContentRouteBuilder,
31+
IOutputExpansionStrategyAccessor outputExpansionStrategyAccessor,
32+
IVariationContextAccessor variationContextAccessor)
1433
{
1534
_apiContentNameProvider = apiContentNameProvider;
16-
_apiContentRouteBuilder = apiContentRouteBuilder;
35+
ApiContentRouteBuilder = apiContentRouteBuilder;
1736
_outputExpansionStrategyAccessor = outputExpansionStrategyAccessor;
37+
VariationContextAccessor = variationContextAccessor;
1838
}
1939

40+
protected IApiContentRouteBuilder ApiContentRouteBuilder { get; }
41+
42+
protected IVariationContextAccessor VariationContextAccessor { get; }
43+
2044
protected abstract T Create(IPublishedContent content, string name, IApiContentRoute route, IDictionary<string, object?> properties);
2145

2246
public virtual T? Build(IPublishedContent content)
2347
{
24-
IApiContentRoute? route = _apiContentRouteBuilder.Build(content);
48+
IApiContentRoute? route = ApiContentRouteBuilder.Build(content);
2549
if (route is null)
2650
{
2751
return default;
2852
}
2953

54+
// If a segment is requested and no segmented properties have any values, we consider the segment as not created or non-existing and return null.
55+
// This aligns the behaviour of the API when it comes to "Accept-Segment" and "Accept-Language" requests, so 404 is returned for both when
56+
// the segment or language is not created or does not exist.
57+
// It also aligns with what we show in the backoffice for whether a segment is "Published" or "Not created".
58+
// Requested languages that aren't created or don't exist will already have exited early in the route builder.
59+
var segment = VariationContextAccessor.VariationContext?.Segment;
60+
if (segment.IsNullOrWhiteSpace() is false
61+
&& content.ContentType.VariesBySegment()
62+
&& content
63+
.Properties
64+
.Where(p => p.PropertyType.VariesBySegment())
65+
.All(p => p.HasValue(VariationContextAccessor.VariationContext?.Culture, segment) is false))
66+
{
67+
return default;
68+
}
69+
3070
IDictionary<string, object?> properties =
3171
_outputExpansionStrategyAccessor.TryGetValue(out IOutputExpansionStrategy? outputExpansionStrategy)
3272
? outputExpansionStrategy.MapContentProperties(content)

‎src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using Umbraco.Cms.Core.DependencyInjection;
1+
using Umbraco.Cms.Core.DependencyInjection;
22
using Umbraco.Cms.Core.Models.DeliveryApi;
33
using Umbraco.Cms.Core.Models.PublishedContent;
44
using Umbraco.Extensions;
@@ -7,9 +7,6 @@ namespace Umbraco.Cms.Core.DeliveryApi;
77

88
public class ApiContentResponseBuilder : ApiContentBuilderBase<IApiContentResponse>, IApiContentResponseBuilder
99
{
10-
private readonly IApiContentRouteBuilder _apiContentRouteBuilder;
11-
private readonly IVariationContextAccessor _variationContextAccessor;
12-
1310
[Obsolete("Please use the constructor that takes an IVariationContextAccessor instead. Scheduled for removal in V17.")]
1411
public ApiContentResponseBuilder(
1512
IApiContentNameProvider apiContentNameProvider,
@@ -28,16 +25,14 @@ public ApiContentResponseBuilder(
2825
IApiContentRouteBuilder apiContentRouteBuilder,
2926
IOutputExpansionStrategyAccessor outputExpansionStrategyAccessor,
3027
IVariationContextAccessor variationContextAccessor)
31-
: base(apiContentNameProvider, apiContentRouteBuilder, outputExpansionStrategyAccessor)
28+
: base(apiContentNameProvider, apiContentRouteBuilder, outputExpansionStrategyAccessor, variationContextAccessor)
3229
{
33-
_apiContentRouteBuilder = apiContentRouteBuilder;
34-
_variationContextAccessor = variationContextAccessor;
3530
}
3631

3732
protected override IApiContentResponse Create(IPublishedContent content, string name, IApiContentRoute route, IDictionary<string, object?> properties)
3833
{
3934
IDictionary<string, IApiContentRoute> cultures = GetCultures(content);
40-
return new ApiContentResponse(content.Key, name, content.ContentType.Alias, content.CreateDate, content.CultureDate(_variationContextAccessor), route, properties, cultures);
35+
return new ApiContentResponse(content.Key, name, content.ContentType.Alias, content.CreateDate, content.CultureDate(VariationContextAccessor), route, properties, cultures);
4136
}
4237

4338
protected virtual IDictionary<string, IApiContentRoute> GetCultures(IPublishedContent content)
@@ -52,7 +47,7 @@ protected virtual IDictionary<string, IApiContentRoute> GetCultures(IPublishedCo
5247
continue;
5348
}
5449

55-
IApiContentRoute? cultureRoute = _apiContentRouteBuilder.Build(content, publishedCultureInfo.Culture);
50+
IApiContentRoute? cultureRoute = ApiContentRouteBuilder.Build(content, publishedCultureInfo.Culture);
5651
if (cultureRoute == null)
5752
{
5853
// content is un-routable in this culture

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Umbraco.Cms.Core.Models.DeliveryApi;
66
using Umbraco.Cms.Core.Models.PublishedContent;
77
using Umbraco.Cms.Core.PropertyEditors;
8+
using Umbraco.Cms.Core.PropertyEditors.DeliveryApi;
89
using Umbraco.Cms.Core.PublishedCache;
910
using Umbraco.Cms.Core.Services.Navigation;
1011
using Umbraco.Cms.Tests.Common;
@@ -143,4 +144,75 @@ public void ContentBuilder_ReturnsNullForUnRoutableContent()
143144

144145
Assert.Null(result);
145146
}
147+
148+
[TestCase("Shared value", "Segmented value", false)]
149+
[TestCase(null, "Segmented value", false)]
150+
[TestCase("Shared value", null, true)]
151+
[TestCase(null, null, true)]
152+
public void ContentBuilder_ReturnsNullForRequestedSegmentThatIsNotCreated(object? sharedValue, object? segmentedValue, bool expectNull)
153+
{
154+
var content = new Mock<IPublishedContent>();
155+
156+
var sharedPropertyValueConverter = new Mock<IDeliveryApiPropertyValueConverter>();
157+
sharedPropertyValueConverter.Setup(p => p.IsValue(It.IsAny<object?>(), It.IsAny<PropertyValueLevel>())).Returns(sharedValue is not null);
158+
sharedPropertyValueConverter.Setup(p => p.ConvertIntermediateToDeliveryApiObject(
159+
It.IsAny<IPublishedElement>(),
160+
It.IsAny<IPublishedPropertyType>(),
161+
It.IsAny<PropertyCacheLevel>(),
162+
It.IsAny<object?>(),
163+
It.IsAny<bool>(),
164+
It.IsAny<bool>())).Returns(sharedValue);
165+
sharedPropertyValueConverter.Setup(p => p.IsConverter(It.IsAny<IPublishedPropertyType>())).Returns(true);
166+
sharedPropertyValueConverter.Setup(p => p.GetPropertyCacheLevel(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
167+
sharedPropertyValueConverter.Setup(p => p.GetDeliveryApiPropertyCacheLevel(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
168+
169+
var sharedPropertyType = SetupPublishedPropertyType(sharedPropertyValueConverter.Object, "sharedMessage", "Umbraco.Textstring");
170+
var sharedProperty = new PublishedElementPropertyBase(sharedPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
171+
172+
var segmentedPropertyValueConverter = new Mock<IDeliveryApiPropertyValueConverter>();
173+
segmentedPropertyValueConverter.Setup(p => p.IsValue(It.IsAny<object?>(), It.IsAny<PropertyValueLevel>())).Returns(segmentedValue is not null);
174+
segmentedPropertyValueConverter.Setup(p => p.ConvertIntermediateToDeliveryApiObject(
175+
It.IsAny<IPublishedElement>(),
176+
It.IsAny<IPublishedPropertyType>(),
177+
It.IsAny<PropertyCacheLevel>(),
178+
It.IsAny<object?>(),
179+
It.IsAny<bool>(),
180+
It.IsAny<bool>())).Returns(segmentedValue);
181+
segmentedPropertyValueConverter.Setup(p => p.IsConverter(It.IsAny<IPublishedPropertyType>())).Returns(true);
182+
segmentedPropertyValueConverter.Setup(p => p.GetPropertyCacheLevel(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
183+
segmentedPropertyValueConverter.Setup(p => p.GetDeliveryApiPropertyCacheLevel(It.IsAny<IPublishedPropertyType>())).Returns(PropertyCacheLevel.None);
184+
185+
var segmentedPropertyType = SetupPublishedPropertyType(segmentedPropertyValueConverter.Object, "segmentedMessage", "Umbraco.Textstring", contentVariation: ContentVariation.Segment);
186+
var segmentedProperty = new PublishedElementPropertyBase(segmentedPropertyType, content.Object, false, PropertyCacheLevel.None, new VariationContext(), Mock.Of<ICacheManager>());
187+
188+
var contentType = new Mock<IPublishedContentType>();
189+
contentType.SetupGet(c => c.Alias).Returns("thePageType");
190+
contentType.SetupGet(c => c.ItemType).Returns(PublishedItemType.Content);
191+
contentType.SetupGet(c => c.Variations).Returns(ContentVariation.Segment);
192+
193+
var key = Guid.NewGuid();
194+
var urlSegment = "url-segment";
195+
var name = "The page";
196+
ConfigurePublishedContentMock(content, key, name, urlSegment, contentType.Object, [sharedProperty, segmentedProperty]);
197+
198+
var routeBuilderMock = new Mock<IApiContentRouteBuilder>();
199+
routeBuilderMock
200+
.Setup(r => r.Build(content.Object, It.IsAny<string?>()))
201+
.Returns(new ApiContentRoute(content.Object.UrlSegment!, new ApiContentStartItem(Guid.NewGuid(), "/")));
202+
203+
var variationContextAccessorMock = new Mock<IVariationContextAccessor>();
204+
variationContextAccessorMock.Setup(v => v.VariationContext).Returns(new VariationContext(segment: "missingSegment"));
205+
206+
var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilderMock.Object, CreateOutputExpansionStrategyAccessor(), variationContextAccessorMock.Object);
207+
var result = builder.Build(content.Object);
208+
209+
if (expectNull)
210+
{
211+
Assert.IsNull(result);
212+
}
213+
else
214+
{
215+
Assert.IsNotNull(result);
216+
}
217+
}
146218
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ public virtual void Setup()
7373
PublishStatusQueryService = publishStatusQueryService.Object;
7474
}
7575

76-
protected IPublishedPropertyType SetupPublishedPropertyType(IPropertyValueConverter valueConverter, string propertyTypeAlias, string editorAlias, object? dataTypeConfiguration = null)
76+
protected IPublishedPropertyType SetupPublishedPropertyType(
77+
IPropertyValueConverter valueConverter,
78+
string propertyTypeAlias,
79+
string editorAlias,
80+
object? dataTypeConfiguration = null,
81+
ContentVariation contentVariation = ContentVariation.Nothing)
7782
{
7883
var mockPublishedContentTypeFactory = new Mock<IPublishedContentTypeFactory>();
7984
mockPublishedContentTypeFactory.Setup(x => x.GetDataType(It.IsAny<int>()))
@@ -83,7 +88,7 @@ protected IPublishedPropertyType SetupPublishedPropertyType(IPropertyValueConver
8388
propertyTypeAlias,
8489
123,
8590
true,
86-
ContentVariation.Nothing,
91+
contentVariation,
8792
new PropertyValueConverterCollection(() => new[] { valueConverter }),
8893
Mock.Of<IPublishedModelFactory>(),
8994
mockPublishedContentTypeFactory.Object);

0 commit comments

Comments
 (0)