From 71cd86896039815e45c8e68105a00778a19869a7 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Wed, 1 Jun 2016 22:19:27 +0200 Subject: [PATCH] Add CheckingPeriod and Timeout subclasses to WaitForOption --- .../java/com/google/cloud/bigquery/Job.java | 14 +- .../com/google/cloud/compute/Operation.java | 12 +- .../java/com/google/cloud/WaitForOption.java | 160 +++++++++++------- .../com/google/cloud/WaitForOptionTest.java | 49 +++--- 4 files changed, 137 insertions(+), 98 deletions(-) diff --git a/gcloud-java-bigquery/src/main/java/com/google/cloud/bigquery/Job.java b/gcloud-java-bigquery/src/main/java/com/google/cloud/bigquery/Job.java index 158ed25b518a..df0849d4b6f4 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/cloud/bigquery/Job.java +++ b/gcloud-java-bigquery/src/main/java/com/google/cloud/bigquery/Job.java @@ -16,18 +16,15 @@ package com.google.cloud.bigquery; -import static com.google.cloud.WaitForOption.Option.CHECKING_PERIOD; -import static com.google.cloud.WaitForOption.Option.TIMEOUT; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkNotNull; import com.google.cloud.Clock; import com.google.cloud.WaitForOption; import com.google.cloud.WaitForOption.CheckingPeriod; +import com.google.cloud.WaitForOption.Timeout; import java.io.IOException; import java.io.ObjectInputStream; -import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -192,14 +189,13 @@ public boolean isDone() { * this exception is never thrown. */ public Job waitFor(WaitForOption... waitOptions) throws InterruptedException, TimeoutException { - Map optionMap = WaitForOption.asMap(waitOptions); - CheckingPeriod checkingPeriod = firstNonNull(CHECKING_PERIOD.getCheckingPeriod(optionMap), - CheckingPeriod.defaultInstance()); - long timeout = firstNonNull(TIMEOUT.getLong(optionMap), -1L); + Timeout timeout = Timeout.getOrDefault(waitOptions); + CheckingPeriod checkingPeriod = CheckingPeriod.getOrDefault(waitOptions); + long timeoutMillis = timeout.timeoutMillis(); Clock clock = options.clock(); long startTime = clock.millis(); while (!isDone()) { - if (timeout != -1 && (clock.millis() - startTime) >= timeout) { + if (timeoutMillis != -1 && (clock.millis() - startTime) >= timeoutMillis) { throw new TimeoutException(); } checkingPeriod.sleep(); diff --git a/gcloud-java-compute/src/main/java/com/google/cloud/compute/Operation.java b/gcloud-java-compute/src/main/java/com/google/cloud/compute/Operation.java index 8351599dd1d7..78752e9cdaeb 100644 --- a/gcloud-java-compute/src/main/java/com/google/cloud/compute/Operation.java +++ b/gcloud-java-compute/src/main/java/com/google/cloud/compute/Operation.java @@ -16,9 +16,6 @@ package com.google.cloud.compute; -import static com.google.cloud.WaitForOption.Option.CHECKING_PERIOD; -import static com.google.cloud.WaitForOption.Option.TIMEOUT; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkNotNull; import com.google.cloud.Clock; @@ -707,14 +704,13 @@ public boolean isDone() { */ public Operation waitFor(WaitForOption... waitOptions) throws InterruptedException, TimeoutException { - Map optionMap = WaitForOption.asMap(waitOptions); - CheckingPeriod checkingPeriod = firstNonNull(CHECKING_PERIOD.getCheckingPeriod(optionMap), - CheckingPeriod.defaultInstance()); - long timeout = firstNonNull(TIMEOUT.getLong(optionMap), -1L); + WaitForOption.Timeout timeout = WaitForOption.Timeout.getOrDefault(waitOptions); + CheckingPeriod checkingPeriod = CheckingPeriod.getOrDefault(waitOptions); + long timeoutMillis = timeout.timeoutMillis(); Clock clock = options.clock(); long startTime = clock.millis(); while (!isDone()) { - if (timeout != -1 && (clock.millis() - startTime) >= timeout) { + if (timeoutMillis != -1 && (clock.millis() - startTime) >= timeoutMillis) { throw new TimeoutException(); } checkingPeriod.sleep(); diff --git a/gcloud-java-core/src/main/java/com/google/cloud/WaitForOption.java b/gcloud-java-core/src/main/java/com/google/cloud/WaitForOption.java index 85be6bdd0eb1..bcdbe9961478 100644 --- a/gcloud-java-core/src/main/java/com/google/cloud/WaitForOption.java +++ b/gcloud-java-core/src/main/java/com/google/cloud/WaitForOption.java @@ -16,39 +16,47 @@ package com.google.cloud; -import static com.google.cloud.WaitForOption.Option.CHECKING_PERIOD; -import static com.google.cloud.WaitForOption.Option.TIMEOUT; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.MoreObjects; -import com.google.common.collect.Maps; import java.io.Serializable; -import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; /** * This class represents options for methods that wait for changes in the status of a resource. */ -public final class WaitForOption implements Serializable { +public abstract class WaitForOption implements Serializable { private static final long serialVersionUID = 8443451708032349243L; - private final Option option; - private final Object value; + private final OptionType optionType; + + enum OptionType { + CHECKING_PERIOD, + TIMEOUT + } + + private WaitForOption(OptionType optionType) { + this.optionType = optionType; + } /** - * This class holds the actual period and related time unit for the checking period. + * This class represents an option to set how frequently the resource status should be checked. + * Objects of this class keep the actual period and related time unit for the checking period. */ - public static final class CheckingPeriod implements Serializable { + public static final class CheckingPeriod extends WaitForOption { private static final long serialVersionUID = -2481062893220539210L; + private static final CheckingPeriod DEFAULT = new CheckingPeriod(500, TimeUnit.MILLISECONDS); private final long period; private final TimeUnit unit; private CheckingPeriod(long period, TimeUnit unit) { + super(OptionType.CHECKING_PERIOD); this.period = period; this.unit = unit; } @@ -85,12 +93,14 @@ public boolean equals(Object obj) { return false; } CheckingPeriod other = (CheckingPeriod) obj; - return Objects.equals(period, other.period) && Objects.equals(unit, other.unit); + return baseEquals(other) + && Objects.equals(period, other.period) + && Objects.equals(unit, other.unit); } @Override public int hashCode() { - return Objects.hash(period, unit); + return Objects.hash(baseHashCode(), period, unit); } @Override @@ -102,48 +112,77 @@ public String toString() { } /** - * Returns the default checking period (500 milliseconds). + * Returns the {@code CheckingPeriod} option specified in {@code options}. If no + * {@code CheckingPeriod} could be found among {@code options}, the default checking period (500 + * milliseconds) is used. */ - public static CheckingPeriod defaultInstance() { - return new CheckingPeriod(500, TimeUnit.MILLISECONDS); + public static CheckingPeriod getOrDefault(WaitForOption... options) { + return getOrDefaultInternal(OptionType.CHECKING_PERIOD, DEFAULT, options); } } - public enum Option { - CHECKING_PERIOD, - TIMEOUT; + /** + * This class represents an option to set the maximum time to wait for the resource's status to + * reach the desired state. + */ + public static final class Timeout extends WaitForOption { + + private static final long serialVersionUID = -7120401111985321932L; + private static final Timeout DEFAULT = new Timeout(-1); - @SuppressWarnings("unchecked") - T get(Map options) { - return (T) options.get(this); + private final long timeoutMillis; + + private Timeout(long timeoutMillis) { + super(OptionType.TIMEOUT); + this.timeoutMillis = timeoutMillis; } - public Long getLong(Map options) { - return get(options); + /** + * Returns the timeout in milliseconds. + */ + public long timeoutMillis() { + return timeoutMillis; } - public CheckingPeriod getCheckingPeriod(Map options) { - return get(options); + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj == null || !(obj instanceof Timeout)) { + return false; + } + Timeout other = (Timeout) obj; + return baseEquals(other) && Objects.equals(timeoutMillis, other.timeoutMillis); } - } - private WaitForOption(Option option, Object value) { - this.option = option; - this.value = value; + @Override + public int hashCode() { + return Objects.hash(baseHashCode(), timeoutMillis); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("timeoutMillis", timeoutMillis) + .toString(); + } + + /** + * Returns the {@code Timeout} option specified in {@code options}. If no {@code Timeout} could + * be found among {@code options}, no timeout will be used. + */ + public static Timeout getOrDefault(WaitForOption... options) { + return getOrDefaultInternal(OptionType.TIMEOUT, DEFAULT, options); + } } - /** - * Returns the option's type. - */ - public Option option() { - return option; + OptionType optionType() { + return optionType; } - /** - * Returns the option's value. - */ - public Object value() { - return value; + final boolean baseEquals(WaitForOption option) { + return Objects.equals(option.optionType, option.optionType); } @Override @@ -151,24 +190,32 @@ public boolean equals(Object obj) { if (obj == this) { return true; } - if (obj == null || !(obj instanceof WaitForOption)) { + if (obj == null || !(obj.getClass().equals(WaitForOption.class))) { return false; } - WaitForOption other = (WaitForOption) obj; - return Objects.equals(option, other.option) && Objects.equals(value, other.value); + return baseEquals((WaitForOption) obj); + } + + final int baseHashCode() { + return Objects.hash(optionType); } @Override public int hashCode() { - return Objects.hash(option, value); + return baseHashCode(); } - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("name", option.name().toLowerCase()) - .add("value", value) - .toString(); + @SuppressWarnings("unchecked") + private static T getOrDefaultInternal(OptionType optionType, + T defaultValue, WaitForOption... options) { + T foundOption = null; + for (WaitForOption option : options) { + if (option.optionType.equals(optionType)) { + checkArgument(foundOption == null, "Duplicate option %s", option); + foundOption = (T) option; + } + } + return firstNonNull(foundOption, defaultValue); } /** @@ -177,9 +224,9 @@ public String toString() { * @param checkEvery the checking period * @param unit the time unit of the checking period */ - public static WaitForOption checkEvery(long checkEvery, TimeUnit unit) { + public static CheckingPeriod checkEvery(long checkEvery, TimeUnit unit) { checkArgument(checkEvery >= 0, "checkEvery must be >= 0"); - return new WaitForOption(CHECKING_PERIOD, new CheckingPeriod(checkEvery, unit)); + return new CheckingPeriod(checkEvery, unit); } /** @@ -188,17 +235,8 @@ public static WaitForOption checkEvery(long checkEvery, TimeUnit unit) { * @param timeout the maximum time to wait, expressed in {@code unit} * @param unit the time unit of the timeout */ - public static WaitForOption timeout(long timeout, TimeUnit unit) { + public static Timeout timeout(long timeout, TimeUnit unit) { checkArgument(timeout >= 0, "timeout must be >= 0"); - return new WaitForOption(TIMEOUT, TimeUnit.MILLISECONDS.convert(timeout, unit)); - } - - public static Map asMap(WaitForOption... options) { - Map optionMap = Maps.newEnumMap(Option.class); - for (WaitForOption waitOption : options) { - Object prev = optionMap.put(waitOption.option(), waitOption.value()); - checkArgument(prev == null, "Duplicate option %s", waitOption); - } - return optionMap; + return new Timeout(unit.toMillis(timeout)); } } diff --git a/gcloud-java-core/src/test/java/com/google/cloud/WaitForOptionTest.java b/gcloud-java-core/src/test/java/com/google/cloud/WaitForOptionTest.java index c1003bbe467c..82996e1ca3f8 100644 --- a/gcloud-java-core/src/test/java/com/google/cloud/WaitForOptionTest.java +++ b/gcloud-java-core/src/test/java/com/google/cloud/WaitForOptionTest.java @@ -18,16 +18,15 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; import com.google.cloud.WaitForOption.CheckingPeriod; -import com.google.cloud.WaitForOption.Option; +import com.google.cloud.WaitForOption.OptionType; +import com.google.cloud.WaitForOption.Timeout; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import java.util.Map; import java.util.concurrent.TimeUnit; public class WaitForOptionTest { @@ -35,18 +34,15 @@ public class WaitForOptionTest { @Rule public ExpectedException thrown = ExpectedException.none(); - private static final WaitForOption CHECKING_PERIOD_OPTION = + private static final CheckingPeriod CHECKING_PERIOD_OPTION = WaitForOption.checkEvery(42, TimeUnit.MILLISECONDS); - private static final WaitForOption TIMEOUT_OPTION = - WaitForOption.timeout(43, TimeUnit.MILLISECONDS); + private static final Timeout TIMEOUT_OPTION = WaitForOption.timeout(43, TimeUnit.MILLISECONDS); @Test public void testCheckEvery() { - assertEquals(Option.CHECKING_PERIOD, CHECKING_PERIOD_OPTION.option()); - assertTrue(CHECKING_PERIOD_OPTION.value() instanceof CheckingPeriod); - CheckingPeriod checkingPeriod = (CheckingPeriod) CHECKING_PERIOD_OPTION.value(); - assertEquals(42, checkingPeriod.period()); - assertEquals(TimeUnit.MILLISECONDS, checkingPeriod.unit()); + assertEquals(OptionType.CHECKING_PERIOD, CHECKING_PERIOD_OPTION.optionType()); + assertEquals(42, CHECKING_PERIOD_OPTION.period()); + assertEquals(TimeUnit.MILLISECONDS, CHECKING_PERIOD_OPTION.unit()); } @Test @@ -58,8 +54,10 @@ public void testCheckEvery_InvalidPeriod() { @Test public void testTimeout() { - assertEquals(Option.TIMEOUT, TIMEOUT_OPTION.option()); - assertEquals(43L, TIMEOUT_OPTION.value()); + assertEquals(OptionType.TIMEOUT, TIMEOUT_OPTION.optionType()); + assertEquals(43, TIMEOUT_OPTION.timeoutMillis()); + Timeout timeoutOption = WaitForOption.timeout(43, TimeUnit.SECONDS); + assertEquals(43_000, timeoutOption.timeoutMillis()); } @Test @@ -98,18 +96,29 @@ public void testEqualsAndHashCode() { } @Test - public void testAsMap() { - Map optionMap = WaitForOption.asMap(CHECKING_PERIOD_OPTION, TIMEOUT_OPTION); - CheckingPeriod checkingPeriod = Option.CHECKING_PERIOD.getCheckingPeriod(optionMap); - assertEquals(42, checkingPeriod.period()); + public void testGetOrDefault() { + assertEquals(CHECKING_PERIOD_OPTION, + CheckingPeriod.getOrDefault(CHECKING_PERIOD_OPTION, TIMEOUT_OPTION)); + assertEquals(TIMEOUT_OPTION, + Timeout.getOrDefault(CHECKING_PERIOD_OPTION, TIMEOUT_OPTION)); + CheckingPeriod checkingPeriod = CheckingPeriod.getOrDefault(TIMEOUT_OPTION); + assertEquals(500, checkingPeriod.period()); assertEquals(TimeUnit.MILLISECONDS, checkingPeriod.unit()); - assertEquals(43, (long) Option.TIMEOUT.getLong(optionMap)); + Timeout timeout = Timeout.getOrDefault(CHECKING_PERIOD_OPTION); + assertEquals(-1, timeout.timeoutMillis()); } @Test - public void testAsMap_DuplicateOption() { + public void testCheckingPeriodGetOrDefault_DuplicateOption() { thrown.expect(IllegalArgumentException.class); thrown.expectMessage(String.format("Duplicate option %s", CHECKING_PERIOD_OPTION)); - WaitForOption.asMap(CHECKING_PERIOD_OPTION, CHECKING_PERIOD_OPTION); + CheckingPeriod.getOrDefault(CHECKING_PERIOD_OPTION, CHECKING_PERIOD_OPTION); + } + + @Test + public void testTimeoutGetOrDefault_DuplicateOption() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage(String.format("Duplicate option %s", TIMEOUT_OPTION)); + Timeout.getOrDefault(TIMEOUT_OPTION, TIMEOUT_OPTION); } }