From ce107b2c98a18704fcd02c5064e5f3ab3ec136e3 Mon Sep 17 00:00:00 2001 From: aozarov Date: Fri, 15 May 2015 17:42:04 -0700 Subject: [PATCH 1/3] adding an option to page from ListResult --- .../com/google/gcloud/ServiceOptions.java | 2 +- .../datastore/DatastoreServiceOptions.java | 10 +- .../google/gcloud/storage/BatchRequest.java | 6 +- .../google/gcloud/storage/BatchResponse.java | 6 +- .../com/google/gcloud/storage/ListResult.java | 15 +- .../gcloud/storage/StorageServiceImpl.java | 143 +++++++++++++----- .../gcloud/storage/StorageServiceOptions.java | 10 +- 7 files changed, 139 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/google/gcloud/ServiceOptions.java b/src/main/java/com/google/gcloud/ServiceOptions.java index a72cb74185c7..448b617372b0 100644 --- a/src/main/java/com/google/gcloud/ServiceOptions.java +++ b/src/main/java/com/google/gcloud/ServiceOptions.java @@ -283,7 +283,7 @@ public AuthCredentials authCredentials() { } public RetryParams retryParams() { - return retryParams; + return retryParams != null ? retryParams : RetryParams.noRetries(); } public ServiceRpcFactory serviceRpcFactory() { diff --git a/src/main/java/com/google/gcloud/datastore/DatastoreServiceOptions.java b/src/main/java/com/google/gcloud/datastore/DatastoreServiceOptions.java index 6bed641c7636..f7b1965d0b29 100644 --- a/src/main/java/com/google/gcloud/datastore/DatastoreServiceOptions.java +++ b/src/main/java/com/google/gcloud/datastore/DatastoreServiceOptions.java @@ -44,6 +44,7 @@ public class DatastoreServiceOptions extends ServiceOptions { @@ -178,10 +179,15 @@ public boolean equals(Object obj) { } DatastoreRpc datastoreRpc() { + if (datastoreRpc != null) { + return datastoreRpc; + } if (serviceRpcFactory() != null) { - return serviceRpcFactory().create(this); + datastoreRpc = serviceRpcFactory().create(this); + } else { + datastoreRpc = ServiceRpcProvider.datastore(this); } - return ServiceRpcProvider.datastore(this); + return datastoreRpc; } public static DatastoreServiceOptions defaultInstance() { diff --git a/src/main/java/com/google/gcloud/storage/BatchRequest.java b/src/main/java/com/google/gcloud/storage/BatchRequest.java index 6f62d5c51ae4..4d2f0cab8c96 100644 --- a/src/main/java/com/google/gcloud/storage/BatchRequest.java +++ b/src/main/java/com/google/gcloud/storage/BatchRequest.java @@ -96,15 +96,15 @@ public boolean equals(Object obj) { && Objects.equals(toGet, other.toGet); } - Map> toDelete() { + public Map> toDelete() { return toDelete; } - Map> toUpdate() { + public Map> toUpdate() { return toUpdate; } - Map> toGet() { + public Map> toGet() { return toGet; } diff --git a/src/main/java/com/google/gcloud/storage/BatchResponse.java b/src/main/java/com/google/gcloud/storage/BatchResponse.java index f0675e348f72..d4a1cc6f812f 100644 --- a/src/main/java/com/google/gcloud/storage/BatchResponse.java +++ b/src/main/java/com/google/gcloud/storage/BatchResponse.java @@ -43,12 +43,12 @@ public static class Result implements Serializable { private final StorageServiceException exception; - Result(T value) { + public Result(T value) { this.value = value; this.exception = null; } - Result(StorageServiceException exception) { + public Result(StorageServiceException exception) { this.exception = exception; this.value = null; } @@ -108,7 +108,7 @@ static Result empty() { } } - BatchResponse(List> deleteResult, List> updateResult, + public BatchResponse(List> deleteResult, List> updateResult, List> getResult) { this.deleteResult = ImmutableList.copyOf(deleteResult); this.updateResult = ImmutableList.copyOf(updateResult); diff --git a/src/main/java/com/google/gcloud/storage/ListResult.java b/src/main/java/com/google/gcloud/storage/ListResult.java index dd843020376e..55406f90cf0c 100644 --- a/src/main/java/com/google/gcloud/storage/ListResult.java +++ b/src/main/java/com/google/gcloud/storage/ListResult.java @@ -29,8 +29,14 @@ public final class ListResult implements Iterable, Se private final String cursor; private final Iterable results; + private final NextPageFetcher pageFetcher; - ListResult(String cursor, Iterable results) { + interface NextPageFetcher extends Serializable { + ListResult nextPage(); + } + + public ListResult(NextPageFetcher pageFetcher, String cursor, Iterable results) { + this.pageFetcher = pageFetcher; this.cursor = cursor; this.results = results; } @@ -39,6 +45,13 @@ public String nextPageCursor() { return cursor; } + public ListResult nextPage() { + if (cursor == null || pageFetcher == null) { + return null; + } + return pageFetcher.nextPage(); + } + @Override public Iterator iterator() { return results.iterator(); diff --git a/src/main/java/com/google/gcloud/storage/StorageServiceImpl.java b/src/main/java/com/google/gcloud/storage/StorageServiceImpl.java index 1f66401105d0..e0e77b0ec7e5 100644 --- a/src/main/java/com/google/gcloud/storage/StorageServiceImpl.java +++ b/src/main/java/com/google/gcloud/storage/StorageServiceImpl.java @@ -43,7 +43,6 @@ import com.google.gcloud.BaseService; import com.google.gcloud.ExceptionHandler; import com.google.gcloud.ExceptionHandler.Interceptor; -import com.google.gcloud.RetryParams; import com.google.gcloud.spi.StorageRpc; import com.google.gcloud.spi.StorageRpc.Tuple; @@ -83,12 +82,10 @@ public RetryResult beforeEval(Exception exception) { private static final byte[] EMPTY_BYTE_ARRAY = {}; private final StorageRpc storageRpc; - private final RetryParams retryParams; StorageServiceImpl(StorageServiceOptions options) { super(options); storageRpc = options.storageRpc(); - retryParams = firstNonNull(options.retryParams(), RetryParams.noRetries()); // todo: replace nulls with Value.asNull (per toPb) // todo: configure timeouts - https://developers.google.com/api-client-library/java/google-api-java-client/errors // todo: provide rewrite - https://cloud.google.com/storage/docs/json_api/v1/objects/rewrite @@ -106,7 +103,7 @@ public Bucket create(Bucket bucket, BucketTargetOption... options) { public com.google.api.services.storage.model.Bucket call() { return storageRpc.create(bucketPb, optionsMap); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -118,7 +115,7 @@ public Blob create(Blob blob, final byte[] content, BlobTargetOption... options) public StorageObject call() { return storageRpc.create(blobPb, firstNonNull(content, EMPTY_BYTE_ARRAY), optionsMap); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -138,7 +135,7 @@ public com.google.api.services.storage.model.Bucket call() { throw ex; } } - }, retryParams, EXCEPTION_HANDLER); + }, options().retryParams(), EXCEPTION_HANDLER); return answer == null ? null : Bucket.fromPb(answer); } @@ -158,46 +155,111 @@ public StorageObject call() { throw ex; } } - }, retryParams, EXCEPTION_HANDLER); + }, options().retryParams(), EXCEPTION_HANDLER); return storageObject == null ? null : Blob.fromPb(storageObject); } + private static abstract class BasePageFetcher + implements ListResult.NextPageFetcher { + + private static final long serialVersionUID = 8236329004030295223L; + protected final Map requestOptions; + protected final StorageServiceOptions serviceOptions; + + BasePageFetcher(StorageServiceOptions serviceOptions, String cursor, + Map optionMap) { + this.serviceOptions = serviceOptions; + ImmutableMap.Builder builder = ImmutableMap.builder(); + builder.put(StorageRpc.Option.PAGE_TOKEN, cursor); + for (Map.Entry option : optionMap.entrySet()) { + if (option.getKey() != StorageRpc.Option.PAGE_TOKEN) { + builder.put(option.getKey(), option.getValue()); + } + } + this.requestOptions = builder.build(); + } + } + + private static class BucketPageFetcher extends BasePageFetcher { + + private static final long serialVersionUID = -5490616010200159174L; + + BucketPageFetcher(StorageServiceOptions serviceOptions, String cursor, + Map optionMap) { + super(serviceOptions, cursor, optionMap); + } + + @Override + public ListResult nextPage() { + return listBuckets(serviceOptions, requestOptions); + } + } + + private static class BlobPageFetcher extends BasePageFetcher { + + private static final long serialVersionUID = -5490616010200159174L; + private final String bucket; + + BlobPageFetcher(String bucket, StorageServiceOptions serviceOptions, String cursor, + Map optionMap) { + super(serviceOptions, cursor, optionMap); + this.bucket = bucket; + } + + @Override + public ListResult nextPage() { + return listBlobs(bucket, serviceOptions, requestOptions); + } + } + @Override public ListResult list(BucketListOption... options) { - final Map optionsMap = optionMap(options); + return listBuckets(options(), optionMap(options)); + } + + private static ListResult listBuckets(final StorageServiceOptions serviceOptions, + final Map optionsMap) { Tuple> result = runWithRetries( new Callable>>() { @Override public Tuple> call() { - return storageRpc.list(optionsMap); - } - }, retryParams, EXCEPTION_HANDLER); - return new ListResult<>(result.x(), Iterables.transform(result.y(), - new Function() { - @Override - public Bucket apply(com.google.api.services.storage.model.Bucket bucketPb) { - return Bucket.fromPb(bucketPb); + return serviceOptions.storageRpc().list(optionsMap); } - })); + }, serviceOptions.retryParams(), EXCEPTION_HANDLER); + String cursor = result.x(); + return new ListResult<>(new BucketPageFetcher(serviceOptions, cursor, optionsMap), cursor, + Iterables.transform(result.y(), + new Function() { + @Override + public Bucket apply(com.google.api.services.storage.model.Bucket bucketPb) { + return Bucket.fromPb(bucketPb); + } + })); } @Override public ListResult list(final String bucket, BlobListOption... options) { - final Map optionsMap = optionMap(options); + return listBlobs(bucket, options(), optionMap(options)); + } + + private static ListResult listBlobs(final String bucket, + final StorageServiceOptions serviceOptions, final Map optionsMap) { Tuple> result = runWithRetries( new Callable>>() { @Override public Tuple> call() { - return storageRpc.list(bucket, optionsMap); + return serviceOptions.storageRpc().list(bucket, optionsMap); } - }, retryParams, EXCEPTION_HANDLER); - return new ListResult<>(result.x(), Iterables.transform(result.y(), - new Function() { - @Override - public Blob apply(StorageObject storageObject) { - return Blob.fromPb(storageObject); - } - })); + }, serviceOptions.retryParams(), EXCEPTION_HANDLER); + String cursor = result.x(); + return new ListResult<>(new BlobPageFetcher(bucket, serviceOptions, cursor, optionsMap), cursor, + Iterables.transform(result.y(), + new Function() { + @Override + public Blob apply(StorageObject storageObject) { + return Blob.fromPb(storageObject); + } + })); } @Override @@ -210,7 +272,7 @@ public Bucket update(Bucket bucket, BucketTargetOption... options) { public com.google.api.services.storage.model.Bucket call() { return storageRpc.patch(bucketPb, optionsMap); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -222,7 +284,7 @@ public Blob update(Blob blob, BlobTargetOption... options) { public StorageObject call() { return storageRpc.patch(storageObject, optionsMap); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -234,7 +296,7 @@ public boolean delete(String bucket, BucketSourceOption... options) { public Boolean call() { return storageRpc.delete(bucketPb, optionsMap); } - }, retryParams, EXCEPTION_HANDLER); + }, options().retryParams(), EXCEPTION_HANDLER); } @Override @@ -246,7 +308,7 @@ public boolean delete(String bucket, String blob, BlobSourceOption... options) { public Boolean call() { return storageRpc.delete(storageObject, optionsMap); } - }, retryParams, EXCEPTION_HANDLER); + }, options().retryParams(), EXCEPTION_HANDLER); } @Override @@ -265,7 +327,7 @@ public Blob compose(final ComposeRequest composeRequest) { public StorageObject call() { return storageRpc.compose(sources, target, targetOptions); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -283,7 +345,7 @@ public Blob copy(CopyRequest copyRequest) { public StorageObject call() { return storageRpc.copy(source, sourceOptions, target, targetOptions); } - }, retryParams, EXCEPTION_HANDLER)); + }, options().retryParams(), EXCEPTION_HANDLER)); } @Override @@ -295,7 +357,7 @@ public byte[] load(String bucket, String blob, BlobSourceOption... options) { public byte[] call() { return storageRpc.load(storageObject, optionsMap); } - }, retryParams, EXCEPTION_HANDLER); + }, options().retryParams(), EXCEPTION_HANDLER); } @Override @@ -361,6 +423,8 @@ private List> transformBatch private static class BlobReadChannelImpl implements BlobReadChannel { + private static final long serialVersionUID = 1612561791239832259L; + private final StorageServiceOptions serviceOptions; private final Blob blob; private final Map requestOptions; @@ -369,7 +433,6 @@ private static class BlobReadChannelImpl implements BlobReadChannel { private boolean endOfStream; private transient StorageRpc storageRpc; - private transient RetryParams retryParams; private transient StorageObject storageObject; private transient int bufferPos; private transient byte[] buffer; @@ -400,7 +463,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE private void initTransients() { storageRpc = serviceOptions.storageRpc(); - retryParams = firstNonNull(serviceOptions.retryParams(), RetryParams.noRetries()); storageObject = blob.toPb(); } @@ -445,7 +507,7 @@ public int read(ByteBuffer byteBuffer) throws IOException { public byte[] call() { return storageRpc.read(storageObject, requestOptions, position, toRead); } - }, retryParams, EXCEPTION_HANDLER); + }, serviceOptions.retryParams(), EXCEPTION_HANDLER); if (toRead > buffer.length) { endOfStream = true; } @@ -472,6 +534,7 @@ private static class BlobWriterChannelImpl implements BlobWriteChannel { private static final int CHUNK_SIZE = 256 * 1024; private static final int COMPACT_THRESHOLD = (int) Math.round(CHUNK_SIZE * 0.8); + private static final long serialVersionUID = -4067648781804698786L; private final StorageServiceOptions options; private final Blob blob; @@ -482,7 +545,6 @@ private static class BlobWriterChannelImpl implements BlobWriteChannel { private boolean isOpen = true; private transient StorageRpc storageRpc; - private transient RetryParams retryParams; private transient StorageObject storageObject; public BlobWriterChannelImpl(StorageServiceOptions options, Blob blob, @@ -515,7 +577,7 @@ private void flush() { public void run() { storageRpc.write(uploadId, buffer, 0, storageObject, position, length, false); } - })); + }), options.retryParams(), EXCEPTION_HANDLER); position += length; limit -= length; byte[] temp = new byte[CHUNK_SIZE]; @@ -536,7 +598,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE private void initTransients() { storageRpc = options.storageRpc(); - retryParams = firstNonNull(options.retryParams(), RetryParams.noRetries()); storageObject = blob.toPb(); } @@ -575,7 +636,7 @@ public void close() throws IOException { public void run() { storageRpc.write(uploadId, buffer, 0, storageObject, position, limit, true); } - })); + }), options.retryParams(), EXCEPTION_HANDLER); position += buffer.length; isOpen = false; buffer = null; diff --git a/src/main/java/com/google/gcloud/storage/StorageServiceOptions.java b/src/main/java/com/google/gcloud/storage/StorageServiceOptions.java index 13b14a1fbf76..87cac636fa92 100644 --- a/src/main/java/com/google/gcloud/storage/StorageServiceOptions.java +++ b/src/main/java/com/google/gcloud/storage/StorageServiceOptions.java @@ -33,6 +33,7 @@ public class StorageServiceOptions extends ServiceOptions { @@ -68,10 +69,15 @@ protected Set scopes() { } StorageRpc storageRpc() { + if (storageRpc != null) { + return storageRpc; + } if (serviceRpcFactory() != null) { - return serviceRpcFactory().create(this); + storageRpc = serviceRpcFactory().create(this); + } else { + storageRpc = ServiceRpcProvider.storage(this); } - return ServiceRpcProvider.storage(this); + return storageRpc; } public String pathDelimiter() { From 87792edc42c5ca2d2ad23820b93fbd737bc51668 Mon Sep 17 00:00:00 2001 From: ozarov Date: Fri, 15 May 2015 21:44:15 -0700 Subject: [PATCH 2/3] fix test and add some javadoc --- src/main/java/com/google/gcloud/storage/ListResult.java | 6 ++++++ .../java/com/google/gcloud/storage/SerializationTest.java | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/gcloud/storage/ListResult.java b/src/main/java/com/google/gcloud/storage/ListResult.java index 55406f90cf0c..b6e36cd67954 100644 --- a/src/main/java/com/google/gcloud/storage/ListResult.java +++ b/src/main/java/com/google/gcloud/storage/ListResult.java @@ -41,10 +41,16 @@ public ListResult(NextPageFetcher pageFetcher, String cursor, Iterable res this.results = results; } + /** + * Returns the cursor for the nextPage or {@code null} if no more results. + */ public String nextPageCursor() { return cursor; } + /** + * Returns the results of the nextPage or {@code null} if no more result. + */ public ListResult nextPage() { if (cursor == null || pageFetcher == null) { return null; diff --git a/src/test/java/com/google/gcloud/storage/SerializationTest.java b/src/test/java/com/google/gcloud/storage/SerializationTest.java index 365462d3f69a..83bc9e5b25d9 100644 --- a/src/test/java/com/google/gcloud/storage/SerializationTest.java +++ b/src/test/java/com/google/gcloud/storage/SerializationTest.java @@ -51,7 +51,7 @@ public class SerializationTest { Collections.>emptyList(), Collections.>emptyList()); private static final ListResult LIST_RESULT = - new ListResult<>("c", Collections.singletonList(Blob.of("b", "n"))); + new ListResult<>(null, "c", Collections.singletonList(Blob.of("b", "n"))); private static StorageService.BlobListOption BLOB_LIST_OPTIONS = StorageService.BlobListOption.maxResults(100); private static StorageService.BlobSourceOption BLOB_SOURCE_OPTIONS = From 8cfb28657c714f8c7768fcd0240c32896f5ae8e3 Mon Sep 17 00:00:00 2001 From: aozarov Date: Mon, 18 May 2015 10:56:25 -0700 Subject: [PATCH 3/3] fix merge conflict --- .../com/google/gcloud/storage/ListResultTest.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/google/gcloud/storage/ListResultTest.java b/src/test/java/com/google/gcloud/storage/ListResultTest.java index 1345b0ced240..8a2e69d0c084 100644 --- a/src/test/java/com/google/gcloud/storage/ListResultTest.java +++ b/src/test/java/com/google/gcloud/storage/ListResultTest.java @@ -22,13 +22,26 @@ import org.junit.Test; +import java.util.Collections; + public class ListResultTest { @Test public void testListResult() throws Exception { ImmutableList values = ImmutableList.of("1", "2"); - ListResult result = new ListResult("c", values); + final ListResult nextResult = + new ListResult<>(null, "c", Collections.emptyList()); + ListResult.NextPageFetcher fetcher = new ListResult.NextPageFetcher() { + + @Override + public ListResult nextPage() { + return nextResult; + } + }; + ListResult result = new ListResult(fetcher, "c", values); + assertEquals(nextResult, result.nextPage()); assertEquals("c", result.nextPageCursor()); assertEquals(values, ImmutableList.copyOf(result.iterator())); + } }