-
Notifications
You must be signed in to change notification settings - Fork 192
DATACOUCH-623 - Add replace() method to CouchbaseRepository for CAS u… #268
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
DATACOUCH-623 - Add replace() method to CouchbaseRepository for CAS u… #268
Conversation
c205141
to
c158cda
Compare
@@ -43,4 +43,8 @@ | |||
@Override | |||
List<T> findAllById(Iterable<ID> iterable); | |||
|
|||
<S extends T> S replace(S var1); |
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.
Adding technology-specific methods violates somewhat the concept of handling technology-specific aspects inside the repository/template API. I left a comment on DATACOUCH-623. Especially entities annotated with @Version
shouldn't require the user to call a different method for proper handling, instead, the requirement to issue a different store-specific replace call should be determined by the Spring Data code and not the caller.
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.
Thanks Mark. I put the upsert() vs. replace() logic in the save().
@mikereiche after looking into this I agree with @mp911de's comment in https://jira.spring.io/browse/DATACOUCH-623?focusedCommentId=191380&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-191380 .. Let's not add to the repository, but rather in our internal code check if the version is present and if present and set use a replace operation and map to the corresponding exception. |
c158cda
to
f9533b2
Compare
} | ||
} | ||
|
||
private boolean hasVersionProperty(Object entity) { |
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.
I think that method can be DRY'ed up? only provided it once and reuse.. also maybe a name that also clarifies it must be present AND not 0 might be better
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.
Done. I put it in a new Utils class.
I had thought of that, but SimpleReactiveCouchbaseRepository has a ReactiveCouchbaseOperations, while SimpleCouchbaseRepository has a CouchbaseOperations. But it doesn't actually need those - it just needs the mapping conext of the converter held by each *Operations - which is the same type.
src/test/java/org/springframework/data/couchbase/domain/UserRepository.java
Outdated
Show resolved
Hide resolved
c85a555
to
b3e0060
Compare
b3e0060
to
3b236c3
Compare
…sage. (#268) Co-authored-by: mikereiche <[email protected]>
…sage.