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

@RelationshipProperties support for @Id @GeneratedValue private UUID id; #2620

Open
MaurizioCasciano opened this issue Oct 27, 2022 · 16 comments
Labels
type: enhancement A general enhancement

Comments

@MaurizioCasciano
Copy link

MaurizioCasciano commented Oct 27, 2022

The @RelationshipProperties only supports

@Id
@GeneratedValue
private Long id;

Can this be extended to also support the UUID type as it is for @Node annotated classes?

@Id
@GeneratedValue
private UUID id;
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 27, 2022
@michael-simons
Copy link
Collaborator

Thanks, @MaurizioCasciano that is an excellent topic to talk to. Right now, not yet. This is atm strictly the internal marker we need to keep the rel properties internally identifiable without creating proxies for them. We need to work in this area anyhow as Neo4j 5 deprecated support for id() instead for elementId().

Pinging @meistermeier about this too

@meistermeier
Copy link
Collaborator

I would be interested in the use-case, you are trying to cover by assigning custom ids? I am just trying to figure out how this can benefit SDN.
As Michael described it correctly, the internal id is a helper for us to not override existing relationships on save/update that might contain unknown (to SDN) properties.

@meistermeier meistermeier added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 1, 2022
@MaurizioCasciano
Copy link
Author

Hi @meistermeier, the use-case I am trying to cover is the modeling of unique relationships having an external identifier (String | UUID) without having to maintain multiple IDs in the DB.

We could add a property to the relationship (e.g. private UUID uuid;) but then we will have also private Long id; creating confusion. Furthermore, the uuid field would also require a unique constraint to be generated.

Considering as an example a list of software licensed to a list of people, there is the need to model the license as a relationship using the existing identifier.

(s:Software{id:"9149843d-aef1-4795-8dbd-481ca365c3c6"})-[:License{id:"a915af52-961a-4fc0-a5d6-0f51b601e92e", expiration:"2022-11-01T11:24:58Z"}]->(p:Person{id:"ca33c0b9-e61c-4be3-9105-b21db0a68e2d"})

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 1, 2022
@meistermeier
Copy link
Collaborator

Sorry for losing track of this issue. Unfortunately, I can only repeat ourselves and tell you that it is not this easy to add an alternative id type (like we have in the entities) to RelationshipProperties. A huge parts of the handling of those definitions relies on knowing the internal id (query creation, for example).
But I see the benefits it could bring for a cleaner definition of the entity itself. I leave this issue open as an enhancement idea.

@meistermeier meistermeier added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Jan 20, 2023
@pabloaguilarm
Copy link

pabloaguilarm commented Jun 5, 2023

Hi @meistermeier , related with this, I am having some issues because, as @michael-simons said, with the change from id() to elementId(), it appears a lot of warnings when a relation is saved. I have been able to update nodes from long id to string, but RelationshipProperties fail.

@meistermeier
Copy link
Collaborator

Could you tell us which version of SDN you are running? If it is SDN 7.1.0 (with Spring Boot 3.1.0) there was a changed introduce that handles elementId() as first class ids. But you have to tell the underlying Cypher-DSL to use the NEO4J_5 dialect. This will be added to the documentation with the next patch release. Also, there will be a change added that mitigates the situation if the dialect is not explicitly set.
See also #2729 (comment) for reference to the needed Cypher-DSL configuration bean.

@pabloaguilarm
Copy link

We are using Spring Boot 2.7.12. Does this version have the changes or Spring Boot 2.7.12 has SDN 6.3.12?

@pabloaguilarm
Copy link

pabloaguilarm commented Jun 13, 2023

Even with the Cypher-DSL configuration bean, it appears this error for the properties of a relationship is missing a property for the generated, internal ID (@Id @GeneratedValue Long id) which is needed for safely updating properties.. And @meistermeier, for what I am seeing, it appears the same error if I just change the id of the target node of relationship and leave the relation id as long.

@antonydelgado
Copy link

Hi guys, why is not posible add uuid for id in relationship entity (@RelationshipProperties), via spring boob 3.1 if in neo4j 5 browser it is posible.
image

@ioxua
Copy link

ioxua commented Jan 16, 2024

@antonydelgado as I understood, even if it's possible to use uuid IDs in Neo4j, spring-data-neo4j does not support it since it relies on the internal ID for object mapping

On another note, with Spring Boot 3.2.1 and spring-data-neo4j 7.2.2 I have the same issue as @pabloaguilarm

@meistermeier
Copy link
Collaborator

You are right about the internal id (id() via Long or elementId() via String) restriction in SDN, @ioxua.
To be honest, since relationship (properties) are nothing that can be obtained directly via repositories or Neo4jTemplate.
I haven't heard of a real world use case why this is needed (@antonydelgado?). Those id information already exist in the database. The only limitation is that they have to be explicitly defined in the relationship property class.

The error message, @pabloaguilarm is referring to, is not covering the String id, but supports it. The check under the hood is scanning the relationship class for either Long or String. (Currently updating the message)
Could you please provide a minimal reproducer for the problem? Changing it here on my side from Long to String does not create any issue on my side.

@antonydelgado
Copy link

Hi, thanks for your response. I think that is unnecessary when I use a relational database, but I've used neo4j and I need to export an neural network inmutable for effects of the sequencing process in various businesses where the communication of activities is necessary. So the longs ids could repeat. I've given a solution, I've been going to write validation code in Java and it works but I think that is wrong way, because I think that solution should have come from the SDN, just as it is done in cypher.

@michael-simons
Copy link
Collaborator

You can just add any value to a @RelationshipProperties annotated class, you can assign uuids from your Java code. Any value you assign will be saved, the solution is here in SDN already.

The long ids respectively the element ids won't repeat, they are generated by the database.

@antonydelgado
Copy link

Hi, I try to add UUID as a Id in @RelationshipProperties class but ocurre errors when I compile and it doesn't work for me, do you do it like this? and it works for you?

@michael-simons
Copy link
Collaborator

You need to understand the difference between id/element id on one side and any other property on the other side first.

Take a look at this graph:

CREATE (a:A {
  id: 42,
  elementId: 'blah'
})-[r:RELATED_TO {
  id: randomUUID()
}]->(b:B {
  id: 4711,
  elementId: 'foobar'
})

It creates two nodes, with two properties each, and a relationship with one property.
The names of the properties being id and element Id.
Now let's read it:

MATCH (a:A) -[r:RELATED_TO]->(b:B)
RETURN a.id AS a_id_prop, a.elementId AS a_element_id_prop,
       id(a) AS a_id, elementId(a) AS a_element_id,
       r.id AS r_id_prop, id(r) as r_id, elementId(r) AS r_element_id;

The result will be:

+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| a_id_prop | a_element_id_prop | a_id | a_element_id                               | r_id_prop                              | r_id | r_element_id                               |
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| 42        | "blah"            | 0    | "4:010023ce-56e4-4571-9fc9-1208617ae0ae:0" | "7c40cfc4-5427-4eaa-8985-4e4ae7a2c09f" | 0    | "5:010023ce-56e4-4571-9fc9-1208617ae0ae:0" |
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

When you do this on your database, the values will be different.

What do we see here? there is a difference between a property named id or element id in the graph vs the ids / or modern Neo4j elementIds of elements of the graph.
We do use the latter, which are generated by the database, to track entities in Java.

We also do use them to make Nodes addressable in Java, unless - and btw recommend - you use an externally assigned UUID.
We never allow relationships to be addressed in isolation through SDN - which @meistermeier already told you, so there we don't give you the option.
BUT that doesn't stop you from adding a property, fill it with a value, and assign a randomUUID to it from your Java code prior to saving.

@antonydelgado
Copy link

antonydelgado commented Jan 17, 2024

Yeah, for that reason i question, becouse i don't know, but thanks for your time in respond me. Could you please review this code where i try introduce a property id (as you recomend, but in my entity class), as a uuid or String?, but i obtain an error that say " ...for the properties of a relationship is missing a property for the generated, internal ID (@Id @GeneratedValue Long id) which is needed for safely updating properties". I think that it is fail of SDN, but i don't know.

RELATION

@Getter
@Setter
@ToString
@RequiredArgsConstructor
@AllArgsConstructor(onConstructor = @__(@PersistenceCreator))
@EqualsAndHashCode(callSuper = false)
@Persistent
@RelationshipProperties
public class R {
    @Id
    @GeneratedValue(GeneratedValue.UUIDGenerator.class)
    @Property(name = "id")
    private UUID id;
    @TargetNode
    private C c;
    @NotNull(message = "Is necessary quantity value")
    @Property(name = "quantity")
    private Long quantity = 1L;
}

NODE A


@Getter
@Setter
@ToString
@RequiredArgsConstructor
@AllArgsConstructor(onConstructor = @__(@PersistenceCreator))
@EqualsAndHashCode(callSuper = false)
@Persistent
@Node(labels = {"A"})
public class A {
    @Id
    @GeneratedValue(GeneratedValue.UUIDGenerator.class)
    private UUID id;
    @Relationship(type = "TEST_FOR", direction = Relationship.Direction.OUTGOING)
    private List<R> rForRelations;
}

NODE C

@Node(labels = {"C"})
public class C {
    @Id
    @GeneratedValue(GeneratedValue.UUIDGenerator.class)
    private UUID id;
...
...
}

Thanks for read and try help me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants