Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
opensearch-project · Oct 11, 2023 · 1c184e2 · 1c184e2
1 parent ab4140c
commit 1c184e2
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 21 deletions.
diff --git a/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpChannel.java b/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpChannel.java
@@ -40,6 +40,7 @@
 import org.opensearch.transport.netty4.Netty4TcpChannel;
 
 import java.net.InetSocketAddress;
+import java.util.Optional;
 
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelPipeline;
@@ -100,18 +101,18 @@ public Channel getNettyChannel() {
 
     @SuppressWarnings("unchecked")
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         Object handler = getNettyChannel().pipeline().get(name);
 
         if (handler == null && inboundPipeline() != null) {
             handler = inboundPipeline().get(name);
         }
 
         if (handler != null && clazz.isInstance(handler) == true) {
-            return (T) handler;
+            return Optional.of((T) handler);
         }
 
-        return null;
+        return Optional.empty();
     }
 
     @Override

diff --git a/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/Netty4TcpChannel.java b/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/Netty4TcpChannel.java
@@ -41,6 +41,7 @@
 import org.opensearch.transport.TransportException;
 
 import java.net.InetSocketAddress;
+import java.util.Optional;
 
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
@@ -166,14 +167,14 @@ public void sendMessage(BytesReference reference, ActionListener<Void> listener)
 
     @SuppressWarnings("unchecked")
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         final Object handler = getNettyChannel().pipeline().get(name);
 
         if (handler != null && clazz.isInstance(handler) == true) {
-            return (T) handler;
+            return Optional.of((T) handler);
         }
 
-        return null;
+        return Optional.empty();
     }
 
     public Channel getNettyChannel() {

diff --git a/server/src/main/java/org/opensearch/http/HttpChannel.java b/server/src/main/java/org/opensearch/http/HttpChannel.java
@@ -36,6 +36,7 @@
 import org.opensearch.core.action.ActionListener;
 
 import java.net.InetSocketAddress;
+import java.util.Optional;
 
 /**
  * Represents an HTTP comms channel
@@ -80,10 +81,9 @@ default void handleException(Exception ex) {}
      * @param name the name of the property
      * @param clazz the expected type of the property
      *
-     * @return the value of the property.
-     *         {@code null} if there's no such property or the expected type is not compatible.
+     * @return the value of the property
      */
-    default <T> T get(String name, Class<T> clazz) {
-        return null;
+    default <T> Optional<T> get(String name, Class<T> clazz) {
+        return Optional.empty();
     }
 }
diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java
@@ -18,6 +18,7 @@
 
 import java.net.InetSocketAddress;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Tracer wrapped {@link HttpChannel}
@@ -94,7 +95,7 @@ public InetSocketAddress getRemoteAddress() {
     }
 
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         return delegate.get(name, clazz);
     }
 }
diff --git a/...src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java b/...src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java
@@ -20,6 +20,7 @@
 import org.opensearch.transport.TransportChannel;
 
 import java.io.IOException;
+import java.util.Optional;
 
 /**
  * Tracer wrapped {@link TransportChannel}
@@ -111,7 +112,7 @@ public Version getVersion() {
     }
 
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         return delegate.get(name, clazz);
     }
 }
diff --git a/server/src/main/java/org/opensearch/transport/TaskTransportChannel.java b/server/src/main/java/org/opensearch/transport/TaskTransportChannel.java
@@ -37,6 +37,7 @@
 import org.opensearch.core.transport.TransportResponse;
 
 import java.io.IOException;
+import java.util.Optional;
 
 /**
  * Transport channel for tasks
@@ -91,7 +92,7 @@ public TransportChannel getChannel() {
     }
 
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         return getChannel().get(name, clazz);
     }
 }
diff --git a/server/src/main/java/org/opensearch/transport/TcpChannel.java b/server/src/main/java/org/opensearch/transport/TcpChannel.java
@@ -38,6 +38,7 @@
 import org.opensearch.core.common.bytes.BytesReference;
 
 import java.net.InetSocketAddress;
+import java.util.Optional;
 
 /**
  * This is a tcp channel representing a single channel connection to another node. It is the base channel
@@ -104,11 +105,10 @@ public interface TcpChannel extends CloseableChannel {
      * @param name the name of the property
      * @param clazz the expected type of the property
      *
-     * @return the value of the property.
-     *         {@code null} if there's no such property or the expected type is not compatible.
+     * @return the value of the property
      */
-    default <T> T get(String name, Class<T> clazz) {
-        return null;
+    default <T> Optional<T> get(String name, Class<T> clazz) {
+        return Optional.empty();
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/transport/TcpTransportChannel.java b/server/src/main/java/org/opensearch/transport/TcpTransportChannel.java
@@ -38,6 +38,7 @@
 import org.opensearch.search.query.QuerySearchResult;
 
 import java.io.IOException;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -131,7 +132,7 @@ public Version getVersion() {
     }
 
     @Override
-    public <T> T get(String name, Class<T> clazz) {
+    public <T> Optional<T> get(String name, Class<T> clazz) {
         return getChannel().get(name, clazz);
     }
 }
diff --git a/server/src/main/java/org/opensearch/transport/TransportChannel.java b/server/src/main/java/org/opensearch/transport/TransportChannel.java
@@ -39,6 +39,7 @@
 import org.opensearch.core.transport.TransportResponse;
 
 import java.io.IOException;
+import java.util.Optional;
 
 /**
  * A transport channel allows to send a response to a request on the channel.
@@ -88,9 +89,8 @@ static void sendErrorResponse(TransportChannel channel, String actionName, Trans
      * @param clazz the expected type of the property
      *
      * @return the value of the property.
-     *         {@code null} if there's no such property or the expected type is not compatible.
      */
-    default <T> T get(String name, Class<T> clazz) {
-        return null;
+    default <T> Optional<T> get(String name, Class<T> clazz) {
+        return Optional.empty();
     }
 }