From a2e60abf83d6603ecf9f0e80b0c95082e8ce0fbb Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Wed, 13 Jan 2016 17:53:25 -0800 Subject: [PATCH 1/5] Populate cursorAfter in datastore v1beta2 --- .../google/gcloud/datastore/QueryResults.java | 5 +- .../gcloud/datastore/QueryResultsImpl.java | 2 +- .../gcloud/datastore/StructuredQuery.java | 22 +++++ .../gcloud/datastore/DatastoreTest.java | 92 +++++++++++++++++++ 4 files changed, 118 insertions(+), 3 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java index b23c56a7c395..6a7e82bd5b2a 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java @@ -35,8 +35,9 @@ public interface QueryResults extends Iterator { Class resultClass(); /** - * Returns the Cursor for point after the value returned in the last {@link #next} call. - * Not currently implemented (depends on v1beta3). + * Returns the Cursor for the point after the value returned in the last {@link #next} call. Until + * v1beta3 is supported, this field should only be used if you have set a limit and the number of + * results is equal to that limit. */ Cursor cursorAfter(); } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java index cd3fe9dd776b..40538af81409 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java @@ -100,6 +100,6 @@ public Class resultClass() { @Override public Cursor cursorAfter() { //return new Cursor(cursor); // only available in v1beta3 - return null; + return new Cursor(queryResultBatchPb.getEndCursor()); } } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java index 293c17cf3c57..c721398066d2 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java @@ -633,6 +633,20 @@ protected static class BaseBuilder> { this.resultType = resultType; } + BaseBuilder(StructuredQuery query) { + resultType = query.type(); + namespace = query.namespace(); + kind = query.kind; + projection.addAll(query.projection); + filter = query.filter; + groupBy.addAll(query.groupBy); + orderBy.addAll(query.orderBy); + startCursor = query.startCursor; + endCursor = query.endCursor; + offset = query.offset; + limit = query.limit; + } + @SuppressWarnings("unchecked") B self() { return (B) this; @@ -773,6 +787,10 @@ static final class Builder extends BaseBuilder> { Builder(ResultType resultType) { super(resultType); } + + Builder(StructuredQuery query) { + super(query); + } } /** @@ -953,6 +971,10 @@ public Integer limit() { return limit; } + public Builder toBuilder() { + return new Builder(this); + } + @Override void populatePb(DatastoreV1.RunQueryRequest.Builder requestPb) { requestPb.setQuery(toPb()); diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java index 636fb3708926..612c97c845fd 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java @@ -27,6 +27,9 @@ import com.google.api.services.datastore.DatastoreV1; import com.google.api.services.datastore.DatastoreV1.EntityResult; +import com.google.api.services.datastore.DatastoreV1.QueryResultBatch; +import com.google.api.services.datastore.DatastoreV1.RunQueryRequest; +import com.google.api.services.datastore.DatastoreV1.RunQueryResponse; import com.google.common.collect.Iterators; import com.google.gcloud.RetryParams; import com.google.gcloud.datastore.Query.ResultType; @@ -462,6 +465,95 @@ public void testRunStructuredQuery() { assertFalse(results4.hasNext()); } + @Test + public void testQueryPaginationWithLimit() throws DatastoreRpcException { + DatastoreRpcFactory rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); + DatastoreRpc rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); + EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) + .andReturn(rpcMock); + List responses = buildResponsesForQueryPaginationWithLimit(); + for (int i = 0; i < responses.size(); i++) { + EasyMock.expect(rpcMock.runQuery(EasyMock.anyObject(RunQueryRequest.class))) + .andReturn(responses.get(i)); + } + EasyMock.replay(rpcFactoryMock, rpcMock); + Datastore mockDatastore = + this.options.toBuilder() + .retryParams(RetryParams.defaultInstance()) + .serviceRpcFactory(rpcFactoryMock) + .build() + .service(); + int limit = 2; + int totalCount = 0; + StructuredQuery query = Query.entityQueryBuilder().limit(limit).build(); + while (true) { + QueryResults results = mockDatastore.run(query); + int resultCount = 0; + while (results.hasNext()) { + results.next(); + resultCount++; + totalCount++; + } + if (resultCount < limit) { + break; + } + query = query.toBuilder().startCursor(results.cursorAfter()).build(); + } + assertEquals(totalCount, 5); + EasyMock.verify(rpcFactoryMock, rpcMock); + } + + private List buildResponsesForQueryPaginationWithLimit() { + Entity entity4 = Entity.builder(KEY4).set("value", StringValue.of("value")).build(); + Entity entity5 = Entity.builder(KEY5).set("value", "value").build(); + datastore.add(ENTITY3, entity4, entity5); + List responses = new ArrayList<>(); + Query query = Query.entityQueryBuilder().build(); + RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder(); + query.populatePb(requestPb); + QueryResultBatch queryResultBatchPb = + RunQueryResponse.newBuilder() + .mergeFrom(((DatastoreImpl) datastore).runQuery(requestPb.build())) + .getBatch(); + QueryResultBatch queryResultBatchPb1 = + QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.NOT_FINISHED) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(0, 1)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(0).getCursor()) + .build(); + responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb1).build()); + QueryResultBatch queryResultBatchPb2 = + QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(1, 2)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(1).getCursor()) + .build(); + responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb2).build()); + QueryResultBatch queryResultBatchPb3 = + QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(2, 4)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(3).getCursor()) + .build(); + responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb3).build()); + QueryResultBatch queryResultBatchPb4 = + QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.NO_MORE_RESULTS) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(4, 5)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(4).getCursor()) + .build(); + responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb4).build()); + return responses; + } + @Test public void testAllocateId() { KeyFactory keyFactory = datastore.newKeyFactory().kind(KIND1); From 2219c82af21997fe66b952b062ff7fe7006503b9 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Wed, 13 Jan 2016 22:08:05 -0800 Subject: [PATCH 2/5] minor fixes to afterCursor + test --- .../main/java/com/google/gcloud/datastore/QueryResults.java | 5 ++--- .../java/com/google/gcloud/datastore/QueryResultsImpl.java | 5 ++++- .../test/java/com/google/gcloud/datastore/DatastoreTest.java | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java index 6a7e82bd5b2a..8928398b76e2 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java @@ -35,9 +35,8 @@ public interface QueryResults extends Iterator { Class resultClass(); /** - * Returns the Cursor for the point after the value returned in the last {@link #next} call. Until - * v1beta3 is supported, this field should only be used if you have set a limit and the number of - * results is equal to that limit. + * Returns the Cursor for the point after the value returned in the last {@link #next} call. + * Currently, {@code cursorAfter} returns null in all cases but the last result. */ Cursor cursorAfter(); } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java index 40538af81409..bae2e936f1c1 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java @@ -100,6 +100,9 @@ public Class resultClass() { @Override public Cursor cursorAfter() { //return new Cursor(cursor); // only available in v1beta3 - return new Cursor(queryResultBatchPb.getEndCursor()); + if (!hasNext()) { + return new Cursor(queryResultBatchPb.getEndCursor()); + } + return null; } } diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java index 612c97c845fd..3ee83c1da059 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java @@ -478,7 +478,7 @@ public void testQueryPaginationWithLimit() throws DatastoreRpcException { } EasyMock.replay(rpcFactoryMock, rpcMock); Datastore mockDatastore = - this.options.toBuilder() + options.toBuilder() .retryParams(RetryParams.defaultInstance()) .serviceRpcFactory(rpcFactoryMock) .build() From 663e54b356162b6890fd8175c9cc17a4a70dd097 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Fri, 15 Jan 2016 09:09:23 -0800 Subject: [PATCH 3/5] clarify batching in docs --- .../main/java/com/google/gcloud/datastore/QueryResults.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java index 8928398b76e2..b882131ba939 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResults.java @@ -22,8 +22,9 @@ * The result of a Google Cloud Datastore query submission. * When the result is not typed it is possible to cast it to its appropriate type according to * the {@link #resultClass} value. - * Results are loaded lazily; therefore it is possible to get a {@code DatastoreException} - * upon {@link Iterator#hasNext hasNext} or {@link Iterator#next next} calls. + * Results are loaded lazily in batches, where batch size is set by Cloud Datastore. As a result, it + * is possible to get a {@code DatastoreException} upon {@link Iterator#hasNext hasNext} or + * {@link Iterator#next next} calls. * * @param the type of the results value. */ From e4113c47155b73d5be62e387a224ac43d1b6a43a Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Fri, 15 Jan 2016 10:43:30 -0800 Subject: [PATCH 4/5] avoid calling hasNext from afterCursor --- .../com/google/gcloud/datastore/QueryResultsImpl.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java index bae2e936f1c1..3c2e0d177f80 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/QueryResultsImpl.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.AbstractIterator; import com.google.gcloud.datastore.Query.ResultType; +import com.google.protobuf.ByteString; import java.util.Iterator; import java.util.Objects; @@ -36,7 +37,7 @@ class QueryResultsImpl extends AbstractIterator implements QueryResults private DatastoreV1.QueryResultBatch queryResultBatchPb; private boolean lastBatch; private Iterator entityResultPbIter; - //private ByteString cursor; // only available in v1beta3 + private ByteString cursor; // only available in v1beta3 QueryResultsImpl(DatastoreImpl datastore, DatastoreV1.ReadOptions readOptionsPb, @@ -83,6 +84,7 @@ protected T computeNext() { sendRequest(); } if (!entityResultPbIter.hasNext()) { + cursor = queryResultBatchPb.getEndCursor(); return endOfData(); } DatastoreV1.EntityResult entityResultPb = entityResultPbIter.next(); @@ -99,10 +101,7 @@ public Class resultClass() { @Override public Cursor cursorAfter() { + return cursor == null ? null : new Cursor(cursor); //return new Cursor(cursor); // only available in v1beta3 - if (!hasNext()) { - return new Cursor(queryResultBatchPb.getEndCursor()); - } - return null; } } From 461f37529d3f6efd8d183e611ebbbd086f453f1e Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Fri, 15 Jan 2016 13:54:37 -0800 Subject: [PATCH 5/5] Minor style fixes --- .../gcloud/datastore/StructuredQuery.java | 2 +- .../gcloud/datastore/DatastoreTest.java | 78 +++++++++---------- 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java index c721398066d2..bbb73df4a79c 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/StructuredQuery.java @@ -972,7 +972,7 @@ public Integer limit() { } public Builder toBuilder() { - return new Builder(this); + return new Builder<>(this); } @Override diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java index 3ee83c1da059..e6f84c76ad40 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java @@ -477,12 +477,11 @@ public void testQueryPaginationWithLimit() throws DatastoreRpcException { .andReturn(responses.get(i)); } EasyMock.replay(rpcFactoryMock, rpcMock); - Datastore mockDatastore = - options.toBuilder() - .retryParams(RetryParams.defaultInstance()) - .serviceRpcFactory(rpcFactoryMock) - .build() - .service(); + Datastore mockDatastore = options.toBuilder() + .retryParams(RetryParams.defaultInstance()) + .serviceRpcFactory(rpcFactoryMock) + .build() + .service(); int limit = 2; int totalCount = 0; StructuredQuery query = Query.entityQueryBuilder().limit(limit).build(); @@ -511,45 +510,40 @@ private List buildResponsesForQueryPaginationWithLimit() { Query query = Query.entityQueryBuilder().build(); RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder(); query.populatePb(requestPb); - QueryResultBatch queryResultBatchPb = - RunQueryResponse.newBuilder() - .mergeFrom(((DatastoreImpl) datastore).runQuery(requestPb.build())) - .getBatch(); - QueryResultBatch queryResultBatchPb1 = - QueryResultBatch.newBuilder() - .mergeFrom(queryResultBatchPb) - .setMoreResults(QueryResultBatch.MoreResultsType.NOT_FINISHED) - .clearEntityResult() - .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(0, 1)) - .setEndCursor(queryResultBatchPb.getEntityResultList().get(0).getCursor()) - .build(); + QueryResultBatch queryResultBatchPb = RunQueryResponse.newBuilder() + .mergeFrom(((DatastoreImpl) datastore).runQuery(requestPb.build())) + .getBatch(); + QueryResultBatch queryResultBatchPb1 = QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.NOT_FINISHED) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(0, 1)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(0).getCursor()) + .build(); responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb1).build()); - QueryResultBatch queryResultBatchPb2 = - QueryResultBatch.newBuilder() - .mergeFrom(queryResultBatchPb) - .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) - .clearEntityResult() - .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(1, 2)) - .setEndCursor(queryResultBatchPb.getEntityResultList().get(1).getCursor()) - .build(); + QueryResultBatch queryResultBatchPb2 = QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(1, 2)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(1).getCursor()) + .build(); responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb2).build()); - QueryResultBatch queryResultBatchPb3 = - QueryResultBatch.newBuilder() - .mergeFrom(queryResultBatchPb) - .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) - .clearEntityResult() - .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(2, 4)) - .setEndCursor(queryResultBatchPb.getEntityResultList().get(3).getCursor()) - .build(); + QueryResultBatch queryResultBatchPb3 = QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.MORE_RESULTS_AFTER_LIMIT) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(2, 4)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(3).getCursor()) + .build(); responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb3).build()); - QueryResultBatch queryResultBatchPb4 = - QueryResultBatch.newBuilder() - .mergeFrom(queryResultBatchPb) - .setMoreResults(QueryResultBatch.MoreResultsType.NO_MORE_RESULTS) - .clearEntityResult() - .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(4, 5)) - .setEndCursor(queryResultBatchPb.getEntityResultList().get(4).getCursor()) - .build(); + QueryResultBatch queryResultBatchPb4 = QueryResultBatch.newBuilder() + .mergeFrom(queryResultBatchPb) + .setMoreResults(QueryResultBatch.MoreResultsType.NO_MORE_RESULTS) + .clearEntityResult() + .addAllEntityResult(queryResultBatchPb.getEntityResultList().subList(4, 5)) + .setEndCursor(queryResultBatchPb.getEntityResultList().get(4).getCursor()) + .build(); responses.add(RunQueryResponse.newBuilder().setBatch(queryResultBatchPb4).build()); return responses; }