Spanner added bounded staleness option and tests #1727
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1727 +/- ##
============================================
+ Coverage 67.21% 67.28% +0.07%
- Complexity 1715 1725 +10
============================================
Files 257 257
Lines 6911 6927 +16
Branches 692 694 +2
============================================
+ Hits 4645 4661 +16
- Misses 1948 1949 +1
+ Partials 318 317 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1727 +/- ##
===========================================
+ Coverage 67.21% 67.3% +0.09%
- Complexity 1715 1724 +9
===========================================
Files 257 258 +1
Lines 6911 6931 +20
Branches 692 692
===========================================
+ Hits 4645 4665 +20
- Misses 1948 1949 +1
+ Partials 318 317 -1
Continue to review full report at Codecov.
|
...c/main/java/org/springframework/cloud/gcp/data/spanner/core/SpannerPageableQueryOptions.java
Show resolved
Hide resolved
...c/main/java/org/springframework/cloud/gcp/data/spanner/core/SpannerPageableQueryOptions.java
Outdated
Show resolved
Hide resolved
...anner/src/main/java/org/springframework/cloud/gcp/data/spanner/core/SpannerQueryOptions.java
Show resolved
Hide resolved
| * | ||
| * @author Chengyuan Zhao | ||
| */ | ||
| public class AbstractSpannerRequestOptions<A, B extends AbstractSpannerRequestOptions> { |
There was a problem hiding this comment.
I don't think you need this second class parameter B extends AbstractSpannerRequestOptions. The cast to B is not really safe anyway.
Why not just return AbstractSpannerRequestOptions?
There was a problem hiding this comment.
The B is for the inheriting children class to provide their own class type. The reason is we return this at the end of the setter methods so the user can chain them together. Without the B and returning just the abstract class the child classes' setters would return the parent abstract class on the final setter call so the following wouldn't compile because the final setter returns the parent abstract class type and not the child type:
SpannerReadOptions = new SpannerReadOptions.setblah(xyz).setblahblah(abc).setblahblahblah(123);
The final setter call still just returns the abstract parent class.
There was a problem hiding this comment.
@meltsufin the cast is safe here because B must extend the abstract parent class in the class definition and this is clearly of that same type.
|
|
||
| protected transient List<A> requestOptions = new ArrayList<>(); | ||
|
|
||
| protected Class<A> requestOptionType; |
There was a problem hiding this comment.
Does this actually work? This member is never assigned? Would it not be null?
There was a problem hiding this comment.
It's assigned in the constructors of the child classes. It's needed to create that array of ReadOption or QueryOption that the client lib requires.
| @@ -73,4 +73,13 @@ public SpannerQueryOptions setAllowPartialRead(boolean allowPartialRead) { | |||
| return this; | |||
| } | |||
|
|
|||
| /** | |||
| * Deprecated. Please use {@code getOptions}. | |||
There was a problem hiding this comment.
Can you use @deprecated like we do in other parts of the code base?
fixes #1723
added bounded staleness support and tests and ref doc update