Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle CompositeProperty objects "like nodes" #2315

Open
xenoterracide opened this issue Jul 5, 2021 · 11 comments
Open

handle CompositeProperty objects "like nodes" #2315

xenoterracide opened this issue Jul 5, 2021 · 11 comments
Labels
status: needs-investigation An issue that has been triaged but needs further investigation

Comments

@xenoterracide
Copy link

https://docs.spring.io/spring-data/neo4j/docs/current/reference/html/#custom.conversions.composite-properties

should we be able to convert any map into an object using the keys as the field names and the values as the values, all the way down? isn't this exactly what Jackson and many deserialization libraries do? This seems convoluted for what is probably the most common use case. Seems like you could apply the exact same logic as you do when converting a node into an object, since aren't the properties on the node the same thing, just a map?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 5, 2021
@michael-simons
Copy link
Collaborator

There is no map attribute type inside the Neo4j database. You are free to use Jackson or any other tool in your custom conversion for composite attribute types.

@michael-simons michael-simons added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 5, 2021
@xenoterracide
Copy link
Author

xenoterracide commented Jul 5, 2021

maybe I'm misunderstanding... I thought you could store a map inside of a database https://neo4j.com/docs/cypher-manual/4.3/syntax/maps/ and if you can't why does a CompositeProperty deserialize to a Map? since you'd have to have a map property in order to have a map on a node.

@michael-simons
Copy link
Collaborator

Hi Caleb,

yes, that's a misconception I head to deal with a couple of years as well. No, Neo4j does not store Maps. It supports them as parameters, though.

Here's the list of supported property types https://neo4j.com/docs/cypher-manual/current/syntax/values/#property-types.

As opposed the list of composite types: https://neo4j.com/docs/cypher-manual/current/syntax/values/#composite-types

Cannot be stored as properties

When the core product changes or supports this, we will do to.

@xenoterracide
Copy link
Author

So is a composite type only for the results of a dynamic query then? confusing, but ok, sorry

@michael-simons
Copy link
Collaborator

No, you can have a map attribute on a @Node or @RelationshipProperties on the Java side / client side of things.

This will be stored than as separate properties on the graph database objects, with a common prefix by default or in any form you decompose it if you add a custom method to it.

Here's our canonical test:

https://github.com/spring-projects/spring-data-neo4j/blob/main/src/test/java/org/springframework/data/neo4j/integration/shared/conversion/ThingWithCompositeProperties.java#L73-L133

When stored, we take the graph db node (the internal representation), turn it into a map (which is a driver feature)
https://github.com/spring-projects/spring-data-neo4j/blob/main/src/test/java/org/springframework/data/neo4j/integration/shared/conversion/CompositePropertiesITBase.java#L207
and make sure the contents of the map is written as single properties into the graph.

this is not about dynamic queries.

@xenoterracide
Copy link
Author

ah, well what this ticket is about, was being able to (per the test) have a generic converter for SomeOtherDto instead of having to write one for every single "nested object", it seems like a simple enough problem.

@michael-simons
Copy link
Collaborator

static class SomeOtherDTOToMapConverter implements Neo4jPersistentPropertyToMapConverter<String, SomeOtherDTO> {

Please checkout if you use Object as Value for the interface: Neo4jPersistentPropertyToMapConverter<String, Object>

The key is the prefix (either String or Enum).
And then pipe your Object instance through whatever you like.

I guess that should work.

@xenoterracide
Copy link
Author

xenoterracide commented Jul 5, 2021

yes, the point is that you should be able to have a generic converter that does this, you shouldn't have to reimplement it for every class, there's no reason that I can think of that spring-data-neo4j couldn't provide or even automatically use such a converter behind the scene's. I was trying to figure out how you're doing Node in the first place since it should essentially be the same problem.

@xenoterracide
Copy link
Author

any chance you could point me to the code that allows neo4j to dynamically construct a node or whatever?

@xenoterracide
Copy link
Author

now I see the problem, while this should work for half of the conversion

/*
 * Copyright © 2021 Caleb Cushing.
 * Apache 2.0. See https://github.com/xenoterracide/brix/LICENSE
 * https://choosealicense.com/licenses/apache-2.0/#
 */
package com.xenoterracide.ppm.util.modelgraph;

import org.neo4j.driver.Value;
import org.neo4j.driver.exceptions.ClientException;
import org.springframework.data.neo4j.core.convert.Neo4jConversionService;
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyToMapConverter;
import org.springframework.util.ReflectionUtils;

import java.util.HashMap;
import java.util.Map;

import static org.neo4j.driver.Values.value;

public class MapToObjectConverter implements Neo4jPersistentPropertyToMapConverter<String, Object> {

  @Override
  public Map<String, Value> decompose(
    Object property,
    Neo4jConversionService neo4jConversionService
  ) {
    var map = new HashMap<String, Value>();
    ReflectionUtils.doWithFields(property.getClass(), (f) -> {
      var o = f.get( property );
      var name = f.getName();
      try {
        map.put( name, value( o ) );
      }
      catch ( ClientException e ) {
        map.put( name, value( this.decompose( o, neo4jConversionService ) ) );
      }
    });
    return map;
  }

  @Override
  public Object compose(
    Map<String, Value> source,
    Neo4jConversionService neo4jConversionService
  ) {

    return null;
  }
}

the other half has a problem that it doesn't actually have the type data that Spring Data Neo4j has because it's able to get that from the parent object. This leads me back to begging for a reopen, I could probably even implement it myself if I had some pointers, it could just be a different annotation.

@michael-simons
Copy link
Collaborator

michael-simons commented Jul 7, 2021 via email

@michael-simons michael-simons reopened this Jul 7, 2021
@michael-simons michael-simons added status: needs-investigation An issue that has been triaged but needs further investigation and removed status: declined A suggestion or change that we don't feel we should currently apply labels Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs-investigation An issue that has been triaged but needs further investigation
Projects
None yet
Development

No branches or pull requests

3 participants