Merge pull request #243 from mziccard/sign-url-time-unit

Make signUrl take long duration and TimeUnit parameter

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;
@@ -62,6 +63,7 @@ public abstract class ServiceOptions<
   private final ServiceRpcFactory<ServiceRpcT, OptionsT> serviceRpcFactory;
   private final int connectTimeout;
   private final int readTimeout;
+  private final Clock clock;
 
   public interface HttpTransportFactory extends Serializable {
     HttpTransport create();
@@ -91,7 +93,44 @@ 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();
+      }
+
+      private Object readResolve() throws ObjectStreamException {
+        return DEFAULT_TIME_SOURCE;
+      }
+    }
+  }
 
   protected abstract static class Builder<
       ServiceRpcT,
@@ -106,6 +145,7 @@ protected abstract static class Builder<
     private ServiceRpcFactory<ServiceRpcT, OptionsT> serviceRpcFactory;
     private int connectTimeout = -1;
     private int readTimeout = -1;
+    private Clock clock;
 
     protected Builder() {}
 
@@ -125,6 +165,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 +273,7 @@ protected ServiceOptions(Builder<ServiceRpcT, OptionsT, ?> builder) {
     serviceRpcFactory = builder.serviceRpcFactory;
     connectTimeout = builder.connectTimeout;
     readTimeout = builder.readTimeout;
+    clock = firstNonNull(builder.clock, Clock.defaultClock());
   }
 
   private static AuthCredentials defaultAuthCredentials() {
@@ -419,9 +472,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 +491,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<ServiceRpcT, OptionsT, ?> toBuilder();

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<ServiceAccountAuthCredentials, Blob> 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

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, 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
    * @see <a href="https://cloud.google.com/storage/docs/access-control#Signed-URLs">Signed-URLs</a>
    */
-  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);
   }
 
   /**
@@ -269,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<Blob> get(final Storage storage, BlobInfo... infos) {
@@ -294,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<Blob> update(final Storage storage, BlobInfo... infos) {
@@ -319,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<Boolean> delete(Storage storage, BlobInfo... infos) {

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,22 +644,24 @@ public static Builder builder() {
    * <p>
    * Example usage of creating a signed URL that is valid for 2 weeks:
    * <pre>   {@code
-   *     service.signUrl(BlobInfo.of("bucket", "name"), TimeUnit.DAYS.toSeconds(14));
+   *     service.signUrl(BlobInfo.of("bucket", "name"), 14, TimeUnit.DAYS);
    * }</pre>
    *
-   * @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, finer granularities will be truncated
+   * @param unit time unit of the {@code duration} parameter
    * @param options optional URL signing options
    * @see <a href="https://cloud.google.com/storage/docs/access-control#Signed-URLs">Signed-URLs</a>
    */
-  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.
    *
    * @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<BlobInfo> get(BlobInfo... blobInfos);
@@ -668,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<BlobInfo> update(BlobInfo... blobInfos);
@@ -678,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<Boolean> delete(BlobInfo... blobInfos);

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<StorageOptions> 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().clock().millis() + unit.toMillis(duration), TimeUnit.MILLISECONDS);
     EnumMap<SignUrlOption.Option, Object> optionMap = Maps.newEnumMap(SignUrlOption.Option.class);
     for (SignUrlOption option : options) {
       optionMap.put(option.option(), option.value());

gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java

@@ -47,6 +47,12 @@ 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;
@@ -61,7 +67,6 @@ public StorageOptions build() {
   private StorageOptions(Builder builder) {
     super(builder);
     pathDelimiter = MoreObjects.firstNonNull(builder.pathDelimiter, DEFAULT_PATH_DELIMITER);
-    // todo: consider providing read-timeout
   }
 
   @Override
@@ -84,6 +89,9 @@ StorageRpc storageRpc() {
     return storageRpc;
   }
 
+  /**
+   * Returns the storage service's path delimiter.
+   */
   public String pathDelimiter() {
     return pathDelimiter;
   }

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

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();