Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
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;
Expand Down Expand Up @@ -435,6 +436,13 @@
.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>();

Check warning on line 445 in src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Large Method

AddCoreNotifications increases from 96 to 101 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}

Check warning on line 50 in src/Umbraco.Infrastructure/Persistence/Relations/ContentRelationsUpdate.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Constructor Over-Injection

ContentRelationsUpdate has 6 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.

/// <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);
}
}

Check warning on line 161 in src/Umbraco.Infrastructure/Persistence/Relations/ContentRelationsUpdate.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

PersistRelations has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

private sealed class NodeIdKey
{
[Column("id")]
public int NodeId { get; set; }

[Column("uniqueId")]
public Guid UniqueId { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
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;
Expand Down Expand Up @@ -1080,81 +1079,9 @@

#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.");

Check notice on line 1084 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Method

PersistRelations is no longer above the threshold for cyclomatic complexity. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

/// <summary>
/// Inserts property values for the content entity
Expand Down Expand Up @@ -1230,14 +1157,5 @@
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; }
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Globalization;

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Lines of Code in a Single File

The lines of code decreases from 1178 to 1176, improve code health by reducing it to 1000. The number of Lines of Code in a single file. More Lines of Code lowers the code health.
using Microsoft.Extensions.Logging;
using NPoco;
using Umbraco.Cms.Core;
Expand Down Expand Up @@ -1074,8 +1074,6 @@
ClearEntityTags(entity, _tagRepository);
}

PersistRelations(entity);

entity.ResetDirtyProperties();

// troubleshooting
Expand Down Expand Up @@ -1325,8 +1323,6 @@
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?
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,6 @@ protected override void PersistNewItem(IMedia entity)
// set tags
SetEntityTags(entity, _tagRepository, _serializer);

PersistRelations(entity);

OnUowRefreshedEntity(new MediaRefreshNotification(entity, new EventMessages()));

entity.ResetDirtyProperties();
Expand Down Expand Up @@ -477,8 +475,6 @@ protected override void PersistUpdatedItem(IMedia entity)
ReplacePropertyValues(entity, entity.VersionId, 0, out _, out _);

SetEntityTags(entity, _tagRepository, _serializer);

PersistRelations(entity);
}

OnUowRefreshedEntity(new MediaRefreshNotification(entity, new EventMessages()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,6 @@ protected override void PersistNewItem(IMember entity)

SetEntityTags(entity, _tagRepository, _jsonSerializer);

PersistRelations(entity);

OnUowRefreshedEntity(new MemberRefreshNotification(entity, new EventMessages()));

entity.ResetDirtyProperties();
Expand Down Expand Up @@ -938,8 +936,6 @@ protected override void PersistUpdatedItem(IMember entity)

SetEntityTags(entity, _tagRepository, _jsonSerializer);

PersistRelations(entity);

OnUowRefreshedEntity(new MemberRefreshNotification(entity, new EventMessages()));

_memberByUsernameCachePolicy.DeleteByUserName(CacheKeys.MemberUserNameCachePrefix, entity.Username);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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()
{
Expand Down
Loading