From aadf5e7643cb766e62bbc796a162f70772ab4e14 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 30 Oct 2025 16:14:08 +0100 Subject: [PATCH 1/8] Bump the version to 17.0.0-rc2 --- src/Umbraco.Web.UI.Client/package-lock.json | 4 ++-- src/Umbraco.Web.UI.Client/package.json | 2 +- version.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index ea9fffe506..f2c6855a31 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -1,12 +1,12 @@ { "name": "@umbraco-cms/backoffice", - "version": "17.0.0-rc1", + "version": "17.0.0-rc2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@umbraco-cms/backoffice", - "version": "17.0.0-rc1", + "version": "17.0.0-rc2", "license": "MIT", "workspaces": [ "./src/packages/*", diff --git a/src/Umbraco.Web.UI.Client/package.json b/src/Umbraco.Web.UI.Client/package.json index 09a8a18839..a74c849a81 100644 --- a/src/Umbraco.Web.UI.Client/package.json +++ b/src/Umbraco.Web.UI.Client/package.json @@ -1,7 +1,7 @@ { "name": "@umbraco-cms/backoffice", "license": "MIT", - "version": "17.0.0-rc1", + "version": "17.0.0-rc2", "type": "module", "exports": { ".": null, diff --git a/version.json b/version.json index ab7426e644..0f1c00ee4c 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": "17.0.0-rc1", + "version": "17.0.0-rc2", "assemblyVersion": { "precision": "build" }, From bd945222379af3536d79850fef5423a57bdb9f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Fri, 31 Oct 2025 10:52:10 +0100 Subject: [PATCH 2/8] Collection children: A slim navigation of collection children + higher take above target (#20641) * enforce update of children when collection * only load one above and below collection children * take 50 above a target for default experience * revert reset target * remove old impl --- .../core/tree/tree-item/tree-item-children.manager.ts | 8 +++++++- .../core/tree/tree-item/tree-item-expansion.manager.ts | 6 ------ .../tree/tree-item/document-tree-item.context.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-children.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-children.manager.ts index 164abc3c07..381dadae9b 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-children.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-children.manager.ts @@ -188,6 +188,12 @@ export class UmbTreeItemChildrenManager< * @memberof UmbTreeItemChildrenManager */ public async loadChildren(): Promise { + const target = this.targetPagination.getBaseTarget(); + /* If a new target is set we only want to reload children if the new target isn’t among the already loaded items. */ + if (target && this.isChildLoaded(target)) { + return; + } + return this.#loadChildren(); } @@ -244,7 +250,7 @@ export class UmbTreeItemChildrenManager< ? this.targetPagination.getNumberOfCurrentItemsBeforeBaseTarget() : this.#takeBeforeTarget !== undefined ? this.#takeBeforeTarget - : 5, + : this.targetPagination.getTakeSize(), takeAfter: reload ? this.targetPagination.getNumberOfCurrentItemsAfterBaseTarget() : this.#takeAfterTarget !== undefined diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-expansion.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-expansion.manager.ts index ce620ec08f..4c09f36325 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-expansion.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-expansion.manager.ts @@ -94,12 +94,6 @@ export class UmbTreeItemTargetExpansionManager< return; } - /* If a new target is set we only want to reload children if the new target isn’t among the already loaded items. */ - const targetIsLoaded = this.#childrenManager.isChildLoaded(target); - if (target && targetIsLoaded) { - return; - } - // If we already have children and the target didn't change then we don't have to load new children const isNewTarget = target !== currentBaseTarget; if (isExpanded && this.#childrenManager.hasLoadedChildren() && !isNewTarget) { diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/tree/tree-item/document-tree-item.context.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/tree/tree-item/document-tree-item.context.ts index 29cbc3d63b..2c78868133 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/tree/tree-item/document-tree-item.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/tree/tree-item/document-tree-item.context.ts @@ -40,7 +40,7 @@ export class UmbDocumentTreeItemContext extends UmbDefaultTreeItemContext< this.hasCollection, (hasCollection) => { if (hasCollection) { - this._treeItemChildrenManager.setTargetTakeSize(2, 2); + this._treeItemChildrenManager.setTargetTakeSize(1, 1); this.observe( this.hasActiveDescendant, From 417335a5c6f0e7f204aab6be2788ff9d52bbd267 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Fri, 31 Oct 2025 10:53:57 +0100 Subject: [PATCH 3/8] Task: Dependency track (#20670) * Generate BOM files on build * Upload BOM to Dependency Track * Move Backoffice BOM generation to right after install The build and/or pack steps are deleting files that are needed for the BOM to be generated properly. * Split the BOM uploads into different jobs * Fix wrong usage of parameters * Move order of dependency track stage * Fix wrong umbracoVersion value * Small fixes * Log curl response headers * Correct version sent to dependency track * Adjusted curl flags * Fix bom file path * Fix dotnet bom file name * Add Login UI to dependency track * Generate BOM for E2E Tests * Move dependency track stage * Move acceptance test .env generation to e2e install template Needed as the post install script is expecting this to exist. * Use major version if public release * Missing ')' * Reverted npm install command changes in static assets project --- build/azure-pipelines.yml | 81 ++++++++++++++++++++++++++++ build/nightly-E2E-setup-template.yml | 34 +++--------- build/templates/dependency-track.yml | 56 +++++++++++++++++++ build/templates/e2e-install.yml | 49 +++++++++++++++++ 4 files changed, 193 insertions(+), 27 deletions(-) create mode 100644 build/templates/dependency-track.yml create mode 100644 build/templates/e2e-install.yml diff --git a/build/azure-pipelines.yml b/build/azure-pipelines.yml index 2246e09e7b..f2f18175fa 100644 --- a/build/azure-pipelines.yml +++ b/build/azure-pipelines.yml @@ -34,6 +34,10 @@ parameters: displayName: Upload API docs type: boolean default: false + - name: uploadDependencyTrack + displayName: Upload BOMs to Dependency Track + type: boolean + default: false - name: forceReleaseTestFilter displayName: Force to use the release test filters type: boolean @@ -103,6 +107,15 @@ stages: command: build projects: $(solution) arguments: "--configuration $(buildConfiguration) --no-restore --property:ContinuousIntegrationBuild=true --property:GeneratePackageOnBuild=true --property:PackageOutputPath=$(Build.ArtifactStagingDirectory)/nupkg" + - powershell: | + dotnet tool install --global CycloneDX + dotnet-CycloneDX $(solution) --output $(Build.ArtifactStagingDirectory)/bom --filename bom-dotnet.xml + displayName: 'Generate Backend BOM' + - powershell: | + npm install --global @cyclonedx/cyclonedx-npm + cyclonedx-npm -o $(Build.ArtifactStagingDirectory)\bom\bom-login.xml --ignore-npm-errors --verbose + displayName: Generate Login UI BOM + workingDirectory: src/Umbraco.Web.UI.Login - task: PublishPipelineArtifact@1 displayName: Publish nupkg inputs: @@ -113,6 +126,11 @@ stages: inputs: targetPath: $(Build.SourcesDirectory) artifactName: build_output + - task: PublishPipelineArtifact@1 + displayName: Publish Backend BOM + inputs: + targetPath: $(Build.ArtifactStagingDirectory)/bom + artifactName: bom-backend - job: B displayName: Build Bellissima Package @@ -124,6 +142,11 @@ stages: lfs: false, fetchDepth: 500 - template: templates/backoffice-install.yml + - powershell: | + npm install --global @cyclonedx/cyclonedx-npm + cyclonedx-npm -o $(Build.ArtifactStagingDirectory)/bom/bom-backoffice.xml --ignore-npm-errors --verbose + displayName: Generate Backoffice UI BOM + workingDirectory: src/Umbraco.Web.UI.Client - script: npm run build:for:npm displayName: Run build:for:npm workingDirectory: src/Umbraco.Web.UI.Client @@ -140,6 +163,35 @@ stages: inputs: targetPath: $(Build.ArtifactStagingDirectory)/npm artifactName: npm + - publish: $(Build.ArtifactStagingDirectory)/bom + artifact: bom-frontend + displayName: 'Publish Frontend BOM' + + - stage: E2E_BOM + displayName: E2E Tests BOM Generation + dependsOn: [] + jobs: + - job: + displayName: E2E Generate BOM + pool: + vmImage: "ubuntu-latest" + steps: + - checkout: self + submodules: false + lfs: false, + fetchDepth: 500 + - template: templates/e2e-install.yml + parameters: + nodeVersion: ${{ variables.nodeVersion }} + npm_config_cache: ${{ variables.npm_config_cache }} + - powershell: | + npm install --global @cyclonedx/cyclonedx-npm + cyclonedx-npm -o $(Build.ArtifactStagingDirectory)/bom/bom-e2e.xml --ignore-npm-errors --verbose + displayName: Generate E2E Tests BOM + workingDirectory: tests/Umbraco.Tests.AcceptanceTest + - publish: $(Build.ArtifactStagingDirectory)/bom + artifact: bom-e2e + displayName: 'Publish E2E BOM' - stage: Build_Docs condition: and(succeeded(), or(eq(dependencies.Build.outputs['A.build.NBGV_PublicRelease'], 'True'), ${{parameters.buildApiDocs}})) @@ -668,6 +720,34 @@ stages: ASPNETCORE_URLS: ${{ variables.ASPNETCORE_URLS }} DatabaseType: ${{ variables.DatabaseType }} + - stage: Dependency_Track + displayName: Dependency Track + dependsOn: + - Build + - E2E_BOM + condition: and(succeeded(), or(eq(dependencies.Build.outputs['A.build.NBGV_PublicRelease'], 'True'), ${{parameters.uploadDependencyTrack}})) + variables: + # Determine Umbraco version based on whether it's a public release or not. If public release, use major version, else use full NuGet package version. + umbracoVersion: $[ iif(eq(stageDependencies.Build.A.outputs['build.NBGV_PublicRelease'], 'True'), stageDependencies.Build.A.outputs['build.NBGV_VersionMajor'], stageDependencies.Build.A.outputs['build.NBGV_NuGetPackageVersion']) ] + jobs: + - template: templates/dependency-track.yml + parameters: + projectName: "Umbraco-CMS" + umbracoVersion: $(umbracoVersion) + projects: + - name: "Backend" + artifact: "bom-backend" + bomFilePath: "bom-dotnet.xml" + - name: "Login" + artifact: "bom-backend" + bomFilePath: "bom-login.xml" + - name: "Backoffice" + artifact: "bom-frontend" + bomFilePath: "bom-backoffice.xml" + - name: "E2E" + artifact: "bom-e2e" + bomFilePath: "bom-e2e.xml" + ############################################### ## Release ############################################### @@ -874,3 +954,4 @@ stages: ContainerName: "$web" BlobPrefix: v$(umbracoMajorVersion)/ui-api CleanTargetBeforeCopy: true + diff --git a/build/nightly-E2E-setup-template.yml b/build/nightly-E2E-setup-template.yml index 8085561900..907ace6b72 100644 --- a/build/nightly-E2E-setup-template.yml +++ b/build/nightly-E2E-setup-template.yml @@ -26,38 +26,18 @@ steps: artifact: nupkg path: $(Agent.BuildDirectory)/app/nupkg - - task: NodeTool@0 - displayName: Use Node.js $(nodeVersion) - inputs: - versionSpec: $(nodeVersion) - - task: UseDotNet@2 displayName: Use .NET SDK from global.json inputs: useGlobalJson: true - - pwsh: | - "UMBRACO_USER_LOGIN=${{ parameters.PlaywrightUserEmail }} - UMBRACO_USER_PASSWORD=${{ parameters.PlaywrightPassword }} - URL=${{ parameters.ASPNETCORE_URLS }} - STORAGE_STAGE_PATH=$(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/playwright/.auth/user.json - CONSOLE_ERRORS_PATH=$(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/console-errors.json" | Out-File .env - displayName: Generate .env - workingDirectory: $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest - - # Cache and restore NPM packages - - task: Cache@2 - displayName: Cache NPM packages - inputs: - key: 'npm_e2e | "$(Agent.OS)" | $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/package-lock.json' - restoreKeys: | - npm_e2e | "$(Agent.OS)" - npm_e2e - path: ${{ parameters.npm_config_cache }} - - - script: npm ci --no-fund --no-audit --prefer-offline - workingDirectory: $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest - displayName: Restore NPM packages + - template: templates/e2e-install.yml + parameters: + nodeVersion: ${{ parameters.nodeVersion }} + npm_config_cache: ${{ parameters.npm_config_cache }} + PlaywrightUserEmail: ${{ parameters.PlaywrightUserEmail }} + PlaywrightPassword: ${{ parameters.PlaywrightPassword }} + ASPNETCORE_URLS: ${{ parameters.ASPNETCORE_URLS }} # Install Template - pwsh: | diff --git a/build/templates/dependency-track.yml b/build/templates/dependency-track.yml new file mode 100644 index 0000000000..74968b4616 --- /dev/null +++ b/build/templates/dependency-track.yml @@ -0,0 +1,56 @@ +parameters: + - name: projectName + type: string + - name: umbracoVersion + type: string + - name: projects + type: object + +jobs: +- job: Create_DT_Project + displayName: Create Dependency Track Project + steps: + - checkout: none + + - bash: | + project_id=$(curl --no-progress-meter -H "X-Api-Key: $(DT_API_KEY)" "$(DT_API_URL)/v1/project/lookup?name=${{ parameters.projectName }}&version=${{ parameters.umbracoVersion }}" | jq -r '.uuid') + if [ "$project_id" != "null" ] && [ -n "$project_id" ]; then + echo "Project '${{ parameters.projectName }}' with version '${{ parameters.umbracoVersion }}' already exists (ID: $project_id)." + else + project_id=$(curl --no-progress-meter \ + -X PUT "$(DT_API_URL)/v1/project" \ + -H "X-Api-Key: $(DT_API_KEY)" \ + -H "Content-Type: application/json" \ + -d '{"name": "${{ parameters.projectName }}", "version": "${{ parameters.umbracoVersion }}", "collectionLogic": "AGGREGATE_DIRECT_CHILDREN"}' \ + | jq -r '.uuid') + if [ -z "$project_id" ] || [ "$project_id" == "null" ]; then + echo "Failed to create project '${{ parameters.projectName }}' version '${{ parameters.umbracoVersion }}'." + exit 1 + fi + echo "Created project '${{ parameters.projectName }}' with version '${{ parameters.umbracoVersion }}' (ID: $project_id)." + fi + displayName: Ensure main project exists in Dependency Track + +- ${{ each project in parameters.projects }}: + - job: + displayName: Upload ${{ project.name }} BOM + dependsOn: Create_DT_Project + steps: + - checkout: none + + - download: current + artifact: ${{ project.artifact }} + displayName: Download ${{ project.artifact }} artifact + + - script: | + curl --no-progress-meter --fail-with-body \ + -X POST "$(DT_API_URL)/v1/bom" \ + -H "X-Api-Key: $(DT_API_KEY)" \ + -H "Content-Type: multipart/form-data" \ + -F "autoCreate=true" \ + -F "projectName=${{ parameters.projectName }}-${{ project.name }}" \ + -F "projectVersion=${{ parameters.umbracoVersion }}" \ + -F "parentName=${{ parameters.projectName }}" \ + -F "parentVersion=${{ parameters.umbracoVersion }}" \ + -F "bom=@$(Pipeline.Workspace)/${{ project.artifact }}/${{ project.bomFilePath }}" + displayName: Upload ${{ project.name }} BOM to Dependency Track diff --git a/build/templates/e2e-install.yml b/build/templates/e2e-install.yml new file mode 100644 index 0000000000..3c2909f3fb --- /dev/null +++ b/build/templates/e2e-install.yml @@ -0,0 +1,49 @@ +parameters: + - name: nodeVersion + type: string + default: '' + + - name: npm_config_cache + type: string + default: '' + + - name: PlaywrightUserEmail + type: string + default: '' + + - name: PlaywrightPassword + type: string + default: '' + + - name: ASPNETCORE_URLS + type: string + default: '' + +steps: + - task: NodeTool@0 + displayName: Use Node.js $(nodeVersion) + inputs: + versionSpec: $(nodeVersion) + + - pwsh: | + "UMBRACO_USER_LOGIN=${{ parameters.PlaywrightUserEmail }} + UMBRACO_USER_PASSWORD=${{ parameters.PlaywrightPassword }} + URL=${{ parameters.ASPNETCORE_URLS }} + STORAGE_STAGE_PATH=$(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/playwright/.auth/user.json + CONSOLE_ERRORS_PATH=$(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/console-errors.json" | Out-File .env + displayName: Generate .env + workingDirectory: $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest + + # Cache and restore NPM packages + - task: Cache@2 + displayName: Cache NPM packages + inputs: + key: 'npm_e2e | "$(Agent.OS)" | $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest/package-lock.json' + restoreKeys: | + npm_e2e | "$(Agent.OS)" + npm_e2e + path: ${{ parameters.npm_config_cache }} + + - script: npm ci --no-fund --no-audit --prefer-offline + workingDirectory: $(Build.SourcesDirectory)/tests/Umbraco.Tests.AcceptanceTest + displayName: Restore NPM packages From 95cc6cc67bb60fbdcdb0503b5872005e5aad419e Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 31 Oct 2025 10:49:26 +0100 Subject: [PATCH 4/8] Performance: Request cache referenced entities when saving documents with block editors (#20590) * Added request cache to content and media lookups in mult URL picker. * Allow property editors to cache referenced entities from block data. * Update src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add obsoletions. * Minor spellcheck * Ensure request cache is available before relying on it. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: kjac --- .../PropertyEditors/DataValueEditor.cs | 156 ++++++++++++++++++ .../ICacheReferencedEntities.cs | 19 +++ .../BlockEditorPropertyValueEditor.cs | 2 + .../BlockValuePropertyValueEditorBase.cs | 39 +++++ .../MediaPicker3PropertyEditor.cs | 85 ++++++---- .../MultiUrlPickerValueEditor.cs | 104 +++++++++++- ...ultiUrlPickerValueEditorValidationTests.cs | 4 +- 7 files changed, 368 insertions(+), 41 deletions(-) create mode 100644 src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs diff --git a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs index 211d36f65f..1c32dc3de6 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs @@ -3,12 +3,14 @@ using System.Globalization; using System.Runtime.Serialization; using System.Xml.Linq; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.Models.Validation; using Umbraco.Cms.Core.PropertyEditors.Validators; using Umbraco.Cms.Core.Serialization; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Extensions; @@ -20,6 +22,9 @@ namespace Umbraco.Cms.Core.PropertyEditors; [DataContract] public class DataValueEditor : IDataValueEditor { + private const string ContentCacheKeyFormat = nameof(DataValueEditor) + "_Content_{0}"; + private const string MediaCacheKeyFormat = nameof(DataValueEditor) + "_Media_{0}"; + private readonly IJsonSerializer? _jsonSerializer; private readonly IShortStringHelper _shortStringHelper; @@ -415,4 +420,155 @@ public class DataValueEditor : IDataValueEditor return value.TryConvertTo(valueType); } + + /// + /// Retrieves a instance by its unique identifier, using the provided request cache to avoid redundant + /// lookups within the same request. + /// + /// + /// This method caches content lookups for the duration of the current request to improve performance when the same content + /// item may be accessed multiple times. This is particularly useful in scenarios involving multiple languages or blocks. + /// + /// The unique identifier of the content item to retrieve. + /// The request-scoped cache used to store and retrieve content items for the duration of the current request. + /// The content service used to fetch the content item if it is not found in the cache. + /// The instance corresponding to the specified key, or null if no such content item exists. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static IContent? GetAndCacheContentById(Guid key, IRequestCache requestCache, IContentService contentService) + { + if (requestCache.IsAvailable is false) + { + return contentService.GetById(key); + } + + var cacheKey = string.Format(ContentCacheKeyFormat, key); + IContent? content = requestCache.GetCacheItem(cacheKey); + if (content is null) + { + content = contentService.GetById(key); + if (content is not null) + { + requestCache.Set(cacheKey, content); + } + } + + return content; + } + + /// + /// Adds the specified item to the request cache using its unique key. + /// + /// The content item to cache. + /// The request cache in which to store the content item. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static void CacheContentById(IContent content, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return; + } + + var cacheKey = string.Format(ContentCacheKeyFormat, content.Key); + requestCache.Set(cacheKey, content); + } + + /// + /// Retrieves a instance by its unique identifier, using the provided request cache to avoid redundant + /// lookups within the same request. + /// + /// + /// This method caches media lookups for the duration of the current request to improve performance when the same media + /// item may be accessed multiple times. This is particularly useful in scenarios involving multiple languages or blocks. + /// + /// The unique identifier of the media item to retrieve. + /// The request-scoped cache used to store and retrieve media items for the duration of the current request. + /// The media service used to fetch the media item if it is not found in the cache. + /// The instance corresponding to the specified key, or null if no such media item exists. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static IMedia? GetAndCacheMediaById(Guid key, IRequestCache requestCache, IMediaService mediaService) + { + if (requestCache.IsAvailable is false) + { + return mediaService.GetById(key); + } + + var cacheKey = string.Format(MediaCacheKeyFormat, key); + IMedia? media = requestCache.GetCacheItem(cacheKey); + + if (media is null) + { + media = mediaService.GetById(key); + if (media is not null) + { + requestCache.Set(cacheKey, media); + } + } + + return media; + } + + /// + /// Adds the specified item to the request cache using its unique key. + /// + /// The media item to cache. + /// The request cache in which to store the media item. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static void CacheMediaById(IMedia media, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return; + } + + var cacheKey = string.Format(MediaCacheKeyFormat, media.Key); + requestCache.Set(cacheKey, media); + } + + /// + /// Determines whether the content item identified by the specified key is present in the request cache. + /// + /// The unique identifier for the content item to check for in the cache. + /// The request cache in which to look for the content item. + /// true if the content item is already cached in the request cache; otherwise, false. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static bool IsContentAlreadyCached(Guid key, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return false; + } + + var cacheKey = string.Format(ContentCacheKeyFormat, key); + return requestCache.GetCacheItem(cacheKey) is not null; + } + + /// + /// Determines whether the media item identified by the specified key is present in the request cache. + /// + /// The unique identifier for the media item to check for in the cache. + /// The request cache in which to look for the media item. + /// true if the media item is already cached in the request cache; otherwise, false. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected static bool IsMediaAlreadyCached(Guid key, IRequestCache requestCache) + { + if (requestCache.IsAvailable is false) + { + return false; + } + + var cacheKey = string.Format(MediaCacheKeyFormat, key); + return requestCache.GetCacheItem(cacheKey) is not null; + } } diff --git a/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs b/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs new file mode 100644 index 0000000000..cf655a9167 --- /dev/null +++ b/src/Umbraco.Core/PropertyEditors/ICacheReferencedEntities.cs @@ -0,0 +1,19 @@ +namespace Umbraco.Cms.Core.PropertyEditors; + +/// +/// Optionally implemented by property editors, this defines a contract for caching entities that are referenced in block values. +/// +[Obsolete("This interface is available for support of request caching retrieved entities in property value editors that implement it. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] +public interface ICacheReferencedEntities +{ + /// + /// Caches the entities referenced by the provided block data values. + /// + /// An enumerable collection of block values that may contain the entities to be cached. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + void CacheReferencedEntities(IEnumerable values); +} diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs index a2e7f4bee9..07a735b8e1 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs @@ -107,6 +107,8 @@ public abstract class BlockEditorPropertyValueEditor : BlockVal BlockEditorData? currentBlockEditorData = SafeParseBlockEditorData(currentValue); BlockEditorData? blockEditorData = SafeParseBlockEditorData(editorValue.Value); + CacheReferencedEntities(blockEditorData); + // We can skip MapBlockValueFromEditor if both editorValue and currentValue values are empty. if (IsBlockEditorDataEmpty(currentBlockEditorData) && IsBlockEditorDataEmpty(blockEditorData)) { diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index 8128643d9a..333df828e9 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -43,6 +43,45 @@ public abstract class BlockValuePropertyValueEditorBase : DataV _languageService = languageService; } + /// + /// Caches referenced entities for all property values with supporting property editors within the specified block editor data + /// optimising subsequent retrieval of entities when parsing and converting property values. + /// + /// + /// This method iterates through all property values associated with data editors in the provided + /// block editor data and invokes caching for referenced entities where supported by the property editor. + /// + /// The block editor data containing content and settings property values to analyze for referenced entities. + [Obsolete("This method is available for support of request caching retrieved entities in derived property value editors. " + + "The intention is to supersede this with lazy loaded read locks, which will make this unnecessary. " + + "Scheduled for removal in Umbraco 19.")] + protected void CacheReferencedEntities(BlockEditorData? blockEditorData) + { + // Group property values by their associated data editor alias. + IEnumerable> valuesByDataEditors = (blockEditorData?.BlockValue.ContentData ?? []).Union(blockEditorData?.BlockValue.SettingsData ?? []) + .SelectMany(x => x.Values) + .Where(x => x.EditorAlias is not null && x.Value is not null) + .GroupBy(x => x.EditorAlias!); + + // Iterate through each group and cache referenced entities if supported by the data editor. + foreach (IGrouping valueByDataEditor in valuesByDataEditors) + { + IDataEditor? dataEditor = _propertyEditors[valueByDataEditor.Key]; + if (dataEditor is null) + { + continue; + } + + IDataValueEditor valueEditor = dataEditor.GetValueEditor(); + + if (valueEditor is ICacheReferencedEntities valueEditorWithPrecaching) + { + valueEditorWithPrecaching.CacheReferencedEntities(valueByDataEditor.Select(x => x.Value!)); + } + } + } + + /// public abstract IEnumerable GetReferences(object? value); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs index d4a9e492bd..50fe850e04 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs @@ -52,10 +52,8 @@ public class MediaPicker3PropertyEditor : DataEditor /// /// Defines the value editor for the media picker property editor. /// - internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference + internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference, ICacheReferencedEntities { - private const string MediaCacheKeyFormat = nameof(MediaPicker3PropertyValueEditor) + "_Media_{0}"; - private readonly IDataTypeConfigurationCache _dataTypeReadCache; private readonly IJsonSerializer _jsonSerializer; private readonly IMediaImportService _mediaImportService; @@ -107,6 +105,27 @@ public class MediaPicker3PropertyEditor : DataEditor Validators.Add(validators); } + /// + public void CacheReferencedEntities(IEnumerable values) + { + var mediaKeys = values + .SelectMany(value => Deserialize(_jsonSerializer, value)) + .Select(dto => dto.MediaKey) + .Distinct() + .Where(x => IsMediaAlreadyCached(x, _appCaches.RequestCache) is false) + .ToList(); + if (mediaKeys.Count == 0) + { + return; + } + + IEnumerable mediaItems = _mediaService.GetByIds(mediaKeys); + foreach (IMedia media in mediaItems) + { + CacheMediaById(media, _appCaches.RequestCache); + } + } + /// public IEnumerable GetReferences(object? value) { @@ -208,31 +227,13 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { - IMedia? media = GetMediaById(mediaWithCropsDto.MediaKey); + IMedia? media = GetAndCacheMediaById(mediaWithCropsDto.MediaKey, _appCaches.RequestCache, _mediaService); mediaWithCropsDto.MediaTypeAlias = media?.ContentType.Alias ?? unknownMediaType; } return mediaWithCropsDtos.Where(m => m.MediaTypeAlias != unknownMediaType).ToList(); } - private IMedia? GetMediaById(Guid key) - { - // Cache media lookups in case the same media is handled multiple times across a save operation, - // which is possible, particularly if we have multiple languages and blocks. - var cacheKey = string.Format(MediaCacheKeyFormat, key); - IMedia? media = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (media is null) - { - media = _mediaService.GetById(key); - if (media is not null) - { - _appCaches.RequestCache.Set(cacheKey, media); - } - } - - return media; - } - private List HandleTemporaryMediaUploads(List mediaWithCropsDtos, MediaPicker3Configuration configuration) { var invalidDtos = new List(); @@ -240,7 +241,7 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { // if the media already exist, don't bother with it - if (GetMediaById(mediaWithCropsDto.MediaKey) != null) + if (GetAndCacheMediaById(mediaWithCropsDto.MediaKey, _appCaches.RequestCache, _mediaService) != null) { continue; } @@ -480,18 +481,7 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (var typeAlias in distinctTypeAliases) { - // Cache media type lookups since the same media type is likely to be used multiple times in validation, - // particularly if we have multiple languages and blocks. - var cacheKey = string.Format(MediaTypeCacheKeyFormat, typeAlias); - string? typeKey = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (typeKey is null) - { - typeKey = _mediaTypeService.Get(typeAlias)?.Key.ToString(); - if (typeKey is not null) - { - _appCaches.RequestCache.Set(cacheKey, typeKey); - } - } + string? typeKey = GetMediaTypeKey(typeAlias); if (typeKey is null || allowedTypes.Contains(typeKey) is false) { @@ -506,6 +496,31 @@ public class MediaPicker3PropertyEditor : DataEditor return []; } + + private string? GetMediaTypeKey(string typeAlias) + { + // Cache media type lookups since the same media type is likely to be used multiple times in validation, + // particularly if we have multiple languages and blocks. + string? GetMediaTypeKeyFromService(string typeAlias) => _mediaTypeService.Get(typeAlias)?.Key.ToString(); + + if (_appCaches.RequestCache.IsAvailable is false) + { + return GetMediaTypeKeyFromService(typeAlias); + } + + var cacheKey = string.Format(MediaTypeCacheKeyFormat, typeAlias); + string? typeKey = _appCaches.RequestCache.GetCacheItem(cacheKey); + if (typeKey is null) + { + typeKey = GetMediaTypeKeyFromService(typeAlias); + if (typeKey is not null) + { + _appCaches.RequestCache.Set(cacheKey, typeKey); + } + } + + return typeKey; + } } /// diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs index 2f0a2b279b..2359e5537d 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MultiUrlPickerValueEditor.cs @@ -3,7 +3,10 @@ using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; @@ -19,14 +22,16 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Core.PropertyEditors; -public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference +public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference, ICacheReferencedEntities { private readonly ILogger _logger; private readonly IPublishedUrlProvider _publishedUrlProvider; private readonly IJsonSerializer _jsonSerializer; private readonly IContentService _contentService; private readonly IMediaService _mediaService; + private readonly AppCaches _appCaches; + [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 19.")] public MultiUrlPickerValueEditor( ILogger logger, ILocalizedTextService localizedTextService, @@ -37,19 +42,102 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference IIOHelper ioHelper, IContentService contentService, IMediaService mediaService) + : this( + logger, + localizedTextService, + shortStringHelper, + attribute, + publishedUrlProvider, + jsonSerializer, + ioHelper, + contentService, + mediaService, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + public MultiUrlPickerValueEditor( + ILogger logger, + ILocalizedTextService localizedTextService, + IShortStringHelper shortStringHelper, + DataEditorAttribute attribute, + IPublishedUrlProvider publishedUrlProvider, + IJsonSerializer jsonSerializer, + IIOHelper ioHelper, + IContentService contentService, + IMediaService mediaService, + AppCaches appCaches) : base(shortStringHelper, jsonSerializer, ioHelper, attribute) { - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _logger = logger; _publishedUrlProvider = publishedUrlProvider; - _jsonSerializer = jsonSerializer; _contentService = contentService; _mediaService = mediaService; + _appCaches = appCaches; + Validators.Add(new TypedJsonValidatorRunner( _jsonSerializer, new MinMaxValidator(localizedTextService))); } + /// + public void CacheReferencedEntities(IEnumerable values) + { + var dtos = values + .Select(value => + { + var asString = value is string str ? str : value.ToString(); + if (string.IsNullOrEmpty(asString)) + { + return null; + } + + return _jsonSerializer.Deserialize>(asString); + }) + .WhereNotNull() + .SelectMany(x => x) + .Where(x => x.Type == Constants.UdiEntityType.Document || x.Type == Constants.UdiEntityType.Media) + .ToList(); + + IList contentKeys = GetKeys(Constants.UdiEntityType.Document, dtos); + IList mediaKeys = GetKeys(Constants.UdiEntityType.Media, dtos); + + if (contentKeys.Count > 0) + { + IEnumerable contentItems = _contentService.GetByIds(contentKeys); + foreach (IContent content in contentItems) + { + CacheContentById(content, _appCaches.RequestCache); + } + } + + if (mediaKeys.Count > 0) + { + IEnumerable mediaItems = _mediaService.GetByIds(mediaKeys); + foreach (IMedia media in mediaItems) + { + CacheMediaById(media, _appCaches.RequestCache); + } + } + } + + private IList GetKeys(string entityType, IEnumerable dtos) => + dtos + .Where(x => x.Type == entityType) + .Select(x => x.Unique ?? (x.Udi is not null ? x.Udi.Guid : Guid.Empty)) + .Where(x => x != Guid.Empty) + .Distinct() + .Where(x => IsAlreadyCached(x, entityType) is false) + .ToList(); + + private bool IsAlreadyCached(Guid key, string entityType) => entityType switch + { + Constants.UdiEntityType.Document => IsContentAlreadyCached(key, _appCaches.RequestCache), + Constants.UdiEntityType.Media => IsMediaAlreadyCached(key, _appCaches.RequestCache), + _ => false, + }; + public IEnumerable GetReferences(object? value) { var asString = value == null ? string.Empty : value is string str ? str : value.ToString(); @@ -105,7 +193,7 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference if (dto.Udi.EntityType == Constants.UdiEntityType.Document) { url = _publishedUrlProvider.GetUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IContent? c = _contentService.GetById(dto.Udi.Guid); + IContent? c = GetAndCacheContentById(dto.Udi.Guid, _appCaches.RequestCache, _contentService); if (c is not null) { @@ -119,7 +207,7 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference else if (dto.Udi.EntityType == Constants.UdiEntityType.Media) { url = _publishedUrlProvider.GetMediaUrl(dto.Udi.Guid, UrlMode.Relative, culture); - IMedia? m = _mediaService.GetById(dto.Udi.Guid); + IMedia? m = GetAndCacheMediaById(dto.Udi.Guid, _appCaches.RequestCache, _mediaService); if (m is not null) { published = m.Trashed is false; @@ -207,6 +295,12 @@ public class MultiUrlPickerValueEditor : DataValueEditor, IDataValueReference [DataMember(Name = "target")] public string? Target { get; set; } + [DataMember(Name = "unique")] + public Guid? Unique { get; set; } + + [DataMember(Name = "type")] + public string? Type { get; set; } + [DataMember(Name = "udi")] public GuidUdi? Udi { get; set; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs index dae1d885b2..c9264d5935 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/Validators/MultiUrlPickerValueEditorValidationTests.cs @@ -2,6 +2,7 @@ using System.ComponentModel.DataAnnotations; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models.Validation; using Umbraco.Cms.Core.PropertyEditors; @@ -68,7 +69,8 @@ internal class MultiUrlPickerValueEditorValidationTests new SystemTextJsonSerializer(new DefaultJsonSerializerEncoderFactory()), Mock.Of(), Mock.Of(), - Mock.Of()) + Mock.Of(), + AppCaches.Disabled) { ConfigurationObject = new MultiUrlPickerConfiguration(), }; From 4ee1d7b13ede5512872146e6a3ff786d074b8bc7 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 3 Nov 2025 10:55:23 +0100 Subject: [PATCH 5/8] Performance: Cache published content instances at cache service level (#20681) Cache published content instances at cache service level --- .../Factories/PublishedContentFactory.cs | 97 +----------- .../Services/DocumentCacheService.cs | 35 ++++- .../Services/MediaCacheService.cs | 37 ++++- .../DocumentHybridCacheTests.cs | 43 ++++- .../DocumentPropertyCacheLevelTests.cs | 130 ++++++++++++++++ .../Factories/PublishedContentFactoryTests.cs | 147 ------------------ .../MediaPropertyCacheLevelTests.cs | 111 +++++++++++++ .../MemberPropertyCacheLevelTests.cs | 96 ++++++++++++ .../PropertyCacheLevelTestsBase.cs | 69 ++++++++ 9 files changed, 508 insertions(+), 257 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentPropertyCacheLevelTests.cs delete mode 100644 tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MediaPropertyCacheLevelTests.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MemberPropertyCacheLevelTests.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/PropertyCacheLevelTestsBase.cs diff --git a/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs b/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs index f6ff872ad1..a6c6273645 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactory.cs @@ -1,10 +1,7 @@ -using Microsoft.Extensions.Logging; -using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Extensions; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; -using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.HybridCache.Factories; @@ -16,8 +13,6 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory private readonly IElementsCache _elementsCache; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IPublishedContentTypeCache _publishedContentTypeCache; - private readonly ILogger _logger; - private readonly AppCaches _appCaches; /// /// Initializes a new instance of the class. @@ -25,40 +20,16 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory public PublishedContentFactory( IElementsCache elementsCache, IVariationContextAccessor variationContextAccessor, - IPublishedContentTypeCache publishedContentTypeCache, - ILogger logger, - AppCaches appCaches) + IPublishedContentTypeCache publishedContentTypeCache) { _elementsCache = elementsCache; _variationContextAccessor = variationContextAccessor; _publishedContentTypeCache = publishedContentTypeCache; - _logger = logger; - _appCaches = appCaches; } /// public IPublishedContent? ToIPublishedContent(ContentCacheNode contentCacheNode, bool preview) { - var cacheKey = $"{nameof(PublishedContentFactory)}DocumentCache_{contentCacheNode.Id}_{preview}_{contentCacheNode.Data?.VersionDate.Ticks ?? 0}"; - IPublishedContent? publishedContent = null; - if (_appCaches.RequestCache.IsAvailable) - { - publishedContent = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (publishedContent is not null) - { - _logger.LogDebug( - "Using cached IPublishedContent for document {ContentCacheNodeName} ({ContentCacheNodeId}).", - contentCacheNode.Data?.Name ?? "No Name", - contentCacheNode.Id); - return publishedContent; - } - } - - _logger.LogDebug( - "Creating IPublishedContent for document {ContentCacheNodeName} ({ContentCacheNodeId}).", - contentCacheNode.Data?.Name ?? "No Name", - contentCacheNode.Id); - IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Content, contentCacheNode.ContentTypeId); var contentNode = new ContentNode( @@ -71,44 +42,19 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory preview ? contentCacheNode.Data : null, preview ? null : contentCacheNode.Data); - publishedContent = GetModel(contentNode, preview); + IPublishedContent? publishedContent = GetModel(contentNode, preview); if (preview) { publishedContent ??= GetPublishedContentAsDraft(publishedContent); } - if (_appCaches.RequestCache.IsAvailable && publishedContent is not null) - { - _appCaches.RequestCache.Set(cacheKey, publishedContent); - } - return publishedContent; } /// public IPublishedContent? ToIPublishedMedia(ContentCacheNode contentCacheNode) { - var cacheKey = $"{nameof(PublishedContentFactory)}MediaCache_{contentCacheNode.Id}"; - IPublishedContent? publishedContent = null; - if (_appCaches.RequestCache.IsAvailable) - { - publishedContent = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (publishedContent is not null) - { - _logger.LogDebug( - "Using cached IPublishedContent for media {ContentCacheNodeName} ({ContentCacheNodeId}).", - contentCacheNode.Data?.Name ?? "No Name", - contentCacheNode.Id); - return publishedContent; - } - } - - _logger.LogDebug( - "Creating IPublishedContent for media {ContentCacheNodeName} ({ContentCacheNodeId}).", - contentCacheNode.Data?.Name ?? "No Name", - contentCacheNode.Id); - IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Media, contentCacheNode.ContentTypeId); var contentNode = new ContentNode( @@ -121,40 +67,12 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory null, contentCacheNode.Data); - publishedContent = GetModel(contentNode, false); - - if (_appCaches.RequestCache.IsAvailable && publishedContent is not null) - { - _appCaches.RequestCache.Set(cacheKey, publishedContent); - } - - return publishedContent; + return GetModel(contentNode, false); } /// public IPublishedMember ToPublishedMember(IMember member) { - string cacheKey = $"{nameof(PublishedContentFactory)}MemberCache_{member.Id}"; - IPublishedMember? publishedMember = null; - if (_appCaches.RequestCache.IsAvailable) - { - publishedMember = _appCaches.RequestCache.GetCacheItem(cacheKey); - if (publishedMember is not null) - { - _logger.LogDebug( - "Using cached IPublishedMember for member {MemberName} ({MemberId}).", - member.Username, - member.Id); - - return publishedMember; - } - } - - _logger.LogDebug( - "Creating IPublishedMember for member {MemberName} ({MemberId}).", - member.Username, - member.Id); - IPublishedContentType contentType = _publishedContentTypeCache.Get(PublishedItemType.Member, member.ContentTypeId); @@ -179,14 +97,7 @@ internal sealed class PublishedContentFactory : IPublishedContentFactory contentType, null, contentData); - publishedMember = new PublishedMember(member, contentNode, _elementsCache, _variationContextAccessor); - - if (_appCaches.RequestCache.IsAvailable) - { - _appCaches.RequestCache.Set(cacheKey, publishedMember); - } - - return publishedMember; + return new PublishedMember(member, contentNode, _elementsCache, _variationContextAccessor); } private static Dictionary GetPropertyValues(IPublishedContentType contentType, IMember member) diff --git a/src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs b/src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs index 2879be5c8d..79d9235e28 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs @@ -1,6 +1,7 @@ #if DEBUG using System.Diagnostics; #endif +using System.Collections.Concurrent; using Microsoft.Extensions.Caching.Hybrid; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -35,6 +36,8 @@ internal sealed class DocumentCacheService : IDocumentCacheService private readonly ILogger _logger; private HashSet? _seedKeys; + private readonly ConcurrentDictionary _publishedContentCache = []; + private HashSet SeedKeys { get @@ -108,6 +111,11 @@ internal sealed class DocumentCacheService : IDocumentCacheService { var cacheKey = GetCacheKey(key, preview); + if (preview is false && _publishedContentCache.TryGetValue(cacheKey, out IPublishedContent? cached)) + { + return cached; + } + ContentCacheNode? contentCacheNode = await _hybridCache.GetOrCreateAsync( cacheKey, async cancel => @@ -137,7 +145,13 @@ internal sealed class DocumentCacheService : IDocumentCacheService return null; } - return _publishedContentFactory.ToIPublishedContent(contentCacheNode, preview).CreateModel(_publishedModelFactory); + IPublishedContent? result = _publishedContentFactory.ToIPublishedContent(contentCacheNode, preview).CreateModel(_publishedModelFactory); + if (result is not null) + { + _publishedContentCache[cacheKey] = result; + } + + return result; } private bool GetPreview() => _previewService.IsInPreview(); @@ -174,7 +188,9 @@ internal sealed class DocumentCacheService : IDocumentCacheService ContentCacheNode? publishedNode = await _databaseCacheRepository.GetContentSourceAsync(key, false); if (publishedNode is not null && _publishStatusQueryService.HasPublishedAncestorPath(publishedNode.Key)) { - await _hybridCache.SetAsync(GetCacheKey(publishedNode.Key, false), publishedNode, GetEntryOptions(publishedNode.Key, false), GenerateTags(key)); + var cacheKey = GetCacheKey(publishedNode.Key, false); + await _hybridCache.SetAsync(cacheKey, publishedNode, GetEntryOptions(publishedNode.Key, false), GenerateTags(key)); + _publishedContentCache.Remove(cacheKey, out _); } scope.Complete(); @@ -183,7 +199,7 @@ internal sealed class DocumentCacheService : IDocumentCacheService public async Task RemoveFromMemoryCacheAsync(Guid key) { await _hybridCache.RemoveAsync(GetCacheKey(key, true)); - await _hybridCache.RemoveAsync(GetCacheKey(key, false)); + await ClearPublishedCacheAsync(key); } public async Task SeedAsync(CancellationToken cancellationToken) @@ -300,7 +316,7 @@ internal sealed class DocumentCacheService : IDocumentCacheService if (content.PublishedState == PublishedState.Unpublishing) { - await _hybridCache.RemoveAsync(GetCacheKey(publishedCacheNode.Key, false)); + await ClearPublishedCacheAsync(publishedCacheNode.Key); } } @@ -338,12 +354,19 @@ internal sealed class DocumentCacheService : IDocumentCacheService foreach (ContentCacheNode content in contentByContentTypeKey) { - _hybridCache.RemoveAsync(GetCacheKey(content.Key, true)).GetAwaiter().GetResult(); + await _hybridCache.RemoveAsync(GetCacheKey(content.Key, true)); if (content.IsDraft is false) { - _hybridCache.RemoveAsync(GetCacheKey(content.Key, false)).GetAwaiter().GetResult(); + await ClearPublishedCacheAsync(content.Key); } } } + + private async Task ClearPublishedCacheAsync(Guid key) + { + var cacheKey = GetCacheKey(key, false); + await _hybridCache.RemoveAsync(cacheKey); + _publishedContentCache.Remove(cacheKey, out _); + } } diff --git a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs index 46d782bdbe..9fe3dc5990 100644 --- a/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs +++ b/src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs @@ -1,6 +1,7 @@ #if DEBUG using System.Diagnostics; #endif +using System.Collections.Concurrent; using Microsoft.Extensions.Caching.Hybrid; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -31,6 +32,8 @@ internal sealed class MediaCacheService : IMediaCacheService private readonly ILogger _logger; private readonly CacheSettings _cacheSettings; + private readonly ConcurrentDictionary _publishedContentCache = []; + private HashSet? _seedKeys; private HashSet SeedKeys { @@ -103,6 +106,12 @@ internal sealed class MediaCacheService : IMediaCacheService private async Task GetNodeAsync(Guid key) { var cacheKey = $"{key}"; + + if (_publishedContentCache.TryGetValue(cacheKey, out IPublishedContent? cached)) + { + return cached; + } + ContentCacheNode? contentCacheNode = await _hybridCache.GetOrCreateAsync( cacheKey, // Unique key to the cache entry async cancel => @@ -122,7 +131,13 @@ internal sealed class MediaCacheService : IMediaCacheService return null; } - return _publishedContentFactory.ToIPublishedMedia(contentCacheNode).CreateModel(_publishedModelFactory); + IPublishedContent? result = _publishedContentFactory.ToIPublishedMedia(contentCacheNode).CreateModel(_publishedModelFactory); + if (result is not null) + { + _publishedContentCache[cacheKey] = result; + } + + return result; } public async Task HasContentByIdAsync(int id) @@ -141,6 +156,7 @@ internal sealed class MediaCacheService : IMediaCacheService using ICoreScope scope = _scopeProvider.CreateCoreScope(); var cacheNode = _cacheNodeFactory.ToContentCacheNode(media); await _databaseCacheRepository.RefreshMediaAsync(cacheNode); + _publishedContentCache.Remove(GetCacheKey(media.Key, false), out _); scope.Complete(); } @@ -219,7 +235,9 @@ internal sealed class MediaCacheService : IMediaCacheService ContentCacheNode? publishedNode = await _databaseCacheRepository.GetMediaSourceAsync(key); if (publishedNode is not null) { - await _hybridCache.SetAsync(GetCacheKey(publishedNode.Key, false), publishedNode, GetEntryOptions(publishedNode.Key)); + var cacheKey = GetCacheKey(publishedNode.Key, false); + await _hybridCache.SetAsync(cacheKey, publishedNode, GetEntryOptions(publishedNode.Key)); + _publishedContentCache.Remove(cacheKey, out _); } scope.Complete(); @@ -234,7 +252,7 @@ internal sealed class MediaCacheService : IMediaCacheService } public async Task RemoveFromMemoryCacheAsync(Guid key) - => await _hybridCache.RemoveAsync(GetCacheKey(key, false)); + => await ClearPublishedCacheAsync(key); public async Task RebuildMemoryCacheByContentTypeAsync(IEnumerable mediaTypeIds) { @@ -244,11 +262,11 @@ internal sealed class MediaCacheService : IMediaCacheService foreach (ContentCacheNode content in contentByContentTypeKey) { - _hybridCache.RemoveAsync(GetCacheKey(content.Key, true)).GetAwaiter().GetResult(); + await _hybridCache.RemoveAsync(GetCacheKey(content.Key, true)); if (content.IsDraft is false) { - _hybridCache.RemoveAsync(GetCacheKey(content.Key, false)).GetAwaiter().GetResult(); + await ClearPublishedCacheAsync(content.Key); } } @@ -269,7 +287,7 @@ internal sealed class MediaCacheService : IMediaCacheService foreach (ContentCacheNode media in mediaCacheNodesByContentTypeKey) { - _hybridCache.RemoveAsync(GetCacheKey(media.Key, false)); + ClearPublishedCacheAsync(media.Key).GetAwaiter().GetResult(); } scope.Complete(); @@ -313,4 +331,11 @@ internal sealed class MediaCacheService : IMediaCacheService // We use the tags to be able to clear all cache entries that are related to a given content item. // Tags for now are only content/media, but can be expanded with draft/published later. private static HashSet GenerateTags(Guid? key) => key is null ? [] : [Constants.Cache.Tags.Media]; + + private async Task ClearPublishedCacheAsync(Guid key) + { + var cacheKey = GetCacheKey(key, false); + await _hybridCache.RemoveAsync(cacheKey); + _publishedContentCache.Remove(cacheKey, out _); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs index 45d192587f..4cfd8b5a8f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheTests.cs @@ -85,17 +85,50 @@ internal sealed class DocumentHybridCacheTests : UmbracoIntegrationTestWithConte Assert.IsFalse(textPage.IsPublished()); } - [Test] - public async Task Cannot_get_unpublished_content() + [TestCase(true)] + [TestCase(false)] + public async Task Can_Get_Unpublished_Content_By_Key(bool preview) { // Arrange var unpublishAttempt = await ContentPublishingService.UnpublishAsync(PublishedTextPage.Key.Value, null, Constants.Security.SuperUserKey); + Assert.IsTrue(unpublishAttempt.Success); - //Act - var textPage = await PublishedContentHybridCache.GetByIdAsync(PublishedTextPageId, false); + // Act + var textPage = await PublishedContentHybridCache.GetByIdAsync(PublishedTextPage.Key.Value, preview); // Assert - Assert.IsNull(textPage); + if (preview) + { + Assert.IsNotNull(textPage); + Assert.IsFalse(textPage.IsPublished()); + } + else + { + Assert.IsNull(textPage); + } + } + + [TestCase(true)] + [TestCase(false)] + public async Task Can_Get_Unpublished_Content_By_Id(bool preview) + { + // Arrange + var unpublishAttempt = await ContentPublishingService.UnpublishAsync(PublishedTextPage.Key.Value, null, Constants.Security.SuperUserKey); + Assert.IsTrue(unpublishAttempt.Success); + + // Act + var textPage = await PublishedContentHybridCache.GetByIdAsync(PublishedTextPageId, preview); + + // Assert + if (preview) + { + Assert.IsNotNull(textPage); + Assert.IsFalse(textPage.IsPublished()); + } + else + { + Assert.IsNull(textPage); + } } [Test] diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentPropertyCacheLevelTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentPropertyCacheLevelTests.cs new file mode 100644 index 0000000000..a621290519 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentPropertyCacheLevelTests.cs @@ -0,0 +1,130 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; +using Umbraco.Cms.Tests.Common.Builders; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; + +public class DocumentPropertyCacheLevelTests : PropertyCacheLevelTestsBase +{ + private static readonly Guid _documentKey = new("9A526E75-DE41-4A81-8883-3E63F11A388D"); + + private IDocumentCacheService DocumentCacheService => GetRequiredService(); + + private IContentEditingService ContentEditingService => GetRequiredService(); + + private IContentPublishingService ContentPublishingService => GetRequiredService(); + + private IContentTypeEditingService ContentTypeEditingService => GetRequiredService(); + + [SetUp] + public async Task SetUpTest() + { + PropertyValueLevelDetectionTestsConverter.Reset(); + + var contentTypeCreateModel = ContentTypeEditingBuilder.CreateSimpleContentType(); + var contentTypeAttempt = await ContentTypeEditingService.CreateAsync(contentTypeCreateModel, Constants.Security.SuperUserKey); + Assert.IsTrue(contentTypeAttempt.Success); + + var contentCreateModel = ContentEditingBuilder.CreateSimpleContent(contentTypeAttempt.Result.Key); + contentCreateModel.Key = _documentKey; + var contentAttempt = await ContentEditingService.CreateAsync(contentCreateModel, Constants.Security.SuperUserKey); + Assert.IsTrue(contentAttempt.Success); + + await PublishPage(); + } + + [TestCase(PropertyCacheLevel.None, false, 1, 10)] + [TestCase(PropertyCacheLevel.None, true, 2, 10)] + [TestCase(PropertyCacheLevel.Element, false, 1, 1)] + [TestCase(PropertyCacheLevel.Element, true, 2, 2)] + [TestCase(PropertyCacheLevel.Elements, false, 1, 1)] + [TestCase(PropertyCacheLevel.Elements, true, 1, 1)] + public async Task Property_Value_Conversion_Respects_Property_Cache_Level(PropertyCacheLevel cacheLevel, bool preview, int expectedSourceConverts, int expectedInterConverts) + { + PropertyValueLevelDetectionTestsConverter.SetCacheLevel(cacheLevel); + + var publishedContent1 = await DocumentCacheService.GetByKeyAsync(_documentKey, preview); + Assert.IsNotNull(publishedContent1); + + var publishedContent2 = await DocumentCacheService.GetByKeyAsync(_documentKey, preview); + Assert.IsNotNull(publishedContent2); + + if (preview) + { + Assert.AreNotSame(publishedContent1, publishedContent2); + } + else + { + Assert.AreSame(publishedContent1, publishedContent2); + } + + var titleValue1 = publishedContent1.Value("title"); + Assert.IsNotNull(titleValue1); + + var titleValue2 = publishedContent2.Value("title"); + Assert.IsNotNull(titleValue2); + + Assert.AreEqual(titleValue1, titleValue2); + + // fetch title values 10 times in total, 5 times from each published content instance + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + + Assert.AreEqual(expectedSourceConverts, PropertyValueLevelDetectionTestsConverter.SourceConverts); + Assert.AreEqual(expectedInterConverts, PropertyValueLevelDetectionTestsConverter.InterConverts); + } + + [TestCase(PropertyCacheLevel.None, false)] + [TestCase(PropertyCacheLevel.None, true)] + [TestCase(PropertyCacheLevel.Element, false)] + [TestCase(PropertyCacheLevel.Element, true)] + [TestCase(PropertyCacheLevel.Elements, false)] + [TestCase(PropertyCacheLevel.Elements, true)] + public async Task Property_Value_Conversion_Is_Triggered_After_Cache_Refresh(PropertyCacheLevel cacheLevel, bool preview) + { + PropertyValueLevelDetectionTestsConverter.SetCacheLevel(cacheLevel); + + var publishedContent1 = await DocumentCacheService.GetByKeyAsync(_documentKey, preview); + Assert.IsNotNull(publishedContent1); + + var titleValue1 = publishedContent1.Value("title"); + Assert.IsNotNull(titleValue1); + + // re-publish the page to trigger a cache refresh for the page + await PublishPage(); + + var publishedContent2 = await DocumentCacheService.GetByKeyAsync(_documentKey, preview); + Assert.IsNotNull(publishedContent2); + + Assert.AreNotSame(publishedContent1, publishedContent2); + + var titleValue2 = publishedContent2.Value("title"); + Assert.IsNotNull(titleValue2); + + Assert.AreEqual(titleValue1, titleValue2); + + // expect conversions for each published content instance, due to the cache refresh + Assert.AreEqual(2, PropertyValueLevelDetectionTestsConverter.SourceConverts); + Assert.AreEqual(2, PropertyValueLevelDetectionTestsConverter.InterConverts); + } + + private async Task PublishPage() + { + var publishAttempt = await ContentPublishingService.PublishAsync( + _documentKey, + [new() { Culture = "*", }], + Constants.Security.SuperUserKey); + Assert.IsTrue(publishAttempt.Success); + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs deleted file mode 100644 index f9f604658c..0000000000 --- a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/Factories/PublishedContentFactoryTests.cs +++ /dev/null @@ -1,147 +0,0 @@ -using NUnit.Framework; -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Cache; -using Umbraco.Cms.Core.Models.PublishedContent; -using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Infrastructure.HybridCache; -using Umbraco.Cms.Infrastructure.HybridCache.Factories; -using Umbraco.Cms.Tests.Common.Builders; -using Umbraco.Cms.Tests.Common.Builders.Extensions; -using Umbraco.Cms.Tests.Common.Testing; -using Umbraco.Cms.Tests.Integration.Testing; - -namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; - -[TestFixture] -[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] -internal sealed class PublishedContentFactoryTests : UmbracoIntegrationTestWithContent -{ - private IPublishedContentFactory PublishedContentFactory => GetRequiredService(); - - private IPublishedValueFallback PublishedValueFallback => GetRequiredService(); - - private IMediaService MediaService => GetRequiredService(); - - private IMediaTypeService MediaTypeService => GetRequiredService(); - - private IMemberService MemberService => GetRequiredService(); - - private IMemberTypeService MemberTypeService => GetRequiredService(); - - protected override void CustomTestSetup(IUmbracoBuilder builder) - { - var requestCache = new DictionaryAppCache(); - var appCaches = new AppCaches( - NoAppCache.Instance, - requestCache, - new IsolatedCaches(type => NoAppCache.Instance)); - builder.Services.AddUnique(appCaches); - } - - [Test] - public void Can_Create_Published_Content_For_Document() - { - var contentCacheNode = new ContentCacheNode - { - Id = Textpage.Id, - Key = Textpage.Key, - ContentTypeId = Textpage.ContentType.Id, - CreateDate = Textpage.CreateDate, - CreatorId = Textpage.CreatorId, - SortOrder = Textpage.SortOrder, - Data = new ContentData( - Textpage.Name, - "text-page", - Textpage.VersionId, - Textpage.UpdateDate, - Textpage.WriterId, - Textpage.TemplateId, - true, - new Dictionary - { - { - "title", new[] - { - new PropertyData - { - Value = "Test title", - Culture = string.Empty, - Segment = string.Empty, - }, - } - }, - }, - null), - }; - var result = PublishedContentFactory.ToIPublishedContent(contentCacheNode, false); - Assert.IsNotNull(result); - Assert.AreEqual(Textpage.Id, result.Id); - Assert.AreEqual(Textpage.Name, result.Name); - Assert.AreEqual("Test title", result.Properties.Single(x => x.Alias == "title").Value(PublishedValueFallback)); - - // Verify that requesting the same content again returns the same instance (from request cache). - var result2 = PublishedContentFactory.ToIPublishedContent(contentCacheNode, false); - Assert.AreSame(result, result2); - } - - [Test] - public async Task Can_Create_Published_Content_For_Media() - { - var mediaType = new MediaTypeBuilder().Build(); - mediaType.AllowedAsRoot = true; - await MediaTypeService.CreateAsync(mediaType, Constants.Security.SuperUserKey); - - var media = new MediaBuilder() - .WithMediaType(mediaType) - .WithName("Media 1") - .Build(); - MediaService.Save(media); - - var contentCacheNode = new ContentCacheNode - { - Id = media.Id, - Key = media.Key, - ContentTypeId = media.ContentType.Id, - Data = new ContentData( - media.Name, - null, - 0, - media.UpdateDate, - media.WriterId, - null, - false, - new Dictionary(), - null), - }; - var result = PublishedContentFactory.ToIPublishedMedia(contentCacheNode); - Assert.IsNotNull(result); - Assert.AreEqual(media.Id, result.Id); - Assert.AreEqual(media.Name, result.Name); - - // Verify that requesting the same content again returns the same instance (from request cache). - var result2 = PublishedContentFactory.ToIPublishedMedia(contentCacheNode); - Assert.AreSame(result, result2); - } - - [Test] - public async Task Can_Create_Published_Member_For_Member() - { - var memberType = new MemberTypeBuilder().Build(); - await MemberTypeService.CreateAsync(memberType, Constants.Security.SuperUserKey); - - var member = new MemberBuilder() - .WithMemberType(memberType) - .WithName("Member 1") - .Build(); - MemberService.Save(member); - - var result = PublishedContentFactory.ToPublishedMember(member); - Assert.IsNotNull(result); - Assert.AreEqual(member.Id, result.Id); - Assert.AreEqual(member.Name, result.Name); - - // Verify that requesting the same content again returns the same instance (from request cache). - var result2 = PublishedContentFactory.ToPublishedMember(member); - Assert.AreSame(result, result2); - } -} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MediaPropertyCacheLevelTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MediaPropertyCacheLevelTests.cs new file mode 100644 index 0000000000..dea2f00881 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MediaPropertyCacheLevelTests.cs @@ -0,0 +1,111 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; +using Umbraco.Cms.Tests.Common.Builders; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; + +public class MediaPropertyCacheLevelTests : PropertyCacheLevelTestsBase +{ + private static readonly Guid _mediaKey = new("B4507763-591F-4E32-AD14-7EA67C6AE0D3"); + + private IMediaCacheService MediaCacheService => GetRequiredService(); + + private IMediaEditingService MediaEditingService => GetRequiredService(); + + private IMediaTypeEditingService MediaTypeEditingService => GetRequiredService(); + + [SetUp] + public async Task SetUpTest() + { + PropertyValueLevelDetectionTestsConverter.Reset(); + + var mediaTypeCreateModel = MediaTypeEditingBuilder.CreateMediaTypeWithOneProperty(propertyAlias: "title"); + var mediaTypeAttempt = await MediaTypeEditingService.CreateAsync(mediaTypeCreateModel, Constants.Security.SuperUserKey); + Assert.IsTrue(mediaTypeAttempt.Success); + + var mediaCreateModel = MediaEditingBuilder.CreateMediaWithAProperty(mediaTypeAttempt.Result.Key, "My Media", null, propertyAlias: "title", propertyValue: "The title"); + mediaCreateModel.Key = _mediaKey; + var mediaAttempt = await MediaEditingService.CreateAsync(mediaCreateModel, Constants.Security.SuperUserKey); + Assert.IsTrue(mediaAttempt.Success); + } + + [TestCase(PropertyCacheLevel.None, 1, 10)] + [TestCase(PropertyCacheLevel.Element, 1, 1)] + [TestCase(PropertyCacheLevel.Elements, 1, 1)] + public async Task Property_Value_Conversion_Respects_Property_Cache_Level(PropertyCacheLevel cacheLevel, int expectedSourceConverts, int expectedInterConverts) + { + PropertyValueLevelDetectionTestsConverter.SetCacheLevel(cacheLevel); + + var publishedContent1 = await MediaCacheService.GetByKeyAsync(_mediaKey); + Assert.IsNotNull(publishedContent1); + + var publishedContent2 = await MediaCacheService.GetByKeyAsync(_mediaKey); + Assert.IsNotNull(publishedContent2); + + Assert.AreSame(publishedContent1, publishedContent2); + + var titleValue1 = publishedContent1.Value("title"); + Assert.IsNotNull(titleValue1); + + var titleValue2 = publishedContent2.Value("title"); + Assert.IsNotNull(titleValue2); + + Assert.AreEqual(titleValue1, titleValue2); + + // fetch title values 10 times in total, 5 times from each published content instance + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + titleValue1 = publishedContent1.Value("title"); + + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + titleValue2 = publishedContent2.Value("title"); + + Assert.AreEqual(expectedSourceConverts, PropertyValueLevelDetectionTestsConverter.SourceConverts); + Assert.AreEqual(expectedInterConverts, PropertyValueLevelDetectionTestsConverter.InterConverts); + } + + [TestCase(PropertyCacheLevel.None)] + [TestCase(PropertyCacheLevel.Element)] + [TestCase(PropertyCacheLevel.Elements)] + public async Task Property_Value_Conversion_Is_Triggered_After_Cache_Refresh(PropertyCacheLevel cacheLevel) + { + PropertyValueLevelDetectionTestsConverter.SetCacheLevel(cacheLevel); + + var publishedContent1 = await MediaCacheService.GetByKeyAsync(_mediaKey); + Assert.IsNotNull(publishedContent1); + + var titleValue1 = publishedContent1.Value("title"); + Assert.AreEqual("The title", titleValue1); + + // save the media to trigger a cache refresh for the media + var mediaAttempt = await MediaEditingService.UpdateAsync( + _mediaKey, + new () + { + Properties = [new () { Alias = "title", Value = "New title" }], + Variants = [new() { Name = publishedContent1.Name }], + }, + Constants.Security.SuperUserKey); + Assert.IsTrue(mediaAttempt.Success); + + var publishedContent2 = await MediaCacheService.GetByKeyAsync(_mediaKey); + Assert.IsNotNull(publishedContent2); + + Assert.AreNotSame(publishedContent1, publishedContent2); + + var titleValue2 = publishedContent2.Value("title"); + Assert.AreEqual("New title", titleValue2); + + // expect conversions for each published content instance, due to the cache refresh + Assert.AreEqual(2, PropertyValueLevelDetectionTestsConverter.SourceConverts); + Assert.AreEqual(2, PropertyValueLevelDetectionTestsConverter.InterConverts); + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MemberPropertyCacheLevelTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MemberPropertyCacheLevelTests.cs new file mode 100644 index 0000000000..20ea900414 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/MemberPropertyCacheLevelTests.cs @@ -0,0 +1,96 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.ContentEditing; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.ContentTypeEditing; +using Umbraco.Cms.Infrastructure.HybridCache.Services; +using Umbraco.Cms.Tests.Common.Builders; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; + +public class MemberPropertyCacheLevelTests : PropertyCacheLevelTestsBase +{ + private static readonly Guid _memberKey = new("1ADC9048-E437-460B-95DC-3B8E19239CBD"); + + private IMemberCacheService MemberCacheService => GetRequiredService(); + + private IMemberEditingService MemberEditingService => GetRequiredService(); + + private IMemberTypeService MemberTypeService => GetRequiredService(); + + [SetUp] + public void SetUpTest() + => PropertyValueLevelDetectionTestsConverter.Reset(); + + [TestCase(PropertyCacheLevel.None, 2, 10)] + [TestCase(PropertyCacheLevel.Element, 2, 2)] + [TestCase(PropertyCacheLevel.Elements, 2, 10)] + public async Task Property_Value_Conversion_Respects_Property_Cache_Level(PropertyCacheLevel cacheLevel, int expectedSourceConverts, int expectedInterConverts) + { + PropertyValueLevelDetectionTestsConverter.SetCacheLevel(cacheLevel); + + var member = await CreateMember(); + + var publishedMember1 = await MemberCacheService.Get(member); + Assert.IsNotNull(publishedMember1); + + var publishedMember2 = await MemberCacheService.Get(member); + Assert.IsNotNull(publishedMember2); + + Assert.AreNotSame(publishedMember1, publishedMember2); + + var titleValue1 = publishedMember1.Value("title"); + Assert.AreEqual("The title", titleValue1); + + var titleValue2 = publishedMember2.Value("title"); + Assert.IsNotNull(titleValue2); + + Assert.AreEqual("The title", titleValue2); + + // fetch title values 10 times in total, 5 times from each published member instance + titleValue1 = publishedMember1.Value("title"); + titleValue1 = publishedMember1.Value("title"); + titleValue1 = publishedMember1.Value("title"); + titleValue1 = publishedMember1.Value("title"); + + titleValue2 = publishedMember2.Value("title"); + titleValue2 = publishedMember2.Value("title"); + titleValue2 = publishedMember2.Value("title"); + titleValue2 = publishedMember2.Value("title"); + + Assert.AreEqual(expectedSourceConverts, PropertyValueLevelDetectionTestsConverter.SourceConverts); + Assert.AreEqual(expectedInterConverts, PropertyValueLevelDetectionTestsConverter.InterConverts); + } + + private IUser SuperUser() => GetRequiredService().GetAsync(Constants.Security.SuperUserKey).GetAwaiter().GetResult(); + + private async Task CreateMember() + { + IMemberType memberType = MemberTypeBuilder.CreateSimpleMemberType(); + var memberTypeCreateResult = await MemberTypeService.UpdateAsync(memberType, Constants.Security.SuperUserKey); + Assert.IsTrue(memberTypeCreateResult.Success); + + var createModel = new MemberCreateModel + { + Key = _memberKey, + Email = "test@test.com", + Username = "test", + Password = "SuperSecret123", + IsApproved = true, + ContentTypeKey = memberType.Key, + Roles = [], + Variants = [new() { Name = "T. Est" }], + Properties = [new() { Alias = "title", Value = "The title" }], + }; + + var memberCreateResult = await MemberEditingService.CreateAsync(createModel, SuperUser()); + Assert.IsTrue(memberCreateResult.Success); + Assert.IsNotNull(memberCreateResult.Result.Content); + + return memberCreateResult.Result.Content; + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/PropertyCacheLevelTestsBase.cs b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/PropertyCacheLevelTestsBase.cs new file mode 100644 index 0000000000..92836f2aaf --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/PropertyCacheLevelTestsBase.cs @@ -0,0 +1,69 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Composing; +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.Sync; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; +using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.PublishedCache.HybridCache; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +public abstract class PropertyCacheLevelTestsBase : UmbracoIntegrationTest +{ + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + builder.AddNotificationHandler(); + builder.AddNotificationHandler(); + builder.Services.AddUnique(); + + builder.PropertyValueConverters().Append(); + } + + [HideFromTypeFinder] + public class PropertyValueLevelDetectionTestsConverter : PropertyValueConverterBase + { + private static PropertyCacheLevel _cacheLevel; + + public static void Reset() + => SourceConverts = InterConverts = 0; + + public static void SetCacheLevel(PropertyCacheLevel cacheLevel) + => _cacheLevel = cacheLevel; + + public static int SourceConverts { get; private set; } + + public static int InterConverts { get; private set; } + + public override bool IsConverter(IPublishedPropertyType propertyType) + => propertyType.EditorAlias is Constants.PropertyEditors.Aliases.TextBox or Constants.PropertyEditors.Aliases.TextArea; + + public override Type GetPropertyValueType(IPublishedPropertyType propertyType) + => typeof(string); + + public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType propertyType) + => _cacheLevel; + + public override object? ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object source, bool preview) + { + SourceConverts++; + return base.ConvertSourceToIntermediate(owner, propertyType, source, preview); + } + + public override object? ConvertIntermediateToObject( + IPublishedElement owner, + IPublishedPropertyType propertyType, + PropertyCacheLevel referenceCacheLevel, + object inter, + bool preview) + { + InterConverts++; + return base.ConvertIntermediateToObject(owner, propertyType, referenceCacheLevel, inter, preview); + } + } +} From 313b60aca53fc7dd8d4af15750ac17cb2eb876ab Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 3 Nov 2025 15:22:16 +0100 Subject: [PATCH 6/8] Load Balancing: Move temporary files and make them configurable to allow for media upload when load balancing the backoffice (#20717) * make file upload location configurable * Update src/Umbraco.Core/Configuration/Models/HostingSettings.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix default implementation --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Umbraco.Core/Configuration/Models/HostingSettings.cs | 6 ++++++ src/Umbraco.Core/Hosting/IHostingEnvironment.cs | 7 ++++++- .../Implement/LocalFileSystemTemporaryFileRepository.cs | 2 +- .../AspNetCore/AspNetCoreHostingEnvironment.cs | 4 ++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/HostingSettings.cs b/src/Umbraco.Core/Configuration/Models/HostingSettings.cs index c8df39b49a..99c1aa5649 100644 --- a/src/Umbraco.Core/Configuration/Models/HostingSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/HostingSettings.cs @@ -25,6 +25,12 @@ public class HostingSettings [DefaultValue(StaticLocalTempStorageLocation)] public LocalTempStorage LocalTempStorageLocation { get; set; } = Enum.Parse(StaticLocalTempStorageLocation); + /// + /// Gets or sets a value for the location of temporary file uploads. + /// + /// /umbraco/Data/TEMP/TemporaryFile if nothing is specified. + public string? TemporaryFileUploadLocation { get; set; } + /// /// Gets or sets a value indicating whether umbraco is running in [debug mode]. /// diff --git a/src/Umbraco.Core/Hosting/IHostingEnvironment.cs b/src/Umbraco.Core/Hosting/IHostingEnvironment.cs index 1dfa72039c..15a7ef74e3 100644 --- a/src/Umbraco.Core/Hosting/IHostingEnvironment.cs +++ b/src/Umbraco.Core/Hosting/IHostingEnvironment.cs @@ -43,7 +43,12 @@ public interface IHostingEnvironment string LocalTempPath { get; } /// - /// The web application's hosted path + /// Gets the location of temporary file uploads. + /// + public string TemporaryFileUploadPath => Path.Combine(LocalTempPath, "TemporaryFile"); + + /// + /// The web application's hosted path. /// /// /// In most cases this will return "/" but if the site is hosted in a virtual directory then this will return the diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LocalFileSystemTemporaryFileRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LocalFileSystemTemporaryFileRepository.cs index 0190cd2034..a369a6e87d 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LocalFileSystemTemporaryFileRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/LocalFileSystemTemporaryFileRepository.cs @@ -27,7 +27,7 @@ internal sealed class LocalFileSystemTemporaryFileRepository : ITemporaryFileRep private DirectoryInfo GetRootDirectory() { - var path = Path.Combine(_hostingEnvironment.LocalTempPath, "TemporaryFile"); + var path = _hostingEnvironment.TemporaryFileUploadPath; if (!Directory.Exists(path)) { diff --git a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs index 324781b5a3..9a57147bcc 100644 --- a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs +++ b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs @@ -132,6 +132,10 @@ public class AspNetCoreHostingEnvironment : IHostingEnvironment } } + /// + public string TemporaryFileUploadPath => _hostingSettings.CurrentValue.TemporaryFileUploadLocation + ?? Path.Combine(MapPathContentRoot(Core.Constants.SystemDirectories.TempData), "TemporaryFile"); + /// public string MapPathWebRoot(string path) => _webHostEnvironment.MapPathWebRoot(path); From a4d893a7b49b86fa912b4eebfb041634832fa9a3 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Mon, 3 Nov 2025 16:24:55 +0100 Subject: [PATCH 7/8] Rich text editor: Treat an "empty" value as a non-value (closes #20454) (#20719) * Make the RTE treat an "empty" value as a non-value * Additional tests * Add tests for invariant and variant content. --------- Co-authored-by: Andy Butland --- .../RteBlockRenderingValueConverter.cs | 13 ++ .../RichTextPropertyEditorTests.cs | 113 ++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteBlockRenderingValueConverter.cs b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteBlockRenderingValueConverter.cs index d39d13e243..65c6835f6b 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteBlockRenderingValueConverter.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/RteBlockRenderingValueConverter.cs @@ -72,6 +72,19 @@ public class RteBlockRenderingValueConverter : SimpleRichTextValueConverter, IDe // to be cached at the published snapshot level, because we have no idea what the block renderings may depend on actually. PropertyCacheLevel.Snapshot; + /// + public override bool? IsValue(object? value, PropertyValueLevel level) + => level switch + { + // we cannot determine if an RTE has a value at source level, because some RTEs might + // be saved with an "empty" representation like {"markup":"","blocks":null}. + PropertyValueLevel.Source => null, + // we assume the RTE has a value if the intermediate value has markup beyond an empty paragraph tag. + PropertyValueLevel.Inter => value is IRichTextEditorIntermediateValue { Markup.Length: > 0 } intermediateValue + && intermediateValue.Markup != "

", + _ => throw new ArgumentOutOfRangeException(nameof(level), level, null) + }; + // to counterweigh the cache level, we're going to do as much of the heavy lifting as we can while converting source to intermediate public override object? ConvertSourceToIntermediate(IPublishedElement owner, IPublishedPropertyType propertyType, object? source, bool preview) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditorTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditorTests.cs index b1f908d852..41ee397548 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditorTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditorTests.cs @@ -1,13 +1,19 @@ using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Blocks; +using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; +using Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.PropertyEditors; @@ -23,6 +29,14 @@ internal sealed class RichTextPropertyEditorTests : UmbracoIntegrationTest private IJsonSerializer JsonSerializer => GetRequiredService(); + private IPublishedContentCache PublishedContentCache => GetRequiredService(); + + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + builder.AddNotificationHandler(); + builder.Services.AddUnique(); + } + [Test] public void Can_Use_Markup_String_As_Value() { @@ -180,4 +194,103 @@ internal sealed class RichTextPropertyEditorTests : UmbracoIntegrationTest Assert.IsNotNull(tags.Single(tag => tag.Text == "Tag Two")); Assert.IsNotNull(tags.Single(tag => tag.Text == "Tag Three")); } + + [TestCase(null, false)] + [TestCase("", false)] + [TestCase("""{"markup":"","blocks":null}""", false)] + [TestCase("""{"markup":"

","blocks":null}""", false)] + [TestCase("abc", true)] + [TestCase("""{"markup":"abc","blocks":null}""", true)] + public async Task Can_Handle_Empty_Value_Representations_For_Invariant_Content(string? rteValue, bool expectedHasValue) + { + var contentType = await CreateContentTypeForEmptyValueTests(); + + var content = new ContentBuilder() + .WithContentType(contentType) + .WithName("Page") + .WithPropertyValues( + new + { + rte = rteValue + }) + .Build(); + + var contentResult = ContentService.Save(content); + Assert.IsTrue(contentResult.Success); + + var publishResult = ContentService.Publish(content, []); + Assert.IsTrue(publishResult.Success); + + var publishedContent = await PublishedContentCache.GetByIdAsync(content.Key); + Assert.IsNotNull(publishedContent); + + var publishedProperty = publishedContent.Properties.First(property => property.Alias == "rte"); + Assert.AreEqual(expectedHasValue, publishedProperty.HasValue()); + + Assert.AreEqual(expectedHasValue, publishedContent.HasValue("rte")); + } + + [TestCase(null, false)] + [TestCase("", false)] + [TestCase("""{"markup":"","blocks":null}""", false)] + [TestCase("""{"markup":"

","blocks":null}""", false)] + [TestCase("abc", true)] + [TestCase("""{"markup":"abc","blocks":null}""", true)] + public async Task Can_Handle_Empty_Value_Representations_For_Variant_Content(string? rteValue, bool expectedHasValue) + { + var contentType = await CreateContentTypeForEmptyValueTests(ContentVariation.Culture); + + var content = new ContentBuilder() + .WithContentType(contentType) + .WithName("Page") + .WithCultureName("en-US", "Page") + .WithPropertyValues( + new + { + rte = rteValue + }, + "en-US") + .Build(); + + var contentResult = ContentService.Save(content); + Assert.IsTrue(contentResult.Success); + + var publishResult = ContentService.Publish(content, ["en-US"]); + Assert.IsTrue(publishResult.Success); + + var publishedContent = await PublishedContentCache.GetByIdAsync(content.Key); + Assert.IsNotNull(publishedContent); + + var publishedProperty = publishedContent.Properties.First(property => property.Alias == "rte"); + Assert.AreEqual(expectedHasValue, publishedProperty.HasValue("en-US")); + + Assert.AreEqual(expectedHasValue, publishedContent.HasValue("rte", "en-US")); + } + + private async Task CreateContentTypeForEmptyValueTests(ContentVariation contentVariation = ContentVariation.Nothing) + { + var contentType = new ContentTypeBuilder() + .WithAlias("myPage") + .WithName("My Page") + .WithContentVariation(contentVariation) + .AddPropertyGroup() + .WithAlias("content") + .WithName("Content") + .WithSupportsPublishing(true) + .AddPropertyType() + .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.RichText) + .WithDataTypeId(Constants.DataTypes.RichtextEditor) + .WithValueStorageType(ValueStorageType.Ntext) + .WithAlias("rte") + .WithName("RTE") + .WithVariations(contentVariation) + .Done() + .Done() + .Build(); + + var contentTypeResult = await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + Assert.IsTrue(contentTypeResult.Success); + + return contentType; + } } From b68a6a9502dc291abb0b61343dfad84a5ae7531c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Mon, 3 Nov 2025 17:09:31 +0100 Subject: [PATCH 8/8] Data Type: use Property Editor UI label instead over name (#20716) Prioritize using the property editor ui label and localize it. --- .../data-type-picker-flow-modal.element.ts | 5 ++++- .../data-type/workspace/data-type-workspace.context.ts | 2 +- ...type-details-workspace-property-editor-picker.element.ts | 2 +- .../tests/DefaultConfig/DataType/RichTextEditor.spec.ts | 6 ++---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/data-type/modals/data-type-picker-flow/data-type-picker-flow-modal.element.ts b/src/Umbraco.Web.UI.Client/src/packages/data-type/modals/data-type-picker-flow/data-type-picker-flow-modal.element.ts index a04ad47150..ef1cf7fc3a 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/data-type/modals/data-type-picker-flow/data-type-picker-flow-modal.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/data-type/modals/data-type-picker-flow/data-type-picker-flow-modal.element.ts @@ -142,7 +142,10 @@ export class UmbDataTypePickerFlowModalElement extends UmbModalBaseElement< } this.removeUmbController(contentContextConsumer); this.removeUmbController(propContextConsumer); - const propertyEditorName = this.#propertyEditorUIs.find((ui) => ui.alias === params.uiAlias)?.name; + const propertyEditorUiManifest = this.#propertyEditorUIs.find((ui) => ui.alias === params.uiAlias); + const propertyEditorName = this.localize.string( + propertyEditorUiManifest?.meta?.label || propertyEditorUiManifest?.name || '#general_notFound', + ); const dataTypeName = `${contentContext?.getName() ?? ''} - ${propContext.getName() ?? ''} - ${propertyEditorName}`; return { diff --git a/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/data-type-workspace.context.ts b/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/data-type-workspace.context.ts index 19d507f51e..0c924e6458 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/data-type-workspace.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/data-type-workspace.context.ts @@ -208,7 +208,7 @@ export class UmbDataTypeWorkspaceContext umbExtensionsRegistry.byTypeAndAlias('propertyEditorUi', propertyEditorUIAlias), (manifest) => { this.#propertyEditorUiIcon.setValue(manifest?.meta.icon || null); - this.#propertyEditorUiName.setValue(manifest?.name || null); + this.#propertyEditorUiName.setValue(manifest?.meta?.label || manifest?.name || null); // Maps properties to have a weight, so they can be sorted, notice UI properties have a +1000 weight compared to schema properties. this.#propertyEditorUISettingsProperties = (manifest?.meta.settings?.properties ?? []).map((x, i) => ({ diff --git a/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/views/details/data-type-details-workspace-property-editor-picker.element.ts b/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/views/details/data-type-details-workspace-property-editor-picker.element.ts index 706f8a9a37..1f493682bf 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/views/details/data-type-details-workspace-property-editor-picker.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/views/details/data-type-details-workspace-property-editor-picker.element.ts @@ -83,7 +83,7 @@ export class UmbDataTypeDetailsWorkspacePropertyEditorPickerElement extends UmbF #renderPropertyEditorReference() { if (!this.propertyEditorUiAlias || !this.propertyEditorSchemaAlias) return nothing; - let name = this.propertyEditorUiName; + let name = this.localize.string(this.propertyEditorUiName); let alias = this.propertyEditorUiAlias; let error = false; diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/DataType/RichTextEditor.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/DataType/RichTextEditor.spec.ts index 8a02804572..57382d6eac 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/DataType/RichTextEditor.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/DataType/RichTextEditor.spec.ts @@ -2,7 +2,6 @@ import {expect} from "@playwright/test"; const dataTypeName = 'Richtext editor'; -const tipTapPropertyEditorName = 'Rich Text Editor [Tiptap] Property Editor UI'; const tipTapAlias = 'Umbraco.RichText'; const tipTapUiAlias = 'Umb.PropertyEditorUi.Tiptap'; const extensionsDefaultValue = [ @@ -81,10 +80,9 @@ test('tiptap is the default property editor in rich text editor', async ({umbrac // Act await umbracoUi.dataType.goToDataType(dataTypeName); - // Assert + // Assert await umbracoUi.dataType.doesSettingHaveValue(ConstantHelper.tipTapSettings); await umbracoUi.dataType.doesSettingItemsHaveCount(ConstantHelper.tipTapSettings); - await umbracoUi.dataType.doesPropertyEditorHaveName(tipTapPropertyEditorName); await umbracoUi.dataType.doesPropertyEditorHaveAlias(tipTapAlias); await umbracoUi.dataType.doesPropertyEditorHaveUiAlias(tipTapUiAlias); const dataTypeData = await umbracoApi.dataType.getByName(dataTypeName); @@ -95,4 +93,4 @@ test('tiptap is the default property editor in rich text editor', async ({umbrac expect(await umbracoApi.dataType.doesTiptapExtensionsItemsMatchCount(dataTypeName, extensionsDefaultValue.length)).toBeTruthy(); expect(await umbracoApi.dataType.doesTiptapExtensionsHaveItems(dataTypeName, extensionsDefaultValue)).toBeTruthy(); expect(await umbracoApi.dataType.doesTiptapToolbarHaveItems(dataTypeName, toolbarDefaultValue)).toBeTruthy(); -}); \ No newline at end of file +});