neo4j-contrib / neomodel

An Object Graph Mapper (OGM) for the Neo4j graph database.
https://neomodel.readthedocs.io
MIT License
939 stars 231 forks source link

Bug fix in `element_id`. #761

Closed aanastasiou closed 10 months ago

aanastasiou commented 10 months ago

If a node object is initialised but not yet saved to the database, then its element_id should return a suitable value to denote that the object is unsaved. Before switching to element_id, we used to examine the _id of an element to be able to tell if the node was saved or not.

The intention seems to be there but since element_id_property is a dynamical property, it was not getting a default value. (Unsaved nodes do not have their element_id property at all and therefore .element_id was failing. On the same subject, this line had to be modified as well, otherwise the node would not save (better to check the attribute that the property maps rather than the property itself anyway).

Regarding a possible default value for element_id_property, I am not sure that this is better than giving StructuredNode an element_id_attribute with a default value, but it seemed to be the way to go to maintain compatibility. I would suggest that we go with a default property on a subsequent major release.

Following this, the tests were also adapted to ensure that we check that an uninitialised object can be handled properly and that it gives us a chance to interrogate its element_id to understand if it is saved or not.

P.S. Certain tests still fail locally :/ but they do not seem related to this particular edit. Nevertheless, I am just noting here that, although this was a small fix, not all tests completed successfully and because of their nature, I am not sure if they could be related or not (the Aura ones are definitely not related, this is for sure, but a couple of tests on "concurrency" were also failing).

sonarcloud[bot] commented 10 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information