Refactor transport-http response delivery

[netconf.git] / transport / transport-http / src / main / java / org / opendaylight / netconf / transport / http / ClientHttp2RequestDispatcher.java

diff --git a/transport/transport-http/src/main/java/org/opendaylight/netconf/transport/http/ClientHttp2RequestDispatcher.java b/transport/transport-http/src/main/java/org/opendaylight/netconf/transport/http/ClientHttp2RequestDispatcher.java
index 3d5089eb531729b559fab520f38cd07ef23e93f0..6089dbbe3620cd7806e8eb1399c7523f4258e643 100644 (file)
--- a/transport/transport-http/src/main/java/org/opendaylight/netconf/transport/http/ClientHttp2RequestDispatcher.java
+++ b/transport/transport-http/src/main/java/org/opendaylight/netconf/transport/http/ClientHttp2RequestDispatcher.java
@@ -10,11 +10,9 @@ package org.opendaylight.netconf.transport.http;
 import static io.netty.handler.codec.http2.HttpConversionUtil.ExtensionHeaderNames.SCHEME;
 import static io.netty.handler.codec.http2.HttpConversionUtil.ExtensionHeaderNames.STREAM_ID;
 
-import com.google.common.util.concurrent.ListenableFuture;
-import com.google.common.util.concurrent.SettableFuture;
+import com.google.common.util.concurrent.FutureCallback;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.channel.SimpleChannelInboundHandler;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.FullHttpResponse;
 import io.netty.handler.codec.http.HttpScheme;
@@ -32,50 +30,39 @@ import org.slf4j.LoggerFactory;
  * Serves as gateway to Netty {@link Channel}, performs sending requests to server, returns server responses associated.
  * Uses request to response mapping by stream identifier.
  */
-class ClientHttp2RequestDispatcher extends SimpleChannelInboundHandler<FullHttpResponse> implements RequestDispatcher {
+final class ClientHttp2RequestDispatcher extends ClientRequestDispatcher {
     private static final Logger LOG = LoggerFactory.getLogger(ClientHttp2RequestDispatcher.class);
 
-    private final Map<Integer, SettableFuture<FullHttpResponse>> map = new ConcurrentHashMap<>();
+    // TODO: we access the queue only from Netty callbacks: can we use a plain HashMap?
+    private final Map<Integer, FutureCallback<FullHttpResponse>> map = new ConcurrentHashMap<>();
+    // identifier for streams initiated from client require to be odd-numbered, 1 is reserved
+    // see https://datatracker.ietf.org/doc/html/rfc7540#section-5.1.1
     private final AtomicInteger streamIdCounter = new AtomicInteger(3);
 
-    private Channel channel = null;
     private boolean ssl = false;
 
-    ClientHttp2RequestDispatcher() {
-        super(true); // auto-release
-    }
-
-    private Integer nextStreamId() {
-        // identifier for streams initiated from client require to be odd-numbered, 1 is reserved
-        // see https://datatracker.ietf.org/doc/html/rfc7540#section-5.1.1
-        return streamIdCounter.getAndAdd(2);
-    }
-
     @Override
     public void handlerAdded(final ChannelHandlerContext ctx) throws Exception {
-        channel = ctx.channel();
         ssl = ctx.pipeline().get(SslHandler.class) != null;
         super.handlerAdded(ctx);
     }
 
     @Override
-    public ListenableFuture<FullHttpResponse> dispatch(final FullHttpRequest request) {
-        if (channel == null) {
-            throw new IllegalStateException("Connection is not established yet");
-        }
-        final var streamId = nextStreamId();
-        request.headers().setInt(STREAM_ID.text(), streamId);
-        request.headers().set(SCHEME.text(), ssl ? HttpScheme.HTTPS.name() : HttpScheme.HTTP.name());
+    public void dispatch(final Channel channel, final FullHttpRequest request,
+            final FutureCallback<FullHttpResponse> callback) {
+        final var streamId = streamIdCounter.getAndAdd(2);
+        request.headers()
+            .setInt(STREAM_ID.text(), streamId)
+            .set(SCHEME.text(), ssl ? HttpScheme.HTTPS.name() : HttpScheme.HTTP.name());
 
-        final var future = SettableFuture.<FullHttpResponse>create();
         channel.writeAndFlush(request).addListener(sent -> {
-            if (sent.cause() == null) {
-                map.put(streamId, future);
+            final var cause = sent.cause();
+            if (cause == null) {
+                map.put(streamId, callback);
             } else {
-                future.setException(sent.cause());
+                callback.onFailure(cause);
             }
         });
-        return future;
     }
 
     @Override
@@ -85,19 +72,12 @@ class ClientHttp2RequestDispatcher extends SimpleChannelInboundHandler<FullHttpR
             LOG.warn("Unexpected response with no stream ID -- Dropping response object {}", response);
             return;
         }
-        final var future = map.remove(streamId);
-        if (future == null) {
+        final var callback = map.remove(streamId);
+        if (callback != null) {
+            callback.onSuccess(response);
+        } else {
             LOG.warn("Unexpected response with unknown or expired stream ID {} -- Dropping response object {}",
                 streamId, response);
-            return;
-        }
-        if (!future.isDone()) {
-            // NB using response' copy to disconnect the content data from channel's buffer allocated.
-            // this prevents the content data became inaccessible once byte buffer of original message is released
-            // on exit of current method
-            future.set(response.copy());
-        } else {
-            LOG.warn("Future is already in Done state -- Dropping response object {}", response);
         }
     }
 }