VPP-348 Return empty DumpReply instead of null

Change-Id: If44f8d37649e5a9d5033ec2c0ab7452397e22691
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

5 files changed, 98 insertions, 44 deletions

diff --git a/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/FutureApiTest.java b/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/FutureApiTest.java
index 7a671731..dc172471 100644
--- a/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/FutureApiTest.java
+++ b/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/FutureApiTest.java
@@ -17,6 +17,7 @@
 package org.openvpp.jvpp.core.test;
 
 import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -24,6 +25,8 @@ import org.openvpp.jvpp.JVpp;
 import org.openvpp.jvpp.JVppRegistry;
 import org.openvpp.jvpp.JVppRegistryImpl;
 import org.openvpp.jvpp.core.JVppCoreImpl;
+import org.openvpp.jvpp.core.dto.BridgeDomainDetailsReplyDump;
+import org.openvpp.jvpp.core.dto.BridgeDomainDump;
 import org.openvpp.jvpp.core.dto.GetNodeIndex;
 import org.openvpp.jvpp.core.dto.GetNodeIndexReply;
 import org.openvpp.jvpp.core.dto.ShowVersion;
@@ -42,10 +45,29 @@ public class FutureApiTest {
         final Future<ShowVersionReply> replyFuture = jvpp.showVersion(new ShowVersion()).toCompletableFuture();
         final ShowVersionReply reply = replyFuture.get();
         LOG.info(
-            String.format(
-                "Received ShowVersionReply: context=%d, program=%s, version=%s, buildDate=%s, buildDirectory=%s\n",
-                reply.context, new String(reply.program), new String(reply.version), new String(reply.buildDate),
-                new String(reply.buildDirectory)));
+                String.format(
+                        "Received ShowVersionReply: context=%d, program=%s, version=%s, buildDate=%s, buildDirectory=%s\n",
+                        reply.context, new String(reply.program), new String(reply.version), new String(reply.buildDate),
+                        new String(reply.buildDirectory)));
+    }
+
+    private static void testEmptyBridgeDomainDump(final FutureJVppCoreFacade jvpp) throws Exception {
+        LOG.info("Sending ShowVersion request...");
+        final BridgeDomainDump request = new BridgeDomainDump();
+        request.bdId = -1; // dump call
+
+        final CompletableFuture<BridgeDomainDetailsReplyDump>
+                replyFuture = jvpp.bridgeDomainDump(request).toCompletableFuture();
+        final BridgeDomainDetailsReplyDump reply = replyFuture.get();
+
+        if (reply == null || reply.bridgeDomainDetails == null) {
+            LOG.severe("Received null response for empty dump: " + reply);
+        } else {
+            LOG.info(
+                    String.format(
+                            "Received empty bridge-domain dump reply with list of bridge-domains: %s, %s",
+                            reply.bridgeDomainDetails, reply.bridgeDomainSwIfDetails));
+        }
     }
 
     private static void testGetNodeIndex(final FutureJVppCoreFacade jvpp) {
@@ -56,8 +78,8 @@ public class FutureApiTest {
         try {
             final GetNodeIndexReply reply = replyFuture.get();
             LOG.info(
-                String.format(
-                    "Received GetNodeIndexReply: context=%d, nodeIndex=%d\n", reply.context, reply.nodeIndex));
+                    String.format(
+                            "Received GetNodeIndexReply: context=%d, nodeIndex=%d\n", reply.context, reply.nodeIndex));
         } catch (Exception e) {
             LOG.log(Level.SEVERE, "GetNodeIndex request failed", e);
         }
@@ -74,10 +96,10 @@ public class FutureApiTest {
         for (SwInterfaceDetails details : reply.swInterfaceDetails) {
             Objects.requireNonNull(details, "reply.swInterfaceDetails contains null element!");
             LOG.info(
-                String.format("Received SwInterfaceDetails: interfaceName=%s, l2AddressLength=%d, adminUpDown=%d, "
-                        + "linkUpDown=%d, linkSpeed=%d, linkMtu=%d\n",
-                    new String(details.interfaceName), details.l2AddressLength, details.adminUpDown,
-                    details.linkUpDown, details.linkSpeed, (int) details.linkMtu));
+                    String.format("Received SwInterfaceDetails: interfaceName=%s, l2AddressLength=%d, adminUpDown=%d, "
+                                    + "linkUpDown=%d, linkSpeed=%d, linkMtu=%d\n",
+                            new String(details.interfaceName), details.l2AddressLength, details.adminUpDown,
+                            details.linkUpDown, details.linkSpeed, (int) details.linkMtu));
         }
     }
 
@@ -89,6 +111,7 @@ public class FutureApiTest {
         final FutureJVppCoreFacade jvppFacade = new FutureJVppCoreFacade(registry, jvpp);
         LOG.info("Successfully connected to VPP");
 
+        testEmptyBridgeDomainDump(jvppFacade);
         testShowVersion(jvppFacade);
         testGetNodeIndex(jvppFacade);
         testSwInterfaceDump(jvppFacade);
diff --git a/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/Readme.txt b/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/Readme.txt
index 016bde1c..c24af24f 100644
--- a/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/Readme.txt
+++ b/vpp-api/java/jvpp-core/org/openvpp/jvpp/core/test/Readme.txt
@@ -2,7 +2,7 @@ This package contains basic tests for jvpp. To run the tests:
 
 - Make sure VPP is running
 - From VPP's build-root/ folder execute:
-  - sudo java-cp build-vpp_debug-native/vpp-api/java/jvpp-registry-16.09.jar:build-vpp_debug-native/vpp-api/java/jvpp-core-16.09.jar org.openvpp.jvpp.core.test.[test name]
+  - sudo java -cp build-vpp_debug-native/vpp-api/java/jvpp-registry-16.12.jar:build-vpp_debug-native/vpp-api/java/jvpp-core-16.12.jar org.openvpp.jvpp.core.test.[test name]
 
 Available tests:
 CallbackApiTest - Similar to ControlPingTest, invokes more complex calls (e.g. interface dump) using low level JVpp APIs
diff --git a/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/AbstractFutureJVppInvoker.java b/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/AbstractFutureJVppInvoker.java
index d8b105f3..53a445e6 100644
--- a/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/AbstractFutureJVppInvoker.java
+++ b/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/AbstractFutureJVppInvoker.java
@@ -30,8 +30,8 @@ import org.openvpp.jvpp.dto.JVppReplyDump;
 import org.openvpp.jvpp.dto.JVppRequest;
 
 /**
-* Future facade on top of JVpp
-*/
+ * Future facade on top of JVpp
+ */
 public abstract class AbstractFutureJVppInvoker implements FutureJVppInvoker {
 
     private final JVpp jvpp;
@@ -43,7 +43,7 @@ public abstract class AbstractFutureJVppInvoker implements FutureJVppInvoker {
     private final Map<Integer, CompletableFuture<? extends JVppReply<?>>> requests;
 
     protected AbstractFutureJVppInvoker(final JVpp jvpp, final JVppRegistry registry,
-                                     final Map<Integer, CompletableFuture<? extends JVppReply<?>>> requestMap) {
+                                        final Map<Integer, CompletableFuture<? extends JVppReply<?>>> requestMap) {
         this.jvpp =  Objects.requireNonNull(jvpp, "jvpp should not be null");
         this.registry =  Objects.requireNonNull(registry, "registry should not be null");
         // Request map represents the shared state between this facade and it's callback
@@ -66,15 +66,10 @@ public abstract class AbstractFutureJVppInvoker implements FutureJVppInvoker {
                 final int contextId = jvpp.send(req);
 
                 if(req instanceof JVppDump) {
-                    replyCompletableFuture = (CompletableFuture<REPLY>) new CompletableDumpFuture<>(contextId);
-                } else {
-                    replyCompletableFuture = new CompletableFuture<>();
+                    throw new IllegalArgumentException("Send with empty reply dump has to be used in case of dump calls");
                 }
-
+                replyCompletableFuture = new CompletableFuture<>();
                 requests.put(contextId, replyCompletableFuture);
-                if(req instanceof JVppDump) {
-                    requests.put(registry.controlPing(jvpp.getClass()), replyCompletableFuture);
-                }
 
                 // TODO in case of timeouts/missing replies, requests from the map are not removed
                 // consider adding cancel method, that would remove requests from the map and cancel
@@ -89,15 +84,43 @@ public abstract class AbstractFutureJVppInvoker implements FutureJVppInvoker {
         }
     }
 
+    @Override
+    @SuppressWarnings("unchecked")
+    public <REQ extends JVppRequest, REPLY extends JVppReply<REQ>, DUMP extends JVppReplyDump<REQ, REPLY>> CompletionStage<DUMP> send(
+            REQ req, DUMP emptyReplyDump) {
+        synchronized(requests) {
+            try {
+                final CompletableDumpFuture<DUMP> replyCompletableFuture;
+                final int contextId = jvpp.send(req);
+
+                if(!(req instanceof JVppDump)) {
+                    throw new IllegalArgumentException("Send without empty reply dump has to be used in case of regular calls");
+                }
+                replyCompletableFuture = new CompletableDumpFuture<>(contextId, emptyReplyDump);
+
+                requests.put(contextId, replyCompletableFuture);
+                requests.put(registry.controlPing(jvpp.getClass()), replyCompletableFuture);
+
+                // TODO in case of timeouts/missing replies, requests from the map are not removed
+                // consider adding cancel method, that would remove requests from the map and cancel
+                // associated replyCompletableFuture
+
+                return replyCompletableFuture;
+            } catch (VppInvocationException ex) {
+                final CompletableFuture<DUMP> replyCompletableFuture = new CompletableFuture<>();
+                replyCompletableFuture.completeExceptionally(ex);
+                return replyCompletableFuture;
+            }
+        }
+    }
+
     public static final class CompletableDumpFuture<T extends JVppReplyDump<?, ?>> extends CompletableFuture<T> {
-        // The reason why this is not final is the instantiation of ReplyDump DTOs
-        // Their instantiation must be generated, so currently the DTOs are created in callback and set when first dump reponses
-        // is handled in the callback.
-        private T replyDump;
+        private final T replyDump;
         private final long contextId;
 
-        public CompletableDumpFuture(final long contextId) {
+        public CompletableDumpFuture(final long contextId, final T emptyDump) {
             this.contextId = contextId;
+            this.replyDump = emptyDump;
         }
 
         public long getContextId() {
@@ -107,10 +130,6 @@ public abstract class AbstractFutureJVppInvoker implements FutureJVppInvoker {
         public T getReplyDump() {
             return replyDump;
         }
-
-        public void setReplyDump(final T replyDump) {
-            this.replyDump = replyDump;
-        }
     }
 
     @Override
diff --git a/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/FutureJVppInvoker.java b/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/FutureJVppInvoker.java
index 1683bd75..721f95c9 100644
--- a/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/FutureJVppInvoker.java
+++ b/vpp-api/java/jvpp-registry/org/openvpp/jvpp/future/FutureJVppInvoker.java
@@ -18,6 +18,7 @@ package org.openvpp.jvpp.future;
 
 
 import org.openvpp.jvpp.dto.JVppReply;
+import org.openvpp.jvpp.dto.JVppReplyDump;
 import org.openvpp.jvpp.dto.JVppRequest;
 
 import java.util.concurrent.CompletionStage;
@@ -36,4 +37,13 @@ public interface FutureJVppInvoker extends NotificationRegistryProvider, AutoClo
      */
     <REQ extends JVppRequest, REPLY extends JVppReply<REQ>> CompletionStage<REPLY> send(REQ req);
 
+
+    /**
+     * Invoke asynchronous dump operation on VPP
+     *
+     * @return CompletionStage with aggregated future result of an async VPP dump call
+     * @throws org.openvpp.jvpp.VppInvocationException when send request failed with details
+     */
+    <REQ extends JVppRequest, REPLY extends JVppReply<REQ>, DUMP extends JVppReplyDump<REQ, REPLY>> CompletionStage<DUMP> send(
+            REQ req, DUMP emptyReplyDump);
 }
diff --git a/vpp-api/java/jvpp/gen/jvppgen/jvpp_future_facade_gen.py b/vpp-api/java/jvpp/gen/jvppgen/jvpp_future_facade_gen.py
index 06c1073b..ebb840f7 100644
--- a/vpp-api/java/jvpp/gen/jvppgen/jvpp_future_facade_gen.py
+++ b/vpp-api/java/jvpp/gen/jvppgen/jvpp_future_facade_gen.py
@@ -126,13 +126,7 @@ jvpp_facade_details_callback_method_template = Template("""
         }
 
         if(completableFuture != null) {
-            $plugin_package.$dto_package.$callback_dto_reply_dump replyDump = completableFuture.getReplyDump();
-            if(replyDump == null) {
-                replyDump = new $plugin_package.$dto_package.$callback_dto_reply_dump();
-                completableFuture.setReplyDump(replyDump);
-            }
-
-            replyDump.$callback_dto_field.add(reply);
+            completableFuture.getReplyDump().$callback_dto_field.add(reply);
         }
     }
 """)
@@ -179,13 +173,13 @@ def generate_jvpp(func_list, base_package, plugin_package, plugin_name, dto_pack
                                                                       reply_name=camel_case_reply_name + dto_gen.dump_dto_suffix,
                                                                       request_name=util.remove_reply_suffix(camel_case_reply_name) +
                                                                                    util.underscore_to_camelcase_upper(util.dump_suffix)))
-                methods_impl.append(future_jvpp_method_impl_template.substitute(plugin_package=plugin_package,
-                                                                                dto_package=dto_package,
-                                                                                method_name=camel_case_request_method_name +
-                                                                                            util.underscore_to_camelcase_upper(util.dump_suffix),
-                                                                                reply_name=camel_case_reply_name + dto_gen.dump_dto_suffix,
-                                                                                request_name=util.remove_reply_suffix(camel_case_reply_name) +
-                                                                                             util.underscore_to_camelcase_upper(util.dump_suffix)))
+                methods_impl.append(future_jvpp_dump_method_impl_template.substitute(plugin_package=plugin_package,
+                                                                                     dto_package=dto_package,
+                                                                                     method_name=camel_case_request_method_name +
+                                                                                                 util.underscore_to_camelcase_upper(util.dump_suffix),
+                                                                                     reply_name=camel_case_reply_name + dto_gen.dump_dto_suffix,
+                                                                                     request_name=util.remove_reply_suffix(camel_case_reply_name) +
+                                                                                                  util.underscore_to_camelcase_upper(util.dump_suffix)))
             else:
                 request_name = util.underscore_to_camelcase_upper(util.unconventional_naming_rep_req[func['name']]) \
                     if func['name'] in util.unconventional_naming_rep_req else util.remove_reply_suffix(camel_case_name_with_suffix)
@@ -313,6 +307,14 @@ future_jvpp_method_impl_template = Template('''
     }
 ''')
 
+future_jvpp_dump_method_impl_template = Template('''
+    @Override
+    public java.util.concurrent.CompletionStage<$plugin_package.$dto_package.$reply_name> $method_name($plugin_package.$dto_package.$request_name request) {
+        return send(request, new $plugin_package.$dto_package.$reply_name());
+    }
+''')
+
+
 # Returns request name or special one from unconventional_naming_rep_req map
 def get_standard_dump_reply_name(camel_case_dto_name, func_name):
     # FIXME this is a hotfix for sub-details callbacks