minor cleanups

line · Jan 18, 2024 · 5689068 · 5689068
1 parent e764192
commit 5689068
Show file tree

Hide file tree

Showing 9 changed files with 18 additions and 34 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java
@@ -114,7 +114,7 @@ public ClientRequestContext build() {
         if (timedOut()) {
             responseCancellationScheduler = CancellationScheduler.finished(false);
         } else {
-            responseCancellationScheduler = CancellationScheduler.of(0, false);
+            responseCancellationScheduler = CancellationScheduler.ofClient(0);
             responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask);
         }
 

diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java
@@ -229,7 +229,7 @@ private DefaultClientRequestContext(
               getAttributes(root), options.contextHook());
         assert (eventLoop == null && responseCancellationScheduler == null) ||
                (eventLoop != null && responseCancellationScheduler != null)
-                : "'eventLoop' and 'responseCancellationScheduler' should either be both null or non-null";
+                : "'eventLoop' and 'responseCancellationScheduler' should be both null or non-null";
 
         this.eventLoop = eventLoop;
         this.options = requireNonNull(options, "options");
@@ -244,7 +244,8 @@ private DefaultClientRequestContext(
                 responseTimeoutMillis = options().responseTimeoutMillis();
             }
             this.responseCancellationScheduler =
-                    CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), false);
+                    CancellationScheduler.ofClient(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis));
+            // the cancellationScheduler is not initialized here since the eventLoop is guaranteed to be null
         } else {
             this.responseCancellationScheduler = responseCancellationScheduler;
         }
@@ -522,7 +523,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx,
         log = RequestLog.builder(this);
         log.startRequest();
         responseCancellationScheduler =
-                CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis()), false);
+                CancellationScheduler.ofClient(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis()));
         writeTimeoutMillis = ctx.writeTimeoutMillis();
         maxResponseLength = ctx.maxResponseLength();
 

diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java
@@ -18,17 +18,19 @@
 
 import java.util.concurrent.CompletableFuture;
 
-import com.google.common.annotations.VisibleForTesting;
-
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.util.TimeoutMode;
 
 import io.netty.util.concurrent.EventExecutor;
 
 public interface CancellationScheduler {
 
-    static CancellationScheduler of(long timeoutNanos, boolean server) {
-        return new DefaultCancellationScheduler(timeoutNanos, server);
+    static CancellationScheduler ofClient(long timeoutNanos) {
+        return new DefaultCancellationScheduler(timeoutNanos, false);
+    }
+
+    static CancellationScheduler ofServer(long timeoutNanos) {
+        return new DefaultCancellationScheduler(timeoutNanos, true);
     }
 
     /**
@@ -93,9 +95,6 @@ public void run(Throwable cause) { /* no-op */ }
     @Deprecated
     CompletableFuture<Void> whenTimedOut();
 
-    @VisibleForTesting
-    boolean isInitialized();
-
     enum State {
         INIT,
         INACTIVE,

diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java
@@ -39,7 +39,6 @@
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.ImmediateEventExecutor;
 
-@SuppressWarnings("UnstableApiUsage")
 final class DefaultCancellationScheduler implements CancellationScheduler {
 
     private static final AtomicReferenceFieldUpdater<DefaultCancellationScheduler, CancellationFuture>
@@ -141,7 +140,7 @@ public void start(CancellationTask task) {
             return;
         }
         if (this.task != null) {
-            // just replace the task, there is already a pending timeout schedule running
+            // just replace the task
             this.task = task;
             return;
         }
@@ -456,8 +455,7 @@ public CompletableFuture<Void> whenTimedOut() {
         }
     }
 
-    @VisibleForTesting
-    public boolean isInitialized() {
+    private boolean isInitialized() {
         return pendingTask == noopPendingTask && eventLoop != null;
     }
 
@@ -542,12 +540,6 @@ State state() {
         return state;
     }
 
-    @Nullable
-    @VisibleForTesting
-    EventExecutor eventLoop() {
-        return eventLoop;
-    }
-
     private static class CancellationFuture extends UnmodifiableFuture<Throwable> {
         @Override
         protected void doComplete(@Nullable Throwable cause) {
@@ -562,9 +554,8 @@ void doComplete() {
     }
 
     private static CancellationScheduler finished0(boolean server) {
-        final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0, server);
-        cancellationScheduler
-                .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask);
+        final CancellationScheduler cancellationScheduler = new DefaultCancellationScheduler(0, server);
+        cancellationScheduler.initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask);
         cancellationScheduler.finishNow();
         return cancellationScheduler;
     }

diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java
@@ -108,9 +108,4 @@ public CompletableFuture<Void> whenTimingOut() {
     public CompletableFuture<Void> whenTimedOut() {
         return VOID_FUTURE;
     }
-
-    @Override
-    public boolean isInitialized() {
-        return true;
-    }
 }
diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java
@@ -177,7 +177,7 @@ public DefaultServiceRequestContext(
             this.requestCancellationScheduler = requestCancellationScheduler;
         } else {
             this.requestCancellationScheduler =
-                    CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis()), true);
+                    CancellationScheduler.ofServer(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis()));
             this.requestCancellationScheduler.init(eventLoop());
         }
         this.sslSession = sslSession;

diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java
@@ -232,7 +232,7 @@ public ServiceRequestContext build() {
         if (timedOut()) {
             requestCancellationScheduler = CancellationScheduler.finished(true);
         } else {
-            requestCancellationScheduler = CancellationScheduler.of(0, true);
+            requestCancellationScheduler = CancellationScheduler.ofServer(0);
             requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask);
         }
 

diff --git a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java
@@ -300,7 +300,7 @@ private static DefaultClientRequestContext newContext(ClientOptions clientOption
         return new DefaultClientRequestContext(
                 mock(EventLoop.class), NoopMeterRegistry.get(), SessionProtocol.H2C,
                 RequestId.random(), HttpMethod.POST, reqTarget, clientOptions, httpRequest,
-                null, RequestOptions.of(), CancellationScheduler.of(0, false), System.nanoTime(),
+                null, RequestOptions.of(), CancellationScheduler.ofClient(0), System.nanoTime(),
                 SystemInfo.currentTimeMicros());
     }
 

diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java
@@ -448,7 +448,6 @@ void multiple_ClearTimeoutInWhenCancelling() {
     void immediateFinishTriggersCompletion() {
         final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0);
         scheduler.init(eventExecutor);
-        await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull());
 
         final Throwable throwable = new Throwable();
 
@@ -469,7 +468,6 @@ void immediateFinishWithoutCause(boolean server) {
         final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0, server);
 
         scheduler.init(eventExecutor);
-        await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull());
 
         assertThat(scheduler.whenCancelling()).isNotCompleted();
         assertThat(scheduler.state()).isEqualTo(State.INIT);