Skip to content

Commit ae2ac53

Browse files
schmidt-sebastianjabubake
authored andcommitted
Using the pageable response from ListCollectionIds (#2530)
1 parent ce99df1 commit ae2ac53

13 files changed

Lines changed: 97 additions & 116 deletions

File tree

google-cloud-firestore/pom.xml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@
4343
<groupId>com.google.api</groupId>
4444
<artifactId>api-common</artifactId>
4545
</dependency>
46-
<dependency>
47-
<groupId>${project.groupId}</groupId>
48-
<artifactId>google-cloud-core-http</artifactId>
49-
</dependency>
5046
<dependency>
5147
<groupId>com.google.api.grpc</groupId>
5248
<artifactId>proto-google-cloud-firestore-v1beta1</artifactId>

google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
import com.google.api.core.ApiFunction;
2020
import com.google.api.core.ApiFuture;
2121
import com.google.api.core.ApiFutures;
22+
import com.google.api.gax.rpc.ApiException;
23+
import com.google.api.gax.rpc.ApiExceptions;
24+
import com.google.cloud.firestore.v1beta1.PagedResponseWrappers.ListCollectionIdsPagedResponse;
2225
import com.google.firestore.v1beta1.ListCollectionIdsRequest;
23-
import com.google.firestore.v1beta1.ListCollectionIdsResponse;
24-
import java.util.ArrayList;
26+
import java.util.Iterator;
2527
import java.util.List;
2628
import java.util.Map;
2729
import java.util.Objects;
28-
import java.util.SortedSet;
29-
import java.util.TreeSet;
3030
import javax.annotation.Nonnull;
3131
import javax.annotation.Nullable;
3232

@@ -339,27 +339,46 @@ public ApiFuture<DocumentSnapshot> get() {
339339
/**
340340
* Fetches the subcollections that are direct children of this document.
341341
*
342-
* @return An ApiFuture that will be resolved with the list of subcollections.
342+
* @throws FirestoreException if the Iterable could not be initialized.
343+
* @return An Iterable that can be used to fetch all subcollections.
343344
*/
344-
public ApiFuture<List<CollectionReference>> getCollections() {
345+
public Iterable<CollectionReference> getCollections() {
345346
ListCollectionIdsRequest.Builder request = ListCollectionIdsRequest.newBuilder();
346347
request.setParent(path.toString());
347-
ApiFuture<ListCollectionIdsResponse> response =
348-
firestore.sendRequest(request.build(), firestore.getClient().listCollectionIdsCallable());
348+
final ListCollectionIdsPagedResponse response;
349349

350-
return ApiFutures.transform(
351-
response,
352-
new ApiFunction<ListCollectionIdsResponse, List<CollectionReference>>() {
350+
try {
351+
response =
352+
ApiExceptions.callAndTranslateApiException(
353+
firestore.sendRequest(
354+
request.build(), firestore.getClient().listCollectionIdsPagedCallable()));
355+
} catch (ApiException exception) {
356+
throw FirestoreException.apiException(exception);
357+
}
358+
359+
return new Iterable<CollectionReference>() {
360+
@Override
361+
@Nonnull
362+
public Iterator<CollectionReference> iterator() {
363+
final Iterator<String> iterator = response.iterateAll().iterator();
364+
return new Iterator<CollectionReference>() {
353365
@Override
354-
public List<CollectionReference> apply(ListCollectionIdsResponse response) {
355-
List<CollectionReference> collectionList = new ArrayList<>();
356-
SortedSet<String> sortedIds = new TreeSet<>(response.getCollectionIdsList());
357-
for (String collectionId : sortedIds) {
358-
collectionList.add(DocumentReference.this.collection(collectionId));
359-
}
360-
return collectionList;
366+
public boolean hasNext() {
367+
return iterator.hasNext();
361368
}
362-
});
369+
370+
@Override
371+
public CollectionReference next() {
372+
return DocumentReference.this.collection(iterator.next());
373+
}
374+
375+
@Override
376+
public void remove() {
377+
throw new UnsupportedOperationException("remove");
378+
}
379+
};
380+
}
381+
};
363382
}
364383

365384
@Override

google-cloud-firestore/src/main/java/com/google/cloud/firestore/Firestore.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,11 @@ public interface Firestore extends Service<FirestoreOptions> {
4545
/**
4646
* Fetches the root collections that are associated with this Firestore database.
4747
*
48-
* @return An ApiFuture that will be resolved with the list of collections.
48+
* @throws FirestoreException if the Iterable could not be initialized.
49+
* @return An Iterable that can be used to fetch all collections.
4950
*/
5051
@Nonnull
51-
ApiFuture<List<CollectionReference>> getCollections();
52+
Iterable<CollectionReference> getCollections();
5253

5354
/**
5455
* Executes the given updateFunction and then attempts to commit the changes applied within the

google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreException.java

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -16,62 +16,61 @@
1616

1717
package com.google.cloud.firestore;
1818

19-
import com.google.cloud.http.BaseHttpServiceException;
20-
import com.google.common.collect.ImmutableSet;
19+
import com.google.api.gax.rpc.ApiException;
20+
import com.google.cloud.grpc.BaseGrpcServiceException;
21+
import io.grpc.Status;
2122
import java.io.IOException;
22-
import java.util.Set;
2323

2424
/** A Firestore Service exception. */
25-
public final class FirestoreException extends BaseHttpServiceException {
25+
public final class FirestoreException extends BaseGrpcServiceException {
2626

27-
private static final Set<Error> RETRYABLE_ERRORS =
28-
ImmutableSet.of(
29-
new Error(10, "ABORTED", false),
30-
new Error(4, "DEADLINE_EXCEEDED", false),
31-
new Error(14, "UNAVAILABLE", true));
32-
private static final long serialVersionUID = 9100921023984662143L;
33-
34-
private FirestoreException(int code, String message, String reason) {
35-
this(code, message, reason, true, null);
27+
private FirestoreException(String reason, Status status) {
28+
super(reason, null, status.getCode().value(), false);
3629
}
3730

38-
private FirestoreException(
39-
int code, String message, String reason, boolean idempotent, Throwable cause) {
40-
super(code, message, reason, idempotent, RETRYABLE_ERRORS, cause);
31+
private FirestoreException(IOException exception, boolean retryable) {
32+
super(exception, retryable);
4133
}
4234

43-
private FirestoreException(IOException exception) {
44-
super(exception, true, RETRYABLE_ERRORS);
35+
private FirestoreException(ApiException exception) {
36+
super(exception);
4537
}
4638

4739
/**
48-
* Create a FirestoreException with {@code FAILED_PRECONDITION} reason and the {@code message} in
49-
* a nested exception.
40+
* Creates a FirestoreException with an {@code INVALID_ARGUMENT} status code and the provided
41+
* message in a nested exception.
5042
*
5143
* @return The FirestoreException
5244
*/
5345
static FirestoreException invalidState(String message, Object... params) {
54-
return new FirestoreException(
55-
UNKNOWN_CODE, String.format(message, params), "FAILED_PRECONDITION");
46+
return new FirestoreException(String.format(message, params), Status.INVALID_ARGUMENT);
47+
}
48+
49+
/**
50+
* Creates a FirestoreException with the provided GRPC Status code and message in a nested
51+
* exception.
52+
*
53+
* @return The FirestoreException
54+
*/
55+
static FirestoreException serverRejected(Status status, String message, Object... params) {
56+
return new FirestoreException(String.format(message, params), status);
5657
}
5758

5859
/**
59-
* Create a FirestoreException with {@code FAILED_PRECONDITION} reason and the {@code message} in
60-
* a nested exception.
60+
* Creates a FirestoreException from an IOException.
6161
*
6262
* @return The FirestoreException
6363
*/
64-
static FirestoreException serverRejected(String message, Object... params) {
65-
return new FirestoreException(UNKNOWN_CODE, String.format(message, params), "CANCELLED");
64+
static FirestoreException networkException(IOException exception, boolean retryable) {
65+
return new FirestoreException(exception, retryable);
6666
}
6767

6868
/**
69-
* Create a FirestoreException with {@code FAILED_PRECONDITION} reason and the {@code message} in
70-
* a nested exception.
69+
* Creates a FirestoreException from an ApiException.
7170
*
7271
* @return The FirestoreException
7372
*/
74-
static FirestoreException networkException(IOException exception) {
73+
static FirestoreException apiException(ApiException exception) {
7574
return new FirestoreException(exception);
7675
}
7776
}

google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.protobuf.ByteString;
3535
import com.google.protobuf.NullValue;
3636
import com.google.protobuf.Timestamp;
37+
import io.grpc.Status;
3738
import java.util.ArrayList;
3839
import java.util.Date;
3940
import java.util.HashMap;
@@ -175,7 +176,7 @@ public DocumentReference document(@Nonnull String documentPath) {
175176

176177
@Nonnull
177178
@Override
178-
public ApiFuture<List<CollectionReference>> getCollections() {
179+
public Iterable<CollectionReference> getCollections() {
179180
DocumentReference rootDocument = new DocumentReference(this, this.databasePath);
180181
return rootDocument.getCollections();
181182
}
@@ -357,7 +358,7 @@ private void maybeRetry() {
357358
} else {
358359
rejectTransaction(
359360
FirestoreException.serverRejected(
360-
"Transaction was cancelled because of too many retries."));
361+
Status.ABORTED, "Transaction was cancelled because of too many retries."));
361362
}
362363
}
363364

google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public FirestoreRpc create(@Nonnull FirestoreOptions options) {
7070
try {
7171
return new GrpcFirestoreRpc(options);
7272
} catch (IOException e) {
73-
throw FirestoreException.networkException(e);
73+
throw FirestoreException.networkException(e, false);
7474
}
7575
}
7676
}

google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ private StructuredQuery.Builder buildQuery() {
648648
}
649649

650650
/**
651-
* Executes the query and streams the results as a StreamObserver of DocumentSnapsnots.
651+
* Executes the query and streams the results as a StreamObserver of DocumentSnapshots.
652652
*
653653
* @param responseObserver The observer to be notified when results arrive.
654654
*/
@@ -742,7 +742,7 @@ public ApiFuture<QuerySnapshot> get() {
742742
return get(null);
743743
}
744744

745-
ApiFuture<QuerySnapshot> get(@Nullable ByteString transctionId) {
745+
ApiFuture<QuerySnapshot> get(@Nullable ByteString transactionId) {
746746
final SettableApiFuture<QuerySnapshot> result = SettableApiFuture.create();
747747

748748
stream(
@@ -766,7 +766,7 @@ public void onCompleted() {
766766
result.set(querySnapshot);
767767
}
768768
},
769-
transctionId);
769+
transactionId);
770770

771771
return result;
772772
}

google-cloud-firestore/src/main/java/com/google/cloud/firestore/spi/v1beta1/FirestoreRpc.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
import com.google.api.gax.rpc.ServerStreamingCallable;
2020
import com.google.api.gax.rpc.UnaryCallable;
2121
import com.google.cloud.ServiceRpc;
22+
import com.google.cloud.firestore.v1beta1.PagedResponseWrappers.ListCollectionIdsPagedResponse;
2223
import com.google.firestore.v1beta1.BatchGetDocumentsRequest;
2324
import com.google.firestore.v1beta1.BatchGetDocumentsResponse;
2425
import com.google.firestore.v1beta1.BeginTransactionRequest;
2526
import com.google.firestore.v1beta1.BeginTransactionResponse;
2627
import com.google.firestore.v1beta1.CommitRequest;
2728
import com.google.firestore.v1beta1.CommitResponse;
2829
import com.google.firestore.v1beta1.ListCollectionIdsRequest;
29-
import com.google.firestore.v1beta1.ListCollectionIdsResponse;
3030
import com.google.firestore.v1beta1.RollbackRequest;
3131
import com.google.firestore.v1beta1.RunQueryRequest;
3232
import com.google.firestore.v1beta1.RunQueryResponse;
@@ -72,5 +72,5 @@ public interface FirestoreRpc extends AutoCloseable, ServiceRpc {
7272
/**
7373
* Returns a list of collections IDs.
7474
*/
75-
UnaryCallable<ListCollectionIdsRequest, ListCollectionIdsResponse> listCollectionIdsCallable();
75+
UnaryCallable<ListCollectionIdsRequest, ListCollectionIdsPagedResponse> listCollectionIdsPagedCallable();
7676
}

google-cloud-firestore/src/main/java/com/google/cloud/firestore/spi/v1beta1/GrpcFirestoreRpc.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
import com.google.auth.Credentials;
3232
import com.google.cloud.NoCredentials;
3333
import com.google.cloud.firestore.FirestoreOptions;
34-
import com.google.cloud.firestore.v1beta1.FirestoreClient;
3534
import com.google.cloud.firestore.v1beta1.FirestoreSettings;
35+
import com.google.cloud.firestore.v1beta1.PagedResponseWrappers.ListCollectionIdsPagedResponse;
3636
import com.google.cloud.firestore.v1beta1.stub.FirestoreStub;
3737
import com.google.cloud.firestore.v1beta1.stub.GrpcFirestoreStub;
3838
import com.google.firestore.v1beta1.BatchGetDocumentsRequest;
@@ -43,7 +43,6 @@
4343
import com.google.firestore.v1beta1.CommitResponse;
4444
import com.google.firestore.v1beta1.DatabaseName;
4545
import com.google.firestore.v1beta1.ListCollectionIdsRequest;
46-
import com.google.firestore.v1beta1.ListCollectionIdsResponse;
4746
import com.google.firestore.v1beta1.RollbackRequest;
4847
import com.google.firestore.v1beta1.RunQueryRequest;
4948
import com.google.firestore.v1beta1.RunQueryResponse;
@@ -176,8 +175,8 @@ public UnaryCallable<RollbackRequest, Empty> rollbackCallable() {
176175
}
177176

178177
@Override
179-
public UnaryCallable<ListCollectionIdsRequest, ListCollectionIdsResponse>
180-
listCollectionIdsCallable() {
181-
return firestoreStub.listCollectionIdsCallable();
178+
public UnaryCallable<ListCollectionIdsRequest, ListCollectionIdsPagedResponse>
179+
listCollectionIdsPagedCallable() {
180+
return firestoreStub.listCollectionIdsPagedCallable();
182181
}
183182
}

google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import static com.google.cloud.firestore.LocalFirestoreHelper.delete;
3939
import static com.google.cloud.firestore.LocalFirestoreHelper.get;
4040
import static com.google.cloud.firestore.LocalFirestoreHelper.getAllResponse;
41-
import static com.google.cloud.firestore.LocalFirestoreHelper.listCollectionIdsResponse;
4241
import static com.google.cloud.firestore.LocalFirestoreHelper.map;
4342
import static com.google.cloud.firestore.LocalFirestoreHelper.set;
4443
import static com.google.cloud.firestore.LocalFirestoreHelper.streamingResponse;
@@ -66,15 +65,13 @@
6665
import com.google.firestore.v1beta1.CommitResponse;
6766
import com.google.firestore.v1beta1.ListCollectionIdsRequest;
6867
import com.google.firestore.v1beta1.Value;
69-
import com.google.protobuf.Message;
7068
import com.google.protobuf.Timestamp;
7169
import java.util.ArrayList;
7270
import java.util.Arrays;
7371
import java.util.Collections;
7472
import java.util.HashMap;
7573
import java.util.List;
7674
import java.util.Map;
77-
import java.util.concurrent.ExecutionException;
7875
import org.junit.Before;
7976
import org.junit.Test;
8077
import org.junit.runner.RunWith;
@@ -675,16 +672,4 @@ public void updateDocumentWithPreconditions() throws Exception {
675672
assertCommitEquals(expectedCommit, request);
676673
}
677674
}
678-
679-
@Test
680-
public void getCollections() throws ExecutionException, InterruptedException {
681-
doReturn(listCollectionIdsResponse("second", "first"))
682-
.when(firestoreMock)
683-
.sendRequest(
684-
listCollectionIdsCapture.capture(), Matchers.<UnaryCallable<Message, Message>>any());
685-
686-
List<CollectionReference> collections = firestoreMock.getCollections().get();
687-
assertEquals("first", collections.get(0).getId());
688-
assertEquals("second", collections.get(1).getId());
689-
}
690675
}

0 commit comments

Comments
 (0)