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

Direct Relationship Persistence Issue #951

Closed
pc-dd opened this issue Jul 12, 2023 · 10 comments
Closed

Direct Relationship Persistence Issue #951

pc-dd opened this issue Jul 12, 2023 · 10 comments
Assignees

Comments

@pc-dd
Copy link

pc-dd commented Jul 12, 2023

Say I have nodes of types A, B and C, and I have the following graph:
QD5Y1

My goal is to load the node A, together with the connected B and C nodes.

Currently (in Kotlin code) I am doing session.load(A::class.java, "a1", -1), which uses this implementation: <T,ID extends Serializable> T load(Class<T> type, ID id, int depth) from https://javadoc.io/static/org.neo4j/neo4j-ogm-core/3.2.3/org/neo4j/ogm/session/Session.html

According to https://neo4j.com/docs/ogm-manual/current/reference/#reference:session:loading-entities:load-depth , providing depth = -1 should make it load the node A together with all its connected nodes. However, it seems to only load the node A.

What is the correct method I should be using to achieve my goal here? I have tried looking into the neo4j OGM documentation, but have not found more information.

Expected Behavior

Expected load to load connected nodes

Current Behavior

Load only loads the initial node but not its connected nodes

Context

Your Environment

  • Java version:
  • Neo4j version (The version of the database): 5.9.0
  • OGM Version (The version of this software): 3.2.23
  • OGM Transport used (One of Bolt, HTTP or embedded): Bolt
  • Neo4j Java Driver version (in case of Bolt transport): 3.2.23
  • Link to your project:

Steps to Reproduce

@pc-dd pc-dd changed the title Loading node together with connected nodes Loading a node together with other connected nodes of different types Jul 12, 2023
@pc-dd
Copy link
Author

pc-dd commented Jul 12, 2023

For more context:

@NodeEntity
data class A(
    @Id val id: String? = null,
    @Relationship(type = "HAS_B")
    val b: MutableSet<B> = mutableSetOf<B>()
)

@RelationshipEntity(type = "HAS_B")
data class HasB @JvmOverloads constructor(
    @Id @GeneratedValue val id: Long? = null,
    @StartNode val start: A = A(),
    @EndNode val end: B = B()
)

@NodeEntity
data class B(
    @Id val id: String? = null,
    @Relationship(type = "HAS_C")
    val c: MutableSet<C> = mutableSetOf<C>()
)

...

@meistermeier
Copy link
Collaborator

You are not loading the HasB relationship entity but the B entity directly.

What worked for me:

@NodeEntity
data class A(
    @Id val id: String? = null,
    @Relationship(type = "HAS_B")
    var b: MutableSet<HasB> = mutableSetOf()
)

@RelationshipEntity(type = "HAS_B")
class HasB @JvmOverloads constructor(
    @Id @GeneratedValue val id: Long? = null,
    @StartNode val start: A = A(),
    @EndNode val end: B = B()
) {
    override fun toString(): String {
        return "HasB(id=$id, end=$end)"
    }
}

@NodeEntity
data class B(
    @Id val id: String? = null,
    @Relationship(type = "HAS_C")
    val c: MutableSet<C> = mutableSetOf()
)

@NodeEntity
data class C(
    @Id val id: String? = null
)

as you can see, I removed the data from data class B because the Has_B will create a class cycle that will call e.g. hashCode etc. in an endless loop (until it hits a stack overflow). There might be ways in Kotlin to keep it as a data class but ignore some fields. But my knowledge is limited about the Kotlin language specifics.

With a test

val c1 = C("C1")
val c2 = C("C2")
val b = B("b1", mutableSetOf(c1, c2))
val a = A("a1")
val hasB = HasB(null, a, b)
a.b = mutableSetOf(hasB)

var session = sessionFactory.openSession()
// I like my database empty before testing: session.purgeDatabase()

session.save(a)

// just to be sure to have a completely clean session state
session = sessionFactory.openSession()

val load = session.load(A::class.java, "a1", -1)

println(load)

I get A(id=a1, b=[HasB(id=6, end=B(id=b1, c=[C(id=C1), C(id=C2)]))])

@pc-dd
Copy link
Author

pc-dd commented Jul 13, 2023

Thank you for the response! This seems to be working.

Could you elaborate on why we should reference hasB instead of B? We were following the example here but this example has no RelationshipEntity. What is the underlying difference?

My guess is, it needs to trace from @NodeEntity data class A to @RelationshipEntity(type = "HAS_B") class HasB directly, but not what we thought to be true, i.e. trace from A to @Relationship(type = "HAS_B"), and then via the HAS_B trace to @RelationshipEntity(type = "HAS_B") data class HasB

@meistermeier
Copy link
Collaborator

First of all I somehow came to the conclusion that the @RelationshipEntity was the problem, sorry.
The explanation for this behaviour is that Neo4j-OGM finds the definition of HAS_B on this entity that also connects the types A and B. This brings the relationship entity being found and it eagerly wants to map to this....and fails.
I have to make up my mind about this. Currently I am in the mood to say this is a bug/unneeded limitation (if we don't mention this in the documentation). It was done before I maintained the project, so I have to be a little bit careful to not miss anything that led to the decision.

@pc-dd
Copy link
Author

pc-dd commented Jul 13, 2023

First of all I somehow came to the conclusion that the @RelationshipEntity was the problem, sorry. The explanation for this behaviour is that Neo4j-OGM finds the definition of HAS_B on this entity that also connects the types A and B. This brings the relationship entity being found and it eagerly wants to map to this....and fails. I have to make up my mind about this. Currently I am in the mood to say this is a bug/unneeded limitation (if we don't mention this in the documentation). It was done before I maintained the project, so I have to be a little bit careful to not miss anything that led to the decision.

Thanks! So I guess the key is "what is being annotated with @relationship"
Looks like in order for OGM to go from node entity A to relationship entity HasB, the thing being annotated with @relationship has to refer to the relationship entity itself, instead of node entity B (i.e. the node entity it connects A to). Is my understanding correct?

@meistermeier
Copy link
Collaborator

Yes, that is 100% correct. In your case this will solve the problem. If you don't need the properties on a relationship, you could also just remove the whole definition of the RelationshipEntity and OGM will map A->B.

It is just the definition for the RelationshipEntity named HAS_B in the metadata registry of OGM has precedence over the direct relationship definition in the NodeEntity. I have already discovered a little bit more why this happens and is explicitly defined in the mapping parts and currently testing a possible additional condition to avoid the mapping of the RelationshipEntity instead of the direct relationship.

@pc-dd
Copy link
Author

pc-dd commented Jul 13, 2023

Yes, that is 100% correct. In your case this will solve the problem. If you don't need the properties on a relationship, you could also just remove the whole definition of the RelationshipEntity and OGM will map A->B.

It is just the definition for the RelationshipEntity named HAS_B in the metadata registry of OGM has precedence over the direct relationship definition in the NodeEntity. I have already discovered a little bit more why this happens and is explicitly defined in the mapping parts and currently testing a possible additional condition to avoid the mapping of the RelationshipEntity instead of the direct relationship.

Got it, thanks! Closing this issue

@pc-dd pc-dd closed this as completed Jul 13, 2023
meistermeier added a commit that referenced this issue Jul 14, 2023
If the source model has a relationship of a specific type defined that
is also covered by a relationship entity, Neo4j-OGM would eagerly
try to map everything to this entity before it notices that the source
is not using the entity and does not put in the related node because
the types are mismatching.
This commit will fix this behaviour while keeping already existing functionality in place as it is.
@meistermeier
Copy link
Collaborator

There is now a fix for this in place in 3.2.41: https://github.com/neo4j/neo4j-ogm/releases/tag/v3.2.41

@pc-dd
Copy link
Author

pc-dd commented Jul 14, 2023

There is now a fix for this in place in 3.2.41: https://github.com/neo4j/neo4j-ogm/releases/tag/v3.2.41

Great, thanks! So now having the following would work?

@NodeEntity
data class A(
    @Id val id: String? = null,
    @Relationship(type = "HAS_B")
    val b: MutableSet<B> = mutableSetOf<B>()
)

(instead of var b: MutableSet<HasB> = mutableSetOf())

EDIT: looks like yes, just upgraded and tested. This is great

@pc-dd pc-dd reopened this Jul 14, 2023
@pc-dd pc-dd closed this as completed Jul 14, 2023
@pc-dd pc-dd changed the title Loading a node together with other connected nodes of different types Direct Relationship Persistence Issue Jul 14, 2023
@meistermeier
Copy link
Collaborator

Thanks for the fast feedback on this. Faster than I could reply to the question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants