From ce769ffff4613904bcc8c65103166a722db388a5 Mon Sep 17 00:00:00 2001 From: Erik-Jan Westendorp Date: Tue, 19 Dec 2023 14:40:03 +0100 Subject: [PATCH 01/24] Add health checks results to payload --- .../Events/HealthCheck/HealthCheckCompletedWebhookEvent.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Webhooks/Events/HealthCheck/HealthCheckCompletedWebhookEvent.cs b/src/Umbraco.Core/Webhooks/Events/HealthCheck/HealthCheckCompletedWebhookEvent.cs index b38cb8d22b..e94fc97bcc 100644 --- a/src/Umbraco.Core/Webhooks/Events/HealthCheck/HealthCheckCompletedWebhookEvent.cs +++ b/src/Umbraco.Core/Webhooks/Events/HealthCheck/HealthCheckCompletedWebhookEvent.cs @@ -15,5 +15,10 @@ public class HealthCheckCompletedWebhookEvent : WebhookEventBase Constants.WebhookEvents.Aliases.HealthCheckCompleted; - public override object? ConvertNotificationToRequestPayload(HealthCheckCompletedNotification notification) => notification.HealthCheckResults; + public override object? ConvertNotificationToRequestPayload(HealthCheckCompletedNotification notification) => + new + { + notification.HealthCheckResults.AllChecksSuccessful, + notification.HealthCheckResults.ResultsAsDictionary + }; } From 3344274616c0abb5d85a5c9b38040ca4265f560d Mon Sep 17 00:00:00 2001 From: Awlad Hussain Date: Wed, 6 Mar 2024 17:45:20 +0000 Subject: [PATCH 02/24] Xpath Query on Multinode Treepicker no longer working for new Nodes (#15159) * fix for #14698 * Adjust XPath logic so it uses parentId as $current only when nodeContextId is null or 0 --------- Co-authored-by: Laura Neto <12862535+lauraneto@users.noreply.github.com> --- src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs b/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs index 87d4d459fa..a3b194226c 100644 --- a/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs +++ b/src/Umbraco.Core/Xml/UmbracoXPathPathSyntaxParser.cs @@ -136,12 +136,12 @@ public class UmbracoXPathPathSyntaxParser }); } - // These parameters must have a node id context - if (nodeContextId.HasValue) + if (nodeContextId.HasValue || parentId.HasValue) { + var currentId = nodeContextId.HasValue && nodeContextId.Value != default ? nodeContextId.Value : parentId.GetValueOrDefault(); vars.Add("$current", q => { - var closestPublishedAncestorId = getClosestPublishedAncestor(getPath(nodeContextId.Value)); + var closestPublishedAncestorId = getClosestPublishedAncestor(getPath(currentId)); return q.Replace("$current", string.Format(rootXpath, closestPublishedAncestorId)); }); } From 7813acfa2b20901c9aa3850ab5a3a4a29c6d87ef Mon Sep 17 00:00:00 2001 From: Paul Seal Date: Thu, 7 Mar 2024 15:44:39 +0000 Subject: [PATCH 03/24] Added a check for the runtime mode and showed the relevant error message if --- .../src/views/content/content.create.controller.js | 1 + src/Umbraco.Web.UI.Client/src/views/content/create.html | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js index c36a0a47b8..54753a94f1 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js @@ -22,6 +22,7 @@ function contentCreateController($scope, function initialize() { $scope.loading = true; $scope.allowedTypes = null; + $scope.runtimeModeProduction = Umbraco.Sys.ServerVariables.application.runtimeMode == 'Production'; var getAllowedTypes = contentTypeResource.getAllowedTypes($scope.currentNode.id).then(function (data) { $scope.allowedTypes = iconHelper.formatContentTypeIcons(data); diff --git a/src/Umbraco.Web.UI.Client/src/views/content/create.html b/src/Umbraco.Web.UI.Client/src/views/content/create.html index e2e3fb9453..eb245c06b2 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/create.html +++ b/src/Umbraco.Web.UI.Client/src/views/content/create.html @@ -9,10 +9,10 @@
Select a blueprint
-

+

The selected page in the content tree doesn't allow for any pages to be created below it.

-
+

There are no allowed Document Types available for creating content here. You must enable these in Document Types within the Settings section, by editing the Allowed child node From 90864d8e586c94a50dee265aea77564ed3b91d17 Mon Sep 17 00:00:00 2001 From: Joe Dearsley Date: Thu, 7 Mar 2024 15:32:59 +0000 Subject: [PATCH 04/24] Validation messages for blocklists no longer show HTML markup. #15697 --- .../validation/valpropertymsg.directive.js | 641 +++++++++--------- 1 file changed, 321 insertions(+), 320 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js index 86afda1233..ab7be52f51 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/validation/valpropertymsg.directive.js @@ -13,345 +13,346 @@ **/ function valPropertyMsg(serverValidationManager, localizationService, angularHelper) { - return { - require: ['^^form', '^^valFormManager', '^^umbProperty', '?^^umbVariantContent', '?^^valPropertyMsg'], - replace: true, - restrict: "E", - template: "

{{errorMsg}}
", - scope: {}, - link: function (scope, element, attrs, ctrl) { + return { + require: ['^^form', '^^valFormManager', '^^umbProperty', '?^^umbVariantContent', '?^^valPropertyMsg'], + replace: true, + restrict: "E", + template: "
", + scope: {}, + link: function (scope, element, attrs, ctrl) { - var unsubscribe = []; - var watcher = null; - var hasError = false; // tracks if there is a child error or an explicit error + var unsubscribe = []; + var watcher = null; + var hasError = false; // tracks if there is a child error or an explicit error - //create properties on our custom scope so we can use it in our template - scope.errorMsg = ""; + //create properties on our custom scope so we can use it in our template + scope.errorMsg = ""; - //the property form controller api - var formCtrl = ctrl[0]; - //the valFormManager controller api - var valFormManager = ctrl[1]; - //the property controller api - var umbPropCtrl = ctrl[2]; - //the variants controller api - var umbVariantCtrl = ctrl[3]; + //the property form controller api + var formCtrl = ctrl[0]; + //the valFormManager controller api + var valFormManager = ctrl[1]; + //the property controller api + var umbPropCtrl = ctrl[2]; + //the variants controller api + var umbVariantCtrl = ctrl[3]; - var currentProperty = umbPropCtrl.property; - scope.currentProperty = currentProperty; + var currentProperty = umbPropCtrl.property; + scope.currentProperty = currentProperty; - var currentCulture = currentProperty.culture; - var currentSegment = currentProperty.segment; + var currentCulture = currentProperty.culture; + var currentSegment = currentProperty.segment; - // validation object won't exist when editor loads outside the content form (ie in settings section when modifying a content type) - var isMandatory = currentProperty.validation ? currentProperty.validation.mandatory : undefined; + // validation object won't exist when editor loads outside the content form (ie in settings section when modifying a content type) + var isMandatory = currentProperty.validation ? currentProperty.validation.mandatory : undefined; - var labels = {}; - var showValidation = false; + var labels = {}; + var showValidation = false; - if (umbVariantCtrl) { - //if we are inside of an umbVariantContent directive + if (umbVariantCtrl) { + //if we are inside of an umbVariantContent directive - var currentVariant = umbVariantCtrl.editor.content; + var currentVariant = umbVariantCtrl.editor.content; - // Lets check if we have variants and we are on the default language then ... - if (umbVariantCtrl.content.variants.length > 1 && (!currentVariant.language || !currentVariant.language.isDefault) && !currentCulture && !currentSegment && !currentProperty.unlockInvariantValue) { - //This property is locked cause its a invariant property shown on a non-default language. - //Therefor do not validate this field. - return; - } - } + // Lets check if we have variants and we are on the default language then ... + if (umbVariantCtrl.content.variants.length > 1 && (!currentVariant.language || !currentVariant.language.isDefault) && !currentCulture && !currentSegment && !currentProperty.unlockInvariantValue) { + //This property is locked cause its a invariant property shown on a non-default language. + //Therefor do not validate this field. + return; + } + } - // if we have reached this part, and there is no culture, then lets fallback to invariant. To get the validation feedback for invariant language. - currentCulture = currentCulture || "invariant"; + // if we have reached this part, and there is no culture, then lets fallback to invariant. To get the validation feedback for invariant language. + currentCulture = currentCulture || "invariant"; - // Gets the error message to display - function getErrorMsg() { - //this can be null if no property was assigned - if (scope.currentProperty) { - //first try to get the error msg from the server collection - var err = serverValidationManager.getPropertyError(umbPropCtrl.getValidationPath(), null, "", null); - //if there's an error message use it - if (err && err.errorMsg) { - return err.errorMsg; - } - else { - return scope.currentProperty.propertyErrorMessage ? scope.currentProperty.propertyErrorMessage : labels.propertyHasErrors; - } + // Gets the error message to display + function getErrorMsg() { + //this can be null if no property was assigned + if (scope.currentProperty) { + //first try to get the error msg from the server collection + var err = serverValidationManager.getPropertyError(umbPropCtrl.getValidationPath(), null, "", null); + //if there's an error message use it + if (err && err.errorMsg) { - } - return labels.propertyHasErrors; - } + return err.errorMsg; + } + else { + return scope.currentProperty.propertyErrorMessage ? scope.currentProperty.propertyErrorMessage : labels.propertyHasErrors; + } - // check the current errors in the form (and recursive sub forms), if there is 1 or 2 errors - // we can check if those are valPropertyMsg or valServer and can clear our error in those cases. - function checkAndClearError() { + } + return labels.propertyHasErrors; + } - var errCount = angularHelper.countAllFormErrors(formCtrl); + // check the current errors in the form (and recursive sub forms), if there is 1 or 2 errors + // we can check if those are valPropertyMsg or valServer and can clear our error in those cases. + function checkAndClearError() { - if (errCount === 0) { - return true; - } + var errCount = angularHelper.countAllFormErrors(formCtrl); - if (errCount > 2) { - return false; - } - - var hasValServer = Utilities.isArray(formCtrl.$error.valServer); - if (errCount === 1 && hasValServer) { - return true; - } - - var hasOwnErr = hasExplicitError(); - if ((errCount === 1 && hasOwnErr) || (errCount === 2 && hasOwnErr && hasValServer)) { - - var propertyValidationPath = umbPropCtrl.getValidationPath(); - // check if we can clear it based on child server errors, if we are the only explicit one remaining we can clear ourselves - if (isLastServerError(propertyValidationPath)) { - serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); - return true; - } - return false; - } - - return false; - } - - // returns true if there is an explicit valPropertyMsg validation error on the form - function hasExplicitError() { - return Utilities.isArray(formCtrl.$error.valPropertyMsg); - } - - // returns true if there is only a single server validation error for this property validation key in it's validation path - function isLastServerError(propertyValidationPath) { - var nestedErrs = serverValidationManager.getPropertyErrorsByValidationPath( - propertyValidationPath, - currentCulture, - currentSegment, - { matchType: "prefix" }); - if (nestedErrs.length === 0 || (nestedErrs.length === 1 && nestedErrs[0].propertyAlias === propertyValidationPath)) { - - return true; - } - return false; - } - - // a custom $validator function called on when each child ngModelController changes a value. - function resetServerValidityValidator(fieldController) { - const storedFieldController = fieldController; // pin a reference to this - return (modelValue, viewValue) => { - // if the ngModelController value has changed, then we can check and clear the error - if (storedFieldController.$dirty) { - if (checkAndClearError()) { - resetError(); - } - } - return true; // this validator is always 'valid' - }; - } - - // collect all ng-model controllers recursively within the umbProperty form - // until it reaches the next nested umbProperty form - function collectAllNgModelControllersRecursively(controls, ngModels) { - controls.forEach(ctrl => { - if (angularHelper.isForm(ctrl)) { - // if it's not another umbProperty form then recurse - if (ctrl.$name !== formCtrl.$name) { - collectAllNgModelControllersRecursively(ctrl.$getControls(), ngModels); - } - } - else if (Object.prototype.hasOwnProperty.call(ctrl, '$modelValue') && Utilities.isObject(ctrl.$validators)) { - ngModels.push(ctrl); - } - }); - } - - // We start the watch when there's server validation errors detected. - // We watch on the current form's properties and on first watch or if they are dynamically changed - // we find all ngModel controls recursively on this form (but stop recursing before we get to the next) - // umbProperty form). Then for each ngModelController we assign a new $validator. This $validator - // will execute whenever the value is changed which allows us to check and reset the server validator - function startWatch() { - if (!watcher) { - - watcher = scope.$watchCollection( - () => formCtrl, - function (updatedFormController) { - let childControls = updatedFormController.$getControls(); - let ngModels = []; - collectAllNgModelControllersRecursively(childControls, ngModels); - ngModels.forEach(x => { - if (!x.$validators.serverValidityResetter) { - x.$validators.serverValidityResetter = resetServerValidityValidator(x); - } - }); - }); - } - } - - //clear the watch when the property validator is valid again - function stopWatch() { - if (watcher) { - watcher(); - watcher = null; - } - } - - function resetError() { - stopWatch(); - hasError = false; - formCtrl.$setValidity('valPropertyMsg', true); - scope.errorMsg = ""; - - } - - // This deals with client side validation changes and is executed anytime validators change on the containing - // valFormManager. This allows us to know when to display or clear the property error data for non-server side errors. - function checkValidationStatus() { - if (formCtrl.$invalid) { - //first we need to check if the valPropertyMsg validity is invalid - if (formCtrl.$error.valPropertyMsg && formCtrl.$error.valPropertyMsg.length > 0) { - //since we already have an error we'll just return since this means we've already set the - //hasError and errorMsg properties which occurs below in the serverValidationManager.subscribe - return; - } - //if there are any errors in the current property form that are not valPropertyMsg - else if (_.without(_.keys(formCtrl.$error), "valPropertyMsg").length > 0) { - - // errors exist, but if the property is NOT mandatory and has no value, the errors should be cleared - if (isMandatory !== undefined && isMandatory === false && !currentProperty.value) { - - resetError(); - - // if there's no value, the controls can be reset, which clears the error state on formCtrl - for (let control of formCtrl.$getControls()) { - control.$setValidity(); - } - - return; - } - - hasError = true; - //update the validation message if we don't already have one assigned. - if (showValidation && scope.errorMsg === "") { - scope.errorMsg = getErrorMsg(); - } - } - else { - resetError(); - } - } - else { - resetError(); - } - } - - function onInit() { - - localizationService.localize("errors_propertyHasErrors").then(function (data) { - - labels.propertyHasErrors = data; - - //if there's any remaining errors in the server validation service then we should show them. - showValidation = serverValidationManager.items.length > 0; - if (!showValidation) { - //We can either get the form submitted status by the parent directive valFormManager (if we add a property to it) - //or we can just check upwards in the DOM for the css class (easier for now). - //The initial hidden state can't always be hidden because when we switch variants in the content editor we cannot - //reset the status. - showValidation = element.closest(".show-validation").length > 0; - } - - //listen for form validation changes. - //The alternative is to add a watch to formCtrl.$invalid but that would lead to many more watches then - // subscribing to this single watch. - // TODO: Really? Since valFormManager is watching a countof all errors which is more overhead than watching formCtrl.$invalid - // and there's a TODO there that it should just watch formCtrl.$invalid - valFormManager.onValidationStatusChanged(function () { - checkValidationStatus(); - }); - - //listen for the forms saving event - unsubscribe.push(scope.$on("formSubmitting", function () { - showValidation = true; - if (hasError && scope.errorMsg === "") { - scope.errorMsg = getErrorMsg(); - startWatch(); - } - else if (!hasError) { - resetError(); - } - })); - - //listen for the forms saved event - unsubscribe.push(scope.$on("formSubmitted", function () { - showValidation = false; - resetError(); - })); - - if (scope.currentProperty) { //this can be null if no property was assigned - - // listen for server validation changes for property validation path prefix. - // We pass in "" in order to listen for all validation changes to the content property, not for - // validation changes to fields in the property this is because some server side validators may not - // return the field name for which the error belongs too, just the property for which it belongs. - // It's important to note that we need to subscribe to server validation changes here because we always must - // indicate that a content property is invalid at the property level since developers may not actually implement - // the correct field validation in their property editors. - - const serverValidationManagerCallback = function(isValid, propertyErrors) { - var hadError = hasError; - hasError = !isValid; - if (hasError) { - //set the error message to the server message - scope.errorMsg = propertyErrors.length > 1 ? labels.propertyHasErrors : propertyErrors[0].errorMsg || labels.propertyHasErrors; - //flag that the current validator is invalid - formCtrl.$setValidity('valPropertyMsg', false); - startWatch(); - - // This check is required in order to be able to reset ourselves and is typically for complex editor - // scenarios where the umb-property itself doesn't contain any ng-model controls which means that the - // above serverValidityResetter technique will not work to clear valPropertyMsg errors. - // In order for this to work we rely on the current form controller's $pristine state. This means that anytime - // the form is submitted whether there are validation errors or not the state must be reset... this is automatically - // taken care of with the formHelper.resetForm method that should be used in all cases. $pristine is required because it's - // a value that is cascaded to all form controls based on the hierarchy of child ng-model controls. This allows us to easily - // know if a value has changed. The alternative is what we used to do which was to put a deep $watch on the entire complex value - // which is hugely inefficient. - if (propertyErrors.length === 1 && hadError && !formCtrl.$pristine) { - var propertyValidationPath = umbPropCtrl.getValidationPath(); - serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); - resetError(); - } - } - else { - resetError(); - } - } - - unsubscribe.push(serverValidationManager.subscribe( - umbPropCtrl.getValidationPath(), - currentCulture, - "", - serverValidationManagerCallback, - currentSegment, - { matchType: "prefix" } // match property validation path prefix - )); - } - - }); - } - - //when the scope is disposed we need to unsubscribe - scope.$on('$destroy', function () { - stopWatch(); - unsubscribe.forEach(u => u()); - }); - - onInit(); + if (errCount === 0) { + return true; } + if (errCount > 2) { + return false; + } - }; + var hasValServer = Utilities.isArray(formCtrl.$error.valServer); + if (errCount === 1 && hasValServer) { + return true; + } + + var hasOwnErr = hasExplicitError(); + if ((errCount === 1 && hasOwnErr) || (errCount === 2 && hasOwnErr && hasValServer)) { + + var propertyValidationPath = umbPropCtrl.getValidationPath(); + // check if we can clear it based on child server errors, if we are the only explicit one remaining we can clear ourselves + if (isLastServerError(propertyValidationPath)) { + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); + return true; + } + return false; + } + + return false; + } + + // returns true if there is an explicit valPropertyMsg validation error on the form + function hasExplicitError() { + return Utilities.isArray(formCtrl.$error.valPropertyMsg); + } + + // returns true if there is only a single server validation error for this property validation key in it's validation path + function isLastServerError(propertyValidationPath) { + var nestedErrs = serverValidationManager.getPropertyErrorsByValidationPath( + propertyValidationPath, + currentCulture, + currentSegment, + { matchType: "prefix" }); + if (nestedErrs.length === 0 || (nestedErrs.length === 1 && nestedErrs[0].propertyAlias === propertyValidationPath)) { + + return true; + } + return false; + } + + // a custom $validator function called on when each child ngModelController changes a value. + function resetServerValidityValidator(fieldController) { + const storedFieldController = fieldController; // pin a reference to this + return (modelValue, viewValue) => { + // if the ngModelController value has changed, then we can check and clear the error + if (storedFieldController.$dirty) { + if (checkAndClearError()) { + resetError(); + } + } + return true; // this validator is always 'valid' + }; + } + + // collect all ng-model controllers recursively within the umbProperty form + // until it reaches the next nested umbProperty form + function collectAllNgModelControllersRecursively(controls, ngModels) { + controls.forEach(ctrl => { + if (angularHelper.isForm(ctrl)) { + // if it's not another umbProperty form then recurse + if (ctrl.$name !== formCtrl.$name) { + collectAllNgModelControllersRecursively(ctrl.$getControls(), ngModels); + } + } + else if (Object.prototype.hasOwnProperty.call(ctrl, '$modelValue') && Utilities.isObject(ctrl.$validators)) { + ngModels.push(ctrl); + } + }); + } + + // We start the watch when there's server validation errors detected. + // We watch on the current form's properties and on first watch or if they are dynamically changed + // we find all ngModel controls recursively on this form (but stop recursing before we get to the next) + // umbProperty form). Then for each ngModelController we assign a new $validator. This $validator + // will execute whenever the value is changed which allows us to check and reset the server validator + function startWatch() { + if (!watcher) { + + watcher = scope.$watchCollection( + () => formCtrl, + function (updatedFormController) { + let childControls = updatedFormController.$getControls(); + let ngModels = []; + collectAllNgModelControllersRecursively(childControls, ngModels); + ngModels.forEach(x => { + if (!x.$validators.serverValidityResetter) { + x.$validators.serverValidityResetter = resetServerValidityValidator(x); + } + }); + }); + } + } + + //clear the watch when the property validator is valid again + function stopWatch() { + if (watcher) { + watcher(); + watcher = null; + } + } + + function resetError() { + stopWatch(); + hasError = false; + formCtrl.$setValidity('valPropertyMsg', true); + scope.errorMsg = ""; + + } + + // This deals with client side validation changes and is executed anytime validators change on the containing + // valFormManager. This allows us to know when to display or clear the property error data for non-server side errors. + function checkValidationStatus() { + if (formCtrl.$invalid) { + //first we need to check if the valPropertyMsg validity is invalid + if (formCtrl.$error.valPropertyMsg && formCtrl.$error.valPropertyMsg.length > 0) { + //since we already have an error we'll just return since this means we've already set the + //hasError and errorMsg properties which occurs below in the serverValidationManager.subscribe + return; + } + //if there are any errors in the current property form that are not valPropertyMsg + else if (_.without(_.keys(formCtrl.$error), "valPropertyMsg").length > 0) { + + // errors exist, but if the property is NOT mandatory and has no value, the errors should be cleared + if (isMandatory !== undefined && isMandatory === false && !currentProperty.value) { + + resetError(); + + // if there's no value, the controls can be reset, which clears the error state on formCtrl + for (let control of formCtrl.$getControls()) { + control.$setValidity(); + } + + return; + } + + hasError = true; + //update the validation message if we don't already have one assigned. + if (showValidation && scope.errorMsg === "") { + scope.errorMsg = getErrorMsg(); + } + } + else { + resetError(); + } + } + else { + resetError(); + } + } + + function onInit() { + + localizationService.localize("errors_propertyHasErrors").then(function (data) { + + labels.propertyHasErrors = data; + + //if there's any remaining errors in the server validation service then we should show them. + showValidation = serverValidationManager.items.length > 0; + if (!showValidation) { + //We can either get the form submitted status by the parent directive valFormManager (if we add a property to it) + //or we can just check upwards in the DOM for the css class (easier for now). + //The initial hidden state can't always be hidden because when we switch variants in the content editor we cannot + //reset the status. + showValidation = element.closest(".show-validation").length > 0; + } + + //listen for form validation changes. + //The alternative is to add a watch to formCtrl.$invalid but that would lead to many more watches then + // subscribing to this single watch. + // TODO: Really? Since valFormManager is watching a countof all errors which is more overhead than watching formCtrl.$invalid + // and there's a TODO there that it should just watch formCtrl.$invalid + valFormManager.onValidationStatusChanged(function () { + checkValidationStatus(); + }); + + //listen for the forms saving event + unsubscribe.push(scope.$on("formSubmitting", function () { + showValidation = true; + if (hasError && scope.errorMsg === "") { + scope.errorMsg = getErrorMsg(); + startWatch(); + } + else if (!hasError) { + resetError(); + } + })); + + //listen for the forms saved event + unsubscribe.push(scope.$on("formSubmitted", function () { + showValidation = false; + resetError(); + })); + + if (scope.currentProperty) { //this can be null if no property was assigned + + // listen for server validation changes for property validation path prefix. + // We pass in "" in order to listen for all validation changes to the content property, not for + // validation changes to fields in the property this is because some server side validators may not + // return the field name for which the error belongs too, just the property for which it belongs. + // It's important to note that we need to subscribe to server validation changes here because we always must + // indicate that a content property is invalid at the property level since developers may not actually implement + // the correct field validation in their property editors. + + const serverValidationManagerCallback = function (isValid, propertyErrors) { + var hadError = hasError; + hasError = !isValid; + if (hasError) { + //set the error message to the server message + scope.errorMsg = propertyErrors.length > 1 ? labels.propertyHasErrors : propertyErrors[0].errorMsg || labels.propertyHasErrors; + //flag that the current validator is invalid + formCtrl.$setValidity('valPropertyMsg', false); + startWatch(); + + // This check is required in order to be able to reset ourselves and is typically for complex editor + // scenarios where the umb-property itself doesn't contain any ng-model controls which means that the + // above serverValidityResetter technique will not work to clear valPropertyMsg errors. + // In order for this to work we rely on the current form controller's $pristine state. This means that anytime + // the form is submitted whether there are validation errors or not the state must be reset... this is automatically + // taken care of with the formHelper.resetForm method that should be used in all cases. $pristine is required because it's + // a value that is cascaded to all form controls based on the hierarchy of child ng-model controls. This allows us to easily + // know if a value has changed. The alternative is what we used to do which was to put a deep $watch on the entire complex value + // which is hugely inefficient. + if (propertyErrors.length === 1 && hadError && !formCtrl.$pristine) { + var propertyValidationPath = umbPropCtrl.getValidationPath(); + serverValidationManager.removePropertyError(propertyValidationPath, currentCulture, "", currentSegment); + resetError(); + } + } + else { + resetError(); + } + } + + unsubscribe.push(serverValidationManager.subscribe( + umbPropCtrl.getValidationPath(), + currentCulture, + "", + serverValidationManagerCallback, + currentSegment, + { matchType: "prefix" } // match property validation path prefix + )); + } + + }); + } + + //when the scope is disposed we need to unsubscribe + scope.$on('$destroy', function () { + stopWatch(); + unsubscribe.forEach(u => u()); + }); + + onInit(); + } + + + }; } angular.module('umbraco.directives.validation').directive("valPropertyMsg", valPropertyMsg); From d97cbf486d1040fd311802e136322c85b8e3eef7 Mon Sep 17 00:00:00 2001 From: Stefan Stankovic Date: Tue, 5 Mar 2024 14:04:32 +0100 Subject: [PATCH 05/24] Show BlockGrid Create Dialog if there are items in clipboard Default behavior: If you have only one available module for an area Umbraco will open a dialog for adding the module. If you have more than one module you will get a dialog to choose a module and that dialog will have a Paste option in the top right corner. New behavior: If there is a module in the clipboard, Umbraco will open a dialog with the Paste option even thou there is only one available module in the area. In other cases, it will work as usual. Reported issue: https://github.com/umbraco/Umbraco-CMS/issues/15755 --- .../umbBlockGridPropertyEditor.component.js | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blockgrid/umbBlockGridPropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blockgrid/umbBlockGridPropertyEditor.component.js index 296d98ab7a..f260bd4d31 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blockgrid/umbBlockGridPropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/blockgrid/umbBlockGridPropertyEditor.component.js @@ -605,7 +605,7 @@ // because no columnSpanOptions defined, then use contextual layout columns. layoutEntry.columnSpan = area? area.$config.columnSpan : vm.gridColumns; } - + } else { if(blockObject.config.columnSpanOptions.length > 0) { @@ -621,7 +621,7 @@ // add layout entry at the decided location in layout. if(parentBlock != null) { - + if(!area) { console.error("Could not find area in block creation"); } @@ -678,7 +678,7 @@ } return null; } - + // Used by umbblockgridentries.component to check how many block types that are available for creation in an area: vm.getAllowedTypesOf = getAllowedTypesOf; @@ -705,7 +705,7 @@ } } }); - } else + } else if(allowance.elementTypeKey) { const blockType = vm.availableBlockTypes.find(x => x.blockConfigModel.contentElementTypeKey === allowance.elementTypeKey); if(blockType && allowedElementTypes.indexOf(blockType) === -1) { @@ -777,7 +777,7 @@ */ var wasNotActiveBefore = blockObject.active !== true; - + // don't open the editor overlay if block has hidden its content editor in overlays and we are requesting to open content, not settings. if (openSettings !== true && blockObject.hideContentInOverlay === true) { return; @@ -860,7 +860,7 @@ const availableTypes = getAllowedTypesOf(parentBlock, areaKey); - if (availableTypes.length === 1) { + if (availableTypes.length === 1 && vm.clipboardItems.length === 0) { var wasAdded = false; var blockType = availableTypes[0]; @@ -896,7 +896,7 @@ } const availableBlockGroups = vm.blockGroups.filter(group => !!availableTypes.find(item => item.blockConfigModel.groupKey === group.key)); - + var amountOfAvailableTypes = availableTypes.length; var availableContentTypesAliases = modelObject.getAvailableAliasesOfElementTypeKeys(availableTypes.map(x => x.blockConfigModel.contentElementTypeKey)); var availableClipboardItems = vm.clipboardItems.filter( @@ -917,7 +917,7 @@ createLabel = vm.createLabel; } const headline = createLabel || (amountOfAvailableTypes.length === 1 ? localizationService.tokenReplace(vm.labels.blockEditor_addThis, [availableTypes[0].elementTypeModel.name]) : vm.labels.grid_addElement); - + var blockPickerModel = { $parentScope: $scope, // pass in a $parentScope, this maintains the scope inheritance in infinite editing $parentForm: vm.propertyForm, // pass in a $parentForm, this maintains the FormController hierarchy with the infinite editing view (if it contains a form) @@ -959,7 +959,7 @@ }, close: function() { // if opened by a inline creator button(index less than length), we want to move the focus away, to hide line-creator. - + // add layout entry at the decided location in layout. if(parentBlock != null) { var area = parentBlock.layout.areas.find(x => x.key === areaKey); @@ -976,7 +976,7 @@ vm.setBlockFocus(blockOfInterest); } } - + editorService.close(); vm.blockTypePickerIsOpen = false; @@ -997,7 +997,7 @@ } function userFlowWhenBlockWasCreated(parentBlock, areaKey, createIndex) { var blockObject; - + if (parentBlock) { var area = parentBlock.layout.areas.find(x => x.key === areaKey); if (!area) { @@ -1185,7 +1185,7 @@ if (initializeLayoutEntry(layoutEntry, parentBlock, areaKey) === null) { return null; } - + if (layoutEntry.$block === null) { // Initialization of the Block Object didn't go well, therefor we will fail the paste action. return null; @@ -1224,14 +1224,14 @@ vm.layout.push(layoutEntry); } } - + return {layoutEntry, failed: nestedBlockFailed}; } function requestPasteFromClipboard(parentBlock, areaKey, index, pasteEntry, pasteType) { const data = pasteClipboardEntry(parentBlock, areaKey, index, pasteEntry, pasteType); - if(data) { + if(data) { if(data.failed === true) { // one or more of nested block creation failed. // Ask wether the user likes to continue: @@ -1245,16 +1245,16 @@ closeButtonLabel: localizations[2], submitButtonLabel: localizations[3], close: function () { - // revert: + // revert: deleteBlock(blockToRevert); overlayService.close(); }, submit: function () { - // continue: + // continue: overlayService.close(); } }; - + overlayService.open(overlay); }); } else { @@ -1351,14 +1351,14 @@ document.documentElement.style.setProperty("--umb-block-grid--dragging-mode", ' '); firstLayoutContainer.style.minHeight = firstLayoutContainer.getBoundingClientRect().height + "px"; - + } vm.exitDraggingMode = exitDraggingMode; function exitDraggingMode() { document.documentElement.style.setProperty("--umb-block-grid--dragging-mode", 'initial'); firstLayoutContainer.style.minHeight = ""; - + } function onAmountOfBlocksChanged() { From d8b0616434d1114d0628fb5702206bea39baa88b Mon Sep 17 00:00:00 2001 From: Bishal Tim <143370089+BishalTimalsina12@users.noreply.github.com> Date: Tue, 12 Mar 2024 16:08:22 +0545 Subject: [PATCH 06/24] Update documentation for GetAllContentTags method in ITagQuery interface (#15860) --- src/Umbraco.Core/PublishedCache/ITagQuery.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Umbraco.Core/PublishedCache/ITagQuery.cs b/src/Umbraco.Core/PublishedCache/ITagQuery.cs index 2deaf75108..e0c6a135c9 100644 --- a/src/Umbraco.Core/PublishedCache/ITagQuery.cs +++ b/src/Umbraco.Core/PublishedCache/ITagQuery.cs @@ -33,6 +33,11 @@ public interface ITagQuery /// /// Gets all document tags. /// + /// /// + /// If no culture is specified, it retrieves tags with an invariant culture. + /// If a culture is specified, it only retrieves tags for that culture. + /// Use "*" to retrieve tags for all cultures. + /// IEnumerable GetAllContentTags(string? group = null, string? culture = null); /// From ac5539a8be587c7e59c7a59e1e0c9ae685f5b22a Mon Sep 17 00:00:00 2001 From: nzdev Date: Mon, 11 Mar 2024 21:38:18 +1300 Subject: [PATCH 07/24] Avoid string.Format allocations when profiler is not enabled. --- src/Umbraco.Core/Logging/IProfiler.cs | 5 +++++ src/Umbraco.Core/Logging/LogProfiler.cs | 3 +++ src/Umbraco.Core/Logging/NoopProfiler.cs | 3 +++ src/Umbraco.Web.Common/Profiler/WebProfiler.cs | 3 +++ src/Umbraco.Web.Website/ViewEngines/ProfilingViewEngine.cs | 4 ++-- tests/Umbraco.Tests.Common/TestHelpers/Stubs/TestProfiler.cs | 3 +++ 6 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Logging/IProfiler.cs b/src/Umbraco.Core/Logging/IProfiler.cs index ab580d6aae..d8efdab9d1 100644 --- a/src/Umbraco.Core/Logging/IProfiler.cs +++ b/src/Umbraco.Core/Logging/IProfiler.cs @@ -27,4 +27,9 @@ public interface IProfiler /// authenticated or you want to clear the results, based upon some other mechanism. /// void Stop(bool discardResults = false); + + /// + /// Whether the profiler is enabled. + /// + bool IsEnabled => true; } diff --git a/src/Umbraco.Core/Logging/LogProfiler.cs b/src/Umbraco.Core/Logging/LogProfiler.cs index 84a57979bf..cc93372d75 100644 --- a/src/Umbraco.Core/Logging/LogProfiler.cs +++ b/src/Umbraco.Core/Logging/LogProfiler.cs @@ -35,6 +35,9 @@ public class LogProfiler : IProfiler // the log never stops } + /// + public bool IsEnabled => _logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug); + // a lightweight disposable timer private class LightDisposableTimer : DisposableObjectSlim { diff --git a/src/Umbraco.Core/Logging/NoopProfiler.cs b/src/Umbraco.Core/Logging/NoopProfiler.cs index 821728c7a6..c7bcf34bc0 100644 --- a/src/Umbraco.Core/Logging/NoopProfiler.cs +++ b/src/Umbraco.Core/Logging/NoopProfiler.cs @@ -14,6 +14,9 @@ public class NoopProfiler : IProfiler { } + /// + public bool IsEnabled => false; + private class VoidDisposable : DisposableObjectSlim { protected override void DisposeResources() diff --git a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs index 8767b450fe..a7c0fe6f0e 100644 --- a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs +++ b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs @@ -24,6 +24,9 @@ public class WebProfiler : IProfiler private int _first; private MiniProfiler? _startupProfiler; + /// + public bool IsEnabled => true; + public IDisposable? Step(string name) => MiniProfiler.Current?.Step(name); public void Start() diff --git a/src/Umbraco.Web.Website/ViewEngines/ProfilingViewEngine.cs b/src/Umbraco.Web.Website/ViewEngines/ProfilingViewEngine.cs index f4f2f004e7..5862f90d64 100644 --- a/src/Umbraco.Web.Website/ViewEngines/ProfilingViewEngine.cs +++ b/src/Umbraco.Web.Website/ViewEngines/ProfilingViewEngine.cs @@ -19,7 +19,7 @@ public class ProfilingViewEngine : IViewEngine public ViewEngineResult FindView(ActionContext context, string viewName, bool isMainPage) { - using (_profiler.Step(string.Format("{0}.FindView, {1}, {2}", _name, viewName, isMainPage))) + using (_profiler.IsEnabled ? _profiler.Step(string.Format("{0}.FindView, {1}, {2}", _name, viewName, isMainPage)) : null) { return WrapResult(Inner.FindView(context, viewName, isMainPage)); } @@ -27,7 +27,7 @@ public class ProfilingViewEngine : IViewEngine public ViewEngineResult GetView(string? executingFilePath, string viewPath, bool isMainPage) { - using (_profiler.Step(string.Format("{0}.GetView, {1}, {2}, {3}", _name, executingFilePath, viewPath, isMainPage))) + using (_profiler.IsEnabled ? _profiler.Step(string.Format("{0}.GetView, {1}, {2}, {3}", _name, executingFilePath, viewPath, isMainPage)) : null) { return Inner.GetView(executingFilePath, viewPath, isMainPage); } diff --git a/tests/Umbraco.Tests.Common/TestHelpers/Stubs/TestProfiler.cs b/tests/Umbraco.Tests.Common/TestHelpers/Stubs/TestProfiler.cs index 097201f4fa..f0fc41a584 100644 --- a/tests/Umbraco.Tests.Common/TestHelpers/Stubs/TestProfiler.cs +++ b/tests/Umbraco.Tests.Common/TestHelpers/Stubs/TestProfiler.cs @@ -39,6 +39,9 @@ public class TestProfiler : IProfiler } } + /// + public bool IsEnabled => _enabled; + public static void Enable() => _enabled = true; public static void Disable() => _enabled = false; From 2fbda6e64ba4b2b2cfd84df92b253d1c8ede7cd5 Mon Sep 17 00:00:00 2001 From: Chad Date: Thu, 14 Mar 2024 03:59:28 +1300 Subject: [PATCH 08/24] Dispose IDisposable instances (#15810) --- .../Checks/Security/ExcessiveHeadersCheck.cs | 2 +- .../HealthChecks/Checks/Security/HttpsCheck.cs | 2 +- .../Media/EmbedProviders/OEmbedProviderBase.cs | 2 +- .../Persistence/Repositories/InstallationRepository.cs | 2 +- .../Persistence/Repositories/UpgradeCheckRepository.cs | 4 ++-- src/Umbraco.Core/Security/LegacyPasswordSecurity.cs | 9 ++++++--- src/Umbraco.Core/Security/PasswordGenerator.cs | 5 ++++- 7 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Core/HealthChecks/Checks/Security/ExcessiveHeadersCheck.cs b/src/Umbraco.Core/HealthChecks/Checks/Security/ExcessiveHeadersCheck.cs index e211d7c257..08088251e5 100644 --- a/src/Umbraco.Core/HealthChecks/Checks/Security/ExcessiveHeadersCheck.cs +++ b/src/Umbraco.Core/HealthChecks/Checks/Security/ExcessiveHeadersCheck.cs @@ -51,7 +51,7 @@ public class ExcessiveHeadersCheck : HealthCheck var url = _hostingEnvironment.ApplicationMainUrl?.GetLeftPart(UriPartial.Authority); // Access the site home page and check for the headers - var request = new HttpRequestMessage(HttpMethod.Head, url); + using var request = new HttpRequestMessage(HttpMethod.Head, url); try { using HttpResponseMessage response = await HttpClient.SendAsync(request); diff --git a/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs b/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs index 888b136360..fbe5933b28 100644 --- a/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs +++ b/src/Umbraco.Core/HealthChecks/Checks/Security/HttpsCheck.cs @@ -80,7 +80,7 @@ public class HttpsCheck : HealthCheck var urlBuilder = new UriBuilder(_hostingEnvironment.ApplicationMainUrl) { Scheme = Uri.UriSchemeHttps }; Uri url = urlBuilder.Uri; - var request = new HttpRequestMessage(HttpMethod.Head, url); + using var request = new HttpRequestMessage(HttpMethod.Head, url); try { diff --git a/src/Umbraco.Core/Media/EmbedProviders/OEmbedProviderBase.cs b/src/Umbraco.Core/Media/EmbedProviders/OEmbedProviderBase.cs index 59d0f171ef..e53935f49e 100644 --- a/src/Umbraco.Core/Media/EmbedProviders/OEmbedProviderBase.cs +++ b/src/Umbraco.Core/Media/EmbedProviders/OEmbedProviderBase.cs @@ -60,7 +60,7 @@ public abstract class OEmbedProviderBase : IEmbedProvider using (var request = new HttpRequestMessage(HttpMethod.Get, url)) { - HttpResponseMessage response = _httpClient.SendAsync(request).GetAwaiter().GetResult(); + using HttpResponseMessage response = _httpClient.SendAsync(request).GetAwaiter().GetResult(); return response.Content.ReadAsStringAsync().Result; } } diff --git a/src/Umbraco.Core/Persistence/Repositories/InstallationRepository.cs b/src/Umbraco.Core/Persistence/Repositories/InstallationRepository.cs index c30015a7a0..e79edec7f7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/InstallationRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/InstallationRepository.cs @@ -20,7 +20,7 @@ public class InstallationRepository : IInstallationRepository _httpClient = new HttpClient(); } - var content = new StringContent(_jsonSerializer.Serialize(installLog), Encoding.UTF8, "application/json"); + using var content = new StringContent(_jsonSerializer.Serialize(installLog), Encoding.UTF8, "application/json"); await _httpClient.PostAsync(RestApiInstallUrl, content); } diff --git a/src/Umbraco.Core/Persistence/Repositories/UpgradeCheckRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UpgradeCheckRepository.cs index 4d4e642d9d..9cf0d52251 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UpgradeCheckRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UpgradeCheckRepository.cs @@ -21,10 +21,10 @@ public class UpgradeCheckRepository : IUpgradeCheckRepository _httpClient = new HttpClient(); } - var content = new StringContent(_jsonSerializer.Serialize(new CheckUpgradeDto(version)), Encoding.UTF8, "application/json"); + using var content = new StringContent(_jsonSerializer.Serialize(new CheckUpgradeDto(version)), Encoding.UTF8, "application/json"); _httpClient.Timeout = TimeSpan.FromSeconds(1); - HttpResponseMessage task = await _httpClient.PostAsync(RestApiUpgradeChecklUrl, content); + using HttpResponseMessage task = await _httpClient.PostAsync(RestApiUpgradeChecklUrl, content); var json = await task.Content.ReadAsStringAsync(); UpgradeResult? result = _jsonSerializer.Deserialize(json); diff --git a/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs b/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs index a960fd7998..f50d0b5bdb 100644 --- a/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs +++ b/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs @@ -16,8 +16,11 @@ public class LegacyPasswordSecurity public static string GenerateSalt() { var numArray = new byte[16]; - new RNGCryptoServiceProvider().GetBytes(numArray); - return Convert.ToBase64String(numArray); + using (var rng = new RNGCryptoServiceProvider()) + { + rng.GetBytes(numArray); + return Convert.ToBase64String(numArray); + } } // TODO: Remove v11 @@ -86,7 +89,7 @@ public class LegacyPasswordSecurity /// public bool VerifyLegacyHashedPassword(string password, string dbPassword) { - var hashAlgorithm = new HMACSHA1 + using var hashAlgorithm = new HMACSHA1 { // the legacy salt was actually the password :( Key = Encoding.Unicode.GetBytes(password), diff --git a/src/Umbraco.Core/Security/PasswordGenerator.cs b/src/Umbraco.Core/Security/PasswordGenerator.cs index 7d55e0e39d..1d938d36c8 100644 --- a/src/Umbraco.Core/Security/PasswordGenerator.cs +++ b/src/Umbraco.Core/Security/PasswordGenerator.cs @@ -98,7 +98,10 @@ public class PasswordGenerator var data = new byte[length]; var chArray = new char[length]; var num1 = 0; - new RNGCryptoServiceProvider().GetBytes(data); + using (var rng = new RNGCryptoServiceProvider()) + { + rng.GetBytes(data); + } for (var index = 0; index < length; ++index) { From 4a977b85a278bbe863b2c7032de2f6722619f69a Mon Sep 17 00:00:00 2001 From: albinj Date: Thu, 7 Mar 2024 08:41:03 +0000 Subject: [PATCH 09/24] Fix UmbracoMapper InvalidOperationException due to concurrent modification of inner dictionary (#15830) --- .../Mapping/UmbracoMapper.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Infrastructure/Mapping/UmbracoMapper.cs b/src/Umbraco.Infrastructure/Mapping/UmbracoMapper.cs index 8ebd803aac..85fa7ad38c 100644 --- a/src/Umbraco.Infrastructure/Mapping/UmbracoMapper.cs +++ b/src/Umbraco.Infrastructure/Mapping/UmbracoMapper.cs @@ -43,8 +43,8 @@ public class UmbracoMapper : IUmbracoMapper // note // // the outer dictionary *can* be modified, see GetCtor and GetMap, hence have to be ConcurrentDictionary - // the inner dictionaries are never modified and therefore can be simple Dictionary - private readonly ConcurrentDictionary>> _ctors = + // the inner dictionaries can also be modified, see GetCtor and therefore also needs to be a ConcurrentDictionary + private readonly ConcurrentDictionary>> _ctors = new(); private readonly ConcurrentDictionary>> _maps = @@ -129,7 +129,7 @@ public class UmbracoMapper : IUmbracoMapper Type sourceType = typeof(TSource); Type targetType = typeof(TTarget); - Dictionary> sourceCtors = DefineCtors(sourceType); + ConcurrentDictionary> sourceCtors = DefineCtors(sourceType); if (ctor != null) { sourceCtors[targetType] = (source, context) => ctor((TSource)source, context)!; @@ -139,8 +139,8 @@ public class UmbracoMapper : IUmbracoMapper sourceMaps[targetType] = (source, target, context) => map((TSource)source, (TTarget)target, context); } - private Dictionary> DefineCtors(Type sourceType) => - _ctors.GetOrAdd(sourceType, _ => new Dictionary>()); + private ConcurrentDictionary> DefineCtors(Type sourceType) => + _ctors.GetOrAdd(sourceType, _ => new ConcurrentDictionary>()); private ConcurrentDictionary> DefineMaps(Type sourceType) => _maps.GetOrAdd(sourceType, _ => new ConcurrentDictionary>()); @@ -391,7 +391,7 @@ public class UmbracoMapper : IUmbracoMapper return null; } - if (_ctors.TryGetValue(sourceType, out Dictionary>? sourceCtor) && + if (_ctors.TryGetValue(sourceType, out ConcurrentDictionary>? sourceCtor) && sourceCtor.TryGetValue(targetType, out Func? ctor)) { return ctor; @@ -399,7 +399,7 @@ public class UmbracoMapper : IUmbracoMapper // we *may* run this more than once but it does not matter ctor = null; - foreach ((Type stype, Dictionary> sctors) in _ctors) + foreach ((Type stype, ConcurrentDictionary> sctors) in _ctors) { if (!stype.IsAssignableFrom(sourceType)) { @@ -427,7 +427,7 @@ public class UmbracoMapper : IUmbracoMapper { if (!v.ContainsKey(c.Key)) { - v.Add(c.Key, c.Value); + v.TryAdd(c.Key, c.Value); } } From 801fb5f88561d28f0bf87ef5e462540a4a08af33 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 14 Mar 2024 14:48:09 +0100 Subject: [PATCH 10/24] Bump version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index c04379412a..547407d05f 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": "10.8.4", + "version": "10.8.5", "assemblyVersion": { "precision": "build" }, From a2511ff09b1f05fb47583fc7685e7f1871a86a3c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 15 Mar 2024 09:56:02 +0000 Subject: [PATCH 11/24] Fixing locking issues for document type saves. (#15854) * Added ExecuteNonQuery(DbCommand command) on database to ensure we call OnExecutingCommand and OnExecutedCommand when executing DbCommands * Added Cache Instructions lock, to avoid deadlocks * Optimized read locks for nucache when only one content type is rebuilt * Optimized the SqlServer locks, so only one command is executed (and thereby roundtrip) per lock instead of two * Avoid breaking changes * Cosmetic changes * Take locks if everything is rebuild * Use same lock in scopes, to avoid potential deadlocks between the two * Use eager locks in PublishedSnapshotService.cs * Added timeouts to some of the application locks * Revert "Use eager locks in PublishedSnapshotService.cs" This reverts commit 01873aae978ffa6e6686d253e482c493715e3a96. * Revert "Added Cache Instructions lock, to avoid deadlocks" This reverts commit e3fca7c12a804bb32ca1156b8abd42a957e9dc21. * Use single readlock call to lock many * Use eager locks for reads * Eager write locks * Ignore test of lazy locks * Unique timeout exception messages --------- Co-authored-by: kjac (cherry picked from commit 2c23e67c65a98e0573924cd1a55bf03df6e13410) --- .../SqlServerDistributedLockingMechanism.cs | 10 +++-- .../SqliteDistributedLockingMechanism.cs | 2 +- src/Umbraco.Core/Cache/ObjectCacheAppCache.cs | 39 +++++++++++++++---- .../Persistence/IUmbracoDatabase.cs | 4 ++ .../Persistence/UmbracoDatabase.cs | 8 ++++ src/Umbraco.Infrastructure/Scoping/Scope.cs | 26 ++++++------- .../ContentStore.cs | 9 ++++- .../Persistence/NuCacheContentService.cs | 22 +++++++++-- .../SnapDictionary.cs | 9 ++++- .../Persistence/LocksTests.cs | 38 +++++++++++++++--- 10 files changed, 129 insertions(+), 38 deletions(-) diff --git a/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs b/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs index 358eae2e38..e0bf4767bf 100644 --- a/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs +++ b/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs @@ -134,9 +134,10 @@ public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism const string query = "SELECT value FROM umbracoLock WITH (REPEATABLEREAD) WHERE id=@id"; - db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";"); + var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}"; - var i = db.ExecuteScalar(query, new { id = LockId }); + // execute the lock timeout query and the actual query in a single server roundtrip + var i = db.ExecuteScalar($"{lockTimeoutQuery};{query}", new { id = LockId }); if (i == null) { @@ -169,9 +170,10 @@ public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism const string query = @"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id"; - db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";"); + var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}"; - var i = db.Execute(query, new { id = LockId }); + // execute the lock timeout query and the actual query in a single server roundtrip + var i = db.Execute($"{lockTimeoutQuery};{query}", new { id = LockId }); if (i == 0) { diff --git a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs index c3d6f8b29d..9f25f77c2c 100644 --- a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs +++ b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs @@ -154,7 +154,7 @@ public class SqliteDistributedLockingMechanism : IDistributedLockingMechanism try { - var i = command.ExecuteNonQuery(); + var i = db.ExecuteNonQuery(command); if (i == 0) { diff --git a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs index dcd83ece94..ff1c9b3bfd 100644 --- a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs +++ b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs @@ -9,6 +9,9 @@ namespace Umbraco.Cms.Core.Cache; /// public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { + private static readonly TimeSpan _readLockTimeout = TimeSpan.FromSeconds(5); + private static readonly TimeSpan _writeLockTimeout = TimeSpan.FromSeconds(5); + private readonly ReaderWriterLockSlim _locker = new(LockRecursionPolicy.SupportsRecursion); private bool _disposedValue; @@ -33,7 +36,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable Lazy? result; try { - _locker.EnterReadLock(); + if (_locker.TryEnterReadLock(_readLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when getting item"); + } result = MemoryCache.Get(key) as Lazy; // null if key not found } finally @@ -195,7 +201,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing item"); + } if (MemoryCache[key] == null) { return; @@ -223,8 +232,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable var isInterface = type.IsInterface; try { - _locker.EnterWriteLock(); - + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing by type"); + } // ToArray required to remove foreach (var key in MemoryCache .Where(x => @@ -259,7 +270,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing by generic type"); + } Type typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; @@ -296,7 +310,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing generic type with predicate"); + } Type typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; @@ -338,7 +355,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing with prefix"); + } // ToArray required to remove foreach (var key in MemoryCache @@ -365,7 +385,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cach when clearing by regex"); + } // ToArray required to remove foreach (var key in MemoryCache diff --git a/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs b/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs index 431ddeb5e8..b86648f05c 100644 --- a/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs +++ b/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs @@ -1,3 +1,4 @@ +using System.Data.Common; using NPoco; using Umbraco.Cms.Infrastructure.Migrations.Install; @@ -33,4 +34,7 @@ public interface IUmbracoDatabase : IDatabase bool IsUmbracoInstalled(); DatabaseSchemaResult ValidateSchema(); + + /// The number of rows affected. + int ExecuteNonQuery(DbCommand command) => command.ExecuteNonQuery(); } diff --git a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs index 425629dc4b..74f8c297ef 100644 --- a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs +++ b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs @@ -224,6 +224,14 @@ public class UmbracoDatabase : Database, IUmbracoDatabase return databaseSchemaValidationResult ?? new DatabaseSchemaResult(); } + public int ExecuteNonQuery(DbCommand command) + { + OnExecutingCommand(command); + var i = command.ExecuteNonQuery(); + OnExecutedCommand(command); + return i; + } + /// /// Returns true if Umbraco database tables are detected to be installed /// diff --git a/src/Umbraco.Infrastructure/Scoping/Scope.cs b/src/Umbraco.Infrastructure/Scoping/Scope.cs index 000b6a602e..0db9656fc5 100644 --- a/src/Umbraco.Infrastructure/Scoping/Scope.cs +++ b/src/Umbraco.Infrastructure/Scoping/Scope.cs @@ -23,10 +23,9 @@ namespace Umbraco.Cms.Infrastructure.Scoping private readonly bool _autoComplete; private readonly CoreDebugSettings _coreDebugSettings; - private readonly object _dictionaryLocker; private readonly IEventAggregator _eventAggregator; private readonly IsolationLevel _isolationLevel; - private readonly object _lockQueueLocker = new(); + private readonly object _locker = new(); private readonly ILogger _logger; private readonly MediaFileManager _mediaFileManager; private readonly RepositoryCacheMode _repositoryCacheMode; @@ -87,7 +86,6 @@ namespace Umbraco.Cms.Infrastructure.Scoping _scopeFileSystem = scopeFileSystems; _autoComplete = autoComplete; Detachable = detachable; - _dictionaryLocker = new object(); #if DEBUG_SCOPES _scopeProvider.RegisterScope(this); @@ -562,7 +560,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping DisposeLastScope(); } - lock (_lockQueueLocker) + lock (_locker) { _queuedLocks?.Clear(); } @@ -573,24 +571,24 @@ namespace Umbraco.Cms.Infrastructure.Scoping public void EagerReadLock(params int[] lockIds) => EagerReadLockInner(InstanceId, null, lockIds); /// - public void ReadLock(params int[] lockIds) => LazyReadLockInner(InstanceId, lockIds); + public void ReadLock(params int[] lockIds) => EagerReadLockInner(InstanceId, null, lockIds); public void EagerReadLock(TimeSpan timeout, int lockId) => EagerReadLockInner(InstanceId, timeout, lockId); /// - public void ReadLock(TimeSpan timeout, int lockId) => LazyReadLockInner(InstanceId, timeout, lockId); + public void ReadLock(TimeSpan timeout, int lockId) => EagerReadLockInner(InstanceId, timeout, lockId); public void EagerWriteLock(params int[] lockIds) => EagerWriteLockInner(InstanceId, null, lockIds); /// - public void WriteLock(params int[] lockIds) => LazyWriteLockInner(InstanceId, lockIds); + public void WriteLock(params int[] lockIds) => EagerWriteLockInner(InstanceId, null, lockIds); public void EagerWriteLock(TimeSpan timeout, int lockId) => EagerWriteLockInner(InstanceId, timeout, lockId); /// - public void WriteLock(TimeSpan timeout, int lockId) => LazyWriteLockInner(InstanceId, timeout, lockId); + public void WriteLock(TimeSpan timeout, int lockId) => EagerWriteLockInner(InstanceId, timeout, lockId); /// /// Used for testing. Ensures and gets any queued read locks. @@ -659,7 +657,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping } else { - lock (_lockQueueLocker) + lock (_locker) { if (_queuedLocks?.Count > 0) { @@ -970,7 +968,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping } else { - lock (_dictionaryLocker) + lock (_locker) { _readLocksDictionary?.Remove(instanceId); _writeLocksDictionary?.Remove(instanceId); @@ -1045,7 +1043,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping private void LazyLockInner(DistributedLockType lockType, Guid instanceId, params int[] lockIds) { - lock (_lockQueueLocker) + lock (_locker) { if (_queuedLocks == null) { @@ -1061,7 +1059,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping private void LazyLockInner(DistributedLockType lockType, Guid instanceId, TimeSpan timeout, int lockId) { - lock (_lockQueueLocker) + lock (_locker) { if (_queuedLocks == null) { @@ -1088,7 +1086,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping } else { - lock (_dictionaryLocker) + lock (_locker) { foreach (var lockId in lockIds) { @@ -1122,7 +1120,7 @@ namespace Umbraco.Cms.Infrastructure.Scoping } else { - lock (_dictionaryLocker) + lock (_locker) { foreach (var lockId in lockIds) { diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs index f38b9dd2fc..0230032dc2 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs @@ -27,6 +27,8 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache; /// public class ContentStore { + private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30); + // TODO: collection trigger (ok for now) // see SnapDictionary notes private const long CollectMinGenDelta = 8; @@ -330,7 +332,12 @@ public class ContentStore throw new InvalidOperationException("Recursive locks not allowed"); } - Monitor.Enter(_wlocko, ref lockInfo.Taken); + Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken); + + if (Monitor.IsEntered(_wlocko) is false) + { + throw new TimeoutException("Could not enter monitor before timeout in content store"); + } lock (_rlocko) { diff --git a/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs b/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs index 09855f5682..13e911f137 100644 --- a/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs +++ b/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs @@ -127,9 +127,25 @@ public class NuCacheContentService : RepositoryService, INuCacheContentService { using (ICoreScope scope = ScopeProvider.CreateCoreScope(repositoryCacheMode: RepositoryCacheMode.Scoped)) { - scope.ReadLock(Constants.Locks.ContentTree); - scope.ReadLock(Constants.Locks.MediaTree); - scope.ReadLock(Constants.Locks.MemberTree); + if (contentTypeIds is null && mediaTypeIds is null && memberTypeIds is null) + { + scope.ReadLock(Constants.Locks.ContentTree,Constants.Locks.MediaTree,Constants.Locks.MemberTree); + } + + if (contentTypeIds is not null && contentTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.ContentTree); + } + + if (mediaTypeIds is not null && mediaTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.MediaTree); + } + + if (memberTypeIds is not null && memberTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.MemberTree); + } _repository.Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds); diff --git a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs index 0d042380d2..b6c87e22bb 100644 --- a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs +++ b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs @@ -9,6 +9,8 @@ public class SnapDictionary where TValue : class where TKey : notnull { + private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30); + // minGenDelta to be adjusted // we may want to throttle collects even if delta is reached // we may want to force collect if delta is not reached but very old @@ -198,7 +200,12 @@ public class SnapDictionary throw new InvalidOperationException("Recursive locks not allowed"); } - Monitor.Enter(_wlocko, ref lockInfo.Taken); + Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken); + + if (Monitor.IsEntered(_wlocko) is false) + { + throw new TimeoutException("Could not enter the monitor before timeout in SnapDictionary"); + } lock (_rlocko) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs index a7390ba6ef..0d1b464efa 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs @@ -1,9 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; using System.Text; -using System.Threading; -using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using NPoco; @@ -15,7 +10,6 @@ using Umbraco.Cms.Persistence.Sqlite.Interceptors; using Umbraco.Cms.Tests.Common.Attributes; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; -using Umbraco.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence; @@ -126,6 +120,7 @@ public class LocksTests : UmbracoIntegrationTest } } + [NUnit.Framework.Ignore("We currently do not have a way to force lazy locks")] [Test] public void GivenNonEagerLocking_WhenNoDbIsAccessed_ThenNoSqlIsExecuted() { @@ -155,6 +150,37 @@ public class LocksTests : UmbracoIntegrationTest Assert.AreEqual(0, sqlCount); } + [Test] + public void GivenNonEagerLocking_WhenDbIsAccessed_ThenSqlIsExecuted() + { + var sqlCount = 0; + + using (var scope = ScopeProvider.CreateScope()) + { + var db = ScopeAccessor.AmbientScope.Database; + try + { + db.EnableSqlCount = true; + + // Issue a lock request, but we are using non-eager + // locks so this only queues the request. + // The lock will not be issued unless we resolve + // scope.Database + scope.WriteLock(Constants.Locks.Servers); + + scope.Database.ExecuteScalar("SELECT 1"); + + sqlCount = db.SqlCount; + } + finally + { + db.EnableSqlCount = false; + } + } + + Assert.AreEqual(2,sqlCount); + } + [Test] [LongRunning] public void ConcurrentWritersTest() From 7e1d1a1968000226cd882fff078b122b8d46c44d Mon Sep 17 00:00:00 2001 From: Jey Date: Mon, 18 Mar 2024 08:27:41 +0100 Subject: [PATCH 12/24] Merge pull request from GHSA-552f-97wf-pmpq Co-authored-by: jey (cherry picked from commit b743f6a2df7c4e8bc72d6aaffd2ae1544ed2ad1a) --- src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs index 96f0025efa..231f2b3b1a 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs @@ -134,8 +134,8 @@ public abstract class UmbracoUserManager : UserManager public override async Task CheckPasswordAsync(TUser user, string? password) { - // we cannot proceed if the user passed in does not have an identity - if (user.HasIdentity == false) + // we cannot proceed if the user passed in does not have an identity, or if no password is provided. + if (user.HasIdentity == false || password is null) { return false; } @@ -252,7 +252,7 @@ public abstract class UmbracoUserManager : UserManager ValidateCredentialsAsync(string username, string password) { TUser user = await FindByNameAsync(username); - + if (user == null) { return false; @@ -263,7 +263,7 @@ public abstract class UmbracoUserManager : UserManager)); } - + var result = await VerifyPasswordAsync(userPasswordStore, user, password); return result == PasswordVerificationResult.Success || result == PasswordVerificationResult.SuccessRehashNeeded; From 8789a643ec737a88f22e4fd13155c7b919a2cd17 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 18 Mar 2024 08:38:55 +0100 Subject: [PATCH 13/24] Bump version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index 90f6e87fbf..beca541bdc 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "12.3.7", + "version": "12.3.8", "assemblyVersion": { "precision": "build" }, From 827fa188f45e882aaaf93a541242d53883298109 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 15 Mar 2024 10:56:02 +0100 Subject: [PATCH 14/24] Fixing locking issues for document type saves. (#15854) * Added ExecuteNonQuery(DbCommand command) on database to ensure we call OnExecutingCommand and OnExecutedCommand when executing DbCommands * Added Cache Instructions lock, to avoid deadlocks * Optimized read locks for nucache when only one content type is rebuilt * Optimized the SqlServer locks, so only one command is executed (and thereby roundtrip) per lock instead of two * Avoid breaking changes * Cosmetic changes * Take locks if everything is rebuild * Use same lock in scopes, to avoid potential deadlocks between the two * Use eager locks in PublishedSnapshotService.cs * Added timeouts to some of the application locks * Revert "Use eager locks in PublishedSnapshotService.cs" This reverts commit 01873aae978ffa6e6686d253e482c493715e3a96. * Revert "Added Cache Instructions lock, to avoid deadlocks" This reverts commit e3fca7c12a804bb32ca1156b8abd42a957e9dc21. * Use single readlock call to lock many * Use eager locks for reads * Eager write locks * Ignore test of lazy locks * Unique timeout exception messages --------- Co-authored-by: kjac (cherry picked from commit 2c23e67c65a98e0573924cd1a55bf03df6e13410) --- .../SqlServerDistributedLockingMechanism.cs | 10 +++-- .../SqliteDistributedLockingMechanism.cs | 2 +- src/Umbraco.Core/Cache/ObjectCacheAppCache.cs | 39 +++++++++++++++---- .../Persistence/IUmbracoDatabase.cs | 4 ++ .../Persistence/UmbracoDatabase.cs | 8 ++++ .../ContentStore.cs | 9 ++++- .../Persistence/NuCacheContentService.cs | 22 +++++++++-- .../SnapDictionary.cs | 9 ++++- .../Persistence/LocksTests.cs | 35 +++++++++++++++-- 9 files changed, 117 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs b/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs index cc2e1b5feb..77975e8f31 100644 --- a/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs +++ b/src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs @@ -143,9 +143,10 @@ public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism const string query = "SELECT value FROM umbracoLock WITH (REPEATABLEREAD) WHERE id=@id"; - db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";"); + var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}"; - var i = db.ExecuteScalar(query, new { id = LockId }); + // execute the lock timeout query and the actual query in a single server roundtrip + var i = db.ExecuteScalar($"{lockTimeoutQuery};{query}", new { id = LockId }); if (i == null) { @@ -178,9 +179,10 @@ public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism const string query = @"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id"; - db.Execute("SET LOCK_TIMEOUT " + _timeout.TotalMilliseconds + ";"); + var lockTimeoutQuery = $"SET LOCK_TIMEOUT {_timeout.TotalMilliseconds}"; - var i = db.Execute(query, new { id = LockId }); + // execute the lock timeout query and the actual query in a single server roundtrip + var i = db.Execute($"{lockTimeoutQuery};{query}", new { id = LockId }); if (i == 0) { diff --git a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs index a29a4d1419..54e30d6fa6 100644 --- a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs +++ b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs @@ -162,7 +162,7 @@ public class SqliteDistributedLockingMechanism : IDistributedLockingMechanism try { - var i = command.ExecuteNonQuery(); + var i = db.ExecuteNonQuery(command); if (i == 0) { diff --git a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs index dcd83ece94..ff1c9b3bfd 100644 --- a/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs +++ b/src/Umbraco.Core/Cache/ObjectCacheAppCache.cs @@ -9,6 +9,9 @@ namespace Umbraco.Cms.Core.Cache; /// public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { + private static readonly TimeSpan _readLockTimeout = TimeSpan.FromSeconds(5); + private static readonly TimeSpan _writeLockTimeout = TimeSpan.FromSeconds(5); + private readonly ReaderWriterLockSlim _locker = new(LockRecursionPolicy.SupportsRecursion); private bool _disposedValue; @@ -33,7 +36,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable Lazy? result; try { - _locker.EnterReadLock(); + if (_locker.TryEnterReadLock(_readLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when getting item"); + } result = MemoryCache.Get(key) as Lazy; // null if key not found } finally @@ -195,7 +201,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing item"); + } if (MemoryCache[key] == null) { return; @@ -223,8 +232,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable var isInterface = type.IsInterface; try { - _locker.EnterWriteLock(); - + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing by type"); + } // ToArray required to remove foreach (var key in MemoryCache .Where(x => @@ -259,7 +270,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing by generic type"); + } Type typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; @@ -296,7 +310,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing generic type with predicate"); + } Type typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; @@ -338,7 +355,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable { try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cache when clearing with prefix"); + } // ToArray required to remove foreach (var key in MemoryCache @@ -365,7 +385,10 @@ public class ObjectCacheAppCache : IAppPolicyCache, IDisposable try { - _locker.EnterWriteLock(); + if (_locker.TryEnterWriteLock(_writeLockTimeout) is false) + { + throw new TimeoutException("Timeout exceeded to the memory cach when clearing by regex"); + } // ToArray required to remove foreach (var key in MemoryCache diff --git a/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs b/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs index 431ddeb5e8..b86648f05c 100644 --- a/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs +++ b/src/Umbraco.Infrastructure/Persistence/IUmbracoDatabase.cs @@ -1,3 +1,4 @@ +using System.Data.Common; using NPoco; using Umbraco.Cms.Infrastructure.Migrations.Install; @@ -33,4 +34,7 @@ public interface IUmbracoDatabase : IDatabase bool IsUmbracoInstalled(); DatabaseSchemaResult ValidateSchema(); + + /// The number of rows affected. + int ExecuteNonQuery(DbCommand command) => command.ExecuteNonQuery(); } diff --git a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs index af8eb8e1fe..3abec76ec2 100644 --- a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs +++ b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabase.cs @@ -223,6 +223,14 @@ public class UmbracoDatabase : Database, IUmbracoDatabase return databaseSchemaValidationResult ?? new DatabaseSchemaResult(); } + public int ExecuteNonQuery(DbCommand command) + { + OnExecutingCommand(command); + var i = command.ExecuteNonQuery(); + OnExecutedCommand(command); + return i; + } + /// /// Returns true if Umbraco database tables are detected to be installed /// diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs index fad8722b77..835c2561df 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs @@ -27,6 +27,8 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache; /// public class ContentStore { + private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30); + // TODO: collection trigger (ok for now) // see SnapDictionary notes private const long CollectMinGenDelta = 8; @@ -330,7 +332,12 @@ public class ContentStore throw new InvalidOperationException("Recursive locks not allowed"); } - Monitor.Enter(_wlocko, ref lockInfo.Taken); + Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken); + + if (Monitor.IsEntered(_wlocko) is false) + { + throw new TimeoutException("Could not enter monitor before timeout in content store"); + } lock (_rlocko) { diff --git a/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs b/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs index 09855f5682..13e911f137 100644 --- a/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs +++ b/src/Umbraco.PublishedCache.NuCache/Persistence/NuCacheContentService.cs @@ -127,9 +127,25 @@ public class NuCacheContentService : RepositoryService, INuCacheContentService { using (ICoreScope scope = ScopeProvider.CreateCoreScope(repositoryCacheMode: RepositoryCacheMode.Scoped)) { - scope.ReadLock(Constants.Locks.ContentTree); - scope.ReadLock(Constants.Locks.MediaTree); - scope.ReadLock(Constants.Locks.MemberTree); + if (contentTypeIds is null && mediaTypeIds is null && memberTypeIds is null) + { + scope.ReadLock(Constants.Locks.ContentTree,Constants.Locks.MediaTree,Constants.Locks.MemberTree); + } + + if (contentTypeIds is not null && contentTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.ContentTree); + } + + if (mediaTypeIds is not null && mediaTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.MediaTree); + } + + if (memberTypeIds is not null && memberTypeIds.Any()) + { + scope.ReadLock(Constants.Locks.MemberTree); + } _repository.Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds); diff --git a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs index 0d042380d2..b6c87e22bb 100644 --- a/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs +++ b/src/Umbraco.PublishedCache.NuCache/SnapDictionary.cs @@ -9,6 +9,8 @@ public class SnapDictionary where TValue : class where TKey : notnull { + private static readonly TimeSpan _monitorTimeout = TimeSpan.FromSeconds(30); + // minGenDelta to be adjusted // we may want to throttle collects even if delta is reached // we may want to force collect if delta is not reached but very old @@ -198,7 +200,12 @@ public class SnapDictionary throw new InvalidOperationException("Recursive locks not allowed"); } - Monitor.Enter(_wlocko, ref lockInfo.Taken); + Monitor.TryEnter(_wlocko, _monitorTimeout, ref lockInfo.Taken); + + if (Monitor.IsEntered(_wlocko) is false) + { + throw new TimeoutException("Could not enter the monitor before timeout in SnapDictionary"); + } lock (_rlocko) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs index 88b48dfaae..9e5b943e0c 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs @@ -1,8 +1,6 @@ using System.Collections.Generic; using System.Linq; using System.Text; -using System.Threading; -using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using NPoco; @@ -14,7 +12,6 @@ using Umbraco.Cms.Persistence.Sqlite.Interceptors; using Umbraco.Cms.Tests.Common.Attributes; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; -using Umbraco.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence; @@ -125,6 +122,7 @@ public class LocksTests : UmbracoIntegrationTest } } + [NUnit.Framework.Ignore("We currently do not have a way to force lazy locks")] [Test] public void GivenNonEagerLocking_WhenNoDbIsAccessed_ThenNoSqlIsExecuted() { @@ -154,6 +152,37 @@ public class LocksTests : UmbracoIntegrationTest Assert.AreEqual(0, sqlCount); } + [Test] + public void GivenNonEagerLocking_WhenDbIsAccessed_ThenSqlIsExecuted() + { + var sqlCount = 0; + + using (var scope = ScopeProvider.CreateScope()) + { + var db = ScopeAccessor.AmbientScope.Database; + try + { + db.EnableSqlCount = true; + + // Issue a lock request, but we are using non-eager + // locks so this only queues the request. + // The lock will not be issued unless we resolve + // scope.Database + scope.WriteLock(Constants.Locks.Servers); + + scope.Database.ExecuteScalar("SELECT 1"); + + sqlCount = db.SqlCount; + } + finally + { + db.EnableSqlCount = false; + } + } + + Assert.AreEqual(2,sqlCount); + } + [Test] [LongRunning] public void ConcurrentWritersTest() From da43d2f08c82f7591868bcb0266bd9f694f4d263 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 15 Mar 2024 15:58:22 +0100 Subject: [PATCH 15/24] Fix after merge (cherry picked from commit 3e08ce1efb2a7f73ec84e8140585c0eff1d7cb30) (cherry picked from commit 7e822bb8a1ffc2c344e3a92ee083b2bc3f64ee59) --- src/Umbraco.Core/Scoping/LockingMechanism.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Scoping/LockingMechanism.cs b/src/Umbraco.Core/Scoping/LockingMechanism.cs index e41fe2d874..ad9fd6dd65 100644 --- a/src/Umbraco.Core/Scoping/LockingMechanism.cs +++ b/src/Umbraco.Core/Scoping/LockingMechanism.cs @@ -35,12 +35,12 @@ public class LockingMechanism : ILockingMechanism } /// - public void ReadLock(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds) => LazyReadLockInner(instanceId, timeout, lockIds); + public void ReadLock(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds) => EagerReadLockInner(instanceId, timeout, lockIds); public void ReadLock(Guid instanceId, params int[] lockIds) => ReadLock(instanceId, null, lockIds); /// - public void WriteLock(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds) => LazyWriteLockInner(instanceId, timeout, lockIds); + public void WriteLock(Guid instanceId, TimeSpan? timeout = null, params int[] lockIds) => EagerWriteLockInner(instanceId, timeout, lockIds); public void WriteLock(Guid instanceId, params int[] lockIds) => WriteLock(instanceId, null, lockIds); From 2c41388096e2b539913597bf3d29ee8b1cf31e33 Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Tue, 19 Mar 2024 12:31:31 +0100 Subject: [PATCH 16/24] Preserve user populated link title when selecting nodes in link picker (#14787) * Prevent user populated link names being removed When selecting a node in a link picker, the "name" (Link title) field is always overridden with the selected node's name. This change prevents the field from being overridden if it is user populated. * Don't update for media either If there is already a name, don't update when selecting media * Deselect current node When selecting media, deselect current node if there is one * Change to suggested functionality Update to more elaborate functionality which will always use the *current* node name. i.e If you change the node name then the link picker name changes also. * Reapply changes Repply changes after merge from contrib branch * Use IsCulturePublished() and only track node name * Fallback to node name in preview * Fix setting media name --------- Co-authored-by: Ronald Barendse --- .../Models/ContentEditing/LinkDisplay.cs | 3 + src/Umbraco.Core/Models/Link.cs | 4 + .../MultiUrlPickerValueEditor.cs | 47 +- .../MultiUrlPickerValueConverter.cs | 12 +- .../linkpicker/linkpicker.controller.js | 533 +++++++++--------- .../linkpicker/linkpicker.html | 7 +- .../multiurlpicker.controller.js | 8 +- .../multiurlpicker/multiurlpicker.html | 2 +- 8 files changed, 314 insertions(+), 302 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentEditing/LinkDisplay.cs b/src/Umbraco.Core/Models/ContentEditing/LinkDisplay.cs index 9b7bde570d..5859c00f6d 100644 --- a/src/Umbraco.Core/Models/ContentEditing/LinkDisplay.cs +++ b/src/Umbraco.Core/Models/ContentEditing/LinkDisplay.cs @@ -11,6 +11,9 @@ public class LinkDisplay [DataMember(Name = "name")] public string? Name { get; set; } + [DataMember(Name = "nodeName")] + public string? NodeName { get; set; } + [DataMember(Name = "published")] public bool Published { get; set; } diff --git a/src/Umbraco.Core/Models/Link.cs b/src/Umbraco.Core/Models/Link.cs index 7047b54555..8e46543bfb 100644 --- a/src/Umbraco.Core/Models/Link.cs +++ b/src/Umbraco.Core/Models/Link.cs @@ -1,3 +1,5 @@ +using Umbraco.Cms.Core.Models.PublishedContent; + namespace Umbraco.Cms.Core.Models; public class Link @@ -10,5 +12,7 @@ public class Link public Udi? Udi { get; set; } + public IPublishedContent? Content { get; set; } + public string? Url { get; set; } } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs index 1b76cd89a5..0ee37104c7 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs @@ -16,6 +16,7 @@ using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Cms.Web.Common.DependencyInjection; +using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; @@ -146,38 +147,41 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference foreach (LinkDto dto in links) { - GuidUdi? udi = dto.Udi; var icon = "icon-link"; + string? nodeName = null; var published = true; var trashed = false; var url = dto.Url; - if (dto.Udi != null) + if (dto.Udi is not null) { if (dto.Udi.EntityType == Constants.UdiEntityType.Document) { - url = _publishedUrlProvider.GetUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IContent? c = _contentService.GetById(dto.Udi.Guid); - - if (c is not null) + IContent? content = _contentService.GetById(dto.Udi.Guid); + if (content is not null) { + icon = content.ContentType.Icon; + nodeName = content.Name; published = culture == null - ? c.Published - : c.PublishedCultures.Contains(culture); - icon = c.ContentType.Icon; - trashed = c.Trashed; + ? content.Published + : content.IsCulturePublished(culture); + trashed = content.Trashed; } + + url = _publishedUrlProvider.GetUrl(dto.Udi.Guid, UrlMode.Relative, culture); } else if (dto.Udi.EntityType == Constants.UdiEntityType.Media) { - url = _publishedUrlProvider.GetMediaUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IMedia? m = _mediaService.GetById(dto.Udi.Guid); - if (m is not null) + IMedia? media = _mediaService.GetById(dto.Udi.Guid); + if (media is not null) { - published = m.Trashed is false; - icon = m.ContentType.Icon; - trashed = m.Trashed; + icon = media.ContentType.Icon; + nodeName = media.Name; + published = media.Trashed is false; + trashed = media.Trashed; } + + url = _publishedUrlProvider.GetMediaUrl(dto.Udi.Guid, UrlMode.Relative, culture); } } @@ -185,12 +189,13 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference { Icon = icon, Name = dto.Name, - Target = dto.Target, - Trashed = trashed, + NodeName = nodeName, Published = published, QueryString = dto.QueryString, - Udi = udi, - Url = url ?? string.Empty, + Target = dto.Target, + Trashed = trashed, + Udi = dto.Udi, + Url = url, }); } @@ -229,7 +234,7 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference QueryString = link.QueryString, Target = link.Target, Udi = link.Udi, - Url = link.Udi == null ? link.Url : null, // only save the URL for external links + Url = link.Udi is null ? link.Url : null, // only save the URL for external links }, _linkDisplayJsonSerializerSettings); } diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs index 61cc0e1c7d..32fea25856 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs @@ -107,6 +107,8 @@ public class MultiUrlPickerValueConverter : PropertyValueConverterBase, IDeliver { LinkType type = LinkType.External; var url = dto.Url; + var name = dto.Name; + IPublishedContent? content = null; if (dto.Udi is not null) { @@ -114,7 +116,7 @@ public class MultiUrlPickerValueConverter : PropertyValueConverterBase, IDeliver ? LinkType.Media : LinkType.Content; - IPublishedContent? content = type == LinkType.Media + content = type == LinkType.Media ? publishedSnapshot.Media?.GetById(preview, dto.Udi.Guid) : publishedSnapshot.Content?.GetById(preview, dto.Udi.Guid); @@ -123,16 +125,22 @@ public class MultiUrlPickerValueConverter : PropertyValueConverterBase, IDeliver continue; } + if (string.IsNullOrEmpty(name)) + { + name = content.Name; + } + url = content.Url(_publishedUrlProvider); } links.Add( new Link { - Name = dto.Name, + Name = name, Target = dto.Target, Type = type, Udi = dto.Udi, + Content = content, Url = url + dto.QueryString, }); } diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.controller.js index 3baba2b77a..673e1a5d3d 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.controller.js @@ -1,290 +1,281 @@ //used for the media picker dialog angular.module("umbraco").controller("Umbraco.Editors.LinkPickerController", - function ($scope, eventsService, entityResource, mediaResource, udiParser, userService, localizationService, editorService) { + function ($scope, eventsService, entityResource, mediaResource, udiParser, userService, localizationService, editorService) { - var vm = this; - var dialogOptions = $scope.model; + var vm = this; + var dialogOptions = $scope.model; - vm.submit = submit; - vm.close = close; - vm.toggleOpenInNewWindow = toggleOpenInNewWindow; + vm.submit = submit; + vm.close = close; + vm.toggleOpenInNewWindow = toggleOpenInNewWindow; - vm.labels = {}; - localizationService.localizeMany(["defaultdialogs_openInNewWindow"]).then(function (data) { - vm.labels.openInNewWindow = data[0]; + vm.labels = {}; + localizationService.localizeMany(["defaultdialogs_openInNewWindow"]).then(function (data) { + vm.labels.openInNewWindow = data[0]; + }); + + if (!$scope.model.title) { + localizationService.localize("defaultdialogs_selectLink") + .then(function (value) { + $scope.model.title = value; }); + } - if (!$scope.model.title) { - localizationService.localize("defaultdialogs_selectLink") - .then(function (value) { - $scope.model.title = value; - }); - } - - $scope.customTreeParams = dialogOptions.dataTypeKey ? "dataTypeKey=" + dialogOptions.dataTypeKey : ""; - $scope.dialogTreeApi = {}; - $scope.model.target = {}; - $scope.searchInfo = { - searchFromId: null, - searchFromName: null, - showSearch: false, - dataTypeKey: dialogOptions.dataTypeKey, - results: [], - selectedSearchResults: [] - }; + localizationService.localize("placeholders_entername").then(function (value) { + $scope.placeholders_entername = value; + }); - $scope.userLinkNameInput = ''; + $scope.customTreeParams = dialogOptions.dataTypeKey ? "dataTypeKey=" + dialogOptions.dataTypeKey : ""; + $scope.dialogTreeApi = {}; + $scope.model.target = {}; + $scope.searchInfo = { + searchFromId: null, + searchFromName: null, + showSearch: false, + dataTypeKey: dialogOptions.dataTypeKey, + results: [], + selectedSearchResults: [] + }; - $scope.showTarget = $scope.model.hideTarget !== true; - $scope.showAnchor = $scope.model.hideAnchor !== true; + $scope.showTarget = $scope.model.hideTarget !== true; + $scope.showAnchor = $scope.model.hideAnchor !== true; - // this ensures that we only sync the tree once and only when it's ready - var oneTimeTreeSync = { - executed: false, - treeReady: false, - sync: function () { - // don't run this if: - // - it was already run once - // - the tree isn't ready yet - // - the model path hasn't been loaded yet - if (this.executed || !this.treeReady || !($scope.model.target && $scope.model.target.path)) { - return; - } - - this.executed = true; - // sync the tree to the model path - $scope.dialogTreeApi.syncTree({ - path: $scope.model.target.path, - tree: "content" - }); - } - }; - - if (dialogOptions.currentTarget) { - // clone the current target so we don't accidentally update the caller's model while manipulating $scope.model.target - $scope.model.target = Utilities.copy(dialogOptions.currentTarget); - // if we have a node ID, we fetch the current node to build the form data - if ($scope.model.target.id || $scope.model.target.udi) { - - // will be either a udi or an int - var id = $scope.model.target.udi ? $scope.model.target.udi : $scope.model.target.id; - - if ($scope.model.target.udi) { - // extract the entity type from the udi and set target.isMedia accordingly - var udi = udiParser.parse(id); - - if (udi && udi.entityType === "media") { - $scope.model.target.isMedia = true; - } else { - delete $scope.model.target.isMedia; - } - } - - if ($scope.model.target.isMedia) { - mediaResource.getById(id).then(function (resp) { - $scope.model.target.url = resp.mediaLink; - }); - } else { - // get the content path - entityResource.getPath(id, "Document").then(function (path) { - $scope.model.target.path = path; - oneTimeTreeSync.sync(); - }); - - entityResource.getUrlAndAnchors(id).then(function (resp) { - $scope.anchorValues = resp.anchorValues; - $scope.model.target.url = resp.url; - }); - - } - } else if ($scope.model.target.url && $scope.model.target.url.length) { - // a url but no id/udi indicates an external link - trim the url to remove the anchor/qs - // only do the substring if there's a # or a ? - var indexOfAnchor = $scope.model.target.url.search(/(#|\?)/); - if (indexOfAnchor > -1) { - // populate the anchor - $scope.model.target.anchor = $scope.model.target.url.substring(indexOfAnchor); - // then rewrite the model and populate the link - $scope.model.target.url = $scope.model.target.url.substring(0, indexOfAnchor); - } - } - - // need to translate the link target ("_blank" or "") into a boolean value for umb-checkbox - vm.openInNewWindow = $scope.model.target.target === "_blank"; - } - else if (dialogOptions.anchors) { - $scope.anchorValues = dialogOptions.anchors; + // this ensures that we only sync the tree once and only when it's ready + var oneTimeTreeSync = { + executed: false, + treeReady: false, + sync: function () { + // don't run this if: + // - it was already run once + // - the tree isn't ready yet + // - the model path hasn't been loaded yet + if (this.executed || !this.treeReady || !($scope.model.target && $scope.model.target.path)) { + return; } - function treeLoadedHandler(args) { - oneTimeTreeSync.treeReady = true; - oneTimeTreeSync.sync(); - } - - function nodeSelectHandler(args) { - if (args && args.event) { - args.event.preventDefault(); - args.event.stopPropagation(); - } - - eventsService.emit("dialogs.linkPicker.select", args); - - if ($scope.currentNode) { - if ($scope.currentNode.id == args.node.id && $scope.currentNode.selected) { - $scope.model.target = {}; - $scope.currentNode.selected = false; - - return; - } - - //un-select if there's a current one selected - $scope.currentNode.selected = false; - } - - $scope.currentNode = args.node; - $scope.currentNode.selected = true; - $scope.model.target.id = args.node.id; - $scope.model.target.udi = args.node.udi; - if ($scope.oKToUpdateLinkTargetName()) { - $scope.model.target.name = args.node.name; - } - - if (args.node.id < 0) { - $scope.model.target.url = "/"; - } - else { - entityResource.getUrlAndAnchors(args.node.id).then(function (resp) { - $scope.anchorValues = resp.anchorValues; - $scope.model.target.url = resp.url; - }); - } - - if (!Utilities.isUndefined($scope.model.target.isMedia)) { - delete $scope.model.target.isMedia; - } - } - - - function nodeExpandedHandler(args) { - // open mini list view for list views - if (args.node.metaData.isContainer) { - openMiniListView(args.node); - } - } - $scope.trackUserInput = function (userInput) { - $scope.userLinkNameInput = userInput; - + this.executed = true; + // sync the tree to the model path + $scope.dialogTreeApi.syncTree({ + path: $scope.model.target.path, + tree: "content" + }); } - $scope.oKToUpdateLinkTargetName = function () { - if (!$scope.userLinkNameInput || !$scope.model.target.name) { - return true; + }; + + if (dialogOptions.currentTarget) { + // clone the current target so we don't accidentally update the caller's model while manipulating $scope.model.target + $scope.model.target = Utilities.copy(dialogOptions.currentTarget); + // if we have a node ID, we fetch the current node to build the form data + if ($scope.model.target.id || $scope.model.target.udi) { + + // will be either a udi or an int + var id = $scope.model.target.udi ? $scope.model.target.udi : $scope.model.target.id; + + if ($scope.model.target.udi) { + // extract the entity type from the udi and set target.isMedia accordingly + var udi = udiParser.parse(id); + + if (udi && udi.entityType === "media") { + $scope.model.target.isMedia = true; + } else { + delete $scope.model.target.isMedia; + } + } + + if ($scope.model.target.isMedia) { + mediaResource.getById(id).then(function (resp) { + $scope.model.target.url = resp.mediaLink; + }); + } else { + // get the content path + entityResource.getPath(id, "Document").then(function (path) { + $scope.model.target.path = path; + oneTimeTreeSync.sync(); + }); + + entityResource.getUrlAndAnchors(id).then(function (resp) { + $scope.anchorValues = resp.anchorValues; + $scope.model.target.url = resp.url; + }); + + } + } else if ($scope.model.target.url && $scope.model.target.url.length) { + // a url but no id/udi indicates an external link - trim the url to remove the anchor/qs + // only do the substring if there's a # or a ? + var indexOfAnchor = $scope.model.target.url.search(/(#|\?)/); + if (indexOfAnchor > -1) { + // populate the anchor + $scope.model.target.anchor = $scope.model.target.url.substring(indexOfAnchor); + // then rewrite the model and populate the link + $scope.model.target.url = $scope.model.target.url.substring(0, indexOfAnchor); + } + } + + // need to translate the link target ("_blank" or "") into a boolean value for umb-checkbox + vm.openInNewWindow = $scope.model.target.target === "_blank"; + } + else if (dialogOptions.anchors) { + $scope.anchorValues = dialogOptions.anchors; + } + + function treeLoadedHandler(args) { + oneTimeTreeSync.treeReady = true; + oneTimeTreeSync.sync(); + } + + function nodeSelectHandler(args) { + if (args && args.event) { + args.event.preventDefault(); + args.event.stopPropagation(); + } + + eventsService.emit("dialogs.linkPicker.select", args); + + if ($scope.currentNode) { + if ($scope.currentNode.id == args.node.id && $scope.currentNode.selected) { + $scope.model.target = {}; + $scope.currentNode.selected = false; + + return; + } + + //un-select if there's a current one selected + $scope.currentNode.selected = false; + } + + $scope.currentNode = args.node; + $scope.currentNode.selected = true; + $scope.model.target.id = args.node.id; + $scope.model.target.udi = args.node.udi; + $scope.model.target[$scope.model.useNodeName ? 'nodeName' : 'name'] = args.node.name; + + if (args.node.id < 0) { + $scope.model.target.url = "/"; + } + else { + entityResource.getUrlAndAnchors(args.node.id).then(function (resp) { + $scope.anchorValues = resp.anchorValues; + $scope.model.target.url = resp.url; + }); + } + + if (!Utilities.isUndefined($scope.model.target.isMedia)) { + delete $scope.model.target.isMedia; + } + } + + + function nodeExpandedHandler(args) { + // open mini list view for list views + if (args.node.metaData.isContainer) { + openMiniListView(args.node); + } + } + + $scope.switchToMediaPicker = function () { + userService.getCurrentUser().then(function (userData) { + + var startNodeId, startNodeIsVirtual; + if (dialogOptions.ignoreUserStartNodes === true) { + startNodeId = -1; + startNodeIsVirtual = true; } else { - return false; + startNodeId = userData.startMediaIds.length !== 1 ? -1 : userData.startMediaIds[0]; + startNodeIsVirtual = userData.startMediaIds.length !== 1; } + + var mediaPicker = { + startNodeId: startNodeId, + startNodeIsVirtual: startNodeIsVirtual, + dataTypeKey: dialogOptions.dataTypeKey, + submit: function (model) { + var media = model.selection[0]; + + $scope.model.target.id = media.id; + $scope.model.target.udi = media.udi; + $scope.model.target.isMedia = true; + $scope.model.target[$scope.model.useNodeName ? 'nodeName' : 'name'] = media.name; + $scope.model.target.url = media.image; + + editorService.close(); + + if ($scope.currentNode) { + $scope.currentNode.selected = false; + } + + // make sure the content tree has nothing highlighted + $scope.dialogTreeApi.syncTree({ + path: "-1", + tree: "content" + }); + }, + close: function () { + editorService.close(); + } + }; + editorService.mediaPicker(mediaPicker); + }); + }; + + $scope.hideSearch = function () { + $scope.searchInfo.showSearch = false; + $scope.searchInfo.searchFromId = null; + $scope.searchInfo.searchFromName = null; + $scope.searchInfo.results = []; + } + + // method to select a search result + $scope.selectResult = function (evt, result) { + result.selected = result.selected === true ? false : true; + nodeSelectHandler({ + event: evt, + node: result + }); + }; + + //callback when there are search results + $scope.onSearchResults = function (results) { + $scope.searchInfo.results = results; + $scope.searchInfo.showSearch = true; + }; + + $scope.onTreeInit = function () { + $scope.dialogTreeApi.callbacks.treeLoaded(treeLoadedHandler); + $scope.dialogTreeApi.callbacks.treeNodeSelect(nodeSelectHandler); + $scope.dialogTreeApi.callbacks.treeNodeExpanded(nodeExpandedHandler); + } + + // Mini list view + $scope.selectListViewNode = function (node) { + node.selected = node.selected === true ? false : true; + nodeSelectHandler({ + node: node + }); + }; + + $scope.closeMiniListView = function () { + $scope.miniListView = undefined; + }; + + function openMiniListView(node) { + $scope.miniListView = node; + } + + function toggleOpenInNewWindow(model, value) { + $scope.model.target.target = model ? "_blank" : ""; + } + + function close() { + if ($scope.model && $scope.model.close) { + $scope.model.close(); } - $scope.switchToMediaPicker = function () { - userService.getCurrentUser().then(function (userData) { + } - var startNodeId, startNodeIsVirtual; - if (dialogOptions.ignoreUserStartNodes === true) { - startNodeId = -1; - startNodeIsVirtual = true; - } - else { - startNodeId = userData.startMediaIds.length !== 1 ? -1 : userData.startMediaIds[0]; - startNodeIsVirtual = userData.startMediaIds.length !== 1; - } + function submit() { + if ($scope.model && $scope.model.submit) { + $scope.model.submit($scope.model); + } + } - var mediaPicker = { - startNodeId: startNodeId, - startNodeIsVirtual: startNodeIsVirtual, - dataTypeKey: dialogOptions.dataTypeKey, - submit: function (model) { - var media = model.selection[0]; - - $scope.model.target.id = media.id; - $scope.model.target.udi = media.udi; - $scope.model.target.isMedia = true; - if ($scope.oKToUpdateLinkTargetName()) { - $scope.model.target.name = media.name; - } - $scope.model.target.url = media.image; - - editorService.close(); - - // make sure the content tree has nothing highlighted - $scope.dialogTreeApi.syncTree({ - path: "-1", - tree: "content" - }); - }, - close: function () { - editorService.close(); - } - }; - editorService.mediaPicker(mediaPicker); - }); - }; - - $scope.hideSearch = function () { - $scope.searchInfo.showSearch = false; - $scope.searchInfo.searchFromId = null; - $scope.searchInfo.searchFromName = null; - $scope.searchInfo.results = []; - } - - // method to select a search result - $scope.selectResult = function (evt, result) { - result.selected = result.selected === true ? false : true; - nodeSelectHandler({ - event: evt, - node: result - }); - }; - - //callback when there are search results - $scope.onSearchResults = function (results) { - $scope.searchInfo.results = results; - $scope.searchInfo.showSearch = true; - }; - - $scope.onTreeInit = function () { - $scope.dialogTreeApi.callbacks.treeLoaded(treeLoadedHandler); - $scope.dialogTreeApi.callbacks.treeNodeSelect(nodeSelectHandler); - $scope.dialogTreeApi.callbacks.treeNodeExpanded(nodeExpandedHandler); - } - - // Mini list view - $scope.selectListViewNode = function (node) { - node.selected = node.selected === true ? false : true; - nodeSelectHandler({ - node: node - }); - }; - - $scope.closeMiniListView = function () { - $scope.miniListView = undefined; - }; - - function openMiniListView(node) { - $scope.miniListView = node; - } - - function toggleOpenInNewWindow(model, value) { - $scope.model.target.target = model ? "_blank" : ""; - } - - function close() { - if ($scope.model && $scope.model.close) { - $scope.model.close(); - } - } - - function submit() { - if ($scope.model && $scope.model.submit) { - $scope.model.submit($scope.model); - } - } - - }); + }); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.html b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.html index 6a020f69ff..00b54e3792 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/linkpicker/linkpicker.html @@ -13,10 +13,8 @@ -
- - + { if (model.target.url || model.target.anchor) { // if an anchor exists, check that it is appropriately prefixed @@ -130,13 +132,15 @@ function multiUrlPickerController($scope, localizationService, entityResource, i } if (link) { link.udi = model.target.udi; - link.name = model.target.name || model.target.url || model.target.anchor; + link.name = model.target.name; + link.nodeName = model.target.nodeName; link.queryString = model.target.anchor; link.target = model.target.target; link.url = model.target.url; } else { link = { - name: model.target.name || model.target.url || model.target.anchor, + name: model.target.name, + nodeName: model.target.nodeName, queryString: model.target.anchor, target: model.target.target, udi: model.target.udi, diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/multiurlpicker/multiurlpicker.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/multiurlpicker/multiurlpicker.html index cb0fcc7d9c..c9f0d42b33 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/multiurlpicker/multiurlpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/multiurlpicker/multiurlpicker.html @@ -6,7 +6,7 @@
Date: Tue, 19 Mar 2024 13:57:06 +0100 Subject: [PATCH 17/24] Bump version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index beca541bdc..20b8b28182 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "12.3.8", + "version": "12.3.9", "assemblyVersion": { "precision": "build" }, From ecdbd9653b9faf59b19765222f0fbe57b33f5048 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 19 Mar 2024 13:57:46 +0100 Subject: [PATCH 18/24] Update imagesharp 2 --- .../Umbraco.Cms.Imaging.ImageSharp2.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj b/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj index 14c203bad6..c03dcfdb37 100644 --- a/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj +++ b/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj @@ -5,7 +5,7 @@ - + From d9e890e95437faaf3353de78c33c2d6c7ea312fc Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 19 Mar 2024 13:28:43 +0100 Subject: [PATCH 19/24] Fixed duplicate reference --- .../Umbraco.Cms.Imaging.ImageSharp2.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj b/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj index 1dcba40ab6..4e65e1d573 100644 --- a/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj +++ b/src/Umbraco.Cms.Imaging.ImageSharp2/Umbraco.Cms.Imaging.ImageSharp2.csproj @@ -5,7 +5,7 @@ - + From c48d6f5a38fa9969e231901b14084289d45b82ef Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 19 Mar 2024 13:59:38 +0100 Subject: [PATCH 20/24] Update imagesharp 3 --- Directory.Packages.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index c80183b9c1..02dbaba948 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -73,7 +73,7 @@ - + @@ -90,4 +90,4 @@ - \ No newline at end of file + From 79d241a910670ceb61250d84136906a05c853a8e Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 19 Mar 2024 13:59:49 +0100 Subject: [PATCH 21/24] Bump version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index a2893b768e..46edea832f 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "13.2.1", + "version": "13.2.2", "assemblyVersion": { "precision": "build" }, From 95aba6d6961f63e9f8bffeed112d3a16b7fe874c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 16 Mar 2024 22:38:17 +0000 Subject: [PATCH 22/24] Bump follow-redirects in /tests/Umbraco.Tests.AcceptanceTest Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.4 to 1.15.6. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](https://github.com/follow-redirects/follow-redirects/compare/v1.15.4...v1.15.6) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] --- tests/Umbraco.Tests.AcceptanceTest/package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Umbraco.Tests.AcceptanceTest/package-lock.json b/tests/Umbraco.Tests.AcceptanceTest/package-lock.json index 38d70d4bef..82892582ce 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/package-lock.json +++ b/tests/Umbraco.Tests.AcceptanceTest/package-lock.json @@ -366,9 +366,9 @@ } }, "node_modules/follow-redirects": { - "version": "1.15.4", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.4.tgz", - "integrity": "sha512-Cr4D/5wlrb0z9dgERpUL3LrmPKVDsETIJhaCMeDfuFYcqa5bldGV6wBsAN6X/vxlXQtFBMrXdXxdL8CbDTGniw==", + "version": "1.15.6", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.6.tgz", + "integrity": "sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA==", "dev": true, "funding": [ { From d3b96a163feb031f8b33d63ab68098d65aa6686d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 16 Mar 2024 22:39:02 +0000 Subject: [PATCH 23/24] Bump follow-redirects in /src/Umbraco.Web.UI.Client Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.4 to 1.15.6. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](https://github.com/follow-redirects/follow-redirects/compare/v1.15.4...v1.15.6) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] --- src/Umbraco.Web.UI.Client/package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index d40879da24..4d87f71a60 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -7928,9 +7928,9 @@ } }, "node_modules/follow-redirects": { - "version": "1.15.4", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.4.tgz", - "integrity": "sha512-Cr4D/5wlrb0z9dgERpUL3LrmPKVDsETIJhaCMeDfuFYcqa5bldGV6wBsAN6X/vxlXQtFBMrXdXxdL8CbDTGniw==", + "version": "1.15.6", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.6.tgz", + "integrity": "sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA==", "dev": true, "funding": [ { From b1c347385630a45d4e2b5eb30e71301bf0adc620 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Fri, 22 Mar 2024 14:12:33 +0100 Subject: [PATCH 24/24] Introduce path provider and resolver for the Content Delivery API (#15922) --- .../Content/ByRouteContentApiController.cs | 59 ++++++++++++++----- .../DeliveryApi/ApiContentPathProvider.cs | 16 +++++ .../DeliveryApi/ApiContentPathResolver.cs | 26 ++++++++ .../DeliveryApi/ApiContentRouteBuilder.cs | 37 ++++++++++-- .../DeliveryApi/IApiContentPathProvider.cs | 8 +++ .../DeliveryApi/IApiContentPathResolver.cs | 8 +++ .../UmbracoBuilder.CoreServices.cs | 2 + .../DeliveryApi/ContentBuilderTests.cs | 10 ++-- .../ContentPickerValueConverterTests.cs | 2 +- .../DeliveryApi/ContentRouteBuilderTests.cs | 39 +++++++++++- .../DeliveryApi/DeliveryApiTests.cs | 4 +- .../MultiNodeTreePickerValueConverterTests.cs | 2 +- .../MultiUrlPickerValueConverterTests.cs | 2 +- .../OutputExpansionStrategyTestBase.cs | 2 +- .../PropertyValueConverterTests.cs | 4 ++ 15 files changed, 188 insertions(+), 33 deletions(-) create mode 100644 src/Umbraco.Core/DeliveryApi/ApiContentPathProvider.cs create mode 100644 src/Umbraco.Core/DeliveryApi/ApiContentPathResolver.cs create mode 100644 src/Umbraco.Core/DeliveryApi/IApiContentPathProvider.cs create mode 100644 src/Umbraco.Core/DeliveryApi/IApiContentPathResolver.cs diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByRouteContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByRouteContentApiController.cs index b88147999a..4fed35a281 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByRouteContentApiController.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByRouteContentApiController.cs @@ -8,7 +8,6 @@ using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Services; -using Umbraco.Extensions; namespace Umbraco.Cms.Api.Delivery.Controllers.Content; @@ -16,10 +15,11 @@ namespace Umbraco.Cms.Api.Delivery.Controllers.Content; [ApiVersion("2.0")] public class ByRouteContentApiController : ContentApiItemControllerBase { - private readonly IRequestRoutingService _requestRoutingService; + private readonly IApiContentPathResolver _apiContentPathResolver; private readonly IRequestRedirectService _requestRedirectService; private readonly IRequestPreviewService _requestPreviewService; private readonly IRequestMemberAccessService _requestMemberAccessService; + private const string PreviewContentRequestPathPrefix = $"/{Constants.DeliveryApi.Routing.PreviewContentPathPrefix}"; [Obsolete($"Please use the constructor that does not accept {nameof(IPublicAccessService)}. Will be removed in V14.")] public ByRouteContentApiController( @@ -58,7 +58,7 @@ public class ByRouteContentApiController : ContentApiItemControllerBase { } - [ActivatorUtilitiesConstructor] + [Obsolete($"Please use the constructor that accepts {nameof(IApiContentPathResolver)}. Will be removed in V15.")] public ByRouteContentApiController( IApiPublishedContentCache apiPublishedContentCache, IApiContentResponseBuilder apiContentResponseBuilder, @@ -66,12 +66,50 @@ public class ByRouteContentApiController : ContentApiItemControllerBase IRequestRedirectService requestRedirectService, IRequestPreviewService requestPreviewService, IRequestMemberAccessService requestMemberAccessService) + : this( + apiPublishedContentCache, + apiContentResponseBuilder, + requestRedirectService, + requestPreviewService, + requestMemberAccessService, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete($"Please use the non-obsolete constructor. Will be removed in V15.")] + public ByRouteContentApiController( + IApiPublishedContentCache apiPublishedContentCache, + IApiContentResponseBuilder apiContentResponseBuilder, + IPublicAccessService publicAccessService, + IRequestRoutingService requestRoutingService, + IRequestRedirectService requestRedirectService, + IRequestPreviewService requestPreviewService, + IRequestMemberAccessService requestMemberAccessService, + IApiContentPathResolver apiContentPathResolver) + : this( + apiPublishedContentCache, + apiContentResponseBuilder, + requestRedirectService, + requestPreviewService, + requestMemberAccessService, + apiContentPathResolver) + { + } + + [ActivatorUtilitiesConstructor] + public ByRouteContentApiController( + IApiPublishedContentCache apiPublishedContentCache, + IApiContentResponseBuilder apiContentResponseBuilder, + IRequestRedirectService requestRedirectService, + IRequestPreviewService requestPreviewService, + IRequestMemberAccessService requestMemberAccessService, + IApiContentPathResolver apiContentPathResolver) : base(apiPublishedContentCache, apiContentResponseBuilder) { - _requestRoutingService = requestRoutingService; _requestRedirectService = requestRedirectService; _requestPreviewService = requestPreviewService; _requestMemberAccessService = requestMemberAccessService; + _apiContentPathResolver = apiContentPathResolver; } [HttpGet("item/{*path}")] @@ -105,8 +143,6 @@ public class ByRouteContentApiController : ContentApiItemControllerBase private async Task HandleRequest(string path) { path = DecodePath(path); - - path = path.TrimStart("/"); path = path.Length == 0 ? "/" : path; IPublishedContent? contentItem = GetContent(path); @@ -128,17 +164,12 @@ public class ByRouteContentApiController : ContentApiItemControllerBase } private IPublishedContent? GetContent(string path) - => path.StartsWith(Constants.DeliveryApi.Routing.PreviewContentPathPrefix) + => path.StartsWith(PreviewContentRequestPathPrefix) ? GetPreviewContent(path) : GetPublishedContent(path); private IPublishedContent? GetPublishedContent(string path) - { - var contentRoute = _requestRoutingService.GetContentRoute(path); - - IPublishedContent? contentItem = ApiPublishedContentCache.GetByRoute(contentRoute); - return contentItem; - } + => _apiContentPathResolver.ResolveContentPath(path); private IPublishedContent? GetPreviewContent(string path) { @@ -147,7 +178,7 @@ public class ByRouteContentApiController : ContentApiItemControllerBase return null; } - if (Guid.TryParse(path.AsSpan(Constants.DeliveryApi.Routing.PreviewContentPathPrefix.Length).TrimEnd("/"), out Guid contentId) is false) + if (Guid.TryParse(path.AsSpan(PreviewContentRequestPathPrefix.Length).TrimEnd("/"), out Guid contentId) is false) { return null; } diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentPathProvider.cs b/src/Umbraco.Core/DeliveryApi/ApiContentPathProvider.cs new file mode 100644 index 0000000000..853f0030ae --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/ApiContentPathProvider.cs @@ -0,0 +1,16 @@ +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.Routing; + +namespace Umbraco.Cms.Core.DeliveryApi; + +// NOTE: left unsealed on purpose so it is extendable. +public class ApiContentPathProvider : IApiContentPathProvider +{ + private readonly IPublishedUrlProvider _publishedUrlProvider; + + public ApiContentPathProvider(IPublishedUrlProvider publishedUrlProvider) + => _publishedUrlProvider = publishedUrlProvider; + + public virtual string? GetContentPath(IPublishedContent content, string? culture) + => _publishedUrlProvider.GetUrl(content, UrlMode.Relative, culture); +} diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentPathResolver.cs b/src/Umbraco.Core/DeliveryApi/ApiContentPathResolver.cs new file mode 100644 index 0000000000..4ca6be3932 --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/ApiContentPathResolver.cs @@ -0,0 +1,26 @@ +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.DeliveryApi; + +// NOTE: left unsealed on purpose so it is extendable. +public class ApiContentPathResolver : IApiContentPathResolver +{ + private readonly IRequestRoutingService _requestRoutingService; + private readonly IApiPublishedContentCache _apiPublishedContentCache; + + public ApiContentPathResolver(IRequestRoutingService requestRoutingService, IApiPublishedContentCache apiPublishedContentCache) + { + _requestRoutingService = requestRoutingService; + _apiPublishedContentCache = apiPublishedContentCache; + } + + public virtual IPublishedContent? ResolveContentPath(string path) + { + path = path.EnsureStartsWith("/"); + + var contentRoute = _requestRoutingService.GetContentRoute(path); + IPublishedContent? contentItem = _apiPublishedContentCache.GetByRoute(contentRoute); + return contentItem; + } +} diff --git a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs index 0ae310d585..148b466a8b 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs @@ -1,5 +1,7 @@ -using Microsoft.Extensions.Options; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; @@ -10,13 +12,14 @@ namespace Umbraco.Cms.Core.DeliveryApi; public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder { - private readonly IPublishedUrlProvider _publishedUrlProvider; + private readonly IApiContentPathProvider _apiContentPathProvider; private readonly GlobalSettings _globalSettings; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor; private readonly IRequestPreviewService _requestPreviewService; private RequestHandlerSettings _requestSettings; + [Obsolete($"Use the constructor that does not accept {nameof(IPublishedUrlProvider)}. Will be removed in V15.")] public ApiContentRouteBuilder( IPublishedUrlProvider publishedUrlProvider, IOptions globalSettings, @@ -24,8 +27,32 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder IPublishedSnapshotAccessor publishedSnapshotAccessor, IRequestPreviewService requestPreviewService, IOptionsMonitor requestSettings) + : this(StaticServiceProvider.Instance.GetRequiredService(), globalSettings, variationContextAccessor, publishedSnapshotAccessor, requestPreviewService, requestSettings) { - _publishedUrlProvider = publishedUrlProvider; + } + + [Obsolete($"Use the constructor that does not accept {nameof(IPublishedUrlProvider)}. Will be removed in V15.")] + public ApiContentRouteBuilder( + IPublishedUrlProvider publishedUrlProvider, + IApiContentPathProvider apiContentPathProvider, + IOptions globalSettings, + IVariationContextAccessor variationContextAccessor, + IPublishedSnapshotAccessor publishedSnapshotAccessor, + IRequestPreviewService requestPreviewService, + IOptionsMonitor requestSettings) + : this(apiContentPathProvider, globalSettings, variationContextAccessor, publishedSnapshotAccessor, requestPreviewService, requestSettings) + { + } + + public ApiContentRouteBuilder( + IApiContentPathProvider apiContentPathProvider, + IOptions globalSettings, + IVariationContextAccessor variationContextAccessor, + IPublishedSnapshotAccessor publishedSnapshotAccessor, + IRequestPreviewService requestPreviewService, + IOptionsMonitor requestSettings) + { + _apiContentPathProvider = apiContentPathProvider; _variationContextAccessor = variationContextAccessor; _publishedSnapshotAccessor = publishedSnapshotAccessor; _requestPreviewService = requestPreviewService; @@ -72,7 +99,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder } // grab the content path from the URL provider - var contentPath = _publishedUrlProvider.GetUrl(content, UrlMode.Relative, culture); + var contentPath = _apiContentPathProvider.GetContentPath(content, culture); // 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, @@ -96,7 +123,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder private string ContentPreviewPath(IPublishedContent content) => $"{Constants.DeliveryApi.Routing.PreviewContentPathPrefix}{content.Key:D}{(_requestSettings.AddTrailingSlash ? "/" : string.Empty)}"; - private static bool IsInvalidContentPath(string path) => path.IsNullOrWhiteSpace() || "#".Equals(path); + private static bool IsInvalidContentPath(string? path) => path.IsNullOrWhiteSpace() || "#".Equals(path); private IPublishedContent GetRoot(IPublishedContent content, bool isPreview) { diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentPathProvider.cs b/src/Umbraco.Core/DeliveryApi/IApiContentPathProvider.cs new file mode 100644 index 0000000000..11a676873b --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/IApiContentPathProvider.cs @@ -0,0 +1,8 @@ +using Umbraco.Cms.Core.Models.PublishedContent; + +namespace Umbraco.Cms.Core.DeliveryApi; + +public interface IApiContentPathProvider +{ + string? GetContentPath(IPublishedContent content, string? culture); +} diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentPathResolver.cs b/src/Umbraco.Core/DeliveryApi/IApiContentPathResolver.cs new file mode 100644 index 0000000000..391f18998e --- /dev/null +++ b/src/Umbraco.Core/DeliveryApi/IApiContentPathResolver.cs @@ -0,0 +1,8 @@ +using Umbraco.Cms.Core.Models.PublishedContent; + +namespace Umbraco.Cms.Core.DeliveryApi; + +public interface IApiContentPathResolver +{ + IPublishedContent? ResolveContentPath(string path); +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 6f732e1a1e..10fbcc1207 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -453,6 +453,8 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs index 6e0dfd7e6f..cbe8ee09b2 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs @@ -31,12 +31,12 @@ public class ContentBuilderTests : DeliveryApiTests content.SetupGet(c => c.CreateDate).Returns(new DateTime(2023, 06, 01)); content.SetupGet(c => c.UpdateDate).Returns(new DateTime(2023, 07, 12)); - var publishedUrlProvider = new Mock(); - publishedUrlProvider - .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((IPublishedContent content, UrlMode mode, string? culture, Uri? current) => $"url:{content.UrlSegment}"); + var apiContentRouteProvider = new Mock(); + apiContentRouteProvider + .Setup(p => p.GetContentPath(It.IsAny(), It.IsAny())) + .Returns((IPublishedContent c, string? culture) => $"url:{c.UrlSegment}"); - var routeBuilder = CreateContentRouteBuilder(publishedUrlProvider.Object, CreateGlobalSettings()); + var routeBuilder = CreateContentRouteBuilder(apiContentRouteProvider.Object, CreateGlobalSettings()); var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilder, CreateOutputExpansionStrategyAccessor()); var result = builder.Build(content.Object); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs index 61e55b76db..9e34979c6e 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentPickerValueConverterTests.cs @@ -18,7 +18,7 @@ public class ContentPickerValueConverterTests : PropertyValueConverterTests PublishedSnapshotAccessor, new ApiContentBuilder( nameProvider ?? new ApiContentNameProvider(), - CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()), + CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings()), CreateOutputExpansionStrategyAccessor())); [Test] diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs index d60097bf69..8fafb162a2 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs @@ -254,6 +254,34 @@ public class ContentRouteBuilderTests : DeliveryApiTests } } + [Test] + public void CanUseCustomContentPathProvider() + { + var rootKey = Guid.NewGuid(); + var root = SetupInvariantPublishedContent("The Root", rootKey, published: false); + + var childKey = Guid.NewGuid(); + var child = SetupInvariantPublishedContent("The Child", childKey, root); + + var apiContentPathProvider = new Mock(); + apiContentPathProvider + .Setup(p => p.GetContentPath(It.IsAny(), It.IsAny())) + .Returns((IPublishedContent content, string? culture) => $"my-custom-path-for-{content.UrlSegment}"); + + var builder = CreateApiContentRouteBuilder(true, apiContentPathProvider: apiContentPathProvider.Object); + var result = builder.Build(root); + Assert.NotNull(result); + Assert.AreEqual("/my-custom-path-for-the-root", result.Path); + Assert.AreEqual(rootKey, result.StartItem.Id); + Assert.AreEqual("the-root", result.StartItem.Path); + + result = builder.Build(child); + Assert.NotNull(result); + Assert.AreEqual("/my-custom-path-for-the-child", result.Path); + Assert.AreEqual(rootKey, result.StartItem.Id); + Assert.AreEqual("the-root", result.StartItem.Path); + } + private IPublishedContent SetupInvariantPublishedContent(string name, Guid key, IPublishedContent? parent = null, bool published = true) { var publishedContentType = CreatePublishedContentType(); @@ -310,7 +338,10 @@ public class ContentRouteBuilderTests : DeliveryApiTests return publishedUrlProvider.Object; } - private ApiContentRouteBuilder CreateApiContentRouteBuilder(bool hideTopLevelNodeFromPath, bool addTrailingSlash = false, bool isPreview = false, IPublishedSnapshotAccessor? publishedSnapshotAccessor = null) + private IApiContentPathProvider SetupApiContentPathProvider(bool hideTopLevelNodeFromPath) + => new ApiContentPathProvider(SetupPublishedUrlProvider(hideTopLevelNodeFromPath)); + + private ApiContentRouteBuilder CreateApiContentRouteBuilder(bool hideTopLevelNodeFromPath, bool addTrailingSlash = false, bool isPreview = false, IPublishedSnapshotAccessor? publishedSnapshotAccessor = null, IApiContentPathProvider? apiContentPathProvider = null) { var requestHandlerSettings = new RequestHandlerSettings { AddTrailingSlash = addTrailingSlash }; var requestHandlerSettingsMonitorMock = new Mock>(); @@ -320,9 +351,10 @@ public class ContentRouteBuilderTests : DeliveryApiTests requestPreviewServiceMock.Setup(m => m.IsPreview()).Returns(isPreview); publishedSnapshotAccessor ??= CreatePublishedSnapshotAccessorForRoute("#"); + apiContentPathProvider ??= SetupApiContentPathProvider(hideTopLevelNodeFromPath); return CreateContentRouteBuilder( - SetupPublishedUrlProvider(hideTopLevelNodeFromPath), + apiContentPathProvider, CreateGlobalSettings(hideTopLevelNodeFromPath), requestHandlerSettingsMonitor: requestHandlerSettingsMonitorMock.Object, requestPreviewService: requestPreviewServiceMock.Object, @@ -335,12 +367,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests publishedUrlProviderMock .Setup(p => p.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(publishedUrl); + var contentPathProvider = new ApiContentPathProvider(publishedUrlProviderMock.Object); var publishedSnapshotAccessor = CreatePublishedSnapshotAccessorForRoute(routeById); var content = SetupVariantPublishedContent("The Content", Guid.NewGuid()); var builder = CreateContentRouteBuilder( - publishedUrlProviderMock.Object, + contentPathProvider, CreateGlobalSettings(), publishedSnapshotAccessor: publishedSnapshotAccessor); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs index 80733e3cdd..47a7c032c9 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/DeliveryApiTests.cs @@ -114,7 +114,7 @@ public class DeliveryApiTests => $"{name.ToLowerInvariant().Replace(" ", "-")}{(culture.IsNullOrWhiteSpace() ? string.Empty : $"-{culture}")}"; protected ApiContentRouteBuilder CreateContentRouteBuilder( - IPublishedUrlProvider publishedUrlProvider, + IApiContentPathProvider contentPathProvider, IOptions globalSettings, IVariationContextAccessor? variationContextAccessor = null, IPublishedSnapshotAccessor? publishedSnapshotAccessor = null, @@ -129,7 +129,7 @@ public class DeliveryApiTests } return new ApiContentRouteBuilder( - publishedUrlProvider, + contentPathProvider, globalSettings, variationContextAccessor ?? Mock.Of(), publishedSnapshotAccessor ?? Mock.Of(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs index b38aa33c8a..1948756e44 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs @@ -21,7 +21,7 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest var contentNameProvider = new ApiContentNameProvider(); var apiUrProvider = new ApiMediaUrlProvider(PublishedUrlProvider); - routeBuilder = routeBuilder ?? CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); + routeBuilder = routeBuilder ?? CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings()); return new MultiNodeTreePickerValueConverter( PublishedSnapshotAccessor, Mock.Of(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs index a3bd140ce6..b99478f7cd 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiUrlPickerValueConverterTests.cs @@ -294,7 +294,7 @@ public class MultiUrlPickerValueConverterTests : PropertyValueConverterTests private MultiUrlPickerValueConverter MultiUrlPickerValueConverter() { - var routeBuilder = CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); + var routeBuilder = CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings()); return new MultiUrlPickerValueConverter( PublishedSnapshotAccessor, Mock.Of(), diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs index f08ea5545e..18d5885aa3 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/OutputExpansionStrategyTestBase.cs @@ -452,5 +452,5 @@ public abstract class OutputExpansionStrategyTestBase : PropertyValueConverterTe return new PublishedElementPropertyBase(elementPropertyType, parent, false, PropertyCacheLevel.None); } - protected IApiContentRouteBuilder ApiContentRouteBuilder() => CreateContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings()); + protected IApiContentRouteBuilder ApiContentRouteBuilder() => CreateContentRouteBuilder(ApiContentPathProvider, CreateGlobalSettings()); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs index d3831ec2e4..475945c9df 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PropertyValueConverterTests.cs @@ -1,5 +1,6 @@ using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; @@ -12,6 +13,8 @@ public class PropertyValueConverterTests : DeliveryApiTests protected IPublishedUrlProvider PublishedUrlProvider { get; private set; } + protected IApiContentPathProvider ApiContentPathProvider { get; private set; } + protected IPublishedContent PublishedContent { get; private set; } protected IPublishedContent PublishedMedia { get; private set; } @@ -69,6 +72,7 @@ public class PropertyValueConverterTests : DeliveryApiTests .Setup(p => p.GetMediaUrl(publishedMedia.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns("the-media-url"); PublishedUrlProvider = PublishedUrlProviderMock.Object; + ApiContentPathProvider = new ApiContentPathProvider(PublishedUrlProvider); var publishedSnapshotAccessor = new Mock(); var publishedSnapshotObject = publishedSnapshot.Object;