Add new enum to specify the direction of a relationship #785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add Relationship.Direction enum and change type of "direction" member to that new enum * introduce converter branch and new method in AnnotationInfo class to support enum members of annotations * adapt return type of methods which previously returned the relationship direction string to now return an enum constant * adapt @relationship annotations in node entities for integration tests * adapt the docs to reflect the changed type
This removes the new API on the AnnotationInfo. This API was very well done and tested, but it increases our surface again and we don’t want that. It’s needed in two places and it’s ok to duplicate it the instantiation there. There’s one signifacant change we’re we check whether the attribute is an enum or not. For comparision: #644
michael-simons
requested changes
Apr 6, 2020
core/src/main/java/org/neo4j/ogm/context/DirectedRelationshipForType.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/context/EntityGraphMapper.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/context/EntityGraphMapper.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/session/request/PathMatchClause.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/session/request/PathMatchClause.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/session/request/PathMatchClause.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/session/request/RelationshipPropertyMatchClause.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/session/request/RelationshipPropertyMatchClause.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #785 +/- ##
============================================
- Coverage 80.35% 80.35% -0.01%
Complexity 3002 3002
============================================
Files 274 275 +1
Lines 9169 9163 -6
Branches 1379 1370 -9
============================================
- Hits 7368 7363 -5
Misses 1299 1299
+ Partials 502 501 -1
Continue to review full report at Codecov.
|
|
Thanks for letting me push into your branch. |
* replace `Relationship.*` imports with `Relationship.Direction`where possible * use reference equals to compare enum constants * refactor EntityGraphMapper#clearContextRelationships to use a switch statement
|
Looks very good to me now… I'm gonna go through one more time and might push a thing or two. Would you please sign our CLA? https://neo4j.com/developer/cla/ so that I can whitelist you in our CI? Thank you. |
This introduces some aliases on the `@Relationship` for the `Direction` constants. These provide some compatibility for users relying on the previous string constants in their declarations. Furthermore, the availibility of the enum shows some points that should be refactored all too painfull, especially in `ClassInfo` which has been done. The rest is minor polishing.
|
Done ✔️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a new enum
Directionin the@Relationshipannotation to replace String based relationship directionDescription
new enum
enum members of annotations
to now return an enum constant to only use the new enum constants internally
Related Issue
#631
Motivation and Context
Specifying the direction of a relationship through a simple string may lead to confusion and might cause unexpected behaviour when a typo is overlooked
How Has This Been Tested?
Running existing test cases and 4 newly added ones
Types of changes
Checklist: