Avoid throwing an exception on getting references when migrating content with changed data types (16)

src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs

@@ -2,7 +2,11 @@
 // See LICENSE for more details.
 
 using System.Diagnostics.CodeAnalysis;
+using System.Text.Json;
+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.Blocks;
@@ -21,9 +25,12 @@ public abstract class BlockEditorPropertyValueEditor<TValue, TLayout> : BlockVal
     where TValue : BlockValue<TLayout>, new()
     where TLayout : class, IBlockLayoutItem, new()
 {
+    private readonly ILogger _logger;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="BlockEditorPropertyValueEditor{TValue, TLayout}"/> class.
     /// </summary>
+    [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 18.")]
     protected BlockEditorPropertyValueEditor(
         PropertyEditorCollection propertyEditors,
         DataValueReferenceFactoryCollection dataValueReferenceFactories,
@@ -34,8 +41,39 @@ protected BlockEditorPropertyValueEditor(
         ILanguageService languageService,
         IIOHelper ioHelper,
         DataEditorAttribute attribute)
-        : base(propertyEditors, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, dataValueReferenceFactories, blockEditorVarianceHandler, languageService, ioHelper, attribute) =>
+        : this(
+              propertyEditors,
+              dataValueReferenceFactories,
+              dataTypeConfigurationCache,
+              shortStringHelper,
+              jsonSerializer,
+              blockEditorVarianceHandler,
+              languageService,
+              ioHelper,
+              attribute,
+              StaticServiceProvider.Instance.GetRequiredService<ILogger>())
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="BlockEditorPropertyValueEditor{TValue, TLayout}"/> class.
+    /// </summary>
+    protected BlockEditorPropertyValueEditor(
+        PropertyEditorCollection propertyEditors,
+        DataValueReferenceFactoryCollection dataValueReferenceFactories,
+        IDataTypeConfigurationCache dataTypeConfigurationCache,
+        IShortStringHelper shortStringHelper,
+        IJsonSerializer jsonSerializer,
+        BlockEditorVarianceHandler blockEditorVarianceHandler,
+        ILanguageService languageService,
+        IIOHelper ioHelper,
+        DataEditorAttribute attribute,
+        ILogger logger)
+        : base(propertyEditors, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, dataValueReferenceFactories, blockEditorVarianceHandler, languageService, ioHelper, attribute)
+    {
         JsonSerializer = jsonSerializer;
+        _logger = logger;
+    }
 
     /// <summary>
     /// Gets the <see cref="IJsonSerializer"/>.
@@ -63,24 +101,15 @@ public override IEnumerable<ITag> GetTags(object? value, object? dataTypeConfigu
     private TValue? ParseBlockValue(object? value)
     {
         var rawJson = value == null ? string.Empty : value is string str ? str : value.ToString();
-        return BlockEditorValues.DeserializeAndClean(rawJson)?.BlockValue;
+        return SafeParseBlockEditorData(rawJson)?.BlockValue;
     }
 
     /// <inheritdoc />
     public override object ToEditor(IProperty property, string? culture = null, string? segment = null)
     {
         var val = property.GetValue(culture, segment);
 
-        BlockEditorData<TValue, TLayout>? blockEditorData;
-        try
-        {
-            blockEditorData = BlockEditorValues.DeserializeAndClean(val);
-        }
-        catch
-        {
-            // if this occurs it means the data is invalid, shouldn't happen but has happened if we change the data format.
-            return string.Empty;
-        }
+        BlockEditorData<TValue, TLayout>? blockEditorData = SafeParseBlockEditorData(val);
 
         if (blockEditorData == null)
         {
@@ -103,8 +132,8 @@ public override object ToEditor(IProperty property, string? culture = null, stri
         // For most of the properties this is fine, but for properties which contain other state it might be critical (e.g. file upload field).
         // So, we must run MapBlockValueFromEditor even if editorValue is null or string.IsNullOrWhiteSpace(editorValue.Value.ToString()) is true.
 
-        BlockEditorData<TValue, TLayout>? currentBlockEditorData = GetBlockEditorData(currentValue);
-        BlockEditorData<TValue, TLayout>? blockEditorData = GetBlockEditorData(editorValue.Value);
+        BlockEditorData<TValue, TLayout>? currentBlockEditorData = SafeParseBlockEditorData(currentValue);
+        BlockEditorData<TValue, TLayout>? blockEditorData = SafeParseBlockEditorData(editorValue.Value);
 
         // We can skip MapBlockValueFromEditor if both editorValue and currentValue values are empty.
         if (IsBlockEditorDataEmpty(currentBlockEditorData) && IsBlockEditorDataEmpty(blockEditorData))
@@ -122,19 +151,30 @@ public override object ToEditor(IProperty property, string? culture = null, stri
         return JsonSerializer.Serialize(blockEditorData.BlockValue);
     }
 
-    private BlockEditorData<TValue, TLayout>? GetBlockEditorData(object? value)
+    private static bool IsBlockEditorDataEmpty([NotNullWhen(false)] BlockEditorData<TValue, TLayout>? editorData)
+        => editorData is null || editorData.BlockValue.ContentData.Count == 0;
+
+    // We don't throw on error here because we want to be able to parse what we can, even if some of the data is invalid. In cases where migrating
+    // from nested content to blocks, we don't want to trigger a fatal error for retrieving references, as this isn't vital to the operation.
+    // See: https://github.com/umbraco/Umbraco-CMS/issues/19784 and Umbraco support cases.
+    private BlockEditorData<TValue, TLayout>? SafeParseBlockEditorData(object? value)
     {
         try
         {
             return BlockEditorValues.DeserializeAndClean(value);
         }
+        catch (JsonException ex)
+        {
+            _logger.LogWarning(
+                "Could not deserialize the provided property value into a block editor value: {PropertyValue}. Error: {ErrorMessage}.",
+                value,
+                ex.Message);
+            return null;
+        }
         catch
         {
             // If this occurs it means the data is invalid. It shouldn't happen could if we change the data format.
             return null;
         }
     }
-
-    private static bool IsBlockEditorDataEmpty([NotNullWhen(false)] BlockEditorData<TValue, TLayout>? editorData)
-        => editorData is null || editorData.BlockValue.ContentData.Count == 0;
 }

src/Umbraco.Infrastructure/PropertyEditors/BlockGridPropertyEditorBase.cs

@@ -56,7 +56,7 @@ public BlockGridEditorPropertyValueEditor(
             BlockEditorVarianceHandler blockEditorVarianceHandler,
             ILanguageService languageService,
             IIOHelper ioHelper)
-            : base(propertyEditors, dataValueReferenceFactories, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, blockEditorVarianceHandler, languageService, ioHelper, attribute)
+            : base(propertyEditors, dataValueReferenceFactories, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, blockEditorVarianceHandler, languageService, ioHelper, attribute, logger)
         {
             BlockEditorValues = new BlockEditorValues<BlockGridValue, BlockGridLayoutItem>(new BlockGridEditorDataConverter(jsonSerializer), elementTypeCache, logger);
             Validators.Add(new BlockEditorValidator<BlockGridValue, BlockGridLayoutItem>(propertyValidationService, BlockEditorValues, elementTypeCache));

src/Umbraco.Infrastructure/PropertyEditors/BlockListPropertyEditorBase.cs

@@ -69,7 +69,7 @@ public BlockListEditorPropertyValueEditor(
             BlockEditorVarianceHandler blockEditorVarianceHandler,
             ILanguageService languageService,
             IIOHelper ioHelper)
-            : base(propertyEditors, dataValueReferenceFactories, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, blockEditorVarianceHandler, languageService, ioHelper, attribute)
+            : base(propertyEditors, dataValueReferenceFactories, dataTypeConfigurationCache, shortStringHelper, jsonSerializer, blockEditorVarianceHandler, languageService, ioHelper, attribute, logger)
         {
             BlockEditorValues = new BlockEditorValues<BlockListValue, BlockListLayoutItem>(blockEditorDataConverter, elementTypeCache, logger);
             Validators.Add(new BlockEditorValidator<BlockListValue, BlockListLayoutItem>(propertyValidationService, BlockEditorValues, elementTypeCache));