diff --git a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs index c35ae6d2ae..5d137d04c6 100644 --- a/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -1,4 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Umbraco.Cms.Core.Composing; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Editors; @@ -12,14 +15,27 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase _logger; + /// /// Initializes a new instance of the class. /// /// The items. + [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 17.")] public DataValueReferenceFactoryCollection(Func> items) - : base(items) + : this( + items, + StaticServiceProvider.Instance.GetRequiredService>()) { } + /// + /// Initializes a new instance of the class. + /// + /// The items. + /// The logger. + public DataValueReferenceFactoryCollection(Func> items, ILogger logger) + : base(items) => _logger = logger; + /// /// Gets all unique references from the specified properties. /// @@ -33,7 +49,7 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase(); // Group by property editor alias to avoid duplicate lookups and optimize value parsing - foreach (var propertyValuesByPropertyEditorAlias in properties.GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Values)) + foreach (IGrouping> propertyValuesByPropertyEditorAlias in properties.GroupBy(x => x.PropertyType.PropertyEditorAlias, x => x.Values)) { if (!propertyEditors.TryGet(propertyValuesByPropertyEditorAlias.Key, out IDataEditor? dataEditor)) { @@ -48,7 +64,7 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase public ISet GetReferences(IDataEditor dataEditor, IEnumerable values) => - GetReferencesEnumerable(dataEditor, values).ToHashSet(); - private IEnumerable GetReferencesEnumerable(IDataEditor dataEditor, IEnumerable values) + GetReferencesEnumerable(dataEditor, values, null).ToHashSet(); + + private ISet GetReferences(IDataEditor dataEditor, IEnumerable values, string propertyEditorAlias) => + GetReferencesEnumerable(dataEditor, values, propertyEditorAlias).ToHashSet(); + + private IEnumerable GetReferencesEnumerable(IDataEditor dataEditor, IEnumerable values, string? propertyEditorAlias) { // TODO: We will need to change this once we support tracking via variants/segments // for now, we are tracking values from ALL variants if (dataEditor.GetValueEditor() is IDataValueReference dataValueReference) { - foreach (UmbracoEntityReference reference in values.SelectMany(dataValueReference.GetReferences)) + foreach (UmbracoEntityReference reference in GetReferencesFromPropertyValues(values, dataValueReference, propertyEditorAlias)) { yield return reference; } @@ -107,6 +127,38 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase GetReferencesFromPropertyValues(IEnumerable values, IDataValueReference dataValueReference, string? propertyEditorAlias) + { + var result = new List(); + foreach (var value in values) + { + // When property editors on data types are changed, we could have values that are incompatible with the new editor. + // Leading to issues such as: + // - https://github.com/umbraco/Umbraco-CMS/issues/17628 + // - https://github.com/umbraco/Umbraco-CMS/issues/17725 + // Although some changes like this are not intended to be compatible, we should handle them gracefully and not + // error in retrieving references, which would prevent manipulating or deleting the content that uses the data type. + try + { + IEnumerable references = dataValueReference.GetReferences(value); + result.AddRange(references); + } + catch (Exception ex) + { + // Log the exception but don't throw, continue with the next value. + _logger.LogError( + ex, + "Error getting references from value {Value} with data editor {DataEditor} and property editor alias {PropertyEditorAlias}.", + value, + dataValueReference.GetType().FullName, + propertyEditorAlias ?? "n/a"); + throw; + } + } + + return result; + } + /// /// Gets all relation type aliases that are automatically tracked. /// @@ -117,6 +169,7 @@ public class DataValueReferenceFactoryCollection : BuilderCollectionBase GetAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) => GetAllAutomaticRelationTypesAliases(propertyEditors); + public ISet GetAllAutomaticRelationTypesAliases(PropertyEditorCollection propertyEditors) { // Always add default automatic relation types diff --git a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs index dfdb55fc63..fff521d13a 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/BlockValuePropertyValueEditorBase.cs @@ -99,17 +99,17 @@ public abstract class BlockValuePropertyValueEditorBase : DataV continue; } - var districtValues = valuesByPropertyEditorAlias.Distinct().ToArray(); + var distinctValues = valuesByPropertyEditorAlias.Distinct().ToArray(); if (dataEditor.GetValueEditor() is IDataValueReference reference) { - foreach (UmbracoEntityReference value in districtValues.SelectMany(reference.GetReferences)) + foreach (UmbracoEntityReference value in distinctValues.SelectMany(reference.GetReferences)) { result.Add(value); } } - IEnumerable references = _dataValueReferenceFactoryCollection.GetReferences(dataEditor, districtValues); + IEnumerable references = _dataValueReferenceFactoryCollection.GetReferences(dataEditor, distinctValues); foreach (UmbracoEntityReference value in references) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs index e6ce8d467e..37ae7fb071 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/DocumentRepositoryTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; @@ -129,7 +130,7 @@ public class DocumentRepositoryTest : UmbracoIntegrationTest var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => Enumerable.Empty())); var dataValueReferences = - new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); var repository = new DocumentRepository( scopeAccessor, appCaches, diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs index 932be80a01..c01d84de7e 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MediaRepositoryTest.cs @@ -3,6 +3,7 @@ using System.Linq; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; @@ -64,7 +65,7 @@ public class MediaRepositoryTest : UmbracoIntegrationTest new PropertyEditorCollection(new DataEditorCollection(() => Enumerable.Empty())); var mediaUrlGenerators = new MediaUrlGeneratorCollection(() => Enumerable.Empty()); var dataValueReferences = - new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); var repository = new MediaRepository( scopeAccessor, appCaches, diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs index 58dc4bb4b0..a935087701 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Linq; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; using NPoco; @@ -53,7 +54,7 @@ public class MemberRepositoryTest : UmbracoIntegrationTest var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => Enumerable.Empty())); var dataValueReferences = - new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); return new MemberRepository( accessor, AppCaches.Disabled, diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs index 8e4a68af70..02e5bac7c5 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TemplateRepositoryTest.cs @@ -6,6 +6,7 @@ using System.IO; using System.Linq; using System.Text; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; @@ -272,7 +273,7 @@ public class TemplateRepositoryTest : UmbracoIntegrationTest var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => Enumerable.Empty())); var dataValueReferences = - new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); var contentRepo = new DocumentRepository( scopeAccessor, AppCaches.Disabled, diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListEditorPropertyValueEditorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListEditorPropertyValueEditorTests.cs index 879400f79a..fc56535200 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListEditorPropertyValueEditorTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/BlockListEditorPropertyValueEditorTests.cs @@ -1,5 +1,6 @@ using System.Globalization; using System.Text.Json.Nodes; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; using NUnit.Framework; @@ -148,7 +149,7 @@ public class BlockListEditorPropertyValueEditorTests new DataEditorAttribute("alias"), new BlockListEditorDataConverter(jsonSerializer), new(new DataEditorCollection(() => [])), - new DataValueReferenceFactoryCollection(Enumerable.Empty), + new DataValueReferenceFactoryCollection(Enumerable.Empty, Mock.Of>()), Mock.Of(), Mock.Of(), localizedTextServiceMock.Object, diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs index 16ea5c11f9..f83b779c63 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueEditorReuseTests.cs @@ -1,4 +1,5 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using NUnit.Framework; using Umbraco.Cms.Core.Cache; @@ -35,7 +36,7 @@ public class DataValueEditorReuseTests Mock.Of())); _propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); - _dataValueReferenceFactories = new DataValueReferenceFactoryCollection(Enumerable.Empty); + _dataValueReferenceFactories = new DataValueReferenceFactoryCollection(Enumerable.Empty, new NullLogger()); var blockVarianceHandler = new BlockEditorVarianceHandler(Mock.Of(), Mock.Of()); _dataValueEditorFactoryMock diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs index 33f4f307f8..43757f064c 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.Logging.Abstractions; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; @@ -49,7 +50,7 @@ public class DataValueReferenceFactoryCollectionTests [Test] public void GetAllReferences_All_Variants_With_IDataValueReferenceFactory() { - var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield()); + var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield(), new NullLogger()); // label does not implement IDataValueReference var labelEditor = new LabelPropertyEditor( @@ -93,7 +94,7 @@ public class DataValueReferenceFactoryCollectionTests [Test] public void GetAllReferences_All_Variants_With_IDataValueReference_Editor() { - var collection = new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + var collection = new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); // mediaPicker does implement IDataValueReference var mediaPicker = new MediaPicker3PropertyEditor( @@ -137,7 +138,7 @@ public class DataValueReferenceFactoryCollectionTests [Test] public void GetAllReferences_Invariant_With_IDataValueReference_Editor() { - var collection = new DataValueReferenceFactoryCollection(() => Enumerable.Empty()); + var collection = new DataValueReferenceFactoryCollection(() => Enumerable.Empty(), new NullLogger()); // mediaPicker does implement IDataValueReference var mediaPicker = new MediaPicker3PropertyEditor( @@ -181,7 +182,7 @@ public class DataValueReferenceFactoryCollectionTests [Test] public void GetAutomaticRelationTypesAliases_ContainsDefault() { - var collection = new DataValueReferenceFactoryCollection(Enumerable.Empty); + var collection = new DataValueReferenceFactoryCollection(Enumerable.Empty, new NullLogger()); var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty)); var result = collection.GetAllAutomaticRelationTypesAliases(propertyEditors).ToArray(); @@ -193,7 +194,7 @@ public class DataValueReferenceFactoryCollectionTests [Test] public void GetAutomaticRelationTypesAliases_ContainsCustom() { - var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield()); + var collection = new DataValueReferenceFactoryCollection(() => new TestDataValueReferenceFactory().Yield(), new NullLogger()); var labelPropertyEditor = new LabelPropertyEditor(DataValueEditorFactory, IOHelper); var propertyEditors = new PropertyEditorCollection(new DataEditorCollection(() => labelPropertyEditor.Yield()));