Move persistence of relations from repository into notification handlers (#20095)

* Move persistance of relations from repository into notification handlers.

* Applied suggestions from code review.

* Apply suggestions from code review

Co-authored-by: Ronald Barendse <ronald@barend.se>

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fixed build.

* Fixed failing integration tests.

---------

Co-authored-by: Ronald Barendse <ronald@barend.se>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Andy Butland
2025-09-09 16:03:20 +02:00
committed by GitHub
parent 00092b5061
commit be8fee199f
9 changed files with 207 additions and 105 deletions

View File

@@ -52,6 +52,7 @@ using Umbraco.Cms.Infrastructure.Migrations;
using Umbraco.Cms.Infrastructure.Migrations.Install;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.Mappers;
using Umbraco.Cms.Infrastructure.Persistence.Relations;
using Umbraco.Cms.Infrastructure.PropertyEditors.NotificationHandlers;
using Umbraco.Cms.Infrastructure.Routing;
using Umbraco.Cms.Infrastructure.Runtime;
@@ -435,6 +436,13 @@ public static partial class UmbracoBuilderExtensions
.AddNotificationAsyncHandler<ContentTypeSavingNotification, WarnDocumentTypeElementSwitchNotificationHandler>()
.AddNotificationAsyncHandler<ContentTypeSavedNotification, WarnDocumentTypeElementSwitchNotificationHandler>();
// Handles for relation persistence on content save.
builder
.AddNotificationHandler<ContentSavedNotification, ContentRelationsUpdate>()
.AddNotificationHandler<ContentPublishedNotification, ContentRelationsUpdate>()
.AddNotificationHandler<MediaSavedNotification, ContentRelationsUpdate>()
.AddNotificationHandler<MemberSavedNotification, ContentRelationsUpdate>();
return builder;
}

View File

@@ -0,0 +1,171 @@
using Microsoft.Extensions.Logging;
using NPoco;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence.Querying;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Infrastructure.Persistence.Dtos;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Extensions;
namespace Umbraco.Cms.Infrastructure.Persistence.Relations;
/// <summary>
/// Defines a notification handler for content saved operations that persists relations.
/// </summary>
internal sealed class ContentRelationsUpdate :
IDistributedCacheNotificationHandler<ContentSavedNotification>,
IDistributedCacheNotificationHandler<ContentPublishedNotification>,
IDistributedCacheNotificationHandler<MediaSavedNotification>,
IDistributedCacheNotificationHandler<MemberSavedNotification>
{
private readonly IScopeProvider _scopeProvider;
private readonly DataValueReferenceFactoryCollection _dataValueReferenceFactories;
private readonly PropertyEditorCollection _propertyEditors;
private readonly IRelationRepository _relationRepository;
private readonly IRelationTypeRepository _relationTypeRepository;
private readonly ILogger<ContentRelationsUpdate> _logger;
/// <summary>
/// Initializes a new instance of the <see cref="ContentRelationsUpdate"/> class.
/// </summary>
public ContentRelationsUpdate(
IScopeProvider scopeProvider,
DataValueReferenceFactoryCollection dataValueReferenceFactories,
PropertyEditorCollection propertyEditors,
IRelationRepository relationRepository,
IRelationTypeRepository relationTypeRepository,
ILogger<ContentRelationsUpdate> logger)
{
_scopeProvider = scopeProvider;
_dataValueReferenceFactories = dataValueReferenceFactories;
_propertyEditors = propertyEditors;
_relationRepository = relationRepository;
_relationTypeRepository = relationTypeRepository;
_logger = logger;
}
/// <inheritdoc/>
public void Handle(ContentSavedNotification notification) => PersistRelations(notification.SavedEntities);
/// <inheritdoc/>
public void Handle(IEnumerable<ContentSavedNotification> notifications) => PersistRelations(notifications.SelectMany(x => x.SavedEntities));
/// <inheritdoc/>
public void Handle(ContentPublishedNotification notification) => PersistRelations(notification.PublishedEntities);
/// <inheritdoc/>
public void Handle(IEnumerable<ContentPublishedNotification> notifications) => PersistRelations(notifications.SelectMany(x => x.PublishedEntities));
/// <inheritdoc/>
public void Handle(MediaSavedNotification notification) => PersistRelations(notification.SavedEntities);
/// <inheritdoc/>
public void Handle(IEnumerable<MediaSavedNotification> notifications) => PersistRelations(notifications.SelectMany(x => x.SavedEntities));
/// <inheritdoc/>
public void Handle(MemberSavedNotification notification) => PersistRelations(notification.SavedEntities);
/// <inheritdoc/>
public void Handle(IEnumerable<MemberSavedNotification> notifications) => PersistRelations(notifications.SelectMany(x => x.SavedEntities));
private void PersistRelations(IEnumerable<IContentBase> entities)
{
using IScope scope = _scopeProvider.CreateScope();
foreach (IContentBase entity in entities)
{
PersistRelations(scope, entity);
}
scope.Complete();
}
private void PersistRelations(IScope scope, IContentBase entity)
{
// Get all references and automatic relation type aliases.
ISet<UmbracoEntityReference> references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, _propertyEditors);
ISet<string> automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(_propertyEditors);
if (references.Count == 0)
{
// Delete all relations using the automatic relation type aliases.
_relationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray());
// No need to add new references/relations
return;
}
// Lookup all relation type IDs.
var relationTypeLookup = _relationTypeRepository.GetMany(Array.Empty<int>())
.Where(x => automaticRelationTypeAliases.Contains(x.Alias))
.ToDictionary(x => x.Alias, x => x.Id);
// Lookup node IDs for all GUID based UDIs.
IEnumerable<Guid> keys = references.Select(x => x.Udi).OfType<GuidUdi>().Select(x => x.Guid);
var keysLookup = scope.Database.FetchByGroups<NodeIdKey, Guid>(keys, Constants.Sql.MaxParameterCount, guids =>
{
return scope.SqlContext.Sql()
.Select<NodeDto>(x => x.NodeId, x => x.UniqueId)
.From<NodeDto>()
.WhereIn<NodeDto>(x => x.UniqueId, guids);
}).ToDictionary(x => x.UniqueId, x => x.NodeId);
// Get all valid relations.
var relations = new List<(int ChildId, int RelationTypeId)>(references.Count);
foreach (UmbracoEntityReference reference in references)
{
if (string.IsNullOrEmpty(reference.RelationTypeAlias))
{
// Reference does not specify a relation type alias, so skip adding a relation.
_logger.LogDebug("The reference to {Udi} does not specify a relation type alias, so it will not be saved as relation.", reference.Udi);
}
else if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias))
{
// Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code.
_logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias);
}
else if (!relationTypeLookup.TryGetValue(reference.RelationTypeAlias, out int relationTypeId))
{
// A non-existent relation type could be caused by an environment issue (e.g. it was manually removed).
_logger.LogWarning("The reference to {Udi} uses a relation type {RelationTypeAlias} that does not exist.", reference.Udi, reference.RelationTypeAlias);
}
else if (reference.Udi is not GuidUdi udi || !keysLookup.TryGetValue(udi.Guid, out var id))
{
// Relations only support references to items that are stored in the NodeDto table (because of foreign key constraints).
_logger.LogInformation("The reference to {Udi} can not be saved as relation, because it doesn't have a node ID.", reference.Udi);
}
else
{
relations.Add((id, relationTypeId));
}
}
// Get all existing relations (optimize for adding new and keeping existing relations).
IQuery<IRelation> query = scope.SqlContext.Query<IRelation>().Where(x => x.ParentId == entity.Id).WhereIn(x => x.RelationTypeId, relationTypeLookup.Values);
var existingRelations = _relationRepository.GetPagedRelationsByQuery(query, 0, int.MaxValue, out _, null)
.ToDictionary(x => (x.ChildId, x.RelationTypeId)); // Relations are unique by parent ID, child ID and relation type ID.
// Add relations that don't exist yet.
IEnumerable<ReadOnlyRelation> relationsToAdd = relations.Except(existingRelations.Keys).Select(x => new ReadOnlyRelation(entity.Id, x.ChildId, x.RelationTypeId));
_relationRepository.SaveBulk(relationsToAdd);
// Delete relations that don't exist anymore.
foreach (IRelation relation in existingRelations.Where(x => !relations.Contains(x.Key)).Select(x => x.Value))
{
_relationRepository.Delete(relation);
}
}
private sealed class NodeIdKey
{
[Column("id")]
public int NodeId { get; set; }
[Column("uniqueId")]
public Guid UniqueId { get; set; }
}
}

View File

@@ -6,7 +6,6 @@ using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence;
using Umbraco.Cms.Core.Persistence.Querying;
@@ -1080,81 +1079,9 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
#endregion
[Obsolete("This method is no longer used as the persistance of relations has been moved to the ContentRelationsUpdate notification handler. Scheduled for removal in Umbraco 18.")]
protected void PersistRelations(TEntity entity)
{
// Get all references and automatic relation type aliases
ISet<UmbracoEntityReference> references = _dataValueReferenceFactories.GetAllReferences(entity.Properties, PropertyEditors);
ISet<string> automaticRelationTypeAliases = _dataValueReferenceFactories.GetAllAutomaticRelationTypesAliases(PropertyEditors);
if (references.Count == 0)
{
// Delete all relations using the automatic relation type aliases
RelationRepository.DeleteByParent(entity.Id, automaticRelationTypeAliases.ToArray());
// No need to add new references/relations
return;
}
// Lookup all relation type IDs
var relationTypeLookup = RelationTypeRepository.GetMany(Array.Empty<int>())
.Where(x => automaticRelationTypeAliases.Contains(x.Alias))
.ToDictionary(x => x.Alias, x => x.Id);
// Lookup node IDs for all GUID based UDIs
IEnumerable<Guid> keys = references.Select(x => x.Udi).OfType<GuidUdi>().Select(x => x.Guid);
var keysLookup = Database.FetchByGroups<NodeIdKey, Guid>(keys, Constants.Sql.MaxParameterCount, guids =>
{
return Sql()
.Select<NodeDto>(x => x.NodeId, x => x.UniqueId)
.From<NodeDto>()
.WhereIn<NodeDto>(x => x.UniqueId, guids);
}).ToDictionary(x => x.UniqueId, x => x.NodeId);
// Get all valid relations
var relations = new List<(int ChildId, int RelationTypeId)>(references.Count);
foreach (UmbracoEntityReference reference in references)
{
if (string.IsNullOrEmpty(reference.RelationTypeAlias))
{
// Reference does not specify a relation type alias, so skip adding a relation
Logger.LogDebug("The reference to {Udi} does not specify a relation type alias, so it will not be saved as relation.", reference.Udi);
}
else if (!automaticRelationTypeAliases.Contains(reference.RelationTypeAlias))
{
// Returning a reference that doesn't use an automatic relation type is an issue that should be fixed in code
Logger.LogError("The reference to {Udi} uses a relation type {RelationTypeAlias} that is not an automatic relation type.", reference.Udi, reference.RelationTypeAlias);
}
else if (!relationTypeLookup.TryGetValue(reference.RelationTypeAlias, out int relationTypeId))
{
// A non-existent relation type could be caused by an environment issue (e.g. it was manually removed)
Logger.LogWarning("The reference to {Udi} uses a relation type {RelationTypeAlias} that does not exist.", reference.Udi, reference.RelationTypeAlias);
}
else if (reference.Udi is not GuidUdi udi || !keysLookup.TryGetValue(udi.Guid, out var id))
{
// Relations only support references to items that are stored in the NodeDto table (because of foreign key constraints)
Logger.LogInformation("The reference to {Udi} can not be saved as relation, because doesn't have a node ID.", reference.Udi);
}
else
{
relations.Add((id, relationTypeId));
}
}
// Get all existing relations (optimize for adding new and keeping existing relations)
var query = Query<IRelation>().Where(x => x.ParentId == entity.Id).WhereIn(x => x.RelationTypeId, relationTypeLookup.Values);
var existingRelations = RelationRepository.GetPagedRelationsByQuery(query, 0, int.MaxValue, out _, null)
.ToDictionary(x => (x.ChildId, x.RelationTypeId)); // Relations are unique by parent ID, child ID and relation type ID
// Add relations that don't exist yet
var relationsToAdd = relations.Except(existingRelations.Keys).Select(x => new ReadOnlyRelation(entity.Id, x.ChildId, x.RelationTypeId));
RelationRepository.SaveBulk(relationsToAdd);
// Delete relations that don't exist anymore
foreach (IRelation relation in existingRelations.Where(x => !relations.Contains(x.Key)).Select(x => x.Value))
{
RelationRepository.Delete(relation);
}
}
=> Logger.LogWarning("ContentRepositoryBase.PersistRelations was called but this is now an obsolete, no-op method that is unused in Umbraco. No relations were persisted. Relations persistence has moved to the ContentRelationsUpdate notification handler.");
/// <summary>
/// Inserts property values for the content entity
@@ -1230,14 +1157,5 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
Database.Execute(SqlContext.Sql().Delete<PropertyDataDto>().WhereIn<PropertyDataDto>(x => x.Id, existingPropDataIds));
}
}
private sealed class NodeIdKey
{
[Column("id")]
public int NodeId { get; set; }
[Column("uniqueId")]
public Guid UniqueId { get; set; }
}
}
}

View File

@@ -1074,8 +1074,6 @@ public class DocumentRepository : ContentRepositoryBase<int, IContent, DocumentR
ClearEntityTags(entity, _tagRepository);
}
PersistRelations(entity);
entity.ResetDirtyProperties();
// troubleshooting
@@ -1325,8 +1323,6 @@ public class DocumentRepository : ContentRepositoryBase<int, IContent, DocumentR
ClearEntityTags(entity, _tagRepository);
}
PersistRelations(entity);
// TODO: note re. tags: explicitly unpublished entities have cleared tags, but masked or trashed entities *still* have tags in the db - so what?
}

View File

@@ -416,8 +416,6 @@ public class MediaRepository : ContentRepositoryBase<int, IMedia, MediaRepositor
// set tags
SetEntityTags(entity, _tagRepository, _serializer);
PersistRelations(entity);
OnUowRefreshedEntity(new MediaRefreshNotification(entity, new EventMessages()));
entity.ResetDirtyProperties();
@@ -477,8 +475,6 @@ public class MediaRepository : ContentRepositoryBase<int, IMedia, MediaRepositor
ReplacePropertyValues(entity, entity.VersionId, 0, out _, out _);
SetEntityTags(entity, _tagRepository, _serializer);
PersistRelations(entity);
}
OnUowRefreshedEntity(new MediaRefreshNotification(entity, new EventMessages()));

View File

@@ -788,8 +788,6 @@ public class MemberRepository : ContentRepositoryBase<int, IMember, MemberReposi
SetEntityTags(entity, _tagRepository, _jsonSerializer);
PersistRelations(entity);
OnUowRefreshedEntity(new MemberRefreshNotification(entity, new EventMessages()));
entity.ResetDirtyProperties();
@@ -938,8 +936,6 @@ public class MemberRepository : ContentRepositoryBase<int, IMember, MemberReposi
SetEntityTags(entity, _tagRepository, _jsonSerializer);
PersistRelations(entity);
OnUowRefreshedEntity(new MemberRefreshNotification(entity, new EventMessages()));
_memberByUsernameCachePolicy.DeleteByUserName(CacheKeys.MemberUserNameCachePrefix, entity.Username);

View File

@@ -1,16 +1,15 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Infrastructure.Persistence.Relations;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Cms.Tests.Integration.Testing;
using Umbraco.Extensions;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
@@ -32,6 +31,13 @@ internal sealed class RelationServiceTests : UmbracoIntegrationTest
private IRelationService RelationService => GetRequiredService<IRelationService>();
protected override void CustomTestSetup(IUmbracoBuilder builder)
{
base.CustomTestSetup(builder);
builder
.AddNotificationHandler<ContentSavedNotification, ContentRelationsUpdate>();
}
[Test]
public void Get_Paged_Relations_By_Relation_Type()
{

View File

@@ -3,7 +3,9 @@ using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Infrastructure.Persistence.Relations;
using Umbraco.Cms.Tests.Common.Attributes;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Testing;
@@ -29,11 +31,12 @@ internal sealed class TrackRelationsTests : UmbracoIntegrationTestWithContent
private IRelationService RelationService => GetRequiredService<IRelationService>();
// protected override void CustomTestSetup(IUmbracoBuilder builder)
// {
// base.CustomTestSetup(builder);
// builder.AddNuCache();
// }
protected override void CustomTestSetup(IUmbracoBuilder builder)
{
base.CustomTestSetup(builder);
builder
.AddNotificationHandler<ContentSavedNotification, ContentRelationsUpdate>();
}
[Test]
[LongRunning]
@@ -89,6 +92,5 @@ internal sealed class TrackRelationsTests : UmbracoIntegrationTestWithContent
Assert.AreEqual(c1.Id, relations[2].ChildId);
Assert.AreEqual(Constants.Conventions.RelationTypes.RelatedMemberAlias, relations[3].RelationType.Alias);
Assert.AreEqual(member.Id, relations[3].ChildId);
}
}

View File

@@ -3,7 +3,9 @@
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Infrastructure.Persistence.Relations;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
using Umbraco.Cms.Tests.Common.Testing;
@@ -25,6 +27,13 @@ internal class TrackedReferencesServiceTests : UmbracoIntegrationTest
private IContentType ContentType { get; set; }
protected override void CustomTestSetup(IUmbracoBuilder builder)
{
base.CustomTestSetup(builder);
builder
.AddNotificationHandler<ContentSavedNotification, ContentRelationsUpdate>();
}
[SetUp]
public void Setup() => CreateTestData();