From b3bedb4efee8866a7c5cfee9be2414adcf8f0a8a Mon Sep 17 00:00:00 2001 From: karlmacklin <353806+karlmacklin@users.noreply.github.com> Date: Mon, 15 May 2023 08:42:58 +0200 Subject: [PATCH 01/21] XPath can unambiguously use $site/$parent (#14127) * XPath can unambiguously use $site/$parent * add deprecation notices and obsolete methods * Update deprecation description text/instruction Co-authored-by: Mole * Small spelling fix on deprecation description * keep depr. getByQuery and handle legacy usage --------- Co-authored-by: Kalle Macklin Co-authored-by: Mole --- .../Xml/UmbracoXPathPathSyntaxParser.cs | 53 ++++++++++++++----- .../Routing/NotFoundHandlerHelper.cs | 1 + .../Controllers/EntityController.cs | 21 +++++--- .../src/common/resources/entity.resource.js | 24 +++++++-- .../contentpicker/contentpicker.controller.js | 9 ++-- 5 files changed, 81 insertions(+), 27 deletions(-) diff --git a/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs b/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs index bb5c186ca6..2a01d42dc7 100644 --- a/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs +++ b/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs @@ -8,13 +8,23 @@ namespace Umbraco.Cms.Core.Xml; /// public class UmbracoXPathPathSyntaxParser { + [Obsolete("This will be removed in Umbraco 13. Use ParseXPathQuery which accepts a parentId instead")] + public static string ParseXPathQuery( + string xpathExpression, + int? nodeContextId, + Func?> getPath, + Func publishedContentExists) => ParseXPathQuery(xpathExpression, nodeContextId, null, getPath, publishedContentExists); + /// /// Parses custom umbraco xpath expression /// /// The Xpath expression /// - /// The current node id context of executing the query - null if there is no current node, in which case - /// some of the parameters like $current, $parent, $site will be disabled + /// The current node id context of executing the query - null if there is no current node. + /// + /// + /// The parent node id of the current node id context of executing the query. With this we can determine the + /// $parent and $site parameters even if the current node is not yet published. /// /// The callback to create the nodeId path, given a node Id /// The callback to return whether a published node exists based on Id @@ -22,6 +32,7 @@ public class UmbracoXPathPathSyntaxParser public static string ParseXPathQuery( string xpathExpression, int? nodeContextId, + int? parentId, Func?> getPath, Func publishedContentExists) { @@ -84,19 +95,27 @@ public class UmbracoXPathPathSyntaxParser // parseable items: var vars = new Dictionary>(); - // These parameters must have a node id context - if (nodeContextId.HasValue) + if (parentId.HasValue) + { + vars.Add("$parent", q => + { + var path = getPath(parentId.Value)?.ToArray(); + var closestPublishedAncestorId = getClosestPublishedAncestor(path); + return q.Replace("$parent", string.Format(rootXpath, closestPublishedAncestorId)); + }); + + vars.Add("$site", q => + { + var closestPublishedAncestorId = getClosestPublishedAncestor(getPath(parentId.Value)); + return q.Replace( + "$site", + string.Format(rootXpath, closestPublishedAncestorId) + "/ancestor-or-self::*[@level = 1]"); + }); + } + else if (nodeContextId.HasValue) { - vars.Add("$current", q => - { - var closestPublishedAncestorId = getClosestPublishedAncestor(getPath(nodeContextId.Value)); - return q.Replace("$current", string.Format(rootXpath, closestPublishedAncestorId)); - }); - vars.Add("$parent", q => { - // remove the first item in the array if its the current node - // this happens when current is published, but we are looking for its parent specifically var path = getPath(nodeContextId.Value)?.ToArray(); if (path?[0] == nodeContextId.ToString()) { @@ -116,6 +135,16 @@ public class UmbracoXPathPathSyntaxParser }); } + // These parameters must have a node id context + if (nodeContextId.HasValue) + { + vars.Add("$current", q => + { + var closestPublishedAncestorId = getClosestPublishedAncestor(getPath(nodeContextId.Value)); + return q.Replace("$current", string.Format(rootXpath, closestPublishedAncestorId)); + }); + } + // TODO: This used to just replace $root with string.Empty BUT, that would never work // the root is always "/root . Need to confirm with Per why this was string.Empty before! vars.Add("$root", q => q.Replace("$root", "/root")); diff --git a/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs b/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs index f945f03b28..04ca6b4bb2 100644 --- a/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs +++ b/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs @@ -79,6 +79,7 @@ internal class NotFoundHandlerHelper var xpathResult = UmbracoXPathPathSyntaxParser.ParseXPathQuery( errorPage.ContentXPath!, domainContentId, + null, nodeid => { IEntitySlim? ent = entityService.Get(nodeid); diff --git a/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs b/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs index 36e7d46df1..9c4f6aa258 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs @@ -555,6 +555,15 @@ public class EntityController : UmbracoAuthorizedJsonController return Ok(returnUrl); } + /// + /// Gets an entity by a xpath query - OBSOLETE + /// + /// + /// + /// + /// + [Obsolete("This will be removed in Umbraco 13. Use GetByXPath instead")] + public ActionResult? GetByQuery(string query, int nodeContextId, UmbracoEntityTypes type) => GetByXPath(query, nodeContextId, null, type); /// /// Gets an entity by a xpath query @@ -562,19 +571,16 @@ public class EntityController : UmbracoAuthorizedJsonController /// /// /// + /// /// - public ActionResult? GetByQuery(string query, int nodeContextId, UmbracoEntityTypes type) + public ActionResult? GetByXPath(string query, int nodeContextId, int? parentId, UmbracoEntityTypes type) { - // TODO: Rename this!!! It's misleading, it should be GetByXPath - - if (type != UmbracoEntityTypes.Document) { throw new ArgumentException("Get by query is only compatible with entities of type Document"); } - - var q = ParseXPathQuery(query, nodeContextId); + var q = ParseXPathQuery(query, nodeContextId, parentId); IPublishedContent? node = _publishedContentQuery.ContentSingleAtXPath(q); if (node == null) @@ -586,10 +592,11 @@ public class EntityController : UmbracoAuthorizedJsonController } // PP: Work in progress on the query parser - private string ParseXPathQuery(string query, int id) => + private string ParseXPathQuery(string query, int id, int? parentId) => UmbracoXPathPathSyntaxParser.ParseXPathQuery( query, id, + parentId, nodeid => { IEntitySlim? ent = _entityService.Get(nodeid); diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js index 3b2618a82b..1c6f8d6184 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js @@ -318,9 +318,22 @@ function entityResource($q, $http, umbRequestHelper) { 'Failed to retrieve entity data for ids ' + ids); }, + /** + * @deprecated use getByXPath instead. + */ + getByQuery: function (query, nodeContextId, type) { + return umbRequestHelper.resourcePromise( + $http.get( + umbRequestHelper.getApiUrl( + "entityApiBaseUrl", + "GetByQuery", + [{ query: query }, { nodeContextId: nodeContextId }, { type: type }])), + 'Failed to retrieve entity data for query ' + query); + }, + /** * @ngdoc method - * @name umbraco.resources.entityResource#getByQuery + * @name umbraco.resources.entityResource#getByXPath * @methodOf umbraco.resources.entityResource * * @description @@ -329,7 +342,7 @@ function entityResource($q, $http, umbRequestHelper) { * ##usage * 
          * //get content by xpath
-         * entityResource.getByQuery("$current", -1, "Document")
+         * entityResource.getByXPath("$current", -1, -1, "Document")
          *    .then(function(ent) {
          *        var myDoc = ent;
          *        alert('its here!');
@@ -338,17 +351,18 @@ function entityResource($q, $http, umbRequestHelper) {
          *
          * @param {string} query xpath to use in query
          * @param {Int} nodeContextId id id to start from
+         * @param {Int} parentId id id of the parent to the starting point
          * @param {string} type Object type name
          * @returns {Promise} resourcePromise object containing the entity.
          *
          */
-        getByQuery: function (query, nodeContextId, type) {
+        getByXPath: function (query, nodeContextId, parentId, type) {
             return umbRequestHelper.resourcePromise(
                $http.get(
                    umbRequestHelper.getApiUrl(
                        "entityApiBaseUrl",
-                       "GetByQuery",
-                       [{ query: query }, { nodeContextId: nodeContextId }, { type: type }])),
+                       "GetByXPath",
+                       [{ query: query }, { nodeContextId: nodeContextId }, { parentId: parentId }, { type: type }])),
                'Failed to retrieve entity data for query ' + query);
         },
 
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js
index 09b9d2c98b..52f147bce0 100644
--- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js
+++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js
@@ -245,9 +245,12 @@ function contentPickerController($scope, $q, $routeParams, $location, entityReso
         dialogOptions.startNodeId = -1;
     }
     else if ($scope.model.config.startNode.query) {
-        //if we have a query for the startnode, we will use that.
-        var rootId = editorState.current.id;
-        entityResource.getByQuery($scope.model.config.startNode.query, rootId, "Document").then(function (ent) {
+        entityResource.getByXPath(
+            $scope.model.config.startNode.query,
+            editorState.current.id,
+            editorState.current.parentId,
+            "Document"
+        ).then(function (ent) {
             dialogOptions.startNodeId = ($scope.model.config.idType === "udi" ? ent.udi : ent.id).toString();
         });
     }

From 85d46c3e82bf99d56f0a4659b00caf8d1c5a769c Mon Sep 17 00:00:00 2001
From: Kenn Jacobsen 
Date: Mon, 15 May 2023 10:06:24 +0200
Subject: [PATCH 02/21] Merge local and global crops for MediaPicker3 (#14237)

---
 .../MediaPickerWithCropsValueConverter.cs     |  11 +-
 ...MediaPickerWithCropsValueConverterTests.cs | 271 +++++++++++++-----
 2 files changed, 205 insertions(+), 77 deletions(-)

diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MediaPickerWithCropsValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MediaPickerWithCropsValueConverter.cs
index a33bb9870d..19f9b5a908 100644
--- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MediaPickerWithCropsValueConverter.cs
+++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MediaPickerWithCropsValueConverter.cs
@@ -131,7 +131,16 @@ public class MediaPickerWithCropsValueConverter : PropertyValueConverterBase, ID
         ApiMediaWithCrops ToApiMedia(MediaWithCrops media)
         {
             IApiMedia inner = _apiMediaBuilder.Build(media.Content);
-            return new ApiMediaWithCrops(inner, media.LocalCrops.FocalPoint, media.LocalCrops.Crops);
+
+            // make sure we merge crops and focal point defined at media level with the locally defined ones (local ones take precedence in case of a conflict)
+            ImageCropperValue? mediaCrops = media.Content.Value(_publishedValueFallback, Constants.Conventions.Media.File);
+            ImageCropperValue localCrops = media.LocalCrops;
+            if (mediaCrops != null)
+            {
+                localCrops = localCrops.Merge(mediaCrops);
+            }
+
+            return new ApiMediaWithCrops(inner, localCrops.FocalPoint, localCrops.Crops);
         }
 
         // NOTE: eventually we might implement this explicitly instead of piggybacking on the default object conversion. however, this only happens once per cache rebuild,
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MediaPickerWithCropsValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MediaPickerWithCropsValueConverterTests.cs
index 23dc5bb8b3..c02526054e 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MediaPickerWithCropsValueConverterTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MediaPickerWithCropsValueConverterTests.cs
@@ -2,7 +2,6 @@ using Moq;
 using NUnit.Framework;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.DeliveryApi;
-using Umbraco.Cms.Core.DeliveryApi.Accessors;
 using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PropertyEditors;
@@ -63,32 +62,15 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
         var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
         Assert.NotNull(result);
         Assert.AreEqual(1, result.Count());
-        Assert.AreEqual("My media", result.First().Name);
-        Assert.AreEqual("my-media", result.First().Url);
-        Assert.AreEqual(".jpg", result.First().Extension);
-        Assert.AreEqual(200, result.First().Width);
-        Assert.AreEqual(400, result.First().Height);
-        Assert.AreEqual(800, result.First().Bytes);
-        Assert.NotNull(result.First().FocalPoint);
-        Assert.AreEqual(".jpg", result.First().Extension);
-        Assert.AreEqual(200, result.First().Width);
-        Assert.AreEqual(400, result.First().Height);
-        Assert.AreEqual(800, result.First().Bytes);
-        Assert.AreEqual(.2m, result.First().FocalPoint.Left);
-        Assert.AreEqual(.4m, result.First().FocalPoint.Top);
-        Assert.NotNull(result.First().Crops);
-        Assert.AreEqual(1, result.First().Crops.Count());
-        Assert.AreEqual("one", result.First().Crops.First().Alias);
-        Assert.AreEqual(100, result.First().Crops.First().Height);
-        Assert.AreEqual(200, result.First().Crops.First().Width);
-        Assert.NotNull(result.First().Crops.First().Coordinates);
-        Assert.AreEqual(1m, result.First().Crops.First().Coordinates.X1);
-        Assert.AreEqual(2m, result.First().Crops.First().Coordinates.X2);
-        Assert.AreEqual(10m, result.First().Crops.First().Coordinates.Y1);
-        Assert.AreEqual(20m, result.First().Crops.First().Coordinates.Y2);
-        Assert.NotNull(result.First().Properties);
-        Assert.AreEqual(1, result.First().Properties.Count);
-        Assert.AreEqual("My alt text", result.First().Properties["altText"]);
+        var first = result.Single();
+        ValidateMedia(first, "My media", "my-media", ".jpg", 200, 400, 800);
+        ValidateFocalPoint(first.FocalPoint, .2m, .4m);
+        Assert.NotNull(first.Crops);
+        Assert.AreEqual(1, first.Crops.Count());
+        ValidateCrop(first.Crops.First(), "one", 200, 100, 1m, 2m, 10m, 20m);
+        Assert.NotNull(first.Properties);
+        Assert.AreEqual(1, first.Properties.Count);
+        Assert.AreEqual("My alt text", first.Properties["altText"]);
     }
 
     [Test]
@@ -138,52 +120,147 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
         var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
         Assert.NotNull(result);
         Assert.AreEqual(2, result.Count());
+        var first = result.First();
+        var last = result.Last();
 
-        Assert.AreEqual("My media", result.First().Name);
-        Assert.AreEqual("my-media", result.First().Url);
-        Assert.AreEqual(".jpg", result.First().Extension);
-        Assert.AreEqual(200, result.First().Width);
-        Assert.AreEqual(400, result.First().Height);
-        Assert.AreEqual(800, result.First().Bytes);
-        Assert.NotNull(result.First().FocalPoint);
-        Assert.AreEqual(.2m, result.First().FocalPoint.Left);
-        Assert.AreEqual(.4m, result.First().FocalPoint.Top);
-        Assert.NotNull(result.First().Crops);
-        Assert.AreEqual(1, result.First().Crops.Count());
-        Assert.AreEqual("one", result.First().Crops.First().Alias);
-        Assert.AreEqual(100, result.First().Crops.First().Height);
-        Assert.AreEqual(200, result.First().Crops.First().Width);
-        Assert.NotNull(result.First().Crops.First().Coordinates);
-        Assert.AreEqual(1m, result.First().Crops.First().Coordinates.X1);
-        Assert.AreEqual(2m, result.First().Crops.First().Coordinates.X2);
-        Assert.AreEqual(10m, result.First().Crops.First().Coordinates.Y1);
-        Assert.AreEqual(20m, result.First().Crops.First().Coordinates.Y2);
-        Assert.NotNull(result.First().Properties);
-        Assert.AreEqual(1, result.First().Properties.Count);
-        Assert.AreEqual("My alt text", result.First().Properties["altText"]);
+        ValidateMedia(first, "My media", "my-media", ".jpg", 200, 400, 800);
+        ValidateFocalPoint(first.FocalPoint, .2m, .4m);
+        Assert.NotNull(first.Crops);
+        Assert.AreEqual(1, first.Crops.Count());
+        ValidateCrop(first.Crops.First(), "one", 200, 100, 1m, 2m, 10m, 20m);
+        Assert.NotNull(first.Properties);
+        Assert.AreEqual(1, first.Properties.Count);
+        Assert.AreEqual("My alt text", first.Properties["altText"]);
 
-        Assert.AreEqual("My other media", result.Last().Name);
-        Assert.AreEqual("my-other-media", result.Last().Url);
-        Assert.AreEqual(".png", result.Last().Extension);
-        Assert.AreEqual(800, result.Last().Width);
-        Assert.AreEqual(600, result.Last().Height);
-        Assert.AreEqual(200, result.Last().Bytes);
-        Assert.NotNull(result.Last().FocalPoint);
-        Assert.AreEqual(.8m, result.Last().FocalPoint.Left);
-        Assert.AreEqual(.6m, result.Last().FocalPoint.Top);
-        Assert.NotNull(result.Last().Crops);
-        Assert.AreEqual(1, result.Last().Crops.Count());
-        Assert.AreEqual("one", result.Last().Crops.Last().Alias);
-        Assert.AreEqual(100, result.Last().Crops.Last().Height);
-        Assert.AreEqual(200, result.Last().Crops.Last().Width);
-        Assert.NotNull(result.Last().Crops.Last().Coordinates);
-        Assert.AreEqual(40m, result.Last().Crops.Last().Coordinates.X1);
-        Assert.AreEqual(20m, result.Last().Crops.Last().Coordinates.X2);
-        Assert.AreEqual(2m, result.Last().Crops.Last().Coordinates.Y1);
-        Assert.AreEqual(1m, result.Last().Crops.Last().Coordinates.Y2);
-        Assert.NotNull(result.Last().Properties);
-        Assert.AreEqual(1, result.Last().Properties.Count);
-        Assert.AreEqual("My other alt text", result.Last().Properties["altText"]);
+        ValidateMedia(last, "My other media", "my-other-media", ".png", 800, 600, 200);
+        ValidateFocalPoint(last.FocalPoint, .8m, .6m);
+        Assert.NotNull(last.Crops);
+        Assert.AreEqual(1, last.Crops.Count());
+        ValidateCrop(last.Crops.First(), "one", 200, 100, 40m, 20m, 2m, 1m);
+        Assert.NotNull(last.Properties);
+        Assert.AreEqual(1, last.Properties.Count);
+        Assert.AreEqual("My other alt text", last.Properties["altText"]);
+    }
+
+    [Test]
+    public void MediaPickerWithCropsValueConverter_MergesMediaCropsWithLocalCrops()
+    {
+        var publishedPropertyType = SetupMediaPropertyType(false);
+        var mediaCrops = new ImageCropperValue
+        {
+            Crops = new[]
+            {
+                new ImageCropperValue.ImageCropperCrop
+                {
+                    Alias = "mediaOne",
+                    Width = 111,
+                    Height = 222,
+                    Coordinates = new ImageCropperValue.ImageCropperCropCoordinates { X1 = 2m, X2 = 4m, Y1 = 20m, Y2 = 40m }
+                }
+            },
+            FocalPoint = new ImageCropperValue.ImageCropperFocalPoint { Left = .9m, Top = .1m }
+        };
+        var mediaKey = SetupMedia("Some media", ".123", 123, 456, "My alt text", 789, mediaCrops);
+
+        var serializer = new JsonNetSerializer();
+
+        var valueConverter = MediaPickerWithCropsValueConverter();
+        Assert.AreEqual(typeof(IEnumerable), valueConverter.GetDeliveryApiPropertyValueType(publishedPropertyType));
+
+        var inter = serializer.Serialize(new[]
+        {
+            new MediaPicker3PropertyEditor.MediaPicker3PropertyValueEditor.MediaWithCropsDto
+            {
+                Key = Guid.NewGuid(),
+                MediaKey = mediaKey,
+                Crops = new []
+                {
+                    new ImageCropperValue.ImageCropperCrop
+                    {
+                        Alias = "one",
+                        Coordinates = new ImageCropperValue.ImageCropperCropCoordinates { X1 = 1m, X2 = 2m, Y1 = 10m, Y2 = 20m }
+                    }
+                }
+            }
+        });
+
+        var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
+        Assert.NotNull(result);
+        Assert.AreEqual(1, result.Count());
+        var mediaWithCrops = result.Single();
+        ValidateMedia(mediaWithCrops, "Some media", "some-media", ".123", 123, 456, 789);
+
+        // no local focal point, should revert to media focal point
+        ValidateFocalPoint(mediaWithCrops.FocalPoint, .9m, .1m);
+
+        // media crops should be merged with local crops
+        Assert.NotNull(mediaWithCrops.Crops);
+        Assert.AreEqual(2, mediaWithCrops.Crops.Count());
+
+        // local crops should be first, media crops should be last
+        ValidateCrop(mediaWithCrops.Crops.First(), "one", 200, 100, 1m, 2m, 10m, 20m);
+        ValidateCrop(mediaWithCrops.Crops.Last(), "mediaOne", 111, 222, 2m, 4m, 20m, 40m);
+    }
+
+
+    [Test]
+    public void MediaPickerWithCropsValueConverter_LocalCropsAndFocalPointTakesPrecedenceOverMediaCropsAndFocalPoint()
+    {
+        var publishedPropertyType = SetupMediaPropertyType(false);
+        var mediaCrops = new ImageCropperValue
+        {
+            Crops = new[]
+            {
+                new ImageCropperValue.ImageCropperCrop
+                {
+                    Alias = "one",
+                    Width = 111,
+                    Height = 222,
+                    Coordinates = new ImageCropperValue.ImageCropperCropCoordinates { X1 = 2m, X2 = 4m, Y1 = 20m, Y2 = 40m }
+                }
+            },
+            FocalPoint = new ImageCropperValue.ImageCropperFocalPoint { Left = .9m, Top = .1m }
+        };
+        var mediaKey = SetupMedia("Some media", ".123", 123, 456, "My alt text", 789, mediaCrops);
+
+        var serializer = new JsonNetSerializer();
+
+        var valueConverter = MediaPickerWithCropsValueConverter();
+        Assert.AreEqual(typeof(IEnumerable), valueConverter.GetDeliveryApiPropertyValueType(publishedPropertyType));
+
+        var inter = serializer.Serialize(new[]
+        {
+            new MediaPicker3PropertyEditor.MediaPicker3PropertyValueEditor.MediaWithCropsDto
+            {
+                Key = Guid.NewGuid(),
+                MediaKey = mediaKey,
+                Crops = new []
+                {
+                    new ImageCropperValue.ImageCropperCrop
+                    {
+                        Alias = "one",
+                        Coordinates = new ImageCropperValue.ImageCropperCropCoordinates { X1 = 1m, X2 = 2m, Y1 = 10m, Y2 = 20m }
+                    }
+                },
+                FocalPoint = new ImageCropperValue.ImageCropperFocalPoint { Left = .2m, Top = .3m }
+            }
+        });
+
+        var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
+        Assert.NotNull(result);
+        Assert.AreEqual(1, result.Count());
+        var mediaWithCrops = result.Single();
+        ValidateMedia(mediaWithCrops, "Some media", "some-media", ".123", 123, 456, 789);
+
+        // local focal point should take precedence over media focal point
+        ValidateFocalPoint(mediaWithCrops.FocalPoint, .2m, .3m);
+
+        // media crops should be discarded when merging with local crops (matching aliases, local ones take precedence)
+        Assert.NotNull(mediaWithCrops.Crops);
+        Assert.AreEqual(1, mediaWithCrops.Crops.Count());
+
+        // local crops should be first, media crops should be last
+        ValidateCrop(mediaWithCrops.Crops.First(), "one", 200, 100, 1m, 2m, 10m, 20m);
     }
 
     [TestCase("")]
@@ -194,8 +271,6 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
     {
         var publishedPropertyType = SetupMediaPropertyType(false);
 
-        var serializer = new JsonNetSerializer();
-
         var valueConverter = MediaPickerWithCropsValueConverter();
 
         var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
@@ -211,8 +286,6 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
     {
         var publishedPropertyType = SetupMediaPropertyType(true);
 
-        var serializer = new JsonNetSerializer();
-
         var valueConverter = MediaPickerWithCropsValueConverter();
 
         var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType, PropertyCacheLevel.Element, inter, false) as IEnumerable;
@@ -241,7 +314,7 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
         return publishedPropertyType.Object;
     }
 
-    private Guid SetupMedia(string name, string extension, int width, int height, string altText, int bytes)
+    private Guid SetupMedia(string name, string extension, int width, int height, string altText, int bytes, ImageCropperValue? imageCropperValue = null)
     {
         var publishedMediaType = new Mock();
         publishedMediaType.SetupGet(c => c.ItemType).Returns(PublishedItemType.Media);
@@ -266,6 +339,7 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
         AddProperty(Constants.Conventions.Media.Width, width);
         AddProperty(Constants.Conventions.Media.Height, height);
         AddProperty(Constants.Conventions.Media.Bytes, bytes);
+        AddProperty(Constants.Conventions.Media.File, imageCropperValue);
         AddProperty("altText", altText);
 
         PublishedMediaCacheMock
@@ -281,4 +355,49 @@ public class MediaPickerWithCropsValueConverterTests : PropertyValueConverterTes
 
         return mediaKey;
     }
+
+    private void ValidateMedia(
+        ApiMediaWithCrops actual,
+        string expectedName,
+        string expectedUrl,
+        string expectedExtension,
+        int expectedWidth,
+        int expectedHeight,
+        int expectedBytes)
+    {
+        Assert.AreEqual(expectedName, actual.Name);
+        Assert.AreEqual(expectedUrl, actual.Url);
+        Assert.AreEqual(expectedExtension, actual.Extension);
+        Assert.AreEqual(expectedWidth, actual.Width);
+        Assert.AreEqual(expectedHeight, actual.Height);
+        Assert.AreEqual(expectedBytes, actual.Bytes);
+
+    }
+
+    private void ValidateFocalPoint(ImageCropperValue.ImageCropperFocalPoint? actual, decimal expectedLeft, decimal expectedTop)
+    {
+        Assert.NotNull(actual);
+        Assert.AreEqual(expectedLeft, actual.Left);
+        Assert.AreEqual(expectedTop, actual.Top);
+    }
+
+    private void ValidateCrop(
+        ImageCropperValue.ImageCropperCrop actual,
+        string expectedAlias,
+        int expectedWidth,
+        int expectedHeight,
+        decimal expectedX1,
+        decimal expectedX2,
+        decimal expectedY1,
+        decimal expectedY2)
+    {
+        Assert.AreEqual(expectedAlias, actual.Alias);
+        Assert.AreEqual(expectedWidth, actual.Width);
+        Assert.AreEqual(expectedHeight, actual.Height);
+        Assert.NotNull(actual.Coordinates);
+        Assert.AreEqual(expectedX1, actual.Coordinates.X1);
+        Assert.AreEqual(expectedX2, actual.Coordinates.X2);
+        Assert.AreEqual(expectedY1, actual.Coordinates.Y1);
+        Assert.AreEqual(expectedY2, actual.Coordinates.Y2);
+    }
 }

From b32b8c2265185d443d155bc862f316a10d38902e Mon Sep 17 00:00:00 2001
From: Kenn Jacobsen 
Date: Mon, 15 May 2023 10:27:20 +0200
Subject: [PATCH 03/21] Un-routable content should never be accessible in the
 delivery API (#14242)

---
 .../Controllers/ByIdContentApiController.cs   |  8 +-
 .../Controllers/QueryContentApiController.cs  |  3 +-
 .../DeliveryApi/ApiContentBuilderBase.cs      | 10 ++-
 .../DeliveryApi/ApiContentResponseBuilder.cs  | 27 ++++--
 .../DeliveryApi/ApiContentRouteBuilder.cs     | 12 ++-
 .../DeliveryApi/IApiContentBuilder.cs         |  2 +-
 .../DeliveryApi/IApiContentResponseBuilder.cs |  2 +-
 .../DeliveryApi/IApiContentRouteBuilder.cs    |  2 +-
 .../DeliveryApi/ApiRichTextParser.cs          |  7 +-
 .../MultiUrlPickerValueConverter.cs           |  7 +-
 .../DeliveryApi/ContentBuilderTests.cs        | 29 ++++++-
 .../DeliveryApi/ContentRouteBuilderTests.cs   | 84 ++++++++++++-------
 .../MultiNodeTreePickerValueConverterTests.cs | 21 ++++-
 13 files changed, 163 insertions(+), 51 deletions(-)

diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/ByIdContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/ByIdContentApiController.cs
index ce870fc11b..fc2e91e1ba 100644
--- a/src/Umbraco.Cms.Api.Delivery/Controllers/ByIdContentApiController.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Controllers/ByIdContentApiController.cs
@@ -43,6 +43,12 @@ public class ByIdContentApiController : ContentApiItemControllerBase
             return Unauthorized();
         }
 
-        return await Task.FromResult(Ok(ApiContentResponseBuilder.Build(contentItem)));
+        IApiContentResponse? apiContentResponse = ApiContentResponseBuilder.Build(contentItem);
+        if (apiContentResponse is null)
+        {
+            return NotFound();
+        }
+
+        return await Task.FromResult(Ok(apiContentResponse));
     }
 }
diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs
index 8db6cdb454..2b2efb8cda 100644
--- a/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs
@@ -7,6 +7,7 @@ using Umbraco.Cms.Core.DeliveryApi;
 using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.Services.OperationStatus;
+using Umbraco.Extensions;
 using Umbraco.New.Cms.Core.Models;
 
 namespace Umbraco.Cms.Api.Delivery.Controllers;
@@ -53,7 +54,7 @@ public class QueryContentApiController : ContentApiControllerBase
 
         PagedModel pagedResult = queryAttempt.Result;
         IEnumerable contentItems = ApiPublishedContentCache.GetByIds(pagedResult.Items);
-        IApiContentResponse[] apiContentItems = contentItems.Select(ApiContentResponseBuilder.Build).ToArray();
+        IApiContentResponse[] apiContentItems = contentItems.Select(ApiContentResponseBuilder.Build).WhereNotNull().ToArray();
 
         var model = new PagedViewModel
         {
diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs b/src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs
index 8c70c3ff5b..ae70f0fdde 100644
--- a/src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs
+++ b/src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs
@@ -19,8 +19,14 @@ public abstract class ApiContentBuilderBase
 
     protected abstract T Create(IPublishedContent content, Guid id, string name, string contentType, IApiContentRoute route, IDictionary properties);
 
-    public virtual T Build(IPublishedContent content)
+    public virtual T? Build(IPublishedContent content)
     {
+        IApiContentRoute? route = _apiContentRouteBuilder.Build(content);
+        if (route is null)
+        {
+            return default;
+        }
+
         IDictionary properties =
             _outputExpansionStrategyAccessor.TryGetValue(out IOutputExpansionStrategy? outputExpansionStrategy)
                 ? outputExpansionStrategy.MapContentProperties(content)
@@ -31,7 +37,7 @@ public abstract class ApiContentBuilderBase
             content.Key,
             _apiContentNameProvider.GetName(content),
             content.ContentType.Alias,
-            _apiContentRouteBuilder.Build(content),
+            route,
             properties);
     }
 }
diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs
index dfa93e55d0..eb9cea6961 100644
--- a/src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs
+++ b/src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs
@@ -1,5 +1,6 @@
 using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
+using Umbraco.Cms.Core.Routing;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Core.DeliveryApi;
@@ -14,12 +15,26 @@ public sealed class ApiContentResponseBuilder : ApiContentBuilderBase properties)
     {
-        var cultures = content.Cultures.Values
-            .Where(publishedCultureInfo => publishedCultureInfo.Culture.IsNullOrWhiteSpace() == false) // filter out invariant cultures
-            .ToDictionary(
-                publishedCultureInfo => publishedCultureInfo.Culture,
-                publishedCultureInfo => _apiContentRouteBuilder.Build(content, publishedCultureInfo.Culture));
+        var routesByCulture = new Dictionary();
 
-        return new ApiContentResponse(id, name, contentType, route, properties, cultures);
+        foreach (PublishedCultureInfo publishedCultureInfo in content.Cultures.Values)
+        {
+            if (publishedCultureInfo.Culture.IsNullOrWhiteSpace())
+            {
+                // filter out invariant cultures
+                continue;
+            }
+
+            IApiContentRoute? cultureRoute = _apiContentRouteBuilder.Build(content, publishedCultureInfo.Culture);
+            if (cultureRoute == null)
+            {
+                // content is un-routable in this culture
+                continue;
+            }
+
+            routesByCulture[publishedCultureInfo.Culture] = cultureRoute;
+        }
+
+        return new ApiContentResponse(id, name, contentType, route, properties, routesByCulture);
     }
 }
diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs
index 34146bf5cf..80552b6488 100644
--- a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs
+++ b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs
@@ -27,7 +27,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
         _globalSettings = globalSettings.Value;
     }
 
-    public IApiContentRoute Build(IPublishedContent content, string? culture = null)
+    public IApiContentRoute? Build(IPublishedContent content, string? culture = null)
     {
         if (content.ItemType != PublishedItemType.Content)
         {
@@ -42,11 +42,17 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
         // in some scenarios the published content is actually routable, but due to the built-in handling of i.e. lacking culture setup
         // the URL provider resolves the content URL as empty string or "#". since the Delivery API handles routing explicitly,
         // we can perform fallback to the content route.
-        if (contentPath.IsNullOrWhiteSpace() || "#".Equals(contentPath))
+        if (IsInvalidContentPath(contentPath))
         {
             contentPath = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot().Content?.GetRouteById(content.Id, culture) ?? contentPath;
         }
 
+        // if the content path has still not been resolved as a valid path, the content is un-routable in this culture
+        if (IsInvalidContentPath(contentPath))
+        {
+            return null;
+        }
+
         contentPath = contentPath.EnsureStartsWith("/");
         if (_globalSettings.HideTopLevelNodeFromPath == false)
         {
@@ -55,4 +61,6 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
 
         return new ApiContentRoute(contentPath, new ApiContentStartItem(root.Key, rootPath));
     }
+
+    private static bool IsInvalidContentPath(string path) => path.IsNullOrWhiteSpace() || "#".Equals(path);
 }
diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentBuilder.cs b/src/Umbraco.Core/DeliveryApi/IApiContentBuilder.cs
index 784cb29370..946a75cda7 100644
--- a/src/Umbraco.Core/DeliveryApi/IApiContentBuilder.cs
+++ b/src/Umbraco.Core/DeliveryApi/IApiContentBuilder.cs
@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
 
 public interface IApiContentBuilder
 {
-    IApiContent Build(IPublishedContent content);
+    IApiContent? Build(IPublishedContent content);
 }
diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentResponseBuilder.cs b/src/Umbraco.Core/DeliveryApi/IApiContentResponseBuilder.cs
index 82c25f3284..1d025ecadf 100644
--- a/src/Umbraco.Core/DeliveryApi/IApiContentResponseBuilder.cs
+++ b/src/Umbraco.Core/DeliveryApi/IApiContentResponseBuilder.cs
@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
 
 public interface IApiContentResponseBuilder
 {
-    IApiContentResponse Build(IPublishedContent content);
+    IApiContentResponse? Build(IPublishedContent content);
 }
diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/IApiContentRouteBuilder.cs
index 764e7765c2..dbd1103353 100644
--- a/src/Umbraco.Core/DeliveryApi/IApiContentRouteBuilder.cs
+++ b/src/Umbraco.Core/DeliveryApi/IApiContentRouteBuilder.cs
@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
 
 public interface IApiContentRouteBuilder
 {
-    IApiContentRoute Build(IPublishedContent content, string? culture = null);
+    IApiContentRoute? Build(IPublishedContent content, string? culture = null);
 }
diff --git a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParser.cs b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParser.cs
index f2d2181e60..4044b37516 100644
--- a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParser.cs
+++ b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParser.cs
@@ -117,9 +117,12 @@ internal sealed partial class ApiRichTextParser : IApiRichTextParser
         {
             case Constants.UdiEntityType.Document:
                 IPublishedContent? content = publishedSnapshot.Content?.GetById(udi);
-                if (content != null)
+                IApiContentRoute? route = content != null
+                    ? _apiContentRouteBuilder.Build(content)
+                    : null;
+                if (route != null)
                 {
-                    attributes["route"] = _apiContentRouteBuilder.Build(content);
+                    attributes["route"] = route;
                 }
 
                 break;
diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs
index 605c348992..821ece9b33 100644
--- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs
+++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs
@@ -177,14 +177,17 @@ public class MultiUrlPickerValueConverter : PropertyValueConverterBase, IDeliver
             {
                 case Constants.UdiEntityType.Document:
                     IPublishedContent? content = publishedSnapshot.Content?.GetById(item.Udi.Guid);
-                    return content == null
+                    IApiContentRoute? route = content != null
+                        ? _apiContentRouteBuilder.Build(content)
+                        : null;
+                    return content == null || route == null
                         ? null
                         : ApiLink.Content(
                             item.Name.IfNullOrWhiteSpace(_apiContentNameProvider.GetName(content)),
                             item.Target,
                             content.Key,
                             content.ContentType.Alias,
-                            _apiContentRouteBuilder.Build(content));
+                            route);
                 case Constants.UdiEntityType.Media:
                     IPublishedContent? media = publishedSnapshot.Media?.GetById(item.Udi.Guid);
                     return media == null
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs
index 750ac885e0..f214c05446 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs
@@ -1,6 +1,7 @@
 using Moq;
 using NUnit.Framework;
 using Umbraco.Cms.Core.DeliveryApi;
+using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.PublishedCache;
@@ -61,10 +62,36 @@ public class ContentBuilderTests : DeliveryApiTests
         var customNameProvider = new Mock();
         customNameProvider.Setup(n => n.GetName(content.Object)).Returns($"Custom name for: {content.Object.Name}");
 
-        var builder = new ApiContentBuilder(customNameProvider.Object, Mock.Of(), CreateOutputExpansionStrategyAccessor());
+        var routeBuilder = new Mock();
+        routeBuilder
+            .Setup(r => r.Build(content.Object, It.IsAny()))
+            .Returns(new ApiContentRoute(content.Object.UrlSegment!, new ApiContentStartItem(Guid.NewGuid(), "/")));
+
+        var builder = new ApiContentBuilder(customNameProvider.Object, routeBuilder.Object, CreateOutputExpansionStrategyAccessor());
         var result = builder.Build(content.Object);
 
         Assert.NotNull(result);
         Assert.AreEqual("Custom name for: The page", result.Name);
     }
+
+    [Test]
+    public void ContentBuilder_ReturnsNullForUnRoutableContent()
+    {
+        var content = new Mock();
+
+        var contentType = new Mock();
+        contentType.SetupGet(c => c.Alias).Returns("thePageType");
+
+        ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", "the-page", contentType.Object, Array.Empty());
+
+        var routeBuilder = new Mock();
+        routeBuilder
+            .Setup(r => r.Build(content.Object, It.IsAny()))
+            .Returns((ApiContentRoute)null);
+
+        var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilder.Object, CreateOutputExpansionStrategyAccessor());
+        var result = builder.Build(content.Object);
+
+        Assert.Null(result);
+    }
 }
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs
index 202026ed30..90774c5e25 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs
@@ -2,6 +2,7 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core.DeliveryApi;
 using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Routing;
@@ -21,6 +22,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
         var result = builder.Build(root);
+        Assert.IsNotNull(result);
         Assert.AreEqual("/", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root", result.StartItem.Path);
@@ -38,6 +40,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
         var result = builder.Build(child);
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root", result.StartItem.Path);
@@ -58,6 +61,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
         var result = builder.Build(grandchild);
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child/the-grandchild", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root", result.StartItem.Path);
@@ -74,11 +78,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(false);
         var result = builder.Build(child, "en-us");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child-en-us", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root-en-us", result.StartItem.Path);
 
         result = builder.Build(child, "da-dk");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child-da-dk", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root-da-dk", result.StartItem.Path);
@@ -95,11 +101,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(false);
         var result = builder.Build(child, "en-us");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root-en-us", result.StartItem.Path);
 
         result = builder.Build(child, "da-dk");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root-da-dk", result.StartItem.Path);
@@ -116,11 +124,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
 
         var builder = CreateApiContentRouteBuilder(false);
         var result = builder.Build(child, "en-us");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child-en-us", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root", result.StartItem.Path);
 
         result = builder.Build(child, "da-dk");
+        Assert.IsNotNull(result);
         Assert.AreEqual("/the-child-da-dk", result.Path);
         Assert.AreEqual(rootKey, result.StartItem.Id);
         Assert.AreEqual("the-root", result.StartItem.Path);
@@ -144,36 +154,18 @@ public class ContentRouteBuilderTests : DeliveryApiTests
     [TestCase("#")]
     public void FallsBackToContentPathIfUrlProviderCannotResolveUrl(string resolvedUrl)
     {
-        var publishedUrlProviderMock = new Mock();
-        publishedUrlProviderMock
-            .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()))
-            .Returns(resolvedUrl);
+        var result = GetUnRoutableRoute(resolvedUrl, "/the/content/route");
+        Assert.IsNotNull(result);
+        Assert.AreEqual("/the/content/route", result.Path);
+    }
 
-        var publishedContentCacheMock = new Mock();
-        publishedContentCacheMock
-            .Setup(c => c.GetRouteById(It.IsAny(), It.IsAny()))
-            .Returns("/the/content/route");
-
-        var publishedSnapshotMock = new Mock();
-        publishedSnapshotMock
-            .SetupGet(s => s.Content)
-            .Returns(publishedContentCacheMock.Object);
-        var publishedSnapshot = publishedSnapshotMock.Object;
-
-        var publishedSnapshotAccessorMock = new Mock();
-        publishedSnapshotAccessorMock
-            .Setup(a => a.TryGetPublishedSnapshot(out publishedSnapshot))
-            .Returns(true);
-
-        var content = SetupVariantPublishedContent("The Content", Guid.NewGuid());
-
-        var builder = new ApiContentRouteBuilder(
-            publishedUrlProviderMock.Object,
-            CreateGlobalSettings(),
-            Mock.Of(),
-            publishedSnapshotAccessorMock.Object);
-
-        Assert.AreEqual("/the/content/route", builder.Build(content).Path);
+    [TestCase("")]
+    [TestCase(" ")]
+    [TestCase("#")]
+    public void YieldsNullForUnRoutableContent(string contentPath)
+    {
+        var result = GetUnRoutableRoute(contentPath, contentPath);
+        Assert.IsNull(result);
     }
 
     [TestCase(true)]
@@ -253,4 +245,38 @@ public class ContentRouteBuilderTests : DeliveryApiTests
             CreateGlobalSettings(hideTopLevelNodeFromPath),
             Mock.Of(),
             Mock.Of());
+
+    private IApiContentRoute? GetUnRoutableRoute(string publishedUrl, string routeById)
+    {
+        var publishedUrlProviderMock = new Mock();
+        publishedUrlProviderMock
+            .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()))
+            .Returns(publishedUrl);
+
+        var publishedContentCacheMock = new Mock();
+        publishedContentCacheMock
+            .Setup(c => c.GetRouteById(It.IsAny(), It.IsAny()))
+            .Returns(routeById);
+
+        var publishedSnapshotMock = new Mock();
+        publishedSnapshotMock
+            .SetupGet(s => s.Content)
+            .Returns(publishedContentCacheMock.Object);
+        var publishedSnapshot = publishedSnapshotMock.Object;
+
+        var publishedSnapshotAccessorMock = new Mock();
+        publishedSnapshotAccessorMock
+            .Setup(a => a.TryGetPublishedSnapshot(out publishedSnapshot))
+            .Returns(true);
+
+        var content = SetupVariantPublishedContent("The Content", Guid.NewGuid());
+
+        var builder = new ApiContentRouteBuilder(
+            publishedUrlProviderMock.Object,
+            CreateGlobalSettings(),
+            Mock.Of(),
+            publishedSnapshotAccessorMock.Object);
+
+        return builder.Build(content);
+    }
 }
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs
index 320f16dbf6..80e175bf81 100644
--- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs
+++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs
@@ -15,13 +15,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi;
 [TestFixture]
 public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTests
 {
-    private MultiNodeTreePickerValueConverter MultiNodeTreePickerValueConverter()
+    private MultiNodeTreePickerValueConverter MultiNodeTreePickerValueConverter(IApiContentRouteBuilder? routeBuilder = null)
     {
         var expansionStrategyAccessor = CreateOutputExpansionStrategyAccessor();
 
         var contentNameProvider = new ApiContentNameProvider();
         var apiUrProvider = new ApiMediaUrlProvider(PublishedUrlProvider);
-        var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of());
+        routeBuilder = routeBuilder ?? new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of(), Mock.Of());
         return new MultiNodeTreePickerValueConverter(
             PublishedSnapshotAccessor,
             Mock.Of(),
@@ -274,4 +274,21 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest
         Assert.NotNull(result);
         Assert.IsEmpty(result);
     }
+
+    [Test]
+    public void MultiNodeTreePickerValueConverter_YieldsNothingForUnRoutableContent()
+    {
+        var publishedDataType = MultiNodePickerPublishedDataType(false, Constants.UdiEntityType.Document);
+        var publishedPropertyType = new Mock();
+        publishedPropertyType.SetupGet(p => p.DataType).Returns(publishedDataType);
+
+        // mocking the route builder will make it yield null values for all routes, so there is no need to setup anything on the mock
+        var routeBuilder = new Mock();
+        var valueConverter = MultiNodeTreePickerValueConverter(routeBuilder.Object);
+
+        var inter = new Udi[] { new GuidUdi(Constants.UdiEntityType.Document, PublishedContent.Key) };
+        var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of(), publishedPropertyType.Object, PropertyCacheLevel.Element, inter, false) as IEnumerable;
+        Assert.NotNull(result);
+        Assert.IsEmpty(result);
+    }
 }

From e572dcfa2da5c0921c767406dc2d2ad9456d1728 Mon Sep 17 00:00:00 2001
From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com>
Date: Mon, 15 May 2023 10:46:29 +0200
Subject: [PATCH 04/21] Delivery API: Handle more unhappy paths when querying
 (#14245)

* Handle more unhappy paths

* Review comments

---------

Co-authored-by: kjac 
---
 .../Querying/Filters/ContentTypeFilter.cs        |  5 ++++-
 .../Querying/Filters/NameFilter.cs               |  5 ++++-
 .../Querying/QueryOptionBase.cs                  | 16 +++++++++-------
 .../Querying/Selectors/AncestorsSelector.cs      | 11 ++++++-----
 .../Services/RequestRoutingService.cs            |  5 +++++
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Filters/ContentTypeFilter.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Filters/ContentTypeFilter.cs
index e851158b87..13d9dc77ec 100644
--- a/src/Umbraco.Cms.Api.Delivery/Querying/Filters/ContentTypeFilter.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Querying/Filters/ContentTypeFilter.cs
@@ -1,5 +1,6 @@
 using Umbraco.Cms.Api.Delivery.Indexing.Filters;
 using Umbraco.Cms.Core.DeliveryApi;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Delivery.Querying.Filters;
 
@@ -19,7 +20,9 @@ public sealed class ContentTypeFilter : IFilterHandler
         return new FilterOption
         {
             FieldName = ContentTypeFilterIndexer.FieldName,
-            Values = new[] { alias.TrimStart('!') },
+            Values = alias.IsNullOrWhiteSpace() == false
+                ? new[] { alias.TrimStart('!') }
+                : Array.Empty(),
             Operator = alias.StartsWith('!')
                 ? FilterOperation.IsNot
                 : FilterOperation.Is
diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Filters/NameFilter.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Filters/NameFilter.cs
index 64aa5b2776..0bf3d5e460 100644
--- a/src/Umbraco.Cms.Api.Delivery/Querying/Filters/NameFilter.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Querying/Filters/NameFilter.cs
@@ -1,5 +1,6 @@
 using Umbraco.Cms.Api.Delivery.Indexing.Sorts;
 using Umbraco.Cms.Core.DeliveryApi;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Delivery.Querying.Filters;
 
@@ -19,7 +20,9 @@ public sealed class NameFilter : IFilterHandler
         return new FilterOption
         {
             FieldName = NameSortIndexer.FieldName,
-            Values = new[] { value.TrimStart('!') },
+            Values = value.IsNullOrWhiteSpace() == false
+                ? new[] { value.TrimStart('!') }
+                : Array.Empty(),
             Operator = value.StartsWith('!')
                 ? FilterOperation.IsNot
                 : FilterOperation.Is
diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs b/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs
index 8934889715..f29e0465f5 100644
--- a/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Querying/QueryOptionBase.cs
@@ -1,6 +1,7 @@
 using Umbraco.Cms.Core.DeliveryApi;
 using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Delivery.Querying;
 
@@ -16,22 +17,23 @@ public abstract class QueryOptionBase
         _requestRoutingService = requestRoutingService;
     }
 
-    public Guid? GetGuidFromQuery(string queryStringValue)
+    protected Guid? GetGuidFromQuery(string queryStringValue)
     {
+        if (queryStringValue.IsNullOrWhiteSpace())
+        {
+            return null;
+        }
+
         if (Guid.TryParse(queryStringValue, out Guid id))
         {
             return id;
         }
 
-        if (!_publishedSnapshotAccessor.TryGetPublishedSnapshot(out IPublishedSnapshot? publishedSnapshot) ||
-            publishedSnapshot?.Content is null)
-        {
-            return null;
-        }
+        IPublishedSnapshot publishedSnapshot = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot();
 
         // Check if the passed value is a path of a content item
         var contentRoute = _requestRoutingService.GetContentRoute(queryStringValue);
-        IPublishedContent? contentItem = publishedSnapshot.Content.GetByRoute(contentRoute);
+        IPublishedContent? contentItem = publishedSnapshot.Content?.GetByRoute(contentRoute);
 
         return contentItem?.Key;
     }
diff --git a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs
index 67490e38f4..5e8e7e2019 100644
--- a/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Querying/Selectors/AncestorsSelector.cs
@@ -25,9 +25,7 @@ public sealed class AncestorsSelector : QueryOptionBase, ISelectorHandler
         var fieldValue = selector[AncestorsSpecifier.Length..];
         Guid? id = GetGuidFromQuery(fieldValue);
 
-        if (id is null ||
-            !_publishedSnapshotAccessor.TryGetPublishedSnapshot(out IPublishedSnapshot? publishedSnapshot) ||
-            publishedSnapshot?.Content is null)
+        if (id is null)
         {
             // Setting the Value to "" since that would yield no results.
             // It won't be appropriate to return null here since if we reached this,
@@ -39,8 +37,11 @@ public sealed class AncestorsSelector : QueryOptionBase, ISelectorHandler
             };
         }
 
-        // With the previous check we made sure that if we reach this, we already made sure that there is a valid content item
-        IPublishedContent contentItem = publishedSnapshot.Content.GetById((Guid)id)!; // so it can't be null
+        IPublishedSnapshot publishedSnapshot = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot();
+
+        IPublishedContent contentItem = publishedSnapshot.Content?.GetById((Guid)id)
+                                        ?? throw new InvalidOperationException("Could not obtain the content cache");
+
         var ancestorKeys = contentItem.Ancestors().Select(a => a.Key.ToString("D")).ToArray();
 
         return new SelectorOption
diff --git a/src/Umbraco.Cms.Api.Delivery/Services/RequestRoutingService.cs b/src/Umbraco.Cms.Api.Delivery/Services/RequestRoutingService.cs
index 993780680d..6bf0dbc887 100644
--- a/src/Umbraco.Cms.Api.Delivery/Services/RequestRoutingService.cs
+++ b/src/Umbraco.Cms.Api.Delivery/Services/RequestRoutingService.cs
@@ -22,6 +22,11 @@ internal sealed class RequestRoutingService : RoutingServiceBase, IRequestRoutin
     /// 
     public string GetContentRoute(string requestedPath)
     {
+        if (requestedPath.IsNullOrWhiteSpace())
+        {
+            return string.Empty;
+        }
+
         requestedPath = requestedPath.EnsureStartsWith("/");
 
         // do we have an explicit start item?

From 79ae829ac78201435b2230381f6c595dd18c03ae Mon Sep 17 00:00:00 2001
From: Nikolaj 
Date: Mon, 15 May 2023 10:55:43 +0200
Subject: [PATCH 05/21] Set version to rc1

---
 version.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/version.json b/version.json
index 1af6ab23c5..e22da7303c 100644
--- a/version.json
+++ b/version.json
@@ -1,6 +1,6 @@
 {
   "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json",
-  "version": "12.0.0-rc",
+  "version": "12.0.0-rc1",
   "assemblyVersion": {
     "precision": "build"
   },

From e50a3d13409d93e7d30363f0bf9e32108c1ba2a3 Mon Sep 17 00:00:00 2001
From: Nikolaj 
Date: Mon, 15 May 2023 12:43:13 +0200
Subject: [PATCH 06/21] Trust Test DB certificate

---
 build/azure-pipelines.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build/azure-pipelines.yml b/build/azure-pipelines.yml
index 310477fa4c..af8cdea3a3 100644
--- a/build/azure-pipelines.yml
+++ b/build/azure-pipelines.yml
@@ -292,7 +292,7 @@ stages:
             Linux:
               vmImage: 'ubuntu-latest'
               testDb: SqlServer
-              connectionString: 'Server=localhost,1433;User Id=sa;Password=$(SA_PASSWORD);'
+              connectionString: 'Server=localhost,1433;User Id=sa;Password=$(SA_PASSWORD);TrustServerCertificate=true'
         pool:
           vmImage: $(vmImage)
         variables:

From 27e5b55e0575a92ae8c6b7a6ea84796b70889168 Mon Sep 17 00:00:00 2001
From: Bjarke Berg 
Date: Mon, 15 May 2023 13:24:19 +0200
Subject: [PATCH 07/21] Added OpenIddict dependencies for future usage (#14247)

---
 src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj        | 2 ++
 .../Umbraco.Cms.Persistence.EFCore.csproj                       | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj b/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj
index fd5e7d4b83..5a2dfa69ef 100644
--- a/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj
+++ b/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj
@@ -10,6 +10,8 @@
   
     
     
+    
+    
   
   
     
diff --git a/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj b/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj
index af566ab67a..3e6e57983f 100644
--- a/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj
+++ b/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj
@@ -9,6 +9,7 @@
     
     
     
+    
   
 
   

From 0548266d0fa452ad5e5a4eb4b9b1cc7b09008aee Mon Sep 17 00:00:00 2001
From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com>
Date: Mon, 15 May 2023 16:09:36 +0200
Subject: [PATCH 08/21] Check folder exist before trying to delete (#14249)

Co-authored-by: Zeegaan 
---
 .../Migrations/Upgrade/V_12_0_0/ResetCache.cs                | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_12_0_0/ResetCache.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_12_0_0/ResetCache.cs
index ce105bf6b8..a3ec96ad7e 100644
--- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_12_0_0/ResetCache.cs
+++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_12_0_0/ResetCache.cs
@@ -22,6 +22,11 @@ public class ResetCache : MigrationBase
 
     private void DeleteAllFilesInFolder(string path)
     {
+        if (Directory.Exists(path) == false)
+        {
+            return;
+        }
+
         var directoryInfo = new DirectoryInfo(path);
 
         foreach (FileInfo file in directoryInfo.GetFiles())

From c632908a516d87d507d51767353a1ce61f2bd2ef Mon Sep 17 00:00:00 2001
From: Bjarke Berg 
Date: Tue, 16 May 2023 14:28:30 +0200
Subject: [PATCH 09/21] Re-added the postmigration from the migration context
 (#14255)

* Re-added the postmigration from the migration context

* Minor cleanup

* Add executed migration contexts to the result object

---------

Co-authored-by: Nikolaj 
---
 .../Migrations/ExecutedMigrationPlan.cs       |  4 ++
 .../Migrations/IMigrationContext.cs           |  7 +++
 .../Migrations/MigrationContext.cs            | 15 +++++++
 .../Migrations/MigrationPlanExecutor.cs       | 44 +++++++++++++++++--
 4 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs
index 065a8e049f..09d726fd6a 100644
--- a/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs
+++ b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs
@@ -24,6 +24,7 @@ public class ExecutedMigrationPlan
         FinalState = finalState ?? throw new ArgumentNullException(nameof(finalState));
         Successful = successful;
         CompletedTransitions = completedTransitions;
+        ExecutedMigrationContexts = Array.Empty();
     }
 
     public ExecutedMigrationPlan()
@@ -59,4 +60,7 @@ public class ExecutedMigrationPlan
     /// A collection of all the succeeded transition.
     ///
public required IReadOnlyList CompletedTransitions { get; init; } + + [Obsolete("This will be removed in the V13, and replaced with UmbracoPlanExecutedNotification")] + internal IReadOnlyList ExecutedMigrationContexts { get; init; } = Array.Empty(); } diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs index 0001b3381d..5e0766755a 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs @@ -37,4 +37,11 @@ public interface IMigrationContext /// Gets or sets a value indicating whether an expression is being built. /// bool BuildingExpression { get; set; } + + /// + /// Adds a post-migration. + /// + [Obsolete("This will be removed in the V13, and replaced with a RebuildCache flag on the MigrationBase")] + void AddPostMigration() + where TMigration : MigrationBase; } diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs index 4b1900c27c..c07adbec90 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs @@ -8,6 +8,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations; /// internal class MigrationContext : IMigrationContext { + private readonly List _postMigrations = new(); + /// /// Initializes a new instance of the class. /// @@ -18,6 +20,10 @@ internal class MigrationContext : IMigrationContext Logger = logger ?? throw new ArgumentNullException(nameof(logger)); } + // this is only internally exposed + [Obsolete("This will be removed in the V13, and replaced with a RebuildCache flag on the MigrationBase")] + internal IReadOnlyList PostMigrations => _postMigrations; + /// public ILogger Logger { get; } @@ -34,4 +40,13 @@ internal class MigrationContext : IMigrationContext /// public bool BuildingExpression { get; set; } + + + /// + [Obsolete("This will be removed in the V13, and replaced with a RebuildCache flag on the MigrationBase, and a UmbracoPlanExecutedNotification.")] + public void AddPostMigration() + where TMigration : MigrationBase => + + // just adding - will be de-duplicated when executing + _postMigrations.Add(typeof(TMigration)); } diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs index c271088626..faea87bd68 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -99,6 +99,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor ExecutedMigrationPlan result = RunMigrationPlan(plan, fromState); + HandlePostMigrations(result); + // If any completed migration requires us to rebuild cache we'll do that. if (_rebuildCache) { @@ -108,6 +110,33 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor return result; } + [Obsolete] + private void HandlePostMigrations(ExecutedMigrationPlan result) + { + // prepare and de-duplicate post-migrations, only keeping the 1st occurence + var executedTypes = new HashSet(); + + foreach (var executedMigrationContext in result.ExecutedMigrationContexts) + { + if (executedMigrationContext is MigrationContext migrationContext) + { + foreach (Type migrationContextPostMigration in migrationContext.PostMigrations) + { + if (executedTypes.Contains(migrationContextPostMigration)) + { + continue; + } + + _logger.LogInformation($"PostMigration: {migrationContextPostMigration.FullName}."); + MigrationBase postMigration = _migrationBuilder.Build(migrationContextPostMigration, executedMigrationContext); + postMigration.Run(); + + executedTypes.Add(migrationContextPostMigration); + } + } + } + } + private ExecutedMigrationPlan RunMigrationPlan(MigrationPlan plan, string fromState) { _logger.LogInformation("Starting '{MigrationName}'...", plan.Name); @@ -122,6 +151,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor List completedTransitions = new(); + var executedMigrationContexts = new List(); while (transition is not null) { _logger.LogInformation("Execute {MigrationType}", transition.MigrationType.Name); @@ -130,11 +160,11 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor { if (transition.MigrationType.IsAssignableTo(typeof(UnscopedMigrationBase))) { - RunUnscopedMigration(transition.MigrationType, plan); + executedMigrationContexts.Add(RunUnscopedMigration(transition.MigrationType, plan)); } else { - RunScopedMigration(transition.MigrationType, plan); + executedMigrationContexts.Add(RunScopedMigration(transition.MigrationType, plan)); } } catch (Exception exception) @@ -149,6 +179,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor FinalState = transition.SourceState, CompletedTransitions = completedTransitions, Plan = plan, + ExecutedMigrationContexts = executedMigrationContexts }; } @@ -197,18 +228,21 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor FinalState = finalState, CompletedTransitions = completedTransitions, Plan = plan, + ExecutedMigrationContexts = executedMigrationContexts }; } - private void RunUnscopedMigration(Type migrationType, MigrationPlan plan) + private MigrationContext RunUnscopedMigration(Type migrationType, MigrationPlan plan) { using IUmbracoDatabase database = _databaseFactory.CreateDatabase(); var context = new MigrationContext(plan, database, _loggerFactory.CreateLogger()); RunMigration(migrationType, context); + + return context; } - private void RunScopedMigration(Type migrationType, MigrationPlan plan) + private MigrationContext RunScopedMigration(Type migrationType, MigrationPlan plan) { // We want to suppress scope (service, etc...) notifications during a migration plan // execution. This is because if a package that doesn't have their migration plan @@ -225,6 +259,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor RunMigration(migrationType, context); scope.Complete(); + + return context; } } From c5e524c9b9a6c935a52a99780700e0ca5ac9f1f7 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 16 May 2023 19:08:38 +0200 Subject: [PATCH 10/21] Wrap RTE content in a dedicated model for future expansion (#14258) --- src/Umbraco.Core/Models/DeliveryApi/RichTextModel.cs | 6 ++++++ .../ValueConverters/RteMacroRenderingValueConverter.cs | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 src/Umbraco.Core/Models/DeliveryApi/RichTextModel.cs diff --git a/src/Umbraco.Core/Models/DeliveryApi/RichTextModel.cs b/src/Umbraco.Core/Models/DeliveryApi/RichTextModel.cs new file mode 100644 index 0000000000..2af7570183 --- /dev/null +++ b/src/Umbraco.Core/Models/DeliveryApi/RichTextModel.cs @@ -0,0 +1,6 @@ +namespace Umbraco.Cms.Core.Models.DeliveryApi; + +public class RichTextModel +{ + public required string Markup { get; set; } +} diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs index 68d15aa99f..6172fbc629 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs @@ -82,13 +82,16 @@ public class RteMacroRenderingValueConverter : SimpleTinyMceValueConverter, IDel public Type GetDeliveryApiPropertyValueType(IPublishedPropertyType propertyType) => _deliveryApiSettings.RichTextOutputAsJson ? typeof(IRichTextElement) - : typeof(string); + : typeof(RichTextModel); public object? ConvertIntermediateToDeliveryApiObject(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, object? inter, bool preview) { if (_deliveryApiSettings.RichTextOutputAsJson is false) { - return Convert(inter, preview) ?? string.Empty; + return new RichTextModel + { + Markup = Convert(inter, preview) ?? string.Empty + }; } var sourceString = inter?.ToString(); From a4202f352aabbf230b300fad389c8ebefe4c2b6c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 17 May 2023 11:03:27 +0200 Subject: [PATCH 11/21] Ignore macros completely in deliveryapi (#14262) * Hack a fix for MNTP that saves content in configuration instead of document * Ignore macro parsing in delivery api, instead of exploding --- .../ValueConverters/MultiNodeTreePickerValueConverter.cs | 7 +++++++ .../ValueConverters/RteMacroRenderingValueConverter.cs | 9 ++++++--- .../MultiNodeTreePickerValueConverterTests.cs | 6 ++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/ValueConverters/MultiNodeTreePickerValueConverter.cs b/src/Umbraco.Core/PropertyEditors/ValueConverters/MultiNodeTreePickerValueConverter.cs index f863c96151..d0ab92223b 100644 --- a/src/Umbraco.Core/PropertyEditors/ValueConverters/MultiNodeTreePickerValueConverter.cs +++ b/src/Umbraco.Core/PropertyEditors/ValueConverters/MultiNodeTreePickerValueConverter.cs @@ -204,6 +204,13 @@ public class MultiNodeTreePickerValueConverter : PropertyValueConverterBase, IDe IPublishedSnapshot publishedSnapshot = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot(); var entityType = GetEntityType(propertyType); + + if (entityType == "content") + { + // TODO Why do MNTP config saves "content" and not "document"? + entityType = Constants.UdiEntityType.Document; + } + GuidUdi[] entityTypeUdis = udis.Where(udi => udi.EntityType == entityType).OfType().ToArray(); return entityType switch { diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs index 6172fbc629..9d8010ab5f 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs @@ -90,7 +90,7 @@ public class RteMacroRenderingValueConverter : SimpleTinyMceValueConverter, IDel { return new RichTextModel { - Markup = Convert(inter, preview) ?? string.Empty + Markup = Convert(inter, preview, false) ?? string.Empty }; } @@ -129,7 +129,7 @@ public class RteMacroRenderingValueConverter : SimpleTinyMceValueConverter, IDel } } - private string? Convert(object? source, bool preview) + private string? Convert(object? source, bool preview, bool handleMacros = true) { if (source == null) { @@ -144,7 +144,10 @@ public class RteMacroRenderingValueConverter : SimpleTinyMceValueConverter, IDel sourceString = _imageSourceParser.EnsureImageSources(sourceString); // ensure string is parsed for macros and macros are executed correctly - sourceString = RenderRteMacros(sourceString, preview); + if (handleMacros) + { + sourceString = RenderRteMacros(sourceString, preview); + } // find and remove the rel attributes used in the Umbraco UI from img tags var doc = new HtmlDocument(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs index 80e175bf81..276df9a223 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs @@ -93,7 +93,9 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest } [Test] - public void MultiNodeTreePickerValueConverter_RendersContentProperties() + [TestCase(Constants.UdiEntityType.Document)] + [TestCase("content")] + public void MultiNodeTreePickerValueConverter_RendersContentProperties(string entityType) { var content = new Mock(); @@ -112,7 +114,7 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest .Setup(pcc => pcc.GetById(key)) .Returns(content.Object); - var publishedDataType = MultiNodePickerPublishedDataType(false, Constants.UdiEntityType.Document); + var publishedDataType = MultiNodePickerPublishedDataType(false, entityType); var publishedPropertyType = new Mock(); publishedPropertyType.SetupGet(p => p.DataType).Returns(publishedDataType); From 867a8a4066915d0839269865bba69a5b4e274a85 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Wed, 17 May 2023 13:25:08 +0200 Subject: [PATCH 12/21] Bump version to next RC --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index e22da7303c..b465096390 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "12.0.0-rc1", + "version": "12.0.0-rc2", "assemblyVersion": { "precision": "build" }, From 16f448a802f352a9859bf3c45dcca626ed097be9 Mon Sep 17 00:00:00 2001 From: Dhanesh Kumar Mj <58820887+dKumarmj@users.noreply.github.com> Date: Fri, 19 May 2023 14:51:40 +0530 Subject: [PATCH 13/21] [Fix] Block editor labels showing Angular JS on first load. (#14143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dhanesh Kumar <“dhanesh.kumar@phases.io”> (cherry picked from commit 58695b6e9fdafc0539a757e6d842e44f618d833d) --- .../src/common/services/blockeditormodelobject.service.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index abb81c38dc..20661d5d1c 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -639,10 +639,9 @@ mapToPropertyModel(this.settings, this.settingsData); } }; - // first time instant update of label. - blockObject.label = (blockObject.config.label || blockObject.content?.contentTypeName) ?? "" ; - blockObject.index = 0; + blockObject.label = blockObject.content?.contentTypeName || ""; + blockObject.index = 0; if (blockObject.config.label && blockObject.config.label !== "" && blockObject.config.unsupported !== true) { var labelElement = $('
', { text: blockObject.config.label}); From 7d3c22b28a43392ea427de578e97b4935d2b45d9 Mon Sep 17 00:00:00 2001 From: Dhanesh Kumar Mj <58820887+dKumarmj@users.noreply.github.com> Date: Fri, 19 May 2023 14:51:40 +0530 Subject: [PATCH 14/21] [Fix] Block editor labels showing Angular JS on first load. (#14143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dhanesh Kumar <“dhanesh.kumar@phases.io”> (cherry picked from commit 58695b6e9fdafc0539a757e6d842e44f618d833d) --- .../src/common/services/blockeditormodelobject.service.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js index abb81c38dc..20661d5d1c 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/blockeditormodelobject.service.js @@ -639,10 +639,9 @@ mapToPropertyModel(this.settings, this.settingsData); } }; - // first time instant update of label. - blockObject.label = (blockObject.config.label || blockObject.content?.contentTypeName) ?? "" ; - blockObject.index = 0; + blockObject.label = blockObject.content?.contentTypeName || ""; + blockObject.index = 0; if (blockObject.config.label && blockObject.config.label !== "" && blockObject.config.unsupported !== true) { var labelElement = $('
', { text: blockObject.config.label}); From 8bbca79e55e5de0b62c148be45bc827b92eb6a82 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 22 May 2023 11:06:22 +0200 Subject: [PATCH 15/21] Split the Examine specifics from API query service (#14257) * Split the Examine specifics from API query service to a provider based model * Review changes: Use paged model as provider return value + add logging --- .../Controllers/ContentApiControllerBase.cs | 4 - .../UmbracoBuilderExtensions.cs | 1 + .../Services/ApiContentQueryProvider.cs | 164 +++++++++++++ .../Services/ApiContentQueryService.cs | 230 ++++-------------- .../DeliveryApi/IApiContentQueryProvider.cs | 26 ++ .../ApiContentQueryOperationStatus.cs | 1 - 6 files changed, 235 insertions(+), 191 deletions(-) create mode 100644 src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs create mode 100644 src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs index 834332fcbb..e260200d5e 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs @@ -31,10 +31,6 @@ public abstract class ContentApiControllerBase : DeliveryApiControllerBase .WithTitle("Filter option not found") .WithDetail("One of the attempted 'filter' options does not exist") .Build()), - ApiContentQueryOperationStatus.IndexNotFound => BadRequest(new ProblemDetailsBuilder() - .WithTitle("Examine index not found") - .WithDetail($"No index found with name {Constants.UmbracoIndexes.DeliveryApiContentIndexName}") - .Build()), ApiContentQueryOperationStatus.SelectorOptionNotFound => BadRequest(new ProblemDetailsBuilder() .WithTitle("Selector option not found") .WithDetail("The attempted 'fetch' option does not exist") diff --git a/src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs index 6370fc24f9..37d84bc273 100644 --- a/src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Delivery/DependencyInjection/UmbracoBuilderExtensions.cs @@ -28,6 +28,7 @@ public static class UmbracoBuilderExtensions builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder.Services.ConfigureOptions(); builder.AddUmbracoApiOpenApiUI(); diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs new file mode 100644 index 0000000000..d9ad8d0f15 --- /dev/null +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs @@ -0,0 +1,164 @@ +using Examine; +using Examine.Search; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Infrastructure.Examine; +using Umbraco.Extensions; +using Umbraco.New.Cms.Core.Models; + +namespace Umbraco.Cms.Api.Delivery.Services; + +/// +/// This is the Examine implementation of content querying for the Delivery API. +/// +internal sealed class ApiContentQueryProvider : IApiContentQueryProvider +{ + private const string ItemIdFieldName = "itemId"; + private readonly IExamineManager _examineManager; + private readonly ILogger _logger; + private readonly string _fallbackGuidValue; + private readonly Dictionary _fieldTypes; + + public ApiContentQueryProvider( + IExamineManager examineManager, + ContentIndexHandlerCollection indexHandlers, + ILogger logger) + { + _examineManager = examineManager; + _logger = logger; + + // A fallback value is needed for Examine queries in case we don't have a value - we can't pass null or empty string + // It is set to a random guid since this would be highly unlikely to yield any results + _fallbackGuidValue = Guid.NewGuid().ToString("D"); + + // build a look-up dictionary of field types by field name + _fieldTypes = indexHandlers + .SelectMany(handler => handler.GetFields()) + .DistinctBy(field => field.FieldName) + .ToDictionary(field => field.FieldName, field => field.FieldType, StringComparer.InvariantCultureIgnoreCase); + } + + public PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, int skip, int take) + { + if (!_examineManager.TryGetIndex(Constants.UmbracoIndexes.DeliveryApiContentIndexName, out IIndex? index)) + { + _logger.LogError("Could not find the index {IndexName} when attempting to execute a query.", Constants.UmbracoIndexes.DeliveryApiContentIndexName); + return new PagedModel(); + } + + IBooleanOperation queryOperation = BuildSelectorOperation(selectorOption, index, culture); + + ApplyFiltering(filterOptions, queryOperation); + ApplySorting(sortOptions, queryOperation); + + ISearchResults? results = queryOperation + .SelectField(ItemIdFieldName) + .Execute(QueryOptions.SkipTake(skip, take)); + + if (results is null) + { + // The query yield no results + return new PagedModel(); + } + + Guid[] items = results + .Where(r => r.Values.ContainsKey(ItemIdFieldName)) + .Select(r => Guid.Parse(r.Values[ItemIdFieldName])) + .ToArray(); + + return new PagedModel(results.TotalItemCount, items); + } + + public SelectorOption AllContentSelectorOption() => new() + { + FieldName = UmbracoExamineFieldNames.CategoryFieldName, Values = new[] { "content" } + }; + + private IBooleanOperation BuildSelectorOperation(SelectorOption selectorOption, IIndex index, string culture) + { + IQuery query = index.Searcher.CreateQuery(); + + IBooleanOperation selectorOperation = selectorOption.Values.Length == 1 + ? query.Field(selectorOption.FieldName, selectorOption.Values.First()) + : query.GroupedOr(new[] { selectorOption.FieldName }, selectorOption.Values); + + // Item culture must be either the requested culture or "none" + selectorOperation.And().GroupedOr(new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture }, culture.ToLowerInvariant().IfNullOrWhiteSpace(_fallbackGuidValue), "none"); + + return selectorOperation; + } + + private void ApplyFiltering(IList filterOptions, IBooleanOperation queryOperation) + { + void HandleExact(IQuery query, string fieldName, string[] values) + { + if (values.Length == 1) + { + query.Field(fieldName, values[0]); + } + else + { + query.GroupedOr(new[] { fieldName }, values); + } + } + + foreach (FilterOption filterOption in filterOptions) + { + var values = filterOption.Values.Any() + ? filterOption.Values + : new[] { _fallbackGuidValue }; + + switch (filterOption.Operator) + { + case FilterOperation.Is: + // TODO: test this for explicit word matching + HandleExact(queryOperation.And(), filterOption.FieldName, values); + break; + case FilterOperation.IsNot: + // TODO: test this for explicit word matching + HandleExact(queryOperation.Not(), filterOption.FieldName, values); + break; + // TODO: Fix + case FilterOperation.Contains: + break; + // TODO: Fix + case FilterOperation.DoesNotContain: + break; + default: + continue; + } + } + } + + private void ApplySorting(IList sortOptions, IOrdering ordering) + { + foreach (SortOption sort in sortOptions) + { + if (_fieldTypes.TryGetValue(sort.FieldName, out FieldType fieldType) is false) + { + _logger.LogWarning( + "Sort implementation for field name {FieldName} does not match an index handler implementation, cannot resolve field type.", + sort.FieldName); + continue; + } + + SortType sortType = fieldType switch + { + FieldType.Number => SortType.Int, + FieldType.Date => SortType.Long, + FieldType.StringRaw => SortType.String, + FieldType.StringAnalyzed => SortType.String, + FieldType.StringSortable => SortType.String, + _ => throw new ArgumentOutOfRangeException(nameof(fieldType)) + }; + + ordering = sort.Direction switch + { + Direction.Ascending => ordering.OrderBy(new SortableField(sort.FieldName, sortType)), + Direction.Descending => ordering.OrderByDescending(new SortableField(sort.FieldName, sortType)), + _ => ordering + }; + } + } +} diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs index d044d774d1..8aac5db6ee 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs @@ -1,57 +1,35 @@ -using Examine; -using Examine.Search; -using Microsoft.Extensions.Logging; using Umbraco.Cms.Api.Delivery.Indexing.Selectors; using Umbraco.Cms.Core; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Services.OperationStatus; -using Umbraco.Cms.Infrastructure.Examine; -using Umbraco.Extensions; using Umbraco.New.Cms.Core.Models; namespace Umbraco.Cms.Api.Delivery.Services; internal sealed class ApiContentQueryService : IApiContentQueryService { - private const string ItemIdFieldName = "itemId"; - private readonly IExamineManager _examineManager; private readonly IRequestStartItemProviderAccessor _requestStartItemProviderAccessor; private readonly SelectorHandlerCollection _selectorHandlers; private readonly FilterHandlerCollection _filterHandlers; private readonly SortHandlerCollection _sortHandlers; private readonly IVariationContextAccessor _variationContextAccessor; - private readonly ILogger _logger; - private readonly string _fallbackGuidValue; - private readonly Dictionary _fieldTypes; + private readonly IApiContentQueryProvider _apiContentQueryProvider; public ApiContentQueryService( - IExamineManager examineManager, IRequestStartItemProviderAccessor requestStartItemProviderAccessor, SelectorHandlerCollection selectorHandlers, FilterHandlerCollection filterHandlers, SortHandlerCollection sortHandlers, - ContentIndexHandlerCollection indexHandlers, - ILogger logger, - IVariationContextAccessor variationContextAccessor) + IVariationContextAccessor variationContextAccessor, + IApiContentQueryProvider apiContentQueryProvider) { - _examineManager = examineManager; _requestStartItemProviderAccessor = requestStartItemProviderAccessor; _selectorHandlers = selectorHandlers; _filterHandlers = filterHandlers; _sortHandlers = sortHandlers; _variationContextAccessor = variationContextAccessor; - _logger = logger; - - // A fallback value is needed for Examine queries in case we don't have a value - we can't pass null or empty string - // It is set to a random guid since this would be highly unlikely to yield any results - _fallbackGuidValue = Guid.NewGuid().ToString("D"); - - // build a look-up dictionary of field types by field name - _fieldTypes = indexHandlers - .SelectMany(handler => handler.GetFields()) - .DistinctBy(field => field.FieldName) - .ToDictionary(field => field.FieldName, field => field.FieldType, StringComparer.InvariantCultureIgnoreCase); + _apiContentQueryProvider = apiContentQueryProvider; } /// @@ -59,198 +37,78 @@ internal sealed class ApiContentQueryService : IApiContentQueryService { var emptyResult = new PagedModel(); - if (!_examineManager.TryGetIndex(Constants.UmbracoIndexes.DeliveryApiContentIndexName, out IIndex? apiIndex)) - { - return Attempt.FailWithStatus(ApiContentQueryOperationStatus.IndexNotFound, emptyResult); - } - - IQuery baseQuery = apiIndex.Searcher.CreateQuery(); - - // Handle Selecting - IBooleanOperation? queryOperation = HandleSelector(fetch, baseQuery); - - // If no Selector could be found, we return no results - if (queryOperation is null) + SelectorOption? selectorOption = GetSelectorOption(fetch); + if (selectorOption is null) { + // If no Selector could be found, we return no results return Attempt.FailWithStatus(ApiContentQueryOperationStatus.SelectorOptionNotFound, emptyResult); } - // Item culture must be either the requested culture or "none" - var culture = CurrentCulture(); - queryOperation.And().GroupedOr(new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture }, culture.ToLowerInvariant().IfNullOrWhiteSpace(_fallbackGuidValue), "none"); - - // Handle Filtering - var canApplyFiltering = CanHandleFiltering(filters, queryOperation); - - // If there is an invalid Filter option, we return no results - if (canApplyFiltering is false) + var filterOptions = new List(); + foreach (var filter in filters) { - return Attempt.FailWithStatus(ApiContentQueryOperationStatus.FilterOptionNotFound, emptyResult); + FilterOption? filterOption = GetFilterOption(filter); + if (filterOption is null) + { + // If there is an invalid Filter option, we return no results + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.FilterOptionNotFound, emptyResult); + } + + filterOptions.Add(filterOption); } - // Handle Sorting - IOrdering? sortQuery = HandleSorting(sorts, queryOperation); - - // If there is an invalid Sort option, we return no results - if (sortQuery is null) + var sortOptions = new List(); + foreach (var sort in sorts) { - return Attempt.FailWithStatus(ApiContentQueryOperationStatus.SortOptionNotFound, emptyResult); + SortOption? sortOption = GetSortOption(sort); + if (sortOption is null) + { + // If there is an invalid Sort option, we return no results + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.SortOptionNotFound, emptyResult); + } + + sortOptions.Add(sortOption); } - ISearchResults? results = sortQuery - .SelectField(ItemIdFieldName) - .Execute(QueryOptions.SkipTake(skip, take)); + var culture = _variationContextAccessor.VariationContext?.Culture ?? string.Empty; - if (results is null) - { - // The query yield no results - return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, emptyResult); - } - - Guid[] items = results - .Where(r => r.Values.ContainsKey(ItemIdFieldName)) - .Select(r => Guid.Parse(r.Values[ItemIdFieldName])) - .ToArray(); - - return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, new PagedModel(results.TotalItemCount, items)); + PagedModel result = _apiContentQueryProvider.ExecuteQuery(selectorOption, filterOptions, sortOptions, culture, skip, take); + return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, result); } - private IBooleanOperation? HandleSelector(string? fetch, IQuery baseQuery) + private SelectorOption? GetSelectorOption(string? fetch) { - string? fieldName = null; - string[] fieldValues = Array.Empty(); - if (fetch is not null) { ISelectorHandler? selectorHandler = _selectorHandlers.FirstOrDefault(h => h.CanHandle(fetch)); - SelectorOption? selector = selectorHandler?.BuildSelectorOption(fetch); - - if (selector is null) - { - return null; - } - - fieldName = selector.FieldName; - fieldValues = selector.Values.Any() - ? selector.Values - : new[] { _fallbackGuidValue }; + return selectorHandler?.BuildSelectorOption(fetch); } - // Take into account the "start-item" header if present, as it defines a starting root node to query from - if (fieldName is null && _requestStartItemProviderAccessor.TryGetValue(out IRequestStartItemProvider? requestStartItemProvider)) + if (_requestStartItemProviderAccessor.TryGetValue(out IRequestStartItemProvider? requestStartItemProvider)) { IPublishedContent? startItem = requestStartItemProvider.GetStartItem(); if (startItem is not null) { // Reusing the boolean operation of the "Descendants" selector, as we want to get all the nodes from the given starting point - fieldName = DescendantsSelectorIndexer.FieldName; - fieldValues = new [] { startItem.Key.ToString() }; + return new SelectorOption + { + FieldName = DescendantsSelectorIndexer.FieldName, Values = new[] { startItem.Key.ToString() } + }; } } - // If no params or no fetch value, get everything from the index - this is a way to do that with Examine - fieldName ??= UmbracoExamineFieldNames.CategoryFieldName; - fieldValues = fieldValues.Any() ? fieldValues : new [] { "content" }; - - return fieldValues.Length == 1 - ? baseQuery.Field(fieldName, fieldValues.First()) - : baseQuery.GroupedOr(new[] { fieldName }, fieldValues); + return _apiContentQueryProvider.AllContentSelectorOption(); } - private bool CanHandleFiltering(IEnumerable filters, IBooleanOperation queryOperation) + private FilterOption? GetFilterOption(string filter) { - void HandleExact(IQuery query, string fieldName, string[] values) - { - if (values.Length == 1) - { - query.Field(fieldName, values[0]); - } - else - { - query.GroupedOr(new[] { fieldName }, values); - } - } - - foreach (var filterValue in filters) - { - IFilterHandler? filterHandler = _filterHandlers.FirstOrDefault(h => h.CanHandle(filterValue)); - FilterOption? filter = filterHandler?.BuildFilterOption(filterValue); - - if (filter is null) - { - return false; - } - - var values = filter.Values.Any() - ? filter.Values - : new[] { _fallbackGuidValue }; - - switch (filter.Operator) - { - case FilterOperation.Is: - // TODO: test this for explicit word matching - HandleExact(queryOperation.And(), filter.FieldName, values); - break; - case FilterOperation.IsNot: - // TODO: test this for explicit word matching - HandleExact(queryOperation.Not(), filter.FieldName, values); - break; - // TODO: Fix - case FilterOperation.Contains: - break; - // TODO: Fix - case FilterOperation.DoesNotContain: - break; - default: - continue; - } - } - - return true; + IFilterHandler? filterHandler = _filterHandlers.FirstOrDefault(h => h.CanHandle(filter)); + return filterHandler?.BuildFilterOption(filter); } - private IOrdering? HandleSorting(IEnumerable sorts, IBooleanOperation queryCriteria) + private SortOption? GetSortOption(string sort) { - IOrdering? orderingQuery = null; - - foreach (var sortValue in sorts) - { - ISortHandler? sortHandler = _sortHandlers.FirstOrDefault(h => h.CanHandle(sortValue)); - SortOption? sort = sortHandler?.BuildSortOption(sortValue); - - if (sort is null) - { - return null; - } - - if (_fieldTypes.TryGetValue(sort.FieldName, out FieldType fieldType) is false) - { - _logger.LogWarning("Sort implementation for field name {FieldName} does not match an index handler implementation, cannot resolve field type.", sort.FieldName); - continue; - } - - SortType sortType = fieldType switch - { - FieldType.Number => SortType.Int, - FieldType.Date => SortType.Long, - FieldType.StringRaw => SortType.String, - FieldType.StringAnalyzed => SortType.String, - FieldType.StringSortable => SortType.String, - _ => throw new ArgumentOutOfRangeException(nameof(fieldType)) - }; - - orderingQuery = sort.Direction switch - { - Direction.Ascending => queryCriteria.OrderBy(new SortableField(sort.FieldName, sortType)), - Direction.Descending => queryCriteria.OrderByDescending(new SortableField(sort.FieldName, sortType)), - _ => orderingQuery - }; - } - - // Keep the index sorting as default - return orderingQuery ?? queryCriteria.OrderBy(); + ISortHandler? sortHandler = _sortHandlers.FirstOrDefault(h => h.CanHandle(sort)); + return sortHandler?.BuildSortOption(sort); } - - private string CurrentCulture() - => _variationContextAccessor.VariationContext?.Culture ?? string.Empty; } diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs b/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs new file mode 100644 index 0000000000..df889184fd --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/IApiContentQueryProvider.cs @@ -0,0 +1,26 @@ +using Umbraco.New.Cms.Core.Models; + +namespace Umbraco.Cms.Core.DeliveryApi; + +/// +/// Concrete implementation of content querying (e.g. based on Examine) +/// +public interface IApiContentQueryProvider +{ + /// + /// Returns a page of item ids that passed the search criteria. + /// + /// The selector option of the search criteria. + /// The filter options of the search criteria. + /// The sorting options of the search criteria. + /// The requested culture. + /// Number of search results to skip (for pagination). + /// Number of search results to retrieve (for pagination). + /// A paged model containing the resulting IDs and the total number of results that matching the search criteria. + PagedModel ExecuteQuery(SelectorOption selectorOption, IList filterOptions, IList sortOptions, string culture, int skip, int take); + + /// + /// Returns a selector option that can be applied to fetch "all content" (i.e. if a selector option is not present when performing a search). + /// + SelectorOption AllContentSelectorOption(); +} diff --git a/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs index 0c3c859070..2e8da1a0b1 100644 --- a/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs @@ -4,7 +4,6 @@ public enum ApiContentQueryOperationStatus { Success, FilterOptionNotFound, - IndexNotFound, SelectorOptionNotFound, SortOptionNotFound } From 1e6c2206105287d75c84ec3bd76e34bf637253a9 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 22 May 2023 12:29:16 +0200 Subject: [PATCH 16/21] Downgrade OpenIdDict (#14279) --- src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj | 4 ++-- .../Umbraco.Cms.Persistence.EFCore.csproj | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj b/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj index 5a2dfa69ef..9deb226783 100644 --- a/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj +++ b/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj @@ -10,8 +10,8 @@ - - + + diff --git a/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj b/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj index 3e6e57983f..3702db461b 100644 --- a/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj +++ b/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj @@ -9,7 +9,7 @@ - + From 459d664531b742b40674ecbcc7477af69ed10815 Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Mon, 22 May 2023 12:44:52 +0200 Subject: [PATCH 17/21] V12: Map |DataDirectory| to path in connectionString (#14278) * Add Data directory as constants * Use new constants instead of internal ones * Replace data directory in connection string if its there * Update src/Umbraco.Cms.Persistence.EFCore/Extensions/UmbracoEFCoreServiceCollectionExtensions.cs --------- Co-authored-by: Zeegaan Co-authored-by: Mole --- .../UmbracoEFCoreServiceCollectionExtensions.cs | 9 ++++++++- .../Configuration/Models/ConnectionStrings.cs | 2 +- src/Umbraco.Core/Constants-System.cs | 10 ++++++++++ .../Extensions/ConfigurationExtensions.cs | 14 ++------------ 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Cms.Persistence.EFCore/Extensions/UmbracoEFCoreServiceCollectionExtensions.cs b/src/Umbraco.Cms.Persistence.EFCore/Extensions/UmbracoEFCoreServiceCollectionExtensions.cs index 857661fd83..4d47e64448 100644 --- a/src/Umbraco.Cms.Persistence.EFCore/Extensions/UmbracoEFCoreServiceCollectionExtensions.cs +++ b/src/Umbraco.Cms.Persistence.EFCore/Extensions/UmbracoEFCoreServiceCollectionExtensions.cs @@ -1,6 +1,6 @@ using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.DistributedLocking; using Umbraco.Cms.Persistence.EFCore.Locking; using Umbraco.Cms.Persistence.EFCore.Scoping; @@ -16,6 +16,13 @@ public static class UmbracoEFCoreServiceCollectionExtensions { defaultEFCoreOptionsAction ??= DefaultOptionsAction; + // Replace data directory + string? dataDirectory = AppDomain.CurrentDomain.GetData(Constants.System.DataDirectoryName)?.ToString(); + if (string.IsNullOrEmpty(dataDirectory) is false) + { + connectionString = connectionString.Replace(Constants.System.DataDirectoryPlaceholder, dataDirectory); + } + services.AddDbContext( options => { diff --git a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs index a5161eca86..1d7690ab52 100644 --- a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs +++ b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs @@ -15,7 +15,7 @@ public class ConnectionStrings // TODO: Rename to [Umbraco]ConnectionString (sin /// /// The DataDirectory placeholder. /// - public const string DataDirectoryPlaceholder = ConfigurationExtensions.DataDirectoryPlaceholder; + public const string DataDirectoryPlaceholder = Constants.System.DataDirectoryPlaceholder; /// /// The postfix used to identify a connection strings provider setting. diff --git a/src/Umbraco.Core/Constants-System.cs b/src/Umbraco.Core/Constants-System.cs index 43de01995b..4a88da6459 100644 --- a/src/Umbraco.Core/Constants-System.cs +++ b/src/Umbraco.Core/Constants-System.cs @@ -64,5 +64,15 @@ public static partial class Constants public const string UmbracoConnectionName = "umbracoDbDSN"; public const string DefaultUmbracoPath = "~/umbraco"; + + /// + /// The DataDirectory name. + /// + public const string DataDirectoryName = "DataDirectory"; + + /// + /// The DataDirectory placeholder. + /// + public const string DataDirectoryPlaceholder = "|DataDirectory|"; } } diff --git a/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs b/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs index 53f3b76c06..3dffdd8e67 100644 --- a/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs +++ b/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs @@ -10,16 +10,6 @@ namespace Umbraco.Extensions; /// public static class ConfigurationExtensions { - /// - /// The DataDirectory name. - /// - internal const string DataDirectoryName = "DataDirectory"; - - /// - /// The DataDirectory placeholder. - /// - internal const string DataDirectoryPlaceholder = "|DataDirectory|"; - /// /// The postfix used to identify a connection string provider setting. /// @@ -76,10 +66,10 @@ public static class ConfigurationExtensions if (!string.IsNullOrEmpty(connectionString)) { // Replace data directory - string? dataDirectory = AppDomain.CurrentDomain.GetData(DataDirectoryName)?.ToString(); + string? dataDirectory = AppDomain.CurrentDomain.GetData(Constants.System.DataDirectoryName)?.ToString(); if (!string.IsNullOrEmpty(dataDirectory)) { - connectionString = connectionString.Replace(DataDirectoryPlaceholder, dataDirectory); + connectionString = connectionString.Replace(Constants.System.DataDirectoryPlaceholder, dataDirectory); } // Get provider name From b67c7fbe772037acdc7d806d7bb9576a7ef58b5b Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 22 May 2023 15:15:19 +0200 Subject: [PATCH 18/21] Cherry pick b8d6613bd8d382759c50cdab1bdd305d626642d6 accidentally kept both lines instead of just the one, this fixes the problem. --- templates/UmbracoProject/UmbracoProject.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/templates/UmbracoProject/UmbracoProject.csproj b/templates/UmbracoProject/UmbracoProject.csproj index ea6db75798..7ae1dee47d 100644 --- a/templates/UmbracoProject/UmbracoProject.csproj +++ b/templates/UmbracoProject/UmbracoProject.csproj @@ -26,8 +26,6 @@ false false - - From 54af079d97be4c6c85bc536ed896f14284bde511 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 22 May 2023 15:18:14 +0200 Subject: [PATCH 19/21] Fix version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index b465096390..829112f0e2 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "12.0.0-rc2", + "version": "12.1.0-rc", "assemblyVersion": { "precision": "build" }, From 7776cded7386ec1b435edb2c03779aa082edbdbf Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 22 May 2023 15:19:13 +0200 Subject: [PATCH 20/21] Bump version to next RC --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index b465096390..2443aaa69a 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "12.0.0-rc2", + "version": "12.0.0-rc3", "assemblyVersion": { "precision": "build" }, From e79246368bdce66744da7b8d1cf225eb130bc62b Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 23 May 2023 09:38:01 +0200 Subject: [PATCH 21/21] Update JSON schema package references for Forms and Deploy. (#14285) --- src/JsonSchema/JsonSchema.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/JsonSchema/JsonSchema.csproj b/src/JsonSchema/JsonSchema.csproj index 3d31dd265d..613283bd69 100644 --- a/src/JsonSchema/JsonSchema.csproj +++ b/src/JsonSchema/JsonSchema.csproj @@ -12,7 +12,7 @@ - - + +