Improve testing of JRT access filtering

Test that method is not invoked when request is rejected by filter.
Move testing of filter accepting request to async- and sync-specific test classes.

4 files changed, 71 insertions, 33 deletions

diff --git a/jrt/tests/com/yahoo/jrt/InvokeAsyncTest.java b/jrt/tests/com/yahoo/jrt/InvokeAsyncTest.java
index 5e9f426bb17..436b650198e 100644
--- a/jrt/tests/com/yahoo/jrt/InvokeAsyncTest.java
+++ b/jrt/tests/com/yahoo/jrt/InvokeAsyncTest.java
@@ -16,6 +16,7 @@ public class InvokeAsyncTest {
     Supervisor   client;
     Target       target;
     Test.Barrier barrier;
+    SimpleRequestAccessFilter filter;
 
     @Before
     public void setUp() throws ListenFailedException {
@@ -23,11 +24,13 @@ public class InvokeAsyncTest {
         client   = new Supervisor(new Transport());
         acceptor = server.listen(new Spec(0));
         target   = client.connect(new Spec("localhost", acceptor.port()));
+        filter = new SimpleRequestAccessFilter();
         server.addMethod(new Method("concat", "ss", "s", this::rpc_concat)
                          .methodDesc("Concatenate 2 strings")
                          .paramDesc(0, "str1", "a string")
                          .paramDesc(1, "str2", "another string")
-                         .returnDesc(0, "ret", "str1 followed by str2"));
+                         .returnDesc(0, "ret", "str1 followed by str2")
+                         .requestAccessFilter(filter));
         barrier = new Test.Barrier();
     }
 
@@ -65,4 +68,21 @@ public class InvokeAsyncTest {
         assertEquals("abcdef", req.returnValues().get(0).asString());
     }
 
+    @org.junit.Test
+    public void testFilterIsInvoked() {
+        Request req = new Request("concat");
+        req.parameters().add(new StringValue("abc"));
+        req.parameters().add(new StringValue("def"));
+        assertFalse(filter.invoked);
+        Test.Waiter w = new Test.Waiter();
+        target.invokeAsync(req, 10, w);
+        assertFalse(w.isDone());
+        barrier.breakIt();
+        w.waitDone();
+        assertTrue(w.isDone());
+        assertFalse(req.isError());
+        assertEquals("abcdef", req.returnValues().get(0).asString());
+        assertTrue(filter.invoked);
+    }
+
 }
diff --git a/jrt/tests/com/yahoo/jrt/InvokeErrorTest.java b/jrt/tests/com/yahoo/jrt/InvokeErrorTest.java
index 4fa06eec56f..3b58ba2f42e 100644
--- a/jrt/tests/com/yahoo/jrt/InvokeErrorTest.java
+++ b/jrt/tests/com/yahoo/jrt/InvokeErrorTest.java
@@ -18,6 +18,7 @@ public class InvokeErrorTest {
     Target       target;
     Test.Barrier barrier;
     SimpleRequestAccessFilter filter;
+    RpcTestMethod             testMethod;
 
     @Before
     public void setUp() throws ListenFailedException {
@@ -26,7 +27,8 @@ public class InvokeErrorTest {
         acceptor = server.listen(new Spec(0));
         target   = client.connect(new Spec("localhost", acceptor.port()));
         filter = new SimpleRequestAccessFilter();
-        server.addMethod(new Method("test", "iib", "i", this::rpc_test).requestAccessFilter(filter));
+        testMethod = new RpcTestMethod();
+        server.addMethod(new Method("test", "iib", "i", testMethod).requestAccessFilter(filter));
         server.addMethod(new Method("test_barrier", "iib", "i", this::rpc_test_barrier));
         barrier = new Test.Barrier();
     }
@@ -39,22 +41,8 @@ public class InvokeErrorTest {
         server.transport().shutdown().join();
     }
 
-    private void rpc_test(Request req) {
-        int value = req.parameters().get(0).asInt32();
-        int error = req.parameters().get(1).asInt32();
-        int extra = req.parameters().get(2).asInt8();
-
-        req.returnValues().add(new Int32Value(value));
-        if (extra != 0) {
-            req.returnValues().add(new Int32Value(value));
-        }
-        if (error != 0) {
-            req.setError(error, "Custom error");
-        }
-    }
-
     private void rpc_test_barrier(Request req) {
-        rpc_test(req);
+        testMethod.invoke(req);
         barrier.waitFor();
     }
 
@@ -161,18 +149,6 @@ public class InvokeErrorTest {
     }
 
     @org.junit.Test
-    public void testFilterIsInvoked() {
-        Request r = new Request("test");
-        r.parameters().add(new Int32Value(42));
-        r.parameters().add(new Int32Value(0));
-        r.parameters().add(new Int8Value((byte)0));
-        assertFalse(filter.invoked);
-        target.invokeSync(r, timeout);
-        assertFalse(r.isError());
-        assertTrue(filter.invoked);
-    }
-
-    @org.junit.Test
     public void testFilterFailsRequest() {
         Request r = new Request("test");
         r.parameters().add(new Int32Value(42));
@@ -183,13 +159,29 @@ public class InvokeErrorTest {
         target.invokeSync(r, timeout);
         assertTrue(r.isError());
         assertTrue(filter.invoked);
+        assertFalse(testMethod.invoked);
         assertEquals(ErrorCode.PERMISSION_DENIED, r.errorCode());
         assertEquals("Permission denied", r.errorMessage());
     }
 
-    private static class SimpleRequestAccessFilter implements RequestAccessFilter {
-        boolean invoked = false, allowed = true;
-        @Override public boolean allow(Request r) { invoked = true; return allowed; }
+    private static class RpcTestMethod implements MethodHandler {
+        boolean invoked = false;
+
+        @Override public void invoke(Request req) { invoked = true; rpc_test(req); }
+
+        void rpc_test(Request req) {
+            int value = req.parameters().get(0).asInt32();
+            int error = req.parameters().get(1).asInt32();
+            int extra = req.parameters().get(2).asInt8();
+
+            req.returnValues().add(new Int32Value(value));
+            if (extra != 0) {
+                req.returnValues().add(new Int32Value(value));
+            }
+            if (error != 0) {
+                req.setError(error, "Custom error");
+            }
+        }
     }
 
 }
diff --git a/jrt/tests/com/yahoo/jrt/InvokeSyncTest.java b/jrt/tests/com/yahoo/jrt/InvokeSyncTest.java
index ca7d0db129d..ec196bea47c 100644
--- a/jrt/tests/com/yahoo/jrt/InvokeSyncTest.java
+++ b/jrt/tests/com/yahoo/jrt/InvokeSyncTest.java
@@ -12,6 +12,7 @@ import java.io.IOException;
 import java.io.PrintStream;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 
@@ -21,6 +22,7 @@ public class InvokeSyncTest {
     Acceptor   acceptor;
     Supervisor client;
     Target     target;
+    SimpleRequestAccessFilter filter;
     
     @Before
     public void setUp() throws ListenFailedException {
@@ -28,11 +30,13 @@ public class InvokeSyncTest {
         client   = new Supervisor(new Transport());
         acceptor = server.listen(new Spec(0));
         target   = client.connect(new Spec("localhost", acceptor.port()));
+        filter = new SimpleRequestAccessFilter();
         server.addMethod(new Method("concat", "ss", "s", this::rpc_concat)
                          .methodDesc("Concatenate 2 strings")
                          .paramDesc(0, "str1", "a string")
                          .paramDesc(1, "str2", "another string")
-                         .returnDesc(0, "ret", "str1 followed by str2"));
+                         .returnDesc(0, "ret", "str1 followed by str2")
+                          .requestAccessFilter(filter));
         server.addMethod(new Method("alltypes", "bhilfds", "s", this::rpc_alltypes)
                           .methodDesc("Method taking all types of params"));
     }
@@ -84,4 +88,17 @@ public class InvokeSyncTest {
         assertEquals(baos.toString(), "This was alltypes. The string param was: baz\n");
     }
 
+    @org.junit.Test
+    public void testFilterIsInvoked() {
+        Request req = new Request("concat");
+        req.parameters().add(new StringValue("abc"));
+        req.parameters().add(new StringValue("def"));
+        assertFalse(filter.invoked);
+        target.invokeSync(req, 10);
+        assertFalse(req.isError());
+        assertEquals("abcdef", req.returnValues().get(0).asString());
+        assertTrue(filter.invoked);
+    }
+
+
 }
diff --git a/jrt/tests/com/yahoo/jrt/SimpleRequestAccessFilter.java b/jrt/tests/com/yahoo/jrt/SimpleRequestAccessFilter.java
new file mode 100644
index 00000000000..38d59720848
--- /dev/null
+++ b/jrt/tests/com/yahoo/jrt/SimpleRequestAccessFilter.java
@@ -0,0 +1,9 @@
+package com.yahoo.jrt;// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+/**
+ * @author bjorncs
+ */
+class SimpleRequestAccessFilter implements RequestAccessFilter {
+    volatile boolean invoked = false, allowed = true;
+    @Override public boolean allow(Request r) { invoked = true; return allowed; }
+}