Throw IllegalArgumentException when Duration.create receives invalid arguments.

Fixes #1179.  Throwing IllegalArgumentException for invalid arguments is more
consistent with the rest of the opencensus-java API.

2 files changed, 78 insertions, 17 deletions

diff --git a/api/src/main/java/io/opencensus/common/Duration.java b/api/src/main/java/io/opencensus/common/Duration.java
index 4b7bb367..f46cd187 100644
--- a/api/src/main/java/io/opencensus/common/Duration.java
+++ b/api/src/main/java/io/opencensus/common/Duration.java
@@ -35,7 +35,6 @@ import javax.annotation.concurrent.Immutable;
 @Immutable
 @AutoValue
 public abstract class Duration implements Comparable<Duration> {
-  private static final Duration ZERO = create(0, 0);
 
   /**
    * Creates a new time duration from given seconds and nanoseconds.
@@ -47,19 +46,30 @@ public abstract class Duration implements Comparable<Duration> {
    *     negative `nanos` field. For durations of one second or more, a non-zero value for the
    *     `nanos` field must be of the same sign as the `seconds` field. Must be from -999,999,999 to
    *     +999,999,999 inclusive.
-   * @return new {@code Duration} with specified fields. For invalid inputs, a {@code Duration} of
-   *     zero is returned.
+   * @return new {@code Duration} with specified fields.
+   * @throws IllegalArgumentException if the arguments are out of range or have inconsistent sign.
    * @since 0.5
    */
   public static Duration create(long seconds, int nanos) {
-    if (seconds < -MAX_SECONDS || seconds > MAX_SECONDS) {
-      return ZERO;
+    if (seconds < -MAX_SECONDS) {
+      throw new IllegalArgumentException(
+          "'seconds' is less than minimum (" + -MAX_SECONDS + "): " + seconds);
     }
-    if (nanos < -MAX_NANOS || nanos > MAX_NANOS) {
-      return ZERO;
+    if (seconds > MAX_SECONDS) {
+      throw new IllegalArgumentException(
+          "'seconds' is greater than maximum (" + MAX_SECONDS + "): " + seconds);
+    }
+    if (nanos < -MAX_NANOS) {
+      throw new IllegalArgumentException(
+          "'nanos' is less than minimum (" + -MAX_NANOS + "): " + nanos);
+    }
+    if (nanos > MAX_NANOS) {
+      throw new IllegalArgumentException(
+          "'nanos' is greater than maximum (" + MAX_NANOS + "): " + nanos);
     }
     if ((seconds < 0 && nanos > 0) || (seconds > 0 && nanos < 0)) {
-      return ZERO;
+      throw new IllegalArgumentException(
+          "'seconds' and 'nanos' have inconsistent sign: seconds=" + seconds + ", nanos=" + nanos);
     }
     return new AutoValue_Duration(seconds, nanos);
   }
@@ -68,8 +78,9 @@ public abstract class Duration implements Comparable<Duration> {
    * Creates a new {@code Duration} from given milliseconds.
    *
    * @param millis the duration in milliseconds.
-   * @return a new {@code Duration} from given milliseconds. For invalid inputs, a {@code Duration}
-   *     of zero is returned.
+   * @return a new {@code Duration} from given milliseconds.
+   * @throws IllegalArgumentException if the number of milliseconds is out of the range that can be
+   *     represented by {@code Duration}.
    * @since 0.5
    */
   public static Duration fromMillis(long millis) {
diff --git a/api/src/test/java/io/opencensus/common/DurationTest.java b/api/src/test/java/io/opencensus/common/DurationTest.java
index fb5f314e..ea636ca0 100644
--- a/api/src/test/java/io/opencensus/common/DurationTest.java
+++ b/api/src/test/java/io/opencensus/common/DurationTest.java
@@ -18,13 +18,17 @@ package io.opencensus.common;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 /** Unit tests for {@link Duration}. */
 @RunWith(JUnit4.class)
 public class DurationTest {
+  @Rule public ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void testDurationCreate() {
     assertThat(Duration.create(24, 42).getSeconds()).isEqualTo(24);
@@ -38,13 +42,45 @@ public class DurationTest {
   }
 
   @Test
-  public void testDurationCreateInvalidInput() {
-    assertThat(Duration.create(-315576000001L, 0)).isEqualTo(Duration.create(0, 0));
-    assertThat(Duration.create(315576000001L, 0)).isEqualTo(Duration.create(0, 0));
-    assertThat(Duration.create(0, 1000000000)).isEqualTo(Duration.create(0, 0));
-    assertThat(Duration.create(0, -1000000000)).isEqualTo(Duration.create(0, 0));
-    assertThat(Duration.create(-1, 1)).isEqualTo(Duration.create(0, 0));
-    assertThat(Duration.create(1, -1)).isEqualTo(Duration.create(0, 0));
+  public void create_SecondsTooLow() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' is less than minimum (-315576000000): -315576000001");
+    Duration.create(-315576000001L, 0);
+  }
+
+  @Test
+  public void create_SecondsTooHigh() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' is greater than maximum (315576000000): 315576000001");
+    Duration.create(315576000001L, 0);
+  }
+
+  @Test
+  public void create_NanosTooLow() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'nanos' is less than minimum (-999999999): -1000000000");
+    Duration.create(0, -1000000000);
+  }
+
+  @Test
+  public void create_NanosTooHigh() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'nanos' is greater than maximum (999999999): 1000000000");
+    Duration.create(0, 1000000000);
+  }
+
+  @Test
+  public void create_NegativeSecondsPositiveNanos() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' and 'nanos' have inconsistent sign: seconds=-1, nanos=1");
+    Duration.create(-1, 1);
+  }
+
+  @Test
+  public void create_PositiveSecondsNegativeNanos() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' and 'nanos' have inconsistent sign: seconds=1, nanos=-1");
+    Duration.create(1, -1);
   }
 
   @Test
@@ -63,6 +99,20 @@ public class DurationTest {
   }
 
   @Test
+  public void fromMillis_TooLow() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' is less than minimum (-315576000000): -315576000001");
+    Duration.fromMillis(-315576000001000L);
+  }
+
+  @Test
+  public void fromMillis_TooHigh() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("'seconds' is greater than maximum (315576000000): 315576000001");
+    Duration.fromMillis(315576000001000L);
+  }
+
+  @Test
   public void duration_CompareLength() {
     assertThat(Duration.create(0, 0).compareTo(Duration.create(0, 0))).isEqualTo(0);
     assertThat(Duration.create(24, 42).compareTo(Duration.create(24, 42))).isEqualTo(0);