From d60d3f458c4f0b253f12bba5a448192b7c15e5d4 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Mon, 12 Oct 2015 12:31:10 +0200 Subject: [PATCH 1/6] Make signUrl take long duration and TimeUnit parameter - Add TimeSource abstract class to wrap clock functionalities (waiting for Java8 Clock) - Add timeSource setter to StorageOptions builder and getter to StorageOptions - Update StorageImplTest and ITStorageTest - Update javadoc for Blob/Storage signUrl --- .../java/com/google/gcloud/storage/Blob.java | 11 ++-- .../com/google/gcloud/storage/Storage.java | 11 ++-- .../google/gcloud/storage/StorageImpl.java | 5 +- .../google/gcloud/storage/StorageOptions.java | 64 +++++++++++++++++++ .../com/google/gcloud/storage/BlobTest.java | 5 +- .../google/gcloud/storage/ITStorageTest.java | 11 +--- .../gcloud/storage/StorageImplTest.java | 32 +++++++--- 7 files changed, 110 insertions(+), 29 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java index 189ae2ad3065..0f32cc3af0ce 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.concurrent.TimeUnit; /** * A Google cloud storage object. @@ -246,13 +247,15 @@ public BlobWriteChannel writer(BlobTargetOption... options) { * time period. This is particularly useful if you don't want publicly accessible blobs, but don't * want to require users to explicitly log in. * - * @param expirationTimeInSeconds the signed URL expiration (using epoch time) - * @param options signed url options + * @param duration time until the signed URL expires, expressed in {@code unit}. The finer + * granularity supported is 1 second + * @param unit time unit of the {@code duration} parameter + * @param options optional URL signing options * @return a signed URL for this bucket and the specified options * @see Signed-URLs */ - public URL signUrl(long expirationTimeInSeconds, SignUrlOption... options) { - return storage.signUrl(info, expirationTimeInSeconds, options); + public URL signUrl(long duration, TimeUnit unit, SignUrlOption... options) { + return storage.signUrl(info, duration, unit, options); } /** diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index a475f22309c6..1f600f0001da 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -34,6 +34,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; /** * An interface for Google Cloud Storage. @@ -643,15 +644,17 @@ public static Builder builder() { *

* Example usage of creating a signed URL that is valid for 2 weeks: * 

   {@code
-   *     service.signUrl(BlobInfo.of("bucket", "name"), TimeUnit.DAYS.toSeconds(14));
+   *     service.signUrl(BlobInfo.of("bucket", "name"), 14, TimeUnit.DAYS);
    * }
* - * @param blobInfo the blob associated with the signed url - * @param expirationTimeInSeconds the signed URL expiration (using epoch time) + * @param blobInfo the blob associated with the signed URL + * @param duration time until the signed URL expires, expressed in {@code unit}. The finer + * granularity supported is 1 second + * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options * @see Signed-URLs */ - URL signUrl(BlobInfo blobInfo, long expirationTimeInSeconds, SignUrlOption... options); + URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOption... options); /** * Gets the requested blobs. A batch request is used to perform this call. diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index 6e32220746ca..2b820bb6c3fb 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -68,6 +68,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; final class StorageImpl extends BaseService implements Storage { @@ -521,7 +522,9 @@ public BlobWriteChannel writer(BlobInfo blobInfo, BlobTargetOption... options) { } @Override - public URL signUrl(BlobInfo blobInfo, long expiration, SignUrlOption... options) { + public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOption... options) { + long expiration = TimeUnit.SECONDS.convert( + options().timeSource().millis() + unit.toMillis(duration), TimeUnit.MILLISECONDS); EnumMap optionMap = Maps.newEnumMap(SignUrlOption.Option.class); for (SignUrlOption option : options) { optionMap.put(option.option(), option.value()); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java index 102089045a4a..049d3029aa8e 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java @@ -23,6 +23,7 @@ import com.google.gcloud.spi.StorageRpc; import com.google.gcloud.spi.StorageRpcFactory; +import java.io.Serializable; import java.util.Objects; import java.util.Set; @@ -34,12 +35,47 @@ public class StorageOptions extends ServiceOptions { private static final String DEFAULT_PATH_DELIMITER = "/"; private final String pathDelimiter; + private final TimeSource timeSource; private transient StorageRpc storageRpc; + /** + * A class providing access to the current time in milliseconds. + * + * Implementations should implement {@code Serializable} wherever possible and must document + * whether or not they do support serialization. + */ + public static abstract class TimeSource { + + private static TimeSource DEFAULT_TIME_SOURCE = new DefaultTimeSource(); + + /** + * Returns current time in milliseconds according to this time source. + */ + public abstract long millis(); + + /** + * Returns the default time source. + */ + public static TimeSource defaultTimeSource() { + return DEFAULT_TIME_SOURCE; + } + + private static class DefaultTimeSource extends TimeSource implements Serializable { + + private static final long serialVersionUID = -5077300394286703864L; + + @Override + public long millis() { + return System.currentTimeMillis(); + } + } + } + public static class Builder extends ServiceOptions.Builder { private String pathDelimiter; + private TimeSource timeSource; private Builder() {} @@ -47,11 +83,27 @@ private Builder(StorageOptions options) { super(options); } + /** + * Sets the path delimiter for the storage service. + * @param pathDelimiter the path delimiter to set + * @return the builder. + */ public Builder pathDelimiter(String pathDelimiter) { this.pathDelimiter = pathDelimiter; return this; } + /** + * Sets the time source for the storage service. The time source is used by `signUrl` to compute + * URL's expiry time. If no time source is set by default `System.getTimeMillis()` is used. + * @param source the time source to set + * @return the builder. + */ + public Builder timeSource(TimeSource source) { + this.timeSource = source; + return this; + } + @Override public StorageOptions build() { return new StorageOptions(this); @@ -61,6 +113,7 @@ public StorageOptions build() { private StorageOptions(Builder builder) { super(builder); pathDelimiter = MoreObjects.firstNonNull(builder.pathDelimiter, DEFAULT_PATH_DELIMITER); + timeSource = MoreObjects.firstNonNull(builder.timeSource, TimeSource.defaultTimeSource()); // todo: consider providing read-timeout } @@ -84,10 +137,21 @@ StorageRpc storageRpc() { return storageRpc; } + /** + * Returns the storage service's path delimiter. + */ public String pathDelimiter() { return pathDelimiter; } + /** + * Returns the storage service's time source. Default time source uses `System.getTimeMillis()` to + * get current time. + */ + public TimeSource timeSource() { + return timeSource; + } + @Override public Builder toBuilder() { return new Builder(this); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java index ece24eeacd1e..788f3e235169 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java @@ -38,6 +38,7 @@ import java.net.URL; import java.util.Arrays; import java.util.List; +import java.util.concurrent.TimeUnit; public class BlobTest { @@ -161,9 +162,9 @@ public void testWriter() throws Exception { @Test public void testSignUrl() throws Exception { URL url = new URL("http://localhost:123/bla"); - expect(storage.signUrl(BLOB_INFO, 100)).andReturn(url); + expect(storage.signUrl(BLOB_INFO, 100, TimeUnit.SECONDS)).andReturn(url); replay(storage); - assertEquals(url, blob.signUrl(100)); + assertEquals(url, blob.signUrl(100, TimeUnit.SECONDS)); } @Test diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java index 2ca36c8ec1e3..bb23ace98724 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java @@ -435,10 +435,7 @@ public void testGetSignedUrl() throws IOException { String blobName = "test-get-signed-url-blob"; BlobInfo blob = BlobInfo.of(bucket, blobName); assertNotNull(storage.create(BlobInfo.of(bucket, blobName), BLOB_BYTE_CONTENT)); - Calendar calendar = Calendar.getInstance(); - calendar.add(Calendar.HOUR, 1); - long expiration = calendar.getTimeInMillis() / 1000; - URL url = storage.signUrl(blob, expiration); + URL url = storage.signUrl(blob, 1, TimeUnit.HOURS); URLConnection connection = url.openConnection(); byte[] readBytes = new byte[BLOB_BYTE_CONTENT.length]; try (InputStream responseStream = connection.getInputStream()) { @@ -453,10 +450,8 @@ public void testPostSignedUrl() throws IOException { String blobName = "test-post-signed-url-blob"; BlobInfo blob = BlobInfo.of(bucket, blobName); assertNotNull(storage.create(BlobInfo.of(bucket, blobName))); - Calendar calendar = Calendar.getInstance(); - calendar.add(Calendar.HOUR, 1); - long expiration = calendar.getTimeInMillis() / 1000; - URL url = storage.signUrl(blob, expiration, Storage.SignUrlOption.httpMethod(HttpMethod.POST)); + URL url = + storage.signUrl(blob, 1, TimeUnit.HOURS, Storage.SignUrlOption.httpMethod(HttpMethod.POST)); URLConnection connection = url.openConnection(); connection.setDoOutput(true); connection.connect(); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index fa0daa976eb0..6d8fb393f58c 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -65,6 +65,7 @@ import java.security.spec.X509EncodedKeySpec; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; public class StorageImplTest { @@ -171,6 +172,13 @@ public class StorageImplTest { + "EkPPhszldvQTY486uPxyD/D7HdfnGW/Nbw5JUhfvecAdudDEhNAQ3PNabyDMI+TpiHy4NTWOrgdcWrzj6VXcdc" + "+uuABnPwRCdcyJ1xl2kOrPksRnp1auNGMLOe4IpEBjGY7baX9UG8+A45MbG0aHmkR59Op/aR9XowIDAQAB"; + private static final StorageOptions.TimeSource TIME_SOURCE = new StorageOptions.TimeSource() { + @Override + public long millis() { + return 42000L; + } + }; + private static PrivateKey privateKey; private static PublicKey publicKey; @@ -794,24 +802,26 @@ public void testSignUrl() throws NoSuchAlgorithmException, InvalidKeyException, String account = "account"; ServiceAccountAuthCredentials credentialsMock = EasyMock.createMock(ServiceAccountAuthCredentials.class); - EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock).times(1); + EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock); EasyMock.expect(optionsMock.authCredentials()).andReturn(credentialsMock).times(2); + EasyMock.expect(optionsMock.timeSource()).andReturn(TIME_SOURCE); EasyMock.expect(credentialsMock.privateKey()).andReturn(privateKey); EasyMock.expect(credentialsMock.account()).andReturn(account); EasyMock.replay(optionsMock, storageRpcMock, credentialsMock); storage = StorageFactory.instance().get(optionsMock); - URL url = storage.signUrl(BLOB_INFO1, 60); + URL url = storage.signUrl(BLOB_INFO1, 14, TimeUnit.DAYS); String stringUrl = url.toString(); String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1).append("/") .append(BLOB_NAME1).append("?GoogleAccessId=").append(account).append("&Expires=") - .append(60).append("&Signature=").toString(); + .append(42L + 1209600).append("&Signature=").toString(); assertTrue(stringUrl.startsWith(expectedUrl)); String signature = stringUrl.substring(expectedUrl.length()); StringBuilder signedMessageBuilder = new StringBuilder(); - signedMessageBuilder.append(HttpMethod.GET).append('\n').append('\n').append('\n').append(60) - .append('\n').append("/").append(BUCKET_NAME1).append("/").append(BLOB_NAME1); + signedMessageBuilder.append(HttpMethod.GET).append('\n').append('\n').append('\n') + .append(42L + 1209600).append('\n').append("/").append(BUCKET_NAME1).append("/") + .append(BLOB_NAME1); Signature signer = Signature.getInstance("SHA256withRSA"); signer.initVerify(publicKey); @@ -827,27 +837,29 @@ public void testSignUrlWithOptions() throws NoSuchAlgorithmException, InvalidKey String account = "account"; ServiceAccountAuthCredentials credentialsMock = EasyMock.createMock(ServiceAccountAuthCredentials.class); - EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock).times(1); + EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock); EasyMock.expect(optionsMock.authCredentials()).andReturn(credentialsMock).times(2); + EasyMock.expect(optionsMock.timeSource()).andReturn(TIME_SOURCE); EasyMock.expect(credentialsMock.privateKey()).andReturn(privateKey); EasyMock.expect(credentialsMock.account()).andReturn(account); EasyMock.replay(optionsMock, storageRpcMock, credentialsMock); storage = StorageFactory.instance().get(optionsMock); URL url = - storage.signUrl(BLOB_INFO1, 60, Storage.SignUrlOption.httpMethod(HttpMethod.POST), + storage.signUrl(BLOB_INFO1, 14, TimeUnit.DAYS, + Storage.SignUrlOption.httpMethod(HttpMethod.POST), Storage.SignUrlOption.withContentType(), Storage.SignUrlOption.withMd5()); String stringUrl = url.toString(); String expectedUrl = new StringBuilder("https://storage.googleapis.com/").append(BUCKET_NAME1).append("/") .append(BLOB_NAME1).append("?GoogleAccessId=").append(account).append("&Expires=") - .append(60).append("&Signature=").toString(); + .append(42L + 1209600).append("&Signature=").toString(); assertTrue(stringUrl.startsWith(expectedUrl)); String signature = stringUrl.substring(expectedUrl.length()); StringBuilder signedMessageBuilder = new StringBuilder(); signedMessageBuilder.append(HttpMethod.POST).append('\n').append(BLOB_INFO1.md5()).append('\n') - .append(BLOB_INFO1.contentType()).append('\n').append(60).append('\n').append("/") - .append(BUCKET_NAME1).append("/").append(BLOB_NAME1); + .append(BLOB_INFO1.contentType()).append('\n').append(42L + 1209600).append('\n') + .append("/").append(BUCKET_NAME1).append("/").append(BLOB_NAME1); Signature signer = Signature.getInstance("SHA256withRSA"); signer.initVerify(publicKey); From 286065de0e2641314ba76cd1721816855e7e2ed7 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Mon, 12 Oct 2015 13:28:09 +0200 Subject: [PATCH 2/6] Update storage example --- .../java/com/google/gcloud/examples/StorageExample.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gcloud-java-examples/src/main/java/com/google/gcloud/examples/StorageExample.java b/gcloud-java-examples/src/main/java/com/google/gcloud/examples/StorageExample.java index 99b543107483..74bee1b4e076 100644 --- a/gcloud-java-examples/src/main/java/com/google/gcloud/examples/StorageExample.java +++ b/gcloud-java-examples/src/main/java/com/google/gcloud/examples/StorageExample.java @@ -55,6 +55,7 @@ import java.util.Calendar; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeUnit; /** * An example of using the Google Cloud Storage. @@ -499,11 +500,8 @@ public void run(Storage storage, Tuple tupl private void run(Storage storage, ServiceAccountAuthCredentials cred, Blob blob) throws IOException { - Calendar cal = Calendar.getInstance(); - cal.add(Calendar.DATE, 1); - long expiration = cal.getTimeInMillis() / 1000; System.out.println("Signed URL: " + - blob.signUrl(expiration, SignUrlOption.serviceAccount(cred))); + blob.signUrl(1, TimeUnit.DAYS, SignUrlOption.serviceAccount(cred))); } @Override From 64121d88a05adb91eb5ac54442f20ae42847ff87 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Mon, 12 Oct 2015 19:18:53 +0200 Subject: [PATCH 3/6] Enhance storage/blob signUrl javadoc --- .../src/main/java/com/google/gcloud/storage/Blob.java | 2 +- .../src/main/java/com/google/gcloud/storage/Storage.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java index 0f32cc3af0ce..7e2b059e51a2 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java @@ -248,7 +248,7 @@ public BlobWriteChannel writer(BlobTargetOption... options) { * want to require users to explicitly log in. * * @param duration time until the signed URL expires, expressed in {@code unit}. The finer - * granularity supported is 1 second + * granularity supported is 1 second, finer granularities will be truncated * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options * @return a signed URL for this bucket and the specified options diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index 1f600f0001da..ed265ed40447 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -649,7 +649,7 @@ public static Builder builder() { * * @param blobInfo the blob associated with the signed URL * @param duration time until the signed URL expires, expressed in {@code unit}. The finer - * granularity supported is 1 second + * granularity supported is 1 second, finer granularities will be truncated * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options * @see Signed-URLs From 194d2397c3f0a1a90bc91c9348f1e20d707d9be5 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Tue, 13 Oct 2015 12:44:05 +0200 Subject: [PATCH 4/6] Rename TimeSource to Clock and move it to ServiceOptions --- .../com/google/gcloud/ServiceOptions.java | 63 ++++++++++++++++++- .../google/gcloud/storage/StorageImpl.java | 2 +- .../google/gcloud/storage/StorageOptions.java | 58 +---------------- .../gcloud/storage/StorageImplTest.java | 7 ++- 4 files changed, 67 insertions(+), 63 deletions(-) diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java b/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java index b80378ccd72c..ab56c78e107e 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java @@ -62,6 +62,7 @@ public abstract class ServiceOptions< private final ServiceRpcFactory serviceRpcFactory; private final int connectTimeout; private final int readTimeout; + private final Clock clock; public interface HttpTransportFactory extends Serializable { HttpTransport create(); @@ -91,7 +92,40 @@ public HttpTransport create() { } } + /** + * A class providing access to the current time in milliseconds. This class is mainly used for + * testing and will be replaced by Java8's {@code java.time.Clock}. + * + * Implementations should implement {@code Serializable} wherever possible and must document + * whether or not they do support serialization. + */ + public static abstract class Clock { + + private static ServiceOptions.Clock DEFAULT_TIME_SOURCE = new DefaultClock(); + + /** + * Returns current time in milliseconds according to this clock. + */ + public abstract long millis(); + + /** + * Returns the default clock. Default clock uses {@link System#currentTimeMillis()} to get time + * in milliseconds. + */ + public static ServiceOptions.Clock defaultClock() { + return DEFAULT_TIME_SOURCE; + } + + private static class DefaultClock extends ServiceOptions.Clock implements Serializable { + private static final long serialVersionUID = -5077300394286703864L; + + @Override + public long millis() { + return System.currentTimeMillis(); + } + } + } protected abstract static class Builder< ServiceRpcT, @@ -106,6 +140,7 @@ protected abstract static class Builder< private ServiceRpcFactory serviceRpcFactory; private int connectTimeout = -1; private int readTimeout = -1; + private Clock clock; protected Builder() {} @@ -125,6 +160,18 @@ protected B self() { return (B) this; } + /** + * Sets the service's clock. The clock is mainly used for testing purpose. {@link Clock} will be + * replaced by Java8's {@code java.time.Clock}. + * + * @param clock the clock to set + * @return the builder. + */ + public B clock(Clock clock) { + this.clock = clock; + return self(); + } + /** * Sets project id. * @@ -221,6 +268,7 @@ protected ServiceOptions(Builder builder) { serviceRpcFactory = builder.serviceRpcFactory; connectTimeout = builder.connectTimeout; readTimeout = builder.readTimeout; + clock = firstNonNull(builder.clock, Clock.defaultClock()); } private static AuthCredentials defaultAuthCredentials() { @@ -419,9 +467,17 @@ public int readTimeout() { return readTimeout; } + /** + * Returns the service's clock. Default time source uses {@link System#currentTimeMillis()} to + * get current time. + */ + public Clock clock() { + return clock; + } + protected int baseHashCode() { return Objects.hash(projectId, host, httpTransportFactory, authCredentials, retryParams, - serviceRpcFactory); + serviceRpcFactory, connectTimeout, readTimeout, clock); } protected boolean baseEquals(ServiceOptions other) { @@ -430,7 +486,10 @@ protected boolean baseEquals(ServiceOptions other) { && Objects.equals(httpTransportFactory, other.httpTransportFactory) && Objects.equals(authCredentials, other.authCredentials) && Objects.equals(retryParams, other.retryParams) - && Objects.equals(serviceRpcFactory, other.serviceRpcFactory); + && Objects.equals(serviceRpcFactory, other.serviceRpcFactory) + && Objects.equals(connectTimeout, other.connectTimeout) + && Objects.equals(readTimeout, other.readTimeout) + && Objects.equals(clock, clock); } public abstract Builder toBuilder(); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index 2b820bb6c3fb..bdb87dae1271 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -524,7 +524,7 @@ public BlobWriteChannel writer(BlobInfo blobInfo, BlobTargetOption... options) { @Override public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOption... options) { long expiration = TimeUnit.SECONDS.convert( - options().timeSource().millis() + unit.toMillis(duration), TimeUnit.MILLISECONDS); + options().clock().millis() + unit.toMillis(duration), TimeUnit.MILLISECONDS); EnumMap optionMap = Maps.newEnumMap(SignUrlOption.Option.class); for (SignUrlOption option : options) { optionMap.put(option.option(), option.value()); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java index 049d3029aa8e..a439e3c8ae49 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java @@ -23,7 +23,6 @@ import com.google.gcloud.spi.StorageRpc; import com.google.gcloud.spi.StorageRpcFactory; -import java.io.Serializable; import java.util.Objects; import java.util.Set; @@ -35,47 +34,12 @@ public class StorageOptions extends ServiceOptions { private static final String DEFAULT_PATH_DELIMITER = "/"; private final String pathDelimiter; - private final TimeSource timeSource; private transient StorageRpc storageRpc; - /** - * A class providing access to the current time in milliseconds. - * - * Implementations should implement {@code Serializable} wherever possible and must document - * whether or not they do support serialization. - */ - public static abstract class TimeSource { - - private static TimeSource DEFAULT_TIME_SOURCE = new DefaultTimeSource(); - - /** - * Returns current time in milliseconds according to this time source. - */ - public abstract long millis(); - - /** - * Returns the default time source. - */ - public static TimeSource defaultTimeSource() { - return DEFAULT_TIME_SOURCE; - } - - private static class DefaultTimeSource extends TimeSource implements Serializable { - - private static final long serialVersionUID = -5077300394286703864L; - - @Override - public long millis() { - return System.currentTimeMillis(); - } - } - } - public static class Builder extends ServiceOptions.Builder { private String pathDelimiter; - private TimeSource timeSource; private Builder() {} @@ -85,6 +49,7 @@ private Builder(StorageOptions options) { /** * Sets the path delimiter for the storage service. + * * @param pathDelimiter the path delimiter to set * @return the builder. */ @@ -93,17 +58,6 @@ public Builder pathDelimiter(String pathDelimiter) { return this; } - /** - * Sets the time source for the storage service. The time source is used by `signUrl` to compute - * URL's expiry time. If no time source is set by default `System.getTimeMillis()` is used. - * @param source the time source to set - * @return the builder. - */ - public Builder timeSource(TimeSource source) { - this.timeSource = source; - return this; - } - @Override public StorageOptions build() { return new StorageOptions(this); @@ -113,8 +67,6 @@ public StorageOptions build() { private StorageOptions(Builder builder) { super(builder); pathDelimiter = MoreObjects.firstNonNull(builder.pathDelimiter, DEFAULT_PATH_DELIMITER); - timeSource = MoreObjects.firstNonNull(builder.timeSource, TimeSource.defaultTimeSource()); - // todo: consider providing read-timeout } @Override @@ -144,14 +96,6 @@ public String pathDelimiter() { return pathDelimiter; } - /** - * Returns the storage service's time source. Default time source uses `System.getTimeMillis()` to - * get current time. - */ - public TimeSource timeSource() { - return timeSource; - } - @Override public Builder toBuilder() { return new Builder(this); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index 6d8fb393f58c..35920d9815ed 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -33,6 +33,7 @@ import com.google.common.io.BaseEncoding; import com.google.gcloud.AuthCredentials.ServiceAccountAuthCredentials; import com.google.gcloud.RetryParams; +import com.google.gcloud.ServiceOptions; import com.google.gcloud.spi.StorageRpc; import com.google.gcloud.spi.StorageRpc.Tuple; @@ -172,7 +173,7 @@ public class StorageImplTest { + "EkPPhszldvQTY486uPxyD/D7HdfnGW/Nbw5JUhfvecAdudDEhNAQ3PNabyDMI+TpiHy4NTWOrgdcWrzj6VXcdc" + "+uuABnPwRCdcyJ1xl2kOrPksRnp1auNGMLOe4IpEBjGY7baX9UG8+A45MbG0aHmkR59Op/aR9XowIDAQAB"; - private static final StorageOptions.TimeSource TIME_SOURCE = new StorageOptions.TimeSource() { + private static final ServiceOptions.Clock TIME_SOURCE = new ServiceOptions.Clock() { @Override public long millis() { return 42000L; @@ -804,7 +805,7 @@ public void testSignUrl() throws NoSuchAlgorithmException, InvalidKeyException, EasyMock.createMock(ServiceAccountAuthCredentials.class); EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock); EasyMock.expect(optionsMock.authCredentials()).andReturn(credentialsMock).times(2); - EasyMock.expect(optionsMock.timeSource()).andReturn(TIME_SOURCE); + EasyMock.expect(optionsMock.clock()).andReturn(TIME_SOURCE); EasyMock.expect(credentialsMock.privateKey()).andReturn(privateKey); EasyMock.expect(credentialsMock.account()).andReturn(account); EasyMock.replay(optionsMock, storageRpcMock, credentialsMock); @@ -839,7 +840,7 @@ public void testSignUrlWithOptions() throws NoSuchAlgorithmException, InvalidKey EasyMock.createMock(ServiceAccountAuthCredentials.class); EasyMock.expect(optionsMock.storageRpc()).andReturn(storageRpcMock); EasyMock.expect(optionsMock.authCredentials()).andReturn(credentialsMock).times(2); - EasyMock.expect(optionsMock.timeSource()).andReturn(TIME_SOURCE); + EasyMock.expect(optionsMock.clock()).andReturn(TIME_SOURCE); EasyMock.expect(credentialsMock.privateKey()).andReturn(privateKey); EasyMock.expect(credentialsMock.account()).andReturn(account); EasyMock.replay(optionsMock, storageRpcMock, credentialsMock); From 3e57ae795e3b3b0388f2515818d727a5e2b4abc9 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Tue, 13 Oct 2015 12:49:51 +0200 Subject: [PATCH 5/6] Fix javadoc indent in Blob and Storage --- .../src/main/java/com/google/gcloud/storage/Blob.java | 10 +++++----- .../main/java/com/google/gcloud/storage/Storage.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java index 7e2b059e51a2..e8d07e823546 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java @@ -248,7 +248,7 @@ public BlobWriteChannel writer(BlobTargetOption... options) { * want to require users to explicitly log in. * * @param duration time until the signed URL expires, expressed in {@code unit}. The finer - * granularity supported is 1 second, finer granularities will be truncated + * granularity supported is 1 second, finer granularities will be truncated * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options * @return a signed URL for this bucket and the specified options @@ -272,7 +272,7 @@ public Storage storage() { * @param storage the storage service used to issue the request * @param infos the blobs to get * @return an immutable list of {@code Blob} objects. If a blob does not exist or access to it has - * been denied the corresponding item in the list is {@code null}. + * been denied the corresponding item in the list is {@code null}. * @throws StorageException upon failure */ public static List get(final Storage storage, BlobInfo... infos) { @@ -297,7 +297,7 @@ public Blob apply(BlobInfo f) { * @param storage the storage service used to issue the request * @param infos the blobs to update * @return an immutable list of {@code Blob} objects. If a blob does not exist or access to it has - * been denied the corresponding item in the list is {@code null}. + * been denied the corresponding item in the list is {@code null}. * @throws StorageException upon failure */ public static List update(final Storage storage, BlobInfo... infos) { @@ -322,8 +322,8 @@ public Blob apply(BlobInfo f) { * @param storage the storage service used to issue the request * @param infos the blobs to delete * @return an immutable list of booleans. If a blob has been deleted the corresponding item in the - * list is {@code true}. If deletion failed or access to the resource was denied the item is - * {@code false}. + * list is {@code true}. If deletion failed or access to the resource was denied the item is + * {@code false}. * @throws StorageException upon failure */ public static List delete(Storage storage, BlobInfo... infos) { diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index ed265ed40447..78502c91feb8 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -649,7 +649,7 @@ public static Builder builder() { * * @param blobInfo the blob associated with the signed URL * @param duration time until the signed URL expires, expressed in {@code unit}. The finer - * granularity supported is 1 second, finer granularities will be truncated + * granularity supported is 1 second, finer granularities will be truncated * @param unit time unit of the {@code duration} parameter * @param options optional URL signing options * @see Signed-URLs @@ -661,7 +661,7 @@ public static Builder builder() { * * @param blobInfos blobs to get * @return an immutable list of {@code BlobInfo} objects. If a blob does not exist or access to it - * has been denied the corresponding item in the list is {@code null}. + * has been denied the corresponding item in the list is {@code null}. * @throws StorageException upon failure */ List get(BlobInfo... blobInfos); @@ -671,7 +671,7 @@ public static Builder builder() { * * @param blobInfos blobs to update * @return an immutable list of {@code BlobInfo} objects. If a blob does not exist or access to it - * has been denied the corresponding item in the list is {@code null}. + * has been denied the corresponding item in the list is {@code null}. * @throws StorageException upon failure */ List update(BlobInfo... blobInfos); @@ -681,8 +681,8 @@ public static Builder builder() { * * @param blobInfos blobs to delete * @return an immutable list of booleans. If a blob has been deleted the corresponding item in the - * list is {@code true}. If deletion failed or access to the resource was denied the item is - * {@code false}. + * list is {@code true}. If deletion failed or access to the resource was denied the item is + * {@code false}. * @throws StorageException upon failure */ List delete(BlobInfo... blobInfos); From 6bbfb056baa6e69934b3171f541dec54996c8516 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Tue, 13 Oct 2015 18:18:47 +0200 Subject: [PATCH 6/6] Add readResolve to StorageOptions.DefaultClock --- .../src/main/java/com/google/gcloud/ServiceOptions.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java b/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java index ab56c78e107e..b1510ae6a9fd 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/ServiceOptions.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.ObjectStreamException; import java.io.Serializable; import java.lang.reflect.Method; import java.net.HttpURLConnection; @@ -124,6 +125,10 @@ private static class DefaultClock extends ServiceOptions.Clock implements Serial public long millis() { return System.currentTimeMillis(); } + + private Object readResolve() throws ObjectStreamException { + return DEFAULT_TIME_SOURCE; + } } }