-
Notifications
You must be signed in to change notification settings - Fork 349
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
DATAJDBC-218 - Add support for key column in @Column annotation #83
Conversation
@@ -70,6 +72,7 @@ public BasicRelationalPersistentProperty(Property property, PersistentEntity<?, | |||
|
|||
this.context = context; | |||
this.columnName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Column.class)).map(Column::value)); | |||
this.keyColumnName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Column.class)).map(Column::keyColumn)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the default value for the keyColumn field is ""
, wouldn't you want to filter that out? Perhaps something like:
...map(Column::keyColumn).filter(x -> x != null && !x.equals(""))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right, filtering out the default value is important here, I have added that.
Why are you also checking x for being not null, that case can not come up if I'm not mistaken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's my bad. Forgot that null
isn't an allowable value for annotations. It's been a while since I've written a custom annotation processor :)
Added line breaks for better readability. Added braces to if/else statements. Used StringUtils instead of .equals(""). Original pull request: #83.
Also: Added documentation for the new behavior. Made the value of the column annotation optional by providing a default. Original pull request: #83.
Thanks, that is polished and merged. |
This adds the functionality requested in DATAJDBC-218, so you can now customize the key column name for List and Map collections inside the @column annotation.
I have also added a test case to demonstrate the behavior.