diff --git a/build/azure-pipelines.yml b/build/azure-pipelines.yml index 23bc2afc5e..aa2fcca05d 100644 --- a/build/azure-pipelines.yml +++ b/build/azure-pipelines.yml @@ -302,7 +302,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: 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 49d16c3024..b3941b65d5 100644 --- a/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj +++ b/src/Umbraco.Cms.Api.Common/Umbraco.Cms.Api.Common.csproj @@ -13,6 +13,8 @@ + + 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/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/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.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/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/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.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? 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.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj b/src/Umbraco.Cms.Persistence.EFCore/Umbraco.Cms.Persistence.EFCore.csproj index af566ab67a..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,6 +9,7 @@ + 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 24820e58b4..36db06d383 100644 --- a/src/Umbraco.Core/Constants-System.cs +++ b/src/Umbraco.Core/Constants-System.cs @@ -69,5 +69,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/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/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/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.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 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.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.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 } 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/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/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 a5a44a8d8d..f753707770 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 6a5ebd6608..94531c0b96 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) @@ -150,6 +180,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor FinalState = transition.SourceState, CompletedTransitions = completedTransitions, Plan = plan, + ExecutedMigrationContexts = executedMigrationContexts }; } @@ -198,18 +229,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 @@ -226,6 +260,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor RunMigration(migrationType, context); scope.Complete(); + + return context; } } 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()) 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/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/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteMacroRenderingValueConverter.cs index 68d15aa99f..9d8010ab5f 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, false) ?? string.Empty + }; } var sourceString = inter?.ToString(); @@ -126,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) { @@ -141,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/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 fe88701266..750d60d67e 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/EntityController.cs @@ -515,6 +515,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 @@ -522,19 +531,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) @@ -546,10 +552,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 a3c96cf805..6f46268b5b 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/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}); 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(); }); } diff --git a/templates/UmbracoProject/UmbracoProject.csproj b/templates/UmbracoProject/UmbracoProject.csproj index 0989706f68..d50f95a907 100644 --- a/templates/UmbracoProject/UmbracoProject.csproj +++ b/templates/UmbracoProject/UmbracoProject.csproj @@ -26,8 +26,6 @@ false false - - 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/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); + } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs index 320f16dbf6..276df9a223 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(), @@ -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); @@ -274,4 +276,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); + } }