From 560e809b4459f508b756a19493de746e0892389e Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Mon, 19 Sep 2016 15:35:41 +0200 Subject: [PATCH] VPP-348 Return empty DumpReply instead of null Change-Id: If44f8d37649e5a9d5033ec2c0ab7452397e22691 Signed-off-by: Maros Marsalek --- .../org/openvpp/jvpp/core/test/FutureApiTest.java | 43 ++++++++++++---- .../org/openvpp/jvpp/core/test/Readme.txt | 2 +- .../jvpp/future/AbstractFutureJVppInvoker.java | 57 ++++++++++++++-------- .../org/openvpp/jvpp/future/FutureJVppInvoker.java | 10 ++++ .../jvpp/gen/jvppgen/jvpp_future_facade_gen.py | 30 ++++++------ 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 7a671731fdd..dc172471163 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 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 + 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 016bde1cd92..c24af24fb49 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 d8b105f359e..53a445e6977 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>> requests; protected AbstractFutureJVppInvoker(final JVpp jvpp, final JVppRegistry registry, - final Map>> requestMap) { + final Map>> 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) 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 , DUMP extends JVppReplyDump> CompletionStage send( + REQ req, DUMP emptyReplyDump) { + synchronized(requests) { + try { + final CompletableDumpFuture 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 replyCompletableFuture = new CompletableFuture<>(); + replyCompletableFuture.completeExceptionally(ex); + return replyCompletableFuture; + } + } + } + public static final class CompletableDumpFuture> extends CompletableFuture { - // 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 1683bd75139..721f95c9f4d 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 */ > CompletionStage 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 + */ + , DUMP extends JVppReplyDump> CompletionStage 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 06c1073b756..ebb840f795f 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 -- 2.16.6