recorded(match): evaluate on the engine (verify includeRequests) so repeated-key and JSON-body array rules (rift#1553/#1579) match

docs/design/sdk-api.md

@@ -67,7 +67,7 @@ io.github.achirdlabs.rift            Rift, RiftAsync, Imposter, Space, FlowState
                                         InterceptRule, InterceptRuleBuilder, TruststoreFormat
 io.github.achirdlabs.rift.error      RiftException (sealed) + 5 leaves
 io.github.achirdlabs.rift.verify     VerificationException, VerificationTimes, RequestMatch,
-                                        PredicateEvaluator
+                                        PredicateEvaluator (deprecated, #337)
 io.github.achirdlabs.rift.model      wire records (ImposterDefinition, Stub, Predicate, …),
                                         WireFormatException
 io.github.achirdlabs.rift.json       JsonValue + codec (unchanged)
@@ -372,7 +372,7 @@ public interface Imposter {
 
   // -- recorded requests + verification (§8) --
   List<RecordedRequest> recorded();
-  List<RecordedRequest> recorded(RequestMatch match);      // client-side filtered
+  List<RecordedRequest> recorded(RequestMatch match);      // engine-evaluated (verify + includeRequests, #337)
   RecordedPage recordedPage();                             // baseline: retained + cursor (§8)
   RecordedPage recordedPage(MatchClause... filters);       // server-side filtered (§8.2)
   RecordedPage recordedSince(long cursor);                 // tail poll: strictly newer than cursor
@@ -728,9 +728,9 @@ public interface RequestMatch {
 engine's evaluator — the same one the request hot path uses — decides; this SDK never re-implements
 matching, so a verdict can never drift from a stub's. Consequences: `inject`/`xpath` predicates are
 honoured in `verify` (not rejected client-side); a custom `RiftTransport` must implement `verify`;
-remote requires an engine with `POST /imposters/{port}/verify` (rift#494). `recorded(match)` stays a
-client-side convenience filter, and `verifyNoInteractions` a client-side emptiness check — neither
-is a predicate verdict.
+remote requires an engine with `POST /imposters/{port}/verify` (rift#494). `recorded(match)` is the same
+engine call with `includeRequests` (#337), returning the matching requests rather than a count;
+`verifyNoInteractions` is a client-side emptiness check, not a predicate verdict.
 
 `verifyResult` is the same verification as a value — the typed seam a bridge renders its own
 assertion failures from (no diff-string scraping):
@@ -757,10 +757,10 @@ public final class VerificationTimes {                    // factories re-export
 }
 ```
 
-`verify` is evaluated by the engine (above). Core's `PredicateEvaluator` implements Mountebank
-predicate semantics against `RecordedRequest` (all field ops + `and/or/not` +
-`caseSensitive/keyCaseSensitive/except` + `jsonpath` + `xpath` via the JDK — zero external deps),
-but it backs only the client-side `recorded(match)` filter.
+`verify` and `recorded(match)` are evaluated by the engine (above). Core's `PredicateEvaluator`, a
+JVM copy of Mountebank's predicate semantics, is deprecated for removal (#337) and no longer called:
+it had drifted from the engine three times (#330, #335, and rift 0.24.0's array rules, rift#1553 /
+rift#1579), which is the failure mode an engine-evaluated design exists to rule out.
 
 On failure, `VerificationException extends AssertionError` renders the engine's verdict, with its
 closest non-matching request when it reports one:
@@ -874,7 +874,7 @@ still percent-encoded, never carrying the query string, which the engine keeps a
 | What | Mountebank predicate set | journal/stream filter clause |
 | Evaluated by | the engine's predicate engine, via `POST /verify` | the engine's journal filter, via `match=` |
 | Grammar | full — `equals`/`contains`/`matches`/`jsonPath`/`xpath`/`and`/`or`/`not` | closed — exact equality on header, flow id, method, path |
-| Used for | `verify(match, times)`, `recorded(match)` (client-side filter) | `recordedPage`/`recordedSince`/`clearRecorded` filters, `Space.recordedPage`/`recordedSince` (auto-injected `flow_id`, #149), `/events?match=` (#131) |
+| Used for | `verify(match, times)`, `recorded(match)` | `recordedPage`/`recordedSince`/`clearRecorded` filters, `Space.recordedPage`/`recordedSince` (auto-injected `flow_id`, #149), `/events?match=` (#131) |
 
 The grammar is closed because the engine's is: it rejects a clause it cannot parse with a 400 rather
 than serving everything, since a filter that silently widened would cross-contaminate correlated

rift-java-conformance/src/test/java/io/github/achirdlabs/rift/conformance/RecordedMatchArraysIT.java

@@ -0,0 +1,75 @@
+package io.github.achirdlabs.rift.conformance;
+
+import io.github.achirdlabs.rift.Imposter;
+import io.github.achirdlabs.rift.Rift;
+import io.github.achirdlabs.rift.verify.RequestMatch;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.TestFactory;
+
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.time.Duration;
+import java.util.stream.Stream;
+
+import static io.github.achirdlabs.rift.conformance.LiveEngine.engine;
+import static io.github.achirdlabs.rift.conformance.LiveEngine.gated;
+import static io.github.achirdlabs.rift.dsl.RiftDsl.imposter;
+import static io.github.achirdlabs.rift.dsl.RiftDsl.ok;
+import static io.github.achirdlabs.rift.dsl.RiftDsl.onRequest;
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * {@code recorded(match)} answers with the engine's predicate rules (#337): from rift 0.24.0 a repeated
+ * query key or header matches by Mountebank's array rules (rift#1553) and a JSON body by its subset,
+ * number and missing-field rules (rift#1579). Needs {@code RIFT_IT=1}.
+ */
+class RecordedMatchArraysIT {
+
+    private static final HttpClient CLIENT = HttpClient.newBuilder()
+            .version(HttpClient.Version.HTTP_1_1).connectTimeout(Duration.ofSeconds(5)).build();
+
+    private static void send(HttpRequest.Builder request) throws Exception {
+        CLIENT.send(request.timeout(Duration.ofSeconds(5)).build(), HttpResponse.BodyHandlers.discarding());
+    }
+
+    private static int count(Imposter imposter, String predicateArray) {
+        return imposter.recorded(RequestMatch.ofJson(predicateArray)).size();
+    }
+
+    @TestFactory
+    Stream<DynamicTest> arraysMatchAsTheEngineMatchesThem() {
+        return gated("repeated keys and JSON-body arrays match by the engine's rules", () -> {
+            try (Rift rift = engine()) {
+                Imposter imposter = rift.create(imposter("arrays").record().stub(onRequest().willReturn(ok())));
+                String base = imposter.uri().toString();
+                send(HttpRequest.newBuilder(URI.create(base + "/q?k=a&k=b")));
+                send(HttpRequest.newBuilder(URI.create(base + "/q?k=b")));
+                send(HttpRequest.newBuilder(URI.create(base + "/j")).header("Content-Type", "application/json")
+                        .POST(HttpRequest.BodyPublishers.ofString("{\"items\":[{\"id\":1},{\"id\":2}],\"n\":1.0}")));
+                send(HttpRequest.newBuilder(URI.create(base + "/j")).header("Content-Type", "application/json")
+                        .POST(HttpRequest.BodyPublishers.ofString("{\"items\":[]}")));
+                send(HttpRequest.newBuilder(URI.create(base + "/h")).header("X-Tag", "x").header("X-Tag", "y"));
+
+                assertAll(
+                        () -> assertEquals(1, count(imposter, "[{\"equals\":{\"query\":{\"k\":[\"b\",\"a\"]}}}]"),
+                                "an expected array is a subset of the values, in any order"),
+                        () -> assertEquals(1, count(imposter, "[{\"deepEquals\":{\"query\":{\"k\":[\"b\",\"a\"]}}}]"),
+                                "deepEquals compares the values as a multiset"),
+                        () -> assertEquals(2, count(imposter, "[{\"equals\":{\"query\":{\"k\":\"b\"}}}]"),
+                                "a scalar matches any one of a repeated key's values"),
+                        () -> assertEquals(1, count(imposter,
+                                "[{\"equals\":{\"path\":\"/j\",\"body\":{\"items\":[{\"id\":2}]}}}]"),
+                                "an expected body array is a subset of the actual one"),
+                        () -> assertEquals(1, count(imposter, "[{\"equals\":{\"path\":\"/j\",\"body\":{\"n\":1}}}]"),
+                                "numbers compare by their JavaScript spelling (1.0 is 1)"),
+                        () -> assertEquals(1, count(imposter, "[{\"equals\":{\"path\":\"/j\",\"body\":{\"n\":\"\"}}}]"),
+                                "a missing body field reads as \"\""),
+                        () -> assertEquals(1, count(imposter, "[{\"equals\":{\"headers\":{\"X-Tag\":[\"y\",\"x\"]}}}]"),
+                                "an expected array over a repeated header"));
+            }
+        });
+    }
+}

rift-java-core/src/main/java/io/github/achirdlabs/rift/Imposter.java

@@ -86,7 +86,14 @@ public interface Imposter {
 
     List<RecordedRequest> recorded();
 
-    /** Recorded requests filtered client-side against {@code match}'s predicates (equals on method/path only; see {@code ImposterImpl}). */
+    /**
+     * The recorded requests that satisfy every predicate in {@code match}, in journal order. The
+     * engine evaluates them ({@code POST /imposters/{port}/verify} with {@code includeRequests}, one
+     * call), so the result follows the running engine's rules exactly: from rift 0.24.0 Mountebank's
+     * array rules for a repeated query key or header and for JSON bodies, and {@code inject}
+     * predicates too. A malformed predicate fails with the engine's error, as {@code verify} does.
+     * An imposter that does not record answers an empty list.
+     */
     List<RecordedRequest> recorded(RequestMatch match);
 
     /**

rift-java-core/src/main/java/io/github/achirdlabs/rift/ImposterImpl.java

@@ -15,7 +15,6 @@
 import io.github.achirdlabs.rift.transport.HostAuthority;
 import io.github.achirdlabs.rift.transport.RiftTransport;
 import io.github.achirdlabs.rift.transport.StubAddress;
-import io.github.achirdlabs.rift.verify.PredicateEvaluator;
 import io.github.achirdlabs.rift.verify.RequestMatch;
 import io.github.achirdlabs.rift.verify.VerificationException;
 import io.github.achirdlabs.rift.verify.VerificationResult;
@@ -250,7 +249,10 @@ private static RecordedPage page(RiftTransport.RecordedSlice slice) {
 
     @Override
     public List<RecordedRequest> recorded(RequestMatch match) {
-        return recorded().stream().filter(r -> PredicateEvaluator.matches(r, match.predicates())).toList();
+        // The engine's evaluator, not a JVM copy of Mountebank's rules (#337). No requireRecording():
+        // a non-recording imposter answers an empty list here, as recorded() does.
+        return RecordedRequests.readMatched(
+                transport.verify(port, VerifyBody.build(match, Optional.empty(), VerifyDetail.REQUESTS)));
     }
 
     @Override

rift-java-core/src/main/java/io/github/achirdlabs/rift/RecordedRequests.java

@@ -41,6 +41,19 @@ static List<RecordedRequest> readAll(JsonValue body, String context) {
                 + " body: expected a JSON array of recorded requests, or a {\"requests\":[...]} envelope");
     }
 
+    /**
+     * The matching requests out of a {@code verify} reply asked for with {@code includeRequests}. A
+     * reply without them is a {@link CommunicationError}: read as empty, it would say "nothing matched"
+     * beside the engine's own non-zero {@code matched}.
+     */
+    static List<RecordedRequest> readMatched(JsonValue verifyReply) {
+        if (verifyReply instanceof JsonObject obj && obj.get("requests") instanceof JsonArray arr) {
+            return read(arr);
+        }
+        throw new CommunicationError("rift admin API returned a verify body without the requested"
+                + " \"requests\" array: " + verifyReply.toJson());
+    }
+
     private static List<RecordedRequest> read(JsonArray arr) {
         List<RecordedRequest> out = new ArrayList<>(arr.items().size());
         for (JsonValue v : arr.items()) {

rift-java-core/src/main/java/io/github/achirdlabs/rift/Space.java

@@ -28,7 +28,14 @@ public interface Space {
 
     List<RecordedRequest> recorded();
 
-    /** Recorded requests within this space, filtered against {@code match}'s predicates. */
+    /**
+     * The recorded requests that satisfy every predicate in {@code match}, in journal order, within this space (its {@code flowId}). The
+     * engine evaluates them ({@code POST /imposters/{port}/verify} with {@code includeRequests}, one
+     * call), so the result follows the running engine's rules exactly: from rift 0.24.0 Mountebank's
+     * array rules for a repeated query key or header and for JSON bodies, and {@code inject}
+     * predicates too. A malformed predicate fails with the engine's error, as {@code verify} does.
+     * An owning imposter that does not record answers an empty list.
+     */
     List<RecordedRequest> recorded(RequestMatch match);
 
     /**

rift-java-core/src/main/java/io/github/achirdlabs/rift/SpaceImpl.java

@@ -7,7 +7,6 @@
 import io.github.achirdlabs.rift.model.Stub;
 import io.github.achirdlabs.rift.transport.RiftTransport;
 import io.github.achirdlabs.rift.transport.StubAddress;
-import io.github.achirdlabs.rift.verify.PredicateEvaluator;
 import io.github.achirdlabs.rift.verify.RequestMatch;
 import io.github.achirdlabs.rift.verify.VerificationException;
 import io.github.achirdlabs.rift.verify.VerificationResult;
@@ -63,7 +62,8 @@ public List<RecordedRequest> recorded() {
 
     @Override
     public List<RecordedRequest> recorded(RequestMatch match) {
-        return recorded().stream().filter(r -> PredicateEvaluator.matches(r, match.predicates())).toList();
+        return RecordedRequests.readMatched(
+                transport.verify(port, VerifyBody.build(match, Optional.of(flowId), VerifyDetail.REQUESTS)));
     }
 
     @Override

rift-java-core/src/main/java/io/github/achirdlabs/rift/verify/PredicateEvaluator.java

@@ -34,14 +34,19 @@
 import org.xml.sax.InputSource;
 
 /**
- * Client-side evaluation of Mountebank predicate semantics against a recorded request. Verification
- * itself is engine-authoritative (#127); what remains here backs {@code recorded(RequestMatch)}, the
- * client-side convenience filter over an imposter's journal.
+ * Client-side evaluation of Mountebank predicate semantics against a recorded request.
+ *
+ * @deprecated The SDK no longer calls this: {@code verify} (#127) and {@code recorded(RequestMatch)}
+ *     (#337) are evaluated by the engine, whose rules this copy cannot keep up with (from rift
+ *     0.24.0 it gets repeated query keys, repeated headers and JSON-body arrays wrong). Use
+ *     {@link io.github.achirdlabs.rift.Imposter#recorded(RequestMatch)} or
+ *     {@code verifyResult(match, VerifyDetail.REQUESTS)}. To be removed in the next minor release.
  *
  * <p>Uses {@code instanceof} chains rather than pattern-matching {@code switch} throughout: the
  * latter is still a preview feature at the Java 17 release level this module compiles against
  * (finalized only in Java 21).
  */
+@Deprecated(since = "0.3.8", forRemoval = true)
 public final class PredicateEvaluator {
 
     private PredicateEvaluator() {}

rift-java-core/src/test/java/io/github/achirdlabs/rift/transport/RecordedMatchOnEngineTest.java

@@ -0,0 +1,109 @@
+package io.github.achirdlabs.rift.transport;
+
+import io.github.achirdlabs.rift.ConnectOptions;
+import io.github.achirdlabs.rift.Imposter;
+import io.github.achirdlabs.rift.RecordedRequest;
+import io.github.achirdlabs.rift.Rift;
+import io.github.achirdlabs.rift.VersionCheck;
+import io.github.achirdlabs.rift.error.CommunicationError;
+import io.github.achirdlabs.rift.json.JsonValue;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static io.github.achirdlabs.rift.dsl.RiftDsl.imposter;
+import static io.github.achirdlabs.rift.dsl.RiftDsl.onGet;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * {@code recorded(match)} asks the engine which recorded requests match (#337), the way {@code verify}
+ * does (#127), instead of re-evaluating Mountebank's predicate rules in the JVM: one {@code verify}
+ * call with {@code includeRequests}, and no journal fetch.
+ */
+class RecordedMatchOnEngineTest {
+
+    private static final String IMP = "{\"port\":4545,\"protocol\":\"http\",\"recordRequests\":true,\"stubs\":[]}";
+    private static final String REPLY = "{\"matched\":2,\"total\":3,\"requests\":["
+            + "{\"method\":\"GET\",\"path\":\"/b\"},{\"method\":\"GET\",\"path\":\"/a\"}]}";
+
+    private static Rift connect(FakeAdminServer s) {
+        return Rift.connect(ConnectOptions.builder(s.baseUri()).versionCheck(VersionCheck.OFF).build());
+    }
+
+    private static Imposter created(FakeAdminServer s, Rift rift) {
+        s.respond("POST /imposters", 201, IMP);
+        return rift.create(imposter("x").port(4545).record());
+    }
+
+    private static List<FakeAdminServer.Received> verifyCalls(FakeAdminServer s) {
+        return s.received().stream()
+                .filter(r -> r.method().equals("POST") && r.path().equals("/imposters/4545/verify"))
+                .toList();
+    }
+
+    private static boolean fetchedJournal(FakeAdminServer s) {
+        return s.received().stream().anyMatch(r -> r.method().equals("GET") && r.path().contains("savedRequests"));
+    }
+
+    @Test
+    void anImposterAsksTheEngineForTheMatchingRequests() {
+        try (FakeAdminServer s = new FakeAdminServer()) {
+            s.respond("POST /imposters/4545/verify", 200, REPLY);
+            try (Rift rift = connect(s)) {
+                List<RecordedRequest> hits = created(s, rift).recorded(onGet("/a"));
+
+                assertEquals(List.of("/b", "/a"), hits.stream().map(RecordedRequest::path).toList(),
+                        "the engine's matches, in the engine's (journal) order");
+                assertEquals(1, verifyCalls(s).size());
+                assertEquals(
+                        JsonValue.parse("{\"predicates\":[{\"equals\":{\"method\":\"GET\",\"path\":\"/a\"}}],"
+                                + "\"includeRequests\":true}"),
+                        JsonValue.parse(verifyCalls(s).get(0).body()));
+                assertTrue(!fetchedJournal(s), "no client-side filtering over the whole journal: " + s.received());
+            }
+        }
+    }
+
+    @Test
+    void aSpaceScopesTheEngineQueryToItsFlowId() {
+        try (FakeAdminServer s = new FakeAdminServer()) {
+            s.respond("POST /imposters/4545/verify", 200, REPLY);
+            try (Rift rift = connect(s)) {
+                List<RecordedRequest> hits = created(s, rift).space("alice").recorded(onGet("/a"));
+
+                assertEquals(List.of("/b", "/a"), hits.stream().map(RecordedRequest::path).toList());
+                assertEquals(
+                        JsonValue.parse("{\"predicates\":[{\"equals\":{\"method\":\"GET\",\"path\":\"/a\"}}],"
+                                + "\"flowId\":\"alice\",\"includeRequests\":true}"),
+                        JsonValue.parse(verifyCalls(s).get(0).body()));
+                assertTrue(!fetchedJournal(s), "no client-side filtering over the journal: " + s.received());
+            }
+        }
+    }
+
+    @Test
+    void noMatchesIsAnEmptyList() {
+        try (FakeAdminServer s = new FakeAdminServer()) {
+            s.respond("POST /imposters/4545/verify", 200, "{\"matched\":0,\"total\":3,\"requests\":[]}");
+            try (Rift rift = connect(s)) {
+                assertEquals(List.of(), created(s, rift).recorded(onGet("/zzz")));
+            }
+        }
+    }
+
+    @Test
+    void aReplyWithoutTheRequestedRequestsIsAnErrorNotAnEmptyResult() {
+        // An engine that ignored includeRequests would otherwise read as "nothing matched" while
+        // its own count says two did.
+        try (FakeAdminServer s = new FakeAdminServer()) {
+            s.respond("POST /imposters/4545/verify", 200, "{\"matched\":2,\"total\":3}");
+            try (Rift rift = connect(s)) {
+                Imposter imp = created(s, rift);
+                CommunicationError e = assertThrows(CommunicationError.class, () -> imp.recorded(onGet("/a")));
+                assertTrue(e.getMessage().contains("requests"), e.getMessage());
+            }
+        }
+    }
+}