adopt: drop the flow-state no-op-store warning once the engine gives every imposter a real store (rift#1504)

docs/design/sdk-api.md

@@ -452,7 +452,9 @@ caching, every call hits the engine (test code wants truth, not staleness).
 and per-space APIs with a real store only when the def declares one — an explicit `_rift.flowState`,
 a scenario stub (`scenarioName`/`requiredScenarioState`/`newScenarioState`), a `_rift.script`
 stub, or an `is` response with `_rift.stateOps` (§7.3; the engine checks this when the imposter is
-created); otherwise it uses a silent no-op store (reads return empty). Separately, a per-space stub only
+created); otherwise, before rift 0.23.0, it uses a silent no-op store (reads return empty). From
+rift 0.23.0 every imposter gets an in-memory store (300 s TTL) by default (rift#1504), and the SDK
+warns about a trigger-less def only on an older engine. Separately, a per-space stub only
 matches when a header-form `flowIdSource` is configured (`flowIdFromHeader(...)`), because the
 engine's flow-id source defaults to the imposter port. The SDK does **not** auto-inject a flow store
 (a transport is a faithful wire mapping — it never rewrites user input); instead it fails fast and

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

@@ -3,6 +3,7 @@
 import io.github.achirdlabs.rift.FlowState;
 import io.github.achirdlabs.rift.Imposter;
 import io.github.achirdlabs.rift.Rift;
+import io.github.achirdlabs.rift.json.JsonString;
 import io.github.achirdlabs.rift.json.JsonValue;
 import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.TestFactory;
@@ -21,6 +22,19 @@
 /** {@code FlowState.get} returns the stored value on every transport (#236). Needs {@code RIFT_IT=1}. */
 class FlowStateIT {
 
+    @TestFactory
+    Stream<DynamicTest> anImposterWithNoStoreTriggerStillKeepsFlowState() {
+        // rift 0.23.0 gives every imposter an in-memory store (rift#1504); before it, this read empty (#307)
+        return gated("a trigger-less imposter's flow state reads back", () -> {
+            try (Rift rift = engine()) {
+                Imposter imposter = rift.create(imposter("no-trigger").stub(onGet("/").willReturn(ok())));
+                imposter.flowState("f").put("k", "v");
+                assertEquals(Optional.of(new JsonString("v")),
+                        imposter.flowState("f").get("k"));
+            }
+        });
+    }
+
     @TestFactory
     Stream<DynamicTest> getReturnsTheStoredValue() {
         return gated("a value put into flow state reads back as itself", () -> {

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

@@ -176,11 +176,11 @@ public interface Imposter {
     Space space(String flowId);
 
     /**
-     * The runtime flow-state store for a flow id. The engine backs this with a real store only when
-     * the def declares one — an explicit {@code _rift.flowState}, a scenario stub, a {@code
-     * _rift.script} stub, or an {@code is} response with {@code _rift.stateOps}; otherwise reads
-     * return empty (a no-op store). This accessor logs one advisory warning if no such trigger is
-     * present.
+     * The runtime flow-state store for a flow id. From rift 0.23.0 every imposter has an in-memory
+     * store (300 s TTL) unless it declares another (rift#1504). An older engine backs it with a real
+     * store only when the def declares one — an explicit {@code _rift.flowState}, a scenario stub, a
+     * {@code _rift.script} stub, or an {@code is} response with {@code _rift.stateOps}; otherwise
+     * reads return empty (a no-op store), and this accessor logs one advisory warning.
      *
      * @throws IllegalArgumentException if {@code flowId} is blank — a blank id is never the default
      *     flow but a distinct, silently-wrong partition, so it is rejected rather than sent verbatim

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

@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Supplier;
 import java.util.OptionalLong;
 
 final class ImposterImpl implements Imposter {
@@ -48,12 +49,24 @@ final class ImposterImpl implements Imposter {
     /** The imposter's engine protocol, {@code http} or {@code https}: fixed for its lifetime on this port. */
     private final String protocol;
 
+    /** The first rift release that gives every imposter an in-memory flow store (rift#1504). */
+    static final String DEFAULT_FLOW_STORE_SINCE = "0.23.0";
+
+    /** The engine's version for advisories: never throws, empty when it cannot be read. */
+    private final Supplier<Optional<String>> engineVersion;
+
     ImposterImpl(int port, RiftTransport transport, ConnectOptions options) {
         this(port, transport, options, Optional.empty(), ImposterDefinition.DEFAULT_PROTOCOL);
     }
 
     ImposterImpl(int port, RiftTransport transport, ConnectOptions options, Optional<String> boundHost,
             String protocol) {
+        this(port, transport, options, boundHost, protocol, Optional::empty);
+    }
+
+    ImposterImpl(int port, RiftTransport transport, ConnectOptions options, Optional<String> boundHost,
+            String protocol, Supplier<Optional<String>> engineVersion) {
+        this.engineVersion = Objects.requireNonNull(engineVersion, "engineVersion");
         this.port = port;
         this.transport = transport;
         this.options = options;
@@ -301,19 +314,29 @@ private synchronized void warnIfSpacesUnusable() {
     /**
      * Warn (once) if the flow-state API is used on an imposter with no store trigger (an explicit
      * {@code _rift.flowState}, a scenario stub, a {@code _rift.script} stub, or {@code
-     * _rift.stateOps}): the engine uses a no-op store, so reads return empty (#40). Advisory, same
-     * fetch-once/swallow policy as above.
+     * _rift.stateOps}) on an engine before {@value #DEFAULT_FLOW_STORE_SINCE}, which uses a no-op
+     * store, so reads return empty (#40, #307). Advisory, same fetch-once/swallow policy as above.
      */
     private synchronized void warnIfFlowStateUnusable() {
         if (flowStateConfigChecked) {
             return;
         }
         flowStateConfigChecked = true;
+        // Only an engine known to predate rift#1504 gives a trigger-less imposter a no-op store. A newer
+        // engine, an unknown version, or a dev build below the floor ("0.1.0") is not warned about.
+        boolean noOpStoreEngine = engineVersion.get()
+                .filter(v -> EngineVersion.atLeast(v, RiftImpl.MIN_ENGINE_VERSION))
+                .filter(v -> !EngineVersion.atLeast(v, DEFAULT_FLOW_STORE_SINCE))
+                .isPresent();
+        if (!noOpStoreEngine) {
+            return;
+        }
         try {
             if (!FlowStateSupport.hasStoreTrigger(definition())) {
                 LOG.log(Level.WARNING, "imposter on port " + port + " uses the flow-state API but declares no "
                         + "store trigger (_rift.flowState, a scenario stub, a _rift.script stub, or _rift.stateOps); "
-                        + "reads return empty (engine uses a no-op store). Declare flowState(inMemoryFlowState()).");
+                        + "reads return empty (rift < " + DEFAULT_FLOW_STORE_SINCE + " uses a no-op store). Declare "
+                        + "flowState(inMemoryFlowState()), or upgrade the engine.");
             }
         } catch (RiftException e) {
             // Advisory only: the SPI's declared def-fetch failures (engine unavailable, not found,

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

@@ -333,6 +333,16 @@ private Optional<String> engineVersion() {
         return Optional.of(cached);
     }
 
+    /** {@link #engineVersion()} for an advisory: an unreadable version is empty, never a failure. */
+    private Optional<String> engineVersionForAdvisory() {
+        try {
+            return engineVersion();
+        } catch (RiftException e) {
+            LOG.log(Level.DEBUG, "could not read the rift engine version for an advisory", e);
+            return Optional.empty();
+        }
+    }
+
     @Override
     public Imposter create(ImposterSpec spec) {
         return create(spec.build());
@@ -401,7 +411,7 @@ private ImposterImpl imposterAt(int port, JsonValue definition) {
                 ? Optional.of(h.value())
                 : Optional.empty();
         String protocol = obj.get("protocol") instanceof JsonString p ? p.value() : ImposterDefinition.DEFAULT_PROTOCOL;
-        return new ImposterImpl(port, transport, options, host, protocol);
+        return new ImposterImpl(port, transport, options, host, protocol, this::engineVersionForAdvisory);
     }
 
     @Override

rift-java-core/src/main/java/io/github/achirdlabs/rift/model/FlowStateSupport.java

@@ -3,9 +3,10 @@
 /**
  * Predicates over an {@link ImposterDefinition} for flow-state / spaces configuration (#40).
  *
- * <p>The rift engine backs the flow-state and per-space APIs with a real store only when the def
- * declares one — an explicit {@code _rift.flowState}, a scenario-naming stub (rift#514), or a {@code
- * _rift.script} stub (rift#358); otherwise reads silently return {@code found:false}. Separately, a
+ * <p>Before rift 0.23.0 the engine backs the flow-state and per-space APIs with a real store only
+ * when the def declares one — an explicit {@code _rift.flowState}, a scenario-naming stub (rift#514),
+ * or a {@code _rift.script} stub (rift#358); otherwise reads silently return {@code found:false}.
+ * From 0.23.0 every imposter has an in-memory store by default (rift#1504). Separately, a
  * space stub can only match when a header-form {@code flowIdSource} is configured, because the
  * engine's flow-id source defaults to the imposter port. These predicates let the DSL fail fast, and
  * the runtime accessors warn, on those otherwise-silent misconfigurations.
@@ -29,7 +30,7 @@ public static boolean hasSpaceStub(ImposterDefinition def) {
     }
 
     /**
-     * The engine will provision a real (non-NoOp) flow store: the def has an explicit {@code
+     * An engine before rift 0.23.0 will provision a real (non-NoOp) flow store: the def has an explicit {@code
      * _rift.flowState}, a scenario-naming stub, a {@code _rift.script} stub, or an {@code is}
      * response with {@code _rift.stateOps}.
      */

rift-java-core/src/test/java/io/github/achirdlabs/rift/ImposterImplFlowStateWarnTest.java

@@ -27,8 +27,14 @@ class ImposterImplFlowStateWarnTest {
 
     private static final int PORT = 4545;
 
+    /** An engine before rift#1504, whose trigger-less imposters get a no-op flow store. */
     private static ImposterImpl imposter(FakeTransport t) {
-        return new ImposterImpl(PORT, t, ConnectOptions.builder(URI.create("http://127.0.0.1:2525")).build());
+        return imposter(t, Optional.of("0.22.0"));
+    }
+
+    private static ImposterImpl imposter(FakeTransport t, Optional<String> engineVersion) {
+        return new ImposterImpl(PORT, t, ConnectOptions.builder(URI.create("http://127.0.0.1:2525")).build(),
+                Optional.empty(), "http", () -> engineVersion);
     }
 
     private static final String TRIGGERLESS =
@@ -100,6 +106,31 @@ void warnsOnMisconfiguredDefAndIsSilentOnConfigured() {
         }
     }
 
+    @Test
+    void aFlowStoreForEveryImposterNeedsNoTriggerWarningOrDefFetch() {
+        // rift 0.23.0 (rift#1504) gives every imposter an in-memory flow store; an unknown version, or a
+        // below-floor dev build ("0.1.0"), is not known to lack one either
+        for (Optional<String> version : List.of(Optional.of("0.23.0"), Optional.of("1.0.0"), Optional.<String>empty(),
+                Optional.of("0.1.0"))) {
+            Logger jul = Logger.getLogger(ImposterImpl.class.getName());
+            List<LogRecord> records = new CopyOnWriteArrayList<>();
+            Handler handler = new Handler() {
+                @Override public void publish(LogRecord r) { records.add(r); }
+                @Override public void flush() {}
+                @Override public void close() {}
+            };
+            jul.addHandler(handler);
+            try {
+                FakeTransport t = new FakeTransport(TRIGGERLESS);
+                assertNotNull(imposter(t, version).flowState("alice"));
+                assertTrue(records.stream().noneMatch(r -> r.getLevel() == Level.WARNING), version + ": " + records);
+                assertEquals(0, t.getImposterCalls, version + ": the version alone settles it");
+            } finally {
+                jul.removeHandler(handler);
+            }
+        }
+    }
+
     @Test
     void defFetchFailureNeverBreaksTheAccessor() {
         // The advisory check must not propagate a transport failure — the accessor still works.