Cypher types in MapCompositeConverter

[mattharr]

Expected Behavior

I'm looking at storing dates into the Neo4j database as Dates; specifically in my case I want to store ZonedDateTime, which the Bolt driver will accept (I believe). I've saved the data using Cypher queries (I had to for speed reasons), and so when loading that data back into a Map of data it should load back in.

Current Behavior

This throws an exception saying that ZonedDateTime is not a supported type.

Possible Solution

I took a copy of the MapCompositeConverter and adding ZonedDateTime, and this looked to be OK, but I'm not storing data via OGM, so not sure if this would hit issues?

Your Environment

• OGM Version used: 3.1.7
• Java Version used: 8
• Neo4J Version used: 3.5.2 (Community)
• Bolt Driver Version used (if applicable): 3.1.7
• Operating System and Version: Windows 10

[michael-simons]

Hello Matt,
thanks for your interest in OGM and your comment.

First: When I speak about the Bolt driver, I usually refer to the underlying Neo4j Java Driver (See https://github.com/neo4j/neo4j-java-driver), which is used by Neo4j-OGM-Bolt.
And yes, you are correct, the driver supports ZonedDateTime (and others).

Sadly, Neo4j-OGM 3.1 does not.

You have some options to workaround:

Neo4j-OGM 3.1 with Spring Data Lovelace

Provide a custom configuration of OGM and also make the instance of ObjectMapper we use aware of native types:

Here is an example:
https://github.com/michael-simons/bootiful-music/blob/master/knowledge/src/main/java/ac/simons/music/knowledge/config/Neo4jConfiguration.java#L35-L56

Keypoints are builder.withCustomProperty(CONFIG_PARAMETER_CONVERSION_MODE, CONVERT_NON_NATIVE_ONLY); and the ObjectMapper in the link above.

Then, you have to turn of Spring Data Neo4js automatic conversion as well, with some like this

https://github.com/michael-simons/bootiful-music/blob/master/knowledge/src/main/java/ac/simons/music/knowledge/support/AbstractAuditableBaseEntity.java#L41-L42 (See @Convert annotation)

And https://github.com/michael-simons/bootiful-music/blob/master/knowledge/src/main/java/ac/simons/music/knowledge/support/NoOpLocalDateTimeConversion.java

While this is a lot of crud around simple things, it is not changeable in OGM 3.1 and SDN Lovelace.

The same project (https://github.com/michael-simons/bootiful-music/tree/feature/spatial-native) in Branch spatial-native is on

OGM 3.2 alpha and SDN Moore

This is done by setting the appropriate Spring Boot property
https://github.com/michael-simons/bootiful-music/blob/feature/spatial-native/knowledge/pom.xml#L36

And including our new native type support

<dependency>
	<groupId>org.neo4j</groupId>
	<artifactId>neo4j-ogm-bolt-native-types</artifactId>
	<version>${neo4j-ogm.version}</version>
</dependency>

The configuration than is as simple as that:
https://github.com/michael-simons/bootiful-music/blob/feature/spatial-native/knowledge/src/main/java/ac/simons/music/knowledge/config/Neo4jConfiguration.java#L31-L37

There is no Spring Boot property for that.

OGM without SDN

This should be possible as well, but here I really recommend going to 3.2.

https://github.com/neo4j/neo4j-ogm/blob/master/neo4j-ogm-tests/neo4j-ogm-native-types-tests/src/test/java/org/neo4j/ogm/persistence/types/nativetypes/DatesBoltTest.java

[mattharr]

Hi,

Thanks very much for getting back to me. We're not using Spring here at the moment (the projects are Dropwizard based), so I'm not worried about the Spring data layer.
I moved to OGM 3.2 and tried it out, but I think it has the same problem. In that when getting data back out into a Map of properties it gets the following error:

org.neo4j.ogm.exception.core.MappingException: Could not map key=VERSION__CREATION_DATE, value=2014-02-25T00:09:21Z (type = class java.time.ZonedDateTime) because it is not a supported type.
at org.neo4j.ogm.typeconversion.MapCompositeConverter.addMapToProperties(MapCompositeConverter.java:112)
at org.neo4j.ogm.typeconversion.MapCompositeConverter.toGraphProperties(MapCompositeConverter.java:96)
at org.neo4j.ogm.typeconversion.MapCompositeConverter.toGraphProperties(MapCompositeConverter.java:45)
at org.neo4j.ogm.metadata.FieldInfo.readComposite(FieldInfo.java:421)
at org.neo4j.ogm.context.EntitySnapshot$Builder.lambda$extractCompositeProperties$1(EntitySnapshot.java:116)
at org.neo4j.ogm.context.EntitySnapshot$Builder$$Lambda$266/1325022009.apply(Unknown Source)
at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:267)
at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1540)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
at org.neo4j.ogm.context.EntitySnapshot$Builder.extractCompositeProperties(EntitySnapshot.java:117)
at org.neo4j.ogm.context.EntitySnapshot$Builder.take(EntitySnapshot.java:100)
at org.neo4j.ogm.context.IdentityMap.remember(IdentityMap.java:77)
at org.neo4j.ogm.context.MappingContext.remember(MappingContext.java:504)
at org.neo4j.ogm.context.MappingContext.addNodeEntity(MappingContext.java:155)
at org.neo4j.ogm.context.GraphEntityMapper.mapNodes(GraphEntityMapper.java:216)
at org.neo4j.ogm.context.GraphEntityMapper.mapContentOf(GraphEntityMapper.java:138)
at org.neo4j.ogm.context.GraphEntityMapper.lambda$map$1(GraphEntityMapper.java:99)
at org.neo4j.ogm.context.GraphEntityMapper$$Lambda$256/537515544.accept(Unknown Source)
at java.util.ArrayList.forEach(ArrayList.java:1249)
at org.neo4j.ogm.context.GraphEntityMapper.map(GraphEntityMapper.java:101)
at org.neo4j.ogm.context.GraphRowModelMapper.map(GraphRowModelMapper.java:59)
at org.neo4j.ogm.session.delegates.LoadOneDelegate.lambda$load$0(LoadOneDelegate.java:86)
at org.neo4j.ogm.session.delegates.LoadOneDelegate$$Lambda$295/1345169044.doInTransaction(Unknown Source)
at org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:579)
at org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:553)
at org.neo4j.ogm.session.delegates.LoadOneDelegate.load(LoadOneDelegate.java:83)
at org.neo4j.ogm.session.Neo4jSession.load(Neo4jSession.java:178)

And I think that due to the Converter having a fixed set of types it knows about:

Set<Class> types = new HashSet<>();
types.add(Boolean.class);  
types.add(Long.class); 
types.add(Double.class);
types.add(String.class);
types.add(List.class);
cypherTypes = Collections.unmodifiableSet(types);

Does that seem right?

By the way, do you know if there are plans to move 3.2 to beta/release? Or what issues are needed to be resolved before that can happen? Or does it just need some time to bake?

Thanks,

Matt

[michael-simons]

Hi Matt,

good catch, the conversion should not be triggered at all in this case.
I'll have a look next week.

[michael-simons]

Now that I have read your issue a second time, I realize that I gave you an answer to a general problem, not to a specific. Thanks for trying it out nevertheless!

I'm only roughly aware that we have this map converter, but giving a quick lock, this should work with the native types enabled as described above

@Properties(prefix = "myPrefix", allowCast = true)
private Map<String, Object> prefixedProperties;

Or, you can add a custom MapConvert (like here org.neo4j.ogm.domain.convertible.parametrized.JsonNode)

When you have time to try this out, a PR against the Master adding a test would be more than appreaciated. A good place would be the submodule neo4j-ogm-native-types-test, you could add the property map to org.neo4j.ogm.persistence.types.nativetypes.DatesTestBase.Sometime for example.

If you open a branch, we could work together on it, i.e. you could provide the test with the above workaround in place, we could make sure that the workaround is no longer needed for the next alpha or beta.

Looking forward to hear from you,

[mattharr]

Hi,

I've created a branch (https://github.com/mattharr/neo4j-ogm), and tried out adding to the tests. Once I got it building locally (it took a few bits and pieces to setup, let me know if you want to chat about them), it looks like just using the allowCast property doesn't work (I wasn't sure it would). The test is in the branch - hopefully I got it right?
The rest of the commit is around fixing the issue (and the one from #632). I used the predicate 'isSupportedNativeType' which I found whilst looking around in the code. Could this be the right method to use? It sounded right, and my initial thought was that it would have checks for all the types supported, but it just seems to be for the newer ones (so things like String or Boolean aren't there); to that end I just left the existing internal sets describing what was allowed.
In doing the change it looks like I got the wrong formatting, so isn't as clean as it should be, but hopefully it still shows up the areas that might need changing (happy to fix up if we look to go forward to fix this).
The rest of the change is around allowing for lower case names for enum maps (the other issue). Have a look, and see if it helps as a point to discuss from.

Cheers,

Matt

[michael-simons]

Hi Matt,
currently reading through the code and trying to figure out the actual changes. Please don't reformat the whole file, that makes things really difficult.

[mattharr]

Hi,

Yes sorry about that, I didn't realise until I'd committed it (my IDE is set up to auto-format on save). Let me know if its too annoying to read through - I can always redo it without the reformat.

[michael-simons]

First step:

     * Some Java types, like Integer and Float, can be automatically cast into a wider type, like Long and Double, by
     * the Neo4j type systems. This in most cases not what one does expect during mapping. We have however not a way
     * to determine whether the type of the value put into the map was supposed originally when reading the instance
     * containing the map back. Set this attribute to true to allow OGM to accept the automatic cast.
     * 
     * @return True, when the values of map entries are allowed to be cast to a wider datatype.
     */
    boolean allowCast() default false;

So that's the reason why setting the parameter has now effect. Your test was already helpful and I've copied it over.

[michael-simons]

Second feedback: The isSupportedNativeType is the correct way to go. I chose this, but also removed the locally stored types, as they would duplicate things.

That however lead to some fixes and changes for the conversion.

Your finding is quite valuable, thank you!

I'll try to review the other additions in your code and add them separatly. It would make things easier to keep such suggestions separate from the start for reviewing and merging.

Expect a new alpha soon.

[mattharr]

Hi, thanks very much for looking into this, glad it was useful. Sorry about batching everything together - too much time working by myself I think.

Cheers,

Matt

[michael-simons]

Sorry about batching everything together - too much time working by myself I think.

It's all good, no harm done.

My colleague Gerrit was so kind to release alpha06, with the bug fix and the transformation feature in it. If you find the time, please try it out :)

[mattharr]

Hi,

Just to let you know, I pulled the alpha06 version, and it all looks to be working fine now, thanks for getting that sorted.
If its of any use, I reset my fork of the ogm repo, and made a small change to remove the dynamic casting for the valueOf method (master...mattharr:master), I think it works, and removes a dynamic method invocation, if that's of any use (no reformatting this time!).

Thanks,

Matt

[michael-simons]

Thanks for your feedback Matt, much appreciated.