Move the DurationConverter to the common.options package

Also change it to java.time.Duration, rather than Jodatime. Now that we're on
Java 8, we no longer need Jodatime.

PiperOrigin-RevId: 162917526

6 files changed, 133 insertions, 56 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index e0c6a936a6..49992e1c4a 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -377,7 +377,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/buildeventstream/transports",
         "//src/main/java/com/google/devtools/common/options",
         "//third_party:guava",
-        "//third_party:joda_time",
         "//third_party:jsr305",
         "//third_party/grpc:grpc-jar",
         "@com_google_protobuf//:protobuf_java",
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
index 9c67f3046e..e24c204924 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
@@ -14,15 +14,11 @@
 
 package com.google.devtools.build.lib.buildeventservice;
 
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
-import com.google.devtools.common.options.OptionsParsingException;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import org.joda.time.Duration;
+import java.time.Duration;
 
 /** Options used by {@link BuildEventServiceModule}. */
 public class BuildEventServiceOptions extends OptionsBase {
@@ -41,7 +37,6 @@ public class BuildEventServiceOptions extends OptionsBase {
   @Option(
     name = "bes_timeout",
     defaultValue = "0s",
-    converter = DurationConverter.class,
     documentationCategory = OptionDocumentationCategory.LOGGING,
     effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
     help =
@@ -81,47 +76,4 @@ public class BuildEventServiceOptions extends OptionsBase {
     help = "Specifies the BES project identifier. Defaults to null."
   )
   public String projectId;
-
-  /**
-   * Simple String to Duration Converter.
-   */
-  public static class DurationConverter implements Converter<Duration> {
-
-    private final Pattern durationRegex = Pattern.compile("^([0-9]+)(d|h|m|s|ms)$");
-
-    @Override
-    public Duration convert(String input) throws OptionsParsingException {
-      // To be compatible with the previous parser, '0' doesn't need a unit.
-      if ("0".equals(input)) {
-        return Duration.ZERO;
-      }
-      Matcher m = durationRegex.matcher(input);
-      if (!m.matches()) {
-        throw new OptionsParsingException("Illegal duration '" + input + "'.");
-      }
-      long duration = Long.parseLong(m.group(1));
-      String unit = m.group(2);
-      switch(unit) {
-        case "d":
-          return Duration.standardDays(duration);
-        case "h":
-          return Duration.standardHours(duration);
-        case "m":
-          return Duration.standardMinutes(duration);
-        case "s":
-          return Duration.standardSeconds(duration);
-        case "ms":
-          return Duration.millis(duration);
-        default:
-          throw new IllegalStateException("This must not happen. Did you update the regex without "
-              + "the switch case?");
-      }
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "An immutable length of time.";
-    }
-  }
-
 }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
index 0cf67ba959..b7c8ce770e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
@@ -25,7 +25,6 @@ import static com.google.devtools.build.v1.BuildStatus.Result.COMMAND_SUCCEEDED;
 import static com.google.devtools.build.v1.BuildStatus.Result.UNKNOWN_STATUS;
 import static java.lang.String.format;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import static org.joda.time.Duration.standardSeconds;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
@@ -55,6 +54,7 @@ import com.google.devtools.build.v1.PublishBuildToolEventStreamResponse;
 import com.google.devtools.build.v1.PublishLifecycleEventRequest;
 import com.google.protobuf.Any;
 import io.grpc.Status;
+import java.time.Duration;
 import java.util.Deque;
 import java.util.concurrent.BlockingDeque;
 import java.util.concurrent.Callable;
@@ -69,7 +69,6 @@ import java.util.concurrent.locks.LockSupport;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
-import org.joda.time.Duration;
 
 /** A {@link BuildEventTransport} that streams {@link BuildEvent}s to BuildEventService. */
 public class BuildEventServiceTransport implements BuildEventTransport {
@@ -81,7 +80,7 @@ public class BuildEventServiceTransport implements BuildEventTransport {
   private static final Logger logger = Logger.getLogger(BuildEventServiceTransport.class.getName());
 
   /** Max wait time until for the Streaming RPC to finish after all events were enqueued. */
-  private static final Duration PUBLISH_EVENT_STREAM_FINISHED_TIMEOUT = standardSeconds(120);
+  private static final Duration PUBLISH_EVENT_STREAM_FINISHED_TIMEOUT = Duration.ofSeconds(120);
 
   private final ListeningExecutorService uploaderExecutorService;
   private final Duration uploadTimeout;
@@ -206,7 +205,7 @@ public class BuildEventServiceTransport implements BuildEventTransport {
                 if (Duration.ZERO.equals(uploadTimeout)) {
                   uploadComplete.get();
                 } else {
-                  uploadComplete.get(uploadTimeout.getMillis(), MILLISECONDS);
+                  uploadComplete.get(uploadTimeout.toMillis(), MILLISECONDS);
                 }
                 report(INFO, UPLOAD_SUCCEEDED_MESSAGE);
               } catch (Exception e) {
@@ -407,7 +406,7 @@ public class BuildEventServiceTransport implements BuildEventTransport {
     pendingAck = new ConcurrentLinkedDeque<>();
 
     return publishEventStream(pendingAck, pendingSend, besClient)
-        .get(PUBLISH_EVENT_STREAM_FINISHED_TIMEOUT.getMillis(), TimeUnit.MILLISECONDS);
+        .get(PUBLISH_EVENT_STREAM_FINISHED_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS);
   }
 
   /** Method responsible for a single Streaming RPC. */
diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java
index 4584f80bc2..aacc64b108 100644
--- a/src/main/java/com/google/devtools/common/options/Converters.java
+++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -16,10 +16,12 @@ package com.google.devtools.common.options;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
+import java.time.Duration;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
@@ -178,6 +180,47 @@ public final class Converters {
   }
 
   /**
+   * Standard converter for the {@link java.time.Duration} type.
+   */
+  public static class DurationConverter implements Converter<Duration> {
+    private final Pattern durationRegex = Pattern.compile("^([0-9]+)(d|h|m|s|ms)$");
+
+    @Override
+    public Duration convert(String input) throws OptionsParsingException {
+      // To be compatible with the previous parser, '0' doesn't need a unit.
+      if ("0".equals(input)) {
+        return Duration.ZERO;
+      }
+      Matcher m = durationRegex.matcher(input);
+      if (!m.matches()) {
+        throw new OptionsParsingException("Illegal duration '" + input + "'.");
+      }
+      long duration = Long.parseLong(m.group(1));
+      String unit = m.group(2);
+      switch(unit) {
+        case "d":
+          return Duration.ofDays(duration);
+        case "h":
+          return Duration.ofHours(duration);
+        case "m":
+          return Duration.ofMinutes(duration);
+        case "s":
+          return Duration.ofSeconds(duration);
+        case "ms":
+          return Duration.ofMillis(duration);
+        default:
+          throw new IllegalStateException("This must not happen. Did you update the regex without "
+              + "the switch case?");
+      }
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "An immutable length of time.";
+    }
+  }
+
+  /**
    * The converters that are available to the options parser by default. These are used if the
    * {@code @Option} annotation does not specify its own {@code converter}, and its type is one of
    * the following.
@@ -185,12 +228,14 @@ public final class Converters {
   static final Map<Class<?>, Converter<?>> DEFAULT_CONVERTERS = Maps.newHashMap();
 
   static {
+    // 1:1 correspondence with UsesOnlyCoreTypes.CORE_TYPES.
     DEFAULT_CONVERTERS.put(String.class, new Converters.StringConverter());
     DEFAULT_CONVERTERS.put(int.class, new Converters.IntegerConverter());
     DEFAULT_CONVERTERS.put(long.class, new Converters.LongConverter());
     DEFAULT_CONVERTERS.put(double.class, new Converters.DoubleConverter());
     DEFAULT_CONVERTERS.put(boolean.class, new Converters.BooleanConverter());
     DEFAULT_CONVERTERS.put(TriState.class, new Converters.TriStateConverter());
+    DEFAULT_CONVERTERS.put(Duration.class, new Converters.DurationConverter());
     DEFAULT_CONVERTERS.put(Void.class, new Converters.VoidConverter());
   }
 
diff --git a/src/main/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java b/src/main/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
index 6f2714f90f..c40495d17b 100644
--- a/src/main/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
+++ b/src/main/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
@@ -20,6 +20,7 @@ import java.lang.annotation.Inherited;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.time.Duration;
 import java.util.List;
 
 /**
@@ -52,6 +53,7 @@ public @interface UsesOnlyCoreTypes {
       double.class,
       boolean.class,
       TriState.class,
-      Void.class
+      Void.class,
+      Duration.class
   );
 }
diff --git a/src/test/java/com/google/devtools/common/options/DurationConverterTest.java b/src/test/java/com/google/devtools/common/options/DurationConverterTest.java
new file mode 100644
index 0000000000..2d56e01c3b
--- /dev/null
+++ b/src/test/java/com/google/devtools/common/options/DurationConverterTest.java
@@ -0,0 +1,80 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.common.options;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+
+import com.google.devtools.common.options.Converters.DurationConverter;
+import java.time.Duration;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link DurationConverter}. */
+@RunWith(JUnit4.class)
+public class DurationConverterTest {
+
+  @Test
+  public void testDurationConverter_zero() throws OptionsParsingException {
+    DurationConverter converter = new DurationConverter();
+
+    assertThat(converter.convert("0")).isEqualTo(Duration.ZERO);
+    assertThat(converter.convert("0d")).isEqualTo(Duration.ZERO);
+    assertThat(converter.convert("0h")).isEqualTo(Duration.ZERO);
+    assertThat(converter.convert("0m")).isEqualTo(Duration.ZERO);
+    assertThat(converter.convert("0s")).isEqualTo(Duration.ZERO);
+    assertThat(converter.convert("0ms")).isEqualTo(Duration.ZERO);
+  }
+
+  @Test
+  public void testDurationConverter_basic() throws OptionsParsingException {
+    DurationConverter converter = new DurationConverter();
+
+    assertThat(converter.convert("10d")).isEqualTo(Duration.ofDays(10));
+    assertThat(converter.convert("20h")).isEqualTo(Duration.ofHours(20));
+    assertThat(converter.convert("30m")).isEqualTo(Duration.ofMinutes(30));
+    assertThat(converter.convert("40s")).isEqualTo(Duration.ofSeconds(40));
+    assertThat(converter.convert("50ms")).isEqualTo(Duration.ofMillis(50));
+  }
+
+  @Test
+  public void testDurationConverter_invalidInputs() {
+    DurationConverter converter = new DurationConverter();
+
+    try {
+      converter.convert("");
+      fail();
+    } catch (OptionsParsingException expected) {
+    }
+
+    try {
+      converter.convert("-10d");
+      fail();
+    } catch (OptionsParsingException expected) {
+    }
+
+    try {
+      converter.convert("h");
+      fail();
+    } catch (OptionsParsingException expected) {
+    }
+
+    try {
+      converter.convert("1g");
+      fail();
+    } catch (OptionsParsingException expected) {
+    }
+  }
+}