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
feat: add Query.limitToLast() #151
Changes from 4 commits
fa8ddc1
79b5f51
c4d0881
fe20e96
0a5deea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
|
||
package com.google.cloud.firestore; | ||
|
||
import static com.google.common.collect.Lists.reverse; | ||
import static com.google.firestore.v1.StructuredQuery.FieldFilter.Operator.ARRAY_CONTAINS; | ||
import static com.google.firestore.v1.StructuredQuery.FieldFilter.Operator.ARRAY_CONTAINS_ANY; | ||
import static com.google.firestore.v1.StructuredQuery.FieldFilter.Operator.EQUAL; | ||
|
@@ -182,6 +183,12 @@ Order toProto() { | |
} | ||
} | ||
|
||
/** Denotes whether a provided limit is applied to the beginning or the end of the result set. */ | ||
enum LimitType { | ||
First, | ||
Last | ||
} | ||
|
||
/** Options that define a Firestore Query. */ | ||
@AutoValue | ||
abstract static class QueryOptions { | ||
|
@@ -194,6 +201,8 @@ abstract static class QueryOptions { | |
|
||
abstract @Nullable Integer getLimit(); | ||
|
||
abstract @Nullable LimitType getLimitType(); | ||
|
||
abstract @Nullable Integer getOffset(); | ||
|
||
abstract @Nullable Cursor getStartCursor(); | ||
|
@@ -226,6 +235,8 @@ abstract static class Builder { | |
|
||
abstract Builder setLimit(Integer value); | ||
|
||
abstract Builder setLimitType(LimitType value); | ||
|
||
abstract Builder setOffset(Integer value); | ||
|
||
abstract Builder setStartCursor(@Nullable Cursor value); | ||
|
@@ -764,15 +775,33 @@ public Query orderBy(@Nonnull FieldPath fieldPath, @Nonnull Direction direction) | |
} | ||
|
||
/** | ||
* Creates and returns a new Query that's additionally limited to only return up to the specified | ||
* number of documents. | ||
* Creates and returns a new Query that only returns the first matching documents. | ||
* | ||
* @param limit The maximum number of items to return. | ||
* @return The created Query. | ||
*/ | ||
@Nonnull | ||
public Query limit(int limit) { | ||
return new Query(firestore, options.toBuilder().setLimit(limit).build()); | ||
return new Query( | ||
firestore, options.toBuilder().setLimit(limit).setLimitType(LimitType.First).build()); | ||
} | ||
|
||
/** | ||
* Creates and returns a new Query that only returns the last matching documents. | ||
* | ||
* <p>You must specify at least one orderBy clause for limitToLast queries, otherwise an {@link | ||
* java.lang.IllegalStateException} will be thrown during execution. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will be --> is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
* | ||
* <p>Results for limitToLast queries cannot be streamed via the {@link | ||
* #stream(ApiStreamObserver)} API. | ||
* | ||
* @param limit The maximum number of items to return. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the (no caps) and no period at end since this is not a complete sentence There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left it as is for now to be consistent with the rest of the code. I will send a PR once this is merged to clean this up everywhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Google Java practices explicitly rule out consistency with existing code as a justification for violating style guidelines in newly added code: https://engdoc.corp.google.com/eng/doc/devguide/java/style/index.md?cl=head#s8.5.2-newly-added-code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
* @return The created Query. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
*/ | ||
@Nonnull | ||
public Query limitToLast(int limit) { | ||
return new Query( | ||
firestore, options.toBuilder().setLimit(limit).setLimitType(LimitType.Last).build()); | ||
} | ||
|
||
/** | ||
|
@@ -1004,8 +1033,22 @@ StructuredQuery.Builder buildQuery() { | |
|
||
if (!options.getFieldOrders().isEmpty()) { | ||
for (FieldOrder order : options.getFieldOrders()) { | ||
structuredQuery.addOrderBy(order.toProto()); | ||
if (LimitType.Last.equals(options.getLimitType())) { | ||
// Flip the orderBy directions since we want the last results | ||
order = | ||
new FieldOrder( | ||
order.fieldPath, | ||
order.direction.equals(Direction.ASCENDING) | ||
? Direction.DESCENDING | ||
: Direction.ASCENDING); | ||
structuredQuery.addOrderBy(order.toProto()); | ||
} else { | ||
structuredQuery.addOrderBy(order.toProto()); | ||
} | ||
} | ||
} else if (LimitType.Last.equals(options.getLimitType())) { | ||
throw new IllegalStateException( | ||
"limitToLast() queries require specifying at least one orderBy() clause."); | ||
} | ||
|
||
if (!options.getFieldProjections().isEmpty()) { | ||
|
@@ -1021,11 +1064,33 @@ StructuredQuery.Builder buildQuery() { | |
} | ||
|
||
if (options.getStartCursor() != null) { | ||
structuredQuery.setStartAt(options.getStartCursor()); | ||
if (LimitType.Last.equals(options.getLimitType())) { | ||
// Swap the cursors to match the flipped query ordering. | ||
Cursor cursor = | ||
options | ||
.getStartCursor() | ||
.toBuilder() | ||
.setBefore(!options.getStartCursor().getBefore()) | ||
.build(); | ||
structuredQuery.setEndAt(cursor); | ||
} else { | ||
structuredQuery.setStartAt(options.getStartCursor()); | ||
} | ||
BenWhitehead marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (options.getEndCursor() != null) { | ||
structuredQuery.setEndAt(options.getEndCursor()); | ||
if (LimitType.Last.equals(options.getLimitType())) { | ||
// Swap the cursors to match the flipped query ordering. | ||
Cursor cursor = | ||
options | ||
.getEndCursor() | ||
.toBuilder() | ||
.setBefore(!options.getEndCursor().getBefore()) | ||
.build(); | ||
structuredQuery.setStartAt(cursor); | ||
} else { | ||
structuredQuery.setEndAt(options.getEndCursor()); | ||
} | ||
BenWhitehead marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return structuredQuery; | ||
|
@@ -1037,7 +1102,12 @@ StructuredQuery.Builder buildQuery() { | |
* @param responseObserver The observer to be notified when results arrive. | ||
*/ | ||
public void stream(@Nonnull final ApiStreamObserver<DocumentSnapshot> responseObserver) { | ||
stream( | ||
Preconditions.checkState( | ||
!LimitType.Last.equals(Query.this.options.getLimitType()), | ||
"Query results for queries that include limitToLast() constraints cannot be streamed. " | ||
+ "Use Query.get() instead."); | ||
BenWhitehead marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
internalStream( | ||
new QuerySnapshotObserver() { | ||
@Override | ||
public void onNext(QueryDocumentSnapshot documentSnapshot) { | ||
|
@@ -1073,7 +1143,7 @@ Timestamp getReadTime() { | |
} | ||
} | ||
|
||
private void stream( | ||
private void internalStream( | ||
final QuerySnapshotObserver documentObserver, @Nullable ByteString transactionId) { | ||
RunQueryRequest.Builder request = RunQueryRequest.newBuilder(); | ||
request.setStructuredQuery(buildQuery()).setParent(options.getParentPath().toString()); | ||
|
@@ -1178,7 +1248,7 @@ public ListenerRegistration addSnapshotListener( | |
ApiFuture<QuerySnapshot> get(@Nullable ByteString transactionId) { | ||
final SettableApiFuture<QuerySnapshot> result = SettableApiFuture.create(); | ||
|
||
stream( | ||
internalStream( | ||
new QuerySnapshotObserver() { | ||
List<QueryDocumentSnapshot> documentSnapshots = new ArrayList<>(); | ||
|
||
|
@@ -1194,8 +1264,14 @@ public void onError(Throwable throwable) { | |
|
||
@Override | ||
public void onCompleted() { | ||
// The results for limitToLast queries need to be flipped since we reversed the | ||
// ordering constraints before sending the query to the backend. | ||
List<QueryDocumentSnapshot> resultView = | ||
LimitType.Last.equals(Query.this.options.getLimitType()) | ||
? reverse(documentSnapshots) | ||
: documentSnapshots; | ||
QuerySnapshot querySnapshot = | ||
QuerySnapshot.withDocuments(Query.this, this.getReadTime(), documentSnapshots); | ||
QuerySnapshot.withDocuments(Query.this, this.getReadTime(), resultView); | ||
result.set(querySnapshot); | ||
} | ||
}, | ||
|
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.
, otherwise --> . Otherwise
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