Fix default mapper bug (#821)

* Fix default mapper bug

* remove unused using

* Tests

* Reset default mapper now needed when resetting the default mapper config

* wrong exception type in test

Author: RichardIrons-neo4j

…elegateAsyncEnumerableExtensionsTests.cs …apping/AsyncEnumerableExtensionsTests.csNeo4j.Driver/Neo4j.Driver.Tests/Mapping/DelegateAsyncEnumerableExtensionsTests.cs renamed to Neo4j.Driver/Neo4j.Driver.Tests/Mapping/AsyncEnumerableExtensionsTests.cs Neo4j.Driver/Neo4j.Driver.Tests/Mapping/DelegateAsyncEnumerableExtensionsTests.cs renamed to Neo4j.Driver/Neo4j.Driver.Tests/Mapping/AsyncEnumerableExtensionsTests.cs

@@ -14,15 +14,17 @@
 // limitations under the License.
 
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using FluentAssertions;
+using Neo4j.Driver.Mapping;
 using Neo4j.Driver.Preview.Mapping;
 using Neo4j.Driver.Tests.TestUtil;
 using Xunit;
 
 namespace Neo4j.Driver.Tests.Mapping;
 
-public class DelegateAsyncEnumerableExtensionsTests
+public class AsyncEnumerableExtensionsTests
 {
     [Fact]
     public async Task ShouldMapAllRecordsFromCursor_01_Property()
@@ -444,4 +446,52 @@ async IAsyncEnumerable<IRecord> GetRecordsAsync()
                     pet = "Cat", car = "BMW", food = "Burger", sport = "Football"
                 });
     }
+
+    [Fact]
+    public async Task AsObjectsFromBlueprintAsync_ShouldMapRecordsToObjects()
+    {
+        async IAsyncEnumerable<IRecord> GetRecordsAsync()
+        {
+            var record1 = TestRecord.Create(
+                ("name", "Alice"),
+                ("age", 25),
+                ("city", "New York"));
+
+            var record2 = TestRecord.Create(
+                ("name", "Bob"),
+                ("age", 30),
+                ("city", "Los Angeles"));
+
+            await Task.Yield();
+            yield return record1;
+            yield return record2;
+        }
+
+        var result = await GetRecordsAsync()
+            .AsObjectsFromBlueprintAsync(new { name = string.Empty, age = 0, city = string.Empty })
+            .ToArrayAsync();
+
+        result.Should()
+            .BeEquivalentTo(
+            [
+                new { name = "Alice", age = 25, city = "New York" },
+                new { name = "Bob", age = 30, city = "Los Angeles" }
+            ]);
+    }
+
+    [Fact]
+    public async Task AsObjectsFromBlueprintAsync_ShouldHandleEmptyEnumerable()
+    {
+        async IAsyncEnumerable<IRecord> GetRecordsAsync()
+        {
+            await Task.Yield();
+            yield break;
+        }
+
+        var result = await GetRecordsAsync()
+            .AsObjectsFromBlueprintAsync(new { name = string.Empty, age = 0, city = string.Empty })
+            .ToArrayAsync();
+
+        result.Should().BeEmpty();
+    }
 }

Neo4j.Driver/Neo4j.Driver.Tests/Mapping/DefaultMapperTests.cs

@@ -355,7 +355,7 @@ private NoConstructors()
     [Fact]
     public void ShouldThrowIfClassHasNoConstructors()
     {
-        var act = DefaultMapper.Get<NoConstructors>;
+        var act = () => DefaultMapper.Get<NoConstructors>(null);
         act.Should().Throw<InvalidOperationException>();
     }
 }

Neo4j.Driver/Neo4j.Driver.Tests/Mapping/MappingProviderTests.cs

@@ -201,4 +201,52 @@ public void ShouldUseCustomMapper()
         obj2.JobTitle.Should().Be("developer");
         obj2.YearsOfService.Should().Be(5);
     }
+
+    private class NameAndGuid
+    {
+        public string Name { get; set; } = null!;
+        public Guid Guid { get; set; }
+    }
+
+    private class MappingProviderThatUsesDefaultMappingAndOverridesAGuidProperty(bool overrideGuid): IMappingProvider
+    {
+        public void CreateMappers(IMappingRegistry registry)
+        {
+            registry.RegisterMapping<NameAndGuid>(
+                b =>
+                {
+                    b.UseDefaultMapping();
+                    if(overrideGuid)
+                    {
+                        b.Map(x => x.Guid, "Guid", converter: x => Guid.Parse(x.As<string>()));
+                    }
+                });
+        }
+    }
+
+    [Fact]
+    public void ShouldNotFailWhenUsingDefaultMapperButMappingSomePropertiesExplicitly()
+    {
+        var guid = Guid.NewGuid();
+        var testRecord = TestRecord.Create(("Name", "Alice"), ("Guid", guid.ToString()));
+        RecordObjectMapping.RegisterProvider(
+            new MappingProviderThatUsesDefaultMappingAndOverridesAGuidProperty(true));
+
+        var obj = testRecord.AsObject<NameAndGuid>();
+
+        obj.Name.Should().Be("Alice");
+        obj.Guid.Should().Be(guid);
+    }
+
+    [Fact]
+    public void ShouldFailWhenUsingDefaultMapperWithoutOverriding()
+    {
+        var guid = Guid.NewGuid();
+        var testRecord = TestRecord.Create(("Name", "Alice"), ("Guid", guid.ToString()));
+        RecordObjectMapping.RegisterProvider(
+            new MappingProviderThatUsesDefaultMappingAndOverridesAGuidProperty(false));
+
+        var act = () => testRecord.AsObject<NameAndGuid>();
+        act.Should().Throw<MappingFailedException>();
+    }
 }

Neo4j.Driver/Neo4j.Driver.Tests/Neo4j.Driver.Tests.csproj

@@ -22,6 +22,7 @@
         <PackageReference Include="Microsoft.Reactive.Testing" Version="5.0.0" />
         <PackageReference Include="Moq" Version="4.20.69" />
         <PackageReference Include="Moq.AutoMock" Version="3.5.0" />
+        <PackageReference Include="System.Linq.Async" Version="6.0.1" />
         <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
         <PackageReference Include="TeamCity.VSTest.TestAdapter" Version="1.0.23" />
         <PackageReference Include="xunit" Version="$(XunitVersion)" />

Neo4j.Driver/Neo4j.Driver/Public/Mapping/BuiltMapper.cs

@@ -28,6 +28,7 @@ internal class BuiltMapper<T> : IRecordMapper<T>
 
     private Func<IRecord, T> _wholeObjectMapping;
     private readonly List<Action<T, IRecord>> _propertyMappings = new();
+    public HashSet<MethodInfo> MappedSetters { get; } = [];
 
     public T Map(IRecord record)
     {
@@ -139,6 +140,7 @@ public void AddMapping(
     {
         // this part only happens once, at the time of building the mapper
         _propertyMappings.Add(MapFromRecord);
+        MappedSetters.Add(propertySetter);
         return;
 
         // this part happens every time a record is mapped
@@ -152,7 +154,7 @@ void MapFromRecord(T obj, IRecord record)
 
             if (mappableValueFound)
             {
-                propertySetter.Invoke(obj, new[] { mappableValue });
+                propertySetter.Invoke(obj, [mappableValue]);
             }
             else if(!optional)
             {

Neo4j.Driver/Neo4j.Driver/Public/Mapping/DefaultMapper.cs

@@ -23,8 +23,14 @@ internal static class DefaultMapper
 {
     private static readonly Dictionary<Type, object> Mappers = new();
 
-    public static IRecordMapper<T> Get<T>()
+    public static void Reset()
     {
+        Mappers.Clear();
+    }
+
+    public static IRecordMapper<T> Get<T>(HashSet<MethodInfo> mappedSetters = null)
+    {
+        mappedSetters ??= [];
         var type = typeof(T);
 
         // if we already have a mapper for this type, return it
@@ -45,9 +51,11 @@ public static IRecordMapper<T> Get<T>()
         var properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance);
         foreach (var property in properties)
         {
-            // ignore properties without a setter or with MappingIgnoredAttribute, or compiler generated
+            // ignore properties without a setter or with MappingIgnoredAttribute, or compiler generated,
+            // or if the setter has already been mapped elsewhere (e.g. custom mapping config)
             if (property.SetMethod is null ||
-                property.GetCustomAttribute<MappingIgnoredAttribute>() is not null)
+                property.GetCustomAttribute<MappingIgnoredAttribute>() is not null ||
+                mappedSetters.Contains(property.SetMethod))
             {
                 continue;
             }

Neo4j.Driver/Neo4j.Driver/Public/Mapping/MappingBuilder.cs

@@ -74,7 +74,7 @@ internal IMappingBuilder<T> UseConstructor(ConstructorInfo constructor)
     /// <inheritdoc />
     public IMappingBuilder<T> UseDefaultMapping()
     {
-        _builtMapper.AddWholeObjectMapping(r => DefaultMapper.Get<T>().Map(r));
+        _builtMapper.AddWholeObjectMapping(r => DefaultMapper.Get<T>(_builtMapper.MappedSetters).Map(r));
         return this;
     }
 

Neo4j.Driver/Neo4j.Driver/Public/Mapping/RecordObjectMapping.cs

@@ -60,6 +60,7 @@ internal static void Reset()
     {
         // discard the current instance and create a new one, which will have no mappers registered
         Instance = new RecordObjectMapping();
+        DefaultMapper.Reset();
     }
 
     /// <summary>
@@ -84,7 +85,7 @@ private static object GetMapperForType(Type type)
         // no mapper registered for this type, so use the default mapper
         var openGenericMethod = typeof(DefaultMapper).GetMethod(nameof(DefaultMapper.Get));
         var closedGenericMethod = openGenericMethod!.MakeGenericMethod(type);
-        return closedGenericMethod.Invoke(null, null);
+        return closedGenericMethod.Invoke(null, [null]);
     }
 
     /// <summary>