Issue 10245 fix for long, TimeUnit to java.time.Duration to API's

grpc · Sep 26, 2024 · 84a37e9 · 84a37e9
1 parent db6c21f
commit 84a37e9
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 105 deletions.
diff --git a/api/src/context/java/io/grpc/Deadline.java b/api/src/context/java/io/grpc/Deadline.java
@@ -16,6 +16,7 @@
 
 package io.grpc;
 
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.concurrent.ScheduledExecutorService;
@@ -68,6 +69,10 @@ public static Deadline after(long duration, TimeUnit units) {
     return after(duration, units, SYSTEM_TICKER);
   }
 
+  public static Deadline after(Duration duration) {
+    return after(TimeUnit.NANOSECONDS.convert(duration.getSeconds(), TimeUnit.SECONDS), TimeUnit.NANOSECONDS, SYSTEM_TICKER);
+  }
+
   /**
    * Create a deadline that will expire at the specified offset based on the given {@link Ticker}.
    *
@@ -95,6 +100,10 @@ public static Deadline after(long duration, TimeUnit units, Ticker ticker) {
     return new Deadline(ticker, units.toNanos(duration), true);
   }
 
+  public static Deadline after(Duration duration, Ticker ticker) {
+    return after(TimeUnit.NANOSECONDS.convert(duration.getSeconds(), TimeUnit.SECONDS), TimeUnit.NANOSECONDS, ticker);
+  }
+
   private final Ticker ticker;
   private final long deadlineNanos;
   private volatile boolean expired;

diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java
@@ -20,6 +20,7 @@
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -176,6 +177,11 @@ public CallOptions withDeadlineAfter(long duration, TimeUnit unit) {
     return withDeadline(Deadline.after(duration, unit));
   }
 
+  public CallOptions withDeadlineAfter(Duration duration) {
+    return withDeadlineAfter(TimeUnit.NANOSECONDS.convert(duration.getSeconds(), TimeUnit.SECONDS),
+        TimeUnit.NANOSECONDS);
+  }
+
   /**
    * Returns the deadline or {@code null} if the deadline is not set.
    */

diff --git a/api/src/main/java/io/grpc/SynchronizationContext.java b/api/src/main/java/io/grpc/SynchronizationContext.java
@@ -150,16 +150,16 @@ public final ScheduledHandle schedule(
       final Runnable task, long delay, TimeUnit unit, ScheduledExecutorService timerService) {
     final ManagedRunnable runnable = new ManagedRunnable(task);
     ScheduledFuture<?> future = timerService.schedule(new Runnable() {
-        @Override
-        public void run() {
-          execute(runnable);
-        }
+      @Override
+      public void run() {
+        execute(runnable);
+      }
 
-        @Override
-        public String toString() {
-          return task.toString() + "(scheduled in SynchronizationContext)";
-        }
-      }, delay, unit);
+      @Override
+      public String toString() {
+        return task.toString() + "(scheduled in SynchronizationContext)";
+      }
+    }, delay, unit);
     return new ScheduledHandle(runnable, future);
   }
 
@@ -194,30 +194,11 @@ public String toString() {
     return new ScheduledHandle(runnable, future);
   }
 
-  public final ScheduledHandle scheduleWithFixedDelay(
-      final Runnable task, Duration initialDelay, Duration delay, TimeUnit unit,
+  public final ScheduledHandle scheduleWithFixedDelay (
+      final Runnable task, Duration initialDelay, Duration delay,
       ScheduledExecutorService timerService) {
-    final ManagedRunnable runnable = new ManagedRunnable(task);
-    ScheduledFuture<?> future = timerService.scheduleWithFixedDelay(new Runnable() {
-      @Override
-      public void run() {
-        execute(runnable);
-      }
-
-      @Override
-      public String toString() {
-        return task.toString() + "(scheduled in SynchronizationContext with delay of " + delay
-            + ")";
-      }
-    }, toNanosSaturated(initialDelay), toNanosSaturated(delay), unit);
-    return new ScheduledHandle(runnable, future);
-  }
-  static long toNanosSaturated(Duration duration) {
-    try {
-      return duration.toNanos();
-    } catch (ArithmeticException tooBig) {
-      return duration.isNegative() ? Long.MIN_VALUE : Long.MAX_VALUE;
-    }
+    return scheduleWithFixedDelay(task, TimeUnit.NANOSECONDS.convert(initialDelay.getSeconds(), TimeUnit.SECONDS),
+        TimeUnit.NANOSECONDS.convert(delay.getSeconds(), TimeUnit.SECONDS), TimeUnit.NANOSECONDS, timerService);
   }
 
   private static class ManagedRunnable implements Runnable {
@@ -272,4 +253,4 @@ public boolean isPending() {
       return !(runnable.hasStarted || runnable.isCancelled);
     }
   }
-}
+}
diff --git a/api/src/test/java/io/grpc/CallOptionsTest.java b/api/src/test/java/io/grpc/CallOptionsTest.java
@@ -32,6 +32,7 @@
 import com.google.common.base.Objects;
 import io.grpc.ClientStreamTracer.StreamInfo;
 import io.grpc.internal.SerializingExecutor;
+import java.time.Duration;
 import java.util.concurrent.Executor;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -150,6 +151,14 @@ public void withDeadlineAfter() {
     assertAbout(deadline()).that(actual).isWithin(10, MILLISECONDS).of(expected);
   }
 
+ @Test
+  public void withDeadlineAfterDuration() {
+    Deadline actual = CallOptions.DEFAULT.withDeadlineAfter(Duration.ofMinutes(1l)).getDeadline();
+    Deadline expected = Deadline.after(1, MINUTES);
+
+    assertAbout(deadline()).that(actual).isWithin(10, MILLISECONDS).of(expected);
+  }
+
   @Test
   public void toStringMatches_noDeadline_default() {
     String actual = allSet

diff --git a/api/src/test/java/io/grpc/DeadlineTest.java b/api/src/test/java/io/grpc/DeadlineTest.java
@@ -29,6 +29,7 @@
 
 import com.google.common.testing.EqualsTester;
 import com.google.common.truth.Truth;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
@@ -137,6 +138,11 @@ public void deadlineMatchesLongValue() {
     assertEquals(10, Deadline.after(10, TimeUnit.MINUTES, ticker).timeRemaining(TimeUnit.MINUTES));
   }
 
+  @Test
+  public void deadLineMatchesDurationValue(){
+    assertEquals(10, Deadline.after(Duration.ofMinutes(10), ticker).timeRemaining(TimeUnit.MINUTES));
+  }
+
   @Test
   public void pastDeadlineIsExpired() {
     Deadline d = Deadline.after(-1, TimeUnit.SECONDS, ticker);
@@ -305,6 +311,36 @@ public void tickersDontMatch() {
     assertFalse(success);
   }
 
+  @Test
+  public void tickersDontMatchDuration() {
+    Deadline d1 = Deadline.after(Duration.ofSeconds(10));
+    Deadline d2 = Deadline.after(Duration.ofSeconds(10), ticker);
+    boolean success = false;
+    try {
+      d1.compareTo(d2);
+      success = true;
+    } catch (AssertionError e) {
+      // Expected
+    }
+    assertFalse(success);
+
+    try {
+      d1.minimum(d2);
+      success = true;
+    } catch (AssertionError e) {
+      // Expected
+    }
+    assertFalse(success);
+
+    try {
+      d1.isBefore(d2);
+      success = true;
+    } catch (AssertionError e) {
+      // Expected
+    }
+    assertFalse(success);
+  }
+
   @Test
   public void toString_before() {
     Deadline d = Deadline.after(12, TimeUnit.MICROSECONDS, ticker);