neo4j / neo4j-dotnet-driver

Neo4j Bolt driver for .NET
Apache License 2.0
231 stars 69 forks source link

Issue in Default Mapping (ToListAsync<T>()) #812

Open killJoy-03 opened 2 months ago

killJoy-03 commented 2 months ago

I am comparing 2 versions of Neo4J Driver (v5.18.1 and v5.22.0) Note - v5.18.1 user Neo4j.Driver.Preview.Mapping and v.5.22.0 uses Neo4j.Driver.Mapping namespace

  1. Both versions can't map NULL value

    Ex: MATCH (a:COL1)-[:REL1]->(b:COL2)
      OPTIONAL MATCH (c:COL3)
      RETURN { item1 : a , item2 : c.prop1 } // c may be null

    This throws an exception like there is no value for item2.

  2. Older version can map directly to Models but newer version can't Ex:

    MATCH (a:COL1)
     RETURN a

    When I use cursor.ToListAsync(); 2.1 Older version works fine. 2.2 Newer version throws error due to Model A contains extra property which it doesn't find in DB result.

  3. In newer version as I mentioned direct model mapping is not working but there is something more Ex:

    MATCH(a:COL1)
        RETURN {  prop1 : a.prop1 , prop2 : a.prop2 } // this is not working
        RETURN a.prop1 as prop1 , a.prop2 as prop2 // this is working  

Version Info :

  • .NET Version: .NET 9.0.100-preview.6.24328.19
  • .NET Driver Version 5.18.1 and 5.22.0
  • Neo4j Server Version & Edition Neo4J 5.21.0
killJoy-03 commented 2 months ago

Update: @RichardIrons-neo4j There is a bypass for example 3.

Example:

public record struct Collection<T>(List<T> Value);

MATCH(a:COL1)
       RETURN collect({  prop1 : a.prop1 , prop2 : a.prop2 }) as Values

This is working if we are mapping to Collection Model

RichardIrons-neo4j commented 1 month ago

@killJoy-03 Hi.

What has happened is that there were some breaking changes made to the mapping code before it came out of preview. When it was in preview, the default mapper was case-insensitive, and also it was very forgiving as to where it might find the value for a property it was trying to set. For instance, if it was looking for "Name", it might find a property called "name" in a node that's in the "person" field of the record it's trying to map. In testing, we found a lot of cases where this could introduce ambiguity and unpredictability in the mapping code. The decision was made to make the mapper much more strict, and to require mapping to be done explicitly in cases where the property name (or constructor parameter name) doesn't match the name of a field.

For example, in the case above, instead of:

public class Example
{
    public string Name { get; set; }
}

you would need to have:

public class Example
{
    [MappingSource("person.name")]
    public string Name { get; set; }
}

And then it would find the value. If you have a property that is not going to receive a value, then you should mark that property with the [MappingIgnored] or [MappingDefaultValue] attributes. Constructor parameters can also be marked with these attributes (except for [MappingIgnored] as it doesn't make sense).