From 868370d222c920ea360ba638b0efc8e072ff71b9 Mon Sep 17 00:00:00 2001
From: Sipke Schoorstra <sipkeschoorstra@outlook.com>
Date: Fri, 24 Jan 2025 18:54:25 +0100
Subject: [PATCH] Refactor VariableDefinitionMapper to include logging

Refactored the `VariableDefinitionMapper` to utilize dependency injection for `IWellKnownTypeRegistry` and added `ILogger` for improved error handling. Updated value conversion logic to log warnings when conversion fails, ensuring better traceability. Simplified return statements and replaced redundant collections with modern syntax.
---
 .../Mappers/VariableDefinitionMapper.cs       | 34 ++++++++-----------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/modules/Elsa.Workflows.Management/Mappers/VariableDefinitionMapper.cs b/src/modules/Elsa.Workflows.Management/Mappers/VariableDefinitionMapper.cs
index 2d741dbc6..6821f23c1 100644
--- a/src/modules/Elsa.Workflows.Management/Mappers/VariableDefinitionMapper.cs
+++ b/src/modules/Elsa.Workflows.Management/Mappers/VariableDefinitionMapper.cs
@@ -4,30 +4,21 @@ using Elsa.Expressions.Helpers;
 using Elsa.Extensions;
 using Elsa.Workflows.Memory;
 using Elsa.Workflows.Models;
+using Microsoft.Extensions.Logging;
 
 namespace Elsa.Workflows.Management.Mappers;
 
 /// <summary>
 /// Maps <see cref="Variable"/>s to <see cref="VariableDefinition"/>s and vice versa.
 /// </summary>
-public class VariableDefinitionMapper
+public class VariableDefinitionMapper(IWellKnownTypeRegistry wellKnownTypeRegistry, ILogger<VariableDefinitionMapper> logger)
 {
-    private readonly IWellKnownTypeRegistry _wellKnownTypeRegistry;
-
-    /// <summary>
-    /// Constructor.
-    /// </summary>
-    public VariableDefinitionMapper(IWellKnownTypeRegistry wellKnownTypeRegistry)
-    {
-        _wellKnownTypeRegistry = wellKnownTypeRegistry;
-    }
-
     /// <summary>
     /// Maps a <see cref="VariableDefinition"/> to a <see cref="Variable"/>.
     /// </summary>
     public Variable? Map(VariableDefinition source)
     {
-        if (!_wellKnownTypeRegistry.TryGetTypeOrDefault(source.TypeName, out var type))
+        if (!wellKnownTypeRegistry.TryGetTypeOrDefault(source.TypeName, out var type))
             return null;
 
         var valueType = source.IsArray ? type.MakeArrayType() : type;
@@ -38,8 +29,14 @@ public class VariableDefinitionMapper
             variable.Id = source.Id;
         
         variable.Name = source.Name;
-        variable.Value = source.Value.ConvertTo(valueType);
-        variable.StorageDriverType = !string.IsNullOrEmpty(source.StorageDriverTypeName) ? Type.GetType(source.StorageDriverTypeName) : default;
+        source.Value?.TryConvertTo(valueType).OnSuccess(value =>
+        {
+            variable.Value = value;
+        }).OnFailure(ex =>
+        {
+            logger.LogWarning(ex, "Failed to convert variable value.");
+        });
+        variable.StorageDriverType = !string.IsNullOrEmpty(source.StorageDriverTypeName) ? Type.GetType(source.StorageDriverTypeName) : null;
 
         return variable;
     }
@@ -52,7 +49,7 @@ public class VariableDefinitionMapper
             .Select(Map)
             .Where(x => x != null)
             .Select(x => x!)
-        ?? Enumerable.Empty<Variable>();
+        ?? [];
 
     /// <summary>
     /// Maps a <see cref="Variable"/> to a <see cref="VariableDefinition"/>.
@@ -64,16 +61,15 @@ public class VariableDefinitionMapper
         var isArray = valueType.IsCollectionType();
         var elementValueType = isArray ? valueType.GenericTypeArguments[0] : valueType; 
         var value = source.Value;
-        
-        var valueTypeAlias = _wellKnownTypeRegistry.GetAliasOrDefault(elementValueType);
+        var valueTypeAlias = wellKnownTypeRegistry.GetAliasOrDefault(elementValueType);
         var storageDriverTypeName = source.StorageDriverType?.GetSimpleAssemblyQualifiedName();
         var serializedValue = value.Format();
 
-        return new VariableDefinition(source.Id, source.Name, valueTypeAlias, isArray, serializedValue, storageDriverTypeName);
+        return new(source.Id, source.Name, valueTypeAlias, isArray, serializedValue, storageDriverTypeName);
     }
 
     /// <summary>
     /// Maps a list of <see cref="Variable"/>s to a list of <see cref="VariableDefinition"/>s.
     /// </summary>
-    public IEnumerable<VariableDefinition> Map(IEnumerable<Variable>? source) => source?.Select(Map) ?? Enumerable.Empty<VariableDefinition>();
+    public IEnumerable<VariableDefinition> Map(IEnumerable<Variable>? source) => source?.Select(Map) ?? [];
 }
\ No newline at end of file