From 02d7f5803b4538d9521dd0e29e146049fc1defdd Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 14:55:31 +0300 Subject: [PATCH 1/3] test: compare the two storage layouts by replaying one corpus through both #1870 Boots the full stack twice, once on each layout, replays a checked-in corpus through both and fails on any difference a caller could observe: status, body, response headers and etag. Etag is what makes this work - it is derived from content, so matching etags say the stored bytes agree without comparing a single path, which is the point when the paths are supposed to differ. Runs as its own Gradle task and its own CI job rather than inside :server:test. The layout is process-wide static state and this suite exists to flip it, which is what makes unrelated classes fail elsewhere in the same JVM; a separate job also means its minute runs alongside the main suite instead of after it. Differences are governed by a checked-in expectations file. Anything not listed fails, and an entry that stops matching fails too, so the file cannot quietly become a place where divergences go to be forgotten. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pr.yml | 21 ++ server/build.gradle | 20 +- .../aidial/core/server/AiDialLifecycle.java | 20 ++ .../core/server/layout/CorpusRunner.java | 180 ++++++++++++++ .../core/server/layout/DialInstance.java | 229 ++++++++++++++++++ .../aidial/core/server/layout/Divergence.java | 16 ++ .../server/layout/ExpectedDivergences.java | 81 +++++++ .../server/layout/LayoutReplayDiffTest.java | 108 +++++++++ .../aidial/core/server/layout/Multipart.java | 8 + .../core/server/layout/RecordedResponse.java | 11 + .../core/server/layout/ResponseDiffer.java | 51 ++++ .../server/layout/ResponseNormalizer.java | 124 ++++++++++ .../aidial/core/server/layout/Scenario.java | 60 +++++ .../src/test/resources/layout-diff/README.md | 85 +++++++ .../corpus/01-conversations-crud.json | 110 +++++++++ .../corpus/02-prompts-and-folders.json | 57 +++++ .../layout-diff/corpus/03-move-and-copy.json | 79 ++++++ .../layout-diff/corpus/04-files.json | 53 ++++ .../layout-diff/corpus/05-sharing.json | 130 ++++++++++ .../layout-diff/corpus/06-publication.json | 109 +++++++++ .../layout-diff/expected-divergences.json | 9 + 21 files changed, 1560 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/com/epam/aidial/core/server/AiDialLifecycle.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/Divergence.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/ExpectedDivergences.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/Multipart.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/RecordedResponse.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/ResponseDiffer.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/ResponseNormalizer.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/Scenario.java create mode 100644 server/src/test/resources/layout-diff/README.md create mode 100644 server/src/test/resources/layout-diff/corpus/01-conversations-crud.json create mode 100644 server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json create mode 100644 server/src/test/resources/layout-diff/corpus/03-move-and-copy.json create mode 100644 server/src/test/resources/layout-diff/corpus/04-files.json create mode 100644 server/src/test/resources/layout-diff/corpus/05-sharing.json create mode 100644 server/src/test/resources/layout-diff/corpus/06-publication.json create mode 100644 server/src/test/resources/layout-diff/expected-divergences.json diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index add78f090..c80872307 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -17,6 +17,27 @@ jobs: java-version: 21 trivy-limit-severities-for-sarif: false + # Kept out of run_tests: the storage layout is process-wide static state and this suite exists to flip it, + # which is what makes unrelated classes fail elsewhere in the same JVM. Its own job also means the ~1 minute + # it takes runs alongside the main suite rather than after it. + storage_layout_comparison: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: epam/ai-dial-ci/actions/java_prepare@4.10.0 + with: + java-version: 21 + java-distribution: temurin + - name: Compare the legacy and tenant-rooted storage layouts + shell: bash + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_ACTOR: ${{ github.actor }} + run: | + ./gradlew :server:layoutDiffTest \ + -Pgpr.user=$GITHUB_ACTOR \ + -Pgpr.key=$GITHUB_TOKEN + generate_openai_spec: runs-on: ubuntu-24.04 permissions: diff --git a/server/build.gradle b/server/build.gradle index 4ffee57ef..54a3fa0d9 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -84,5 +84,23 @@ test { events "passed", "skipped", "failed", "standardOut", "standardError" exceptionFormat = "full" } - useJUnitPlatform() + // The storage layout is process-wide static state; a suite whose job is flipping it does not share a JVM + // with the rest of the tests. See server/src/test/resources/layout-diff/README.md. + useJUnitPlatform { + excludeTags "layout-diff" + } +} + +tasks.register("layoutDiffTest", Test) { + description = "Replays the layout-diff corpus against both storage layouts and fails on any divergence." + group = "verification" + testClassesDirs = sourceSets.test.output.classesDirs + classpath = sourceSets.test.runtimeClasspath + testLogging { + events "passed", "skipped", "failed", "standardOut", "standardError" + exceptionFormat = "full" + } + useJUnitPlatform { + includeTags "layout-diff" + } } diff --git a/server/src/test/java/com/epam/aidial/core/server/AiDialLifecycle.java b/server/src/test/java/com/epam/aidial/core/server/AiDialLifecycle.java new file mode 100644 index 000000000..7d8c5e77a --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/AiDialLifecycle.java @@ -0,0 +1,20 @@ +package com.epam.aidial.core.server; + +/** + * {@code AiDial.start()} and {@code stop()} are package-private, and the layout comparison harness lives in a + * sub-package because it boots the stack itself rather than through {@code ResourceBaseTest}. A test-only + * bridge keeps that reachable without widening the production API for it. + */ +public final class AiDialLifecycle { + + private AiDialLifecycle() { + } + + public static void start(AiDial dial) throws Exception { + dial.start(); + } + + public static void stop(AiDial dial) throws Exception { + dial.stop(); + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java new file mode 100644 index 000000000..e0da45cc7 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java @@ -0,0 +1,180 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.fasterxml.jackson.databind.JsonNode; +import lombok.SneakyThrows; + +import java.io.InputStream; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Loads the checked-in corpus and replays it against one instance, recording what came back. + * + *

Scenarios share a single instance rather than getting one boot each: a boot costs seconds and the corpus is + * expected to grow. Isolation is by convention instead — every scenario addresses paths under its own name. + */ +public class CorpusRunner { + + public static final String REPLAY_CORPUS = "layout-diff/corpus"; + + private static final String API_KEY_1 = "proxyKey1"; + private static final String API_KEY_2 = "proxyKey2"; + + /** + * Identifies a response across both runs. Scenario and step names are the corpus's own, so a divergence + * points at a line in a checked-in file rather than at an index. + */ + public record StepKey(String scenario, String step) { + @Override + public String toString() { + return scenario + " / " + step; + } + } + + @SneakyThrows + public static List loadCorpus(String dir) { + URI uri = Objects.requireNonNull(CorpusRunner.class.getClassLoader().getResource(dir), + dir + " is missing from the test resources").toURI(); + + List scenarios = new ArrayList<>(); + try (var files = Files.list(Paths.get(uri))) { + for (Path file : files.filter(path -> path.toString().endsWith(".json")).sorted().toList()) { + try (InputStream in = Files.newInputStream(file)) { + scenarios.add(ProxyUtil.MAPPER.readValue(in, Scenario.class)); + } + } + } + + if (scenarios.isEmpty()) { + throw new IllegalStateException("The corpus is empty; a green comparison would mean nothing"); + } + return scenarios; + } + + /** + * One instance's side of the comparison. {@code buckets} is carried alongside the responses because the two + * runs must agree on it before any response comparison means anything — buckets thread through nearly every + * url in the corpus, so a difference there would make every other difference unreadable. + */ + public record Run(Map buckets, Map responses) { + } + + /** + * Replays every scenario in order and returns the response of every step, keyed so that the two runs line + * up. A step that fails to produce a response at all is a failure of the harness, not a divergence, and is + * allowed to propagate. + */ + public static Run replay(DialInstance instance, List scenarios) { + Map buckets = Map.of( + "bucket1", instance.bucket(API_KEY_1), + "bucket2", instance.bucket(API_KEY_2)); + + Map recorded = new LinkedHashMap<>(); + for (Scenario scenario : scenarios) { + Map variables = new HashMap<>(buckets); + Map raw = new LinkedHashMap<>(); + + for (Scenario.Step step : scenario.steps()) { + RecordedResponse response = instance.send( + step.method(), + substitute(step.path(), variables), + substitute(step.query(), variables), + substitute(bodyText(step.body()), variables), + substituteValues(step.headersOrEmpty(), variables), + step.multipart()); + + StepKey key = new StepKey(scenario.name(), step.name()); + if (raw.put(key, response) != null) { + throw new IllegalStateException("Duplicate step name in the corpus: " + key); + } + + capture(step, response, variables); + } + + // Normalisation waits for the end of the scenario: a value is only known to be a generated + // identifier once some step has captured it, and the step that produced it ran before that. + raw.forEach((key, response) -> recorded.put(key, ResponseNormalizer.normalize(response, variables))); + } + return new Run(buckets, recorded); + } + + private static void capture(Scenario.Step step, RecordedResponse response, Map variables) { + step.captureOrEmpty().forEach((name, capture) -> { + JsonNode node = readTree(response.body()).at(capture.at()); + if (node.isMissingNode() || node.isNull()) { + throw new IllegalStateException("Step '" + step.name() + "' cannot capture '" + name + + "' at '" + capture.at() + "' from: " + response.body()); + } + variables.put(name, extract(step, name, capture, node.asText())); + }); + + step.captureHeadersOrEmpty().forEach((name, header) -> { + String value = response.headers().get(header.toLowerCase()); + if (value == null) { + throw new IllegalStateException("Step '" + step.name() + "' cannot capture '" + name + + "' from the missing header '" + header + "'"); + } + variables.put(name, value); + }); + } + + private static String extract(Scenario.Step step, String name, Scenario.Capture capture, String value) { + if (capture.extract() == null) { + return value; + } + + Matcher matcher = Pattern.compile(capture.extract()).matcher(value); + if (!matcher.find()) { + throw new IllegalStateException("Step '" + step.name() + "' cannot capture '" + name + "': '" + + capture.extract() + "' does not match '" + value + "'"); + } + return matcher.group(1); + } + + @SneakyThrows + private static JsonNode readTree(String body) { + return ProxyUtil.MAPPER.readTree(body == null ? "{}" : body); + } + + @SneakyThrows + private static String bodyText(Object body) { + if (body == null) { + return null; + } + return body instanceof String text ? text : ProxyUtil.MAPPER.writeValueAsString(body); + } + + private static Map substituteValues(Map values, Map variables) { + Map resolved = new LinkedHashMap<>(); + values.forEach((name, value) -> resolved.put(name, substitute(value, variables))); + return resolved; + } + + private static String substitute(String template, Map variables) { + if (template == null) { + return null; + } + + String resolved = template; + for (Map.Entry variable : variables.entrySet()) { + resolved = resolved.replace("${" + variable.getKey() + "}", variable.getValue()); + } + + int unresolved = resolved.indexOf("${"); + if (unresolved >= 0) { + throw new IllegalStateException("Unresolved placeholder in: " + resolved.substring(unresolved)); + } + return resolved; + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java new file mode 100644 index 000000000..0e20a2630 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java @@ -0,0 +1,229 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.AiDial; +import com.epam.aidial.core.server.AiDialLifecycle; +import com.epam.aidial.core.server.FileUtil; +import com.epam.aidial.core.server.security.AccessTokenValidator; +import com.epam.aidial.core.server.security.ExtractedClaims; +import com.epam.aidial.core.server.util.ProxyUtil; +import com.fasterxml.jackson.databind.node.ObjectNode; +import io.vertx.core.Future; +import io.vertx.core.json.Json; +import io.vertx.core.json.JsonObject; +import lombok.Getter; +import lombok.SneakyThrows; +import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase; +import org.apache.hc.client5.http.entity.mime.HttpMultipartMode; +import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.mockito.Mockito; +import redis.embedded.RedisServer; + +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +/** + * One full DIAL stack — embedded Redis, filesystem blob store, HTTP server — brought up under a chosen + * storage layout and driven over HTTP. + * + *

This deliberately does not extend {@code ResourceBaseTest}: the layout is process-wide static state chosen + * at start-up, so comparing layouts means two boots inside one test method, which a {@code @BeforeEach} base + * class cannot express. Both boots are given the same fixed clock and the same id generator sequence, so any + * difference in a response is attributable to the layout rather than to timing or generated identifiers. + */ +public class DialInstance implements AutoCloseable { + + private static final String ID_SEED = "0"; + private static final long FIRST_ID = 123; + + private final AtomicLong nextId = new AtomicLong(FIRST_ID); + + private final Path dataDir; + private final RedisServer redis; + private final AiDial dial; + private final CloseableHttpClient client; + + @Getter + private final String name; + private final int port; + + @SneakyThrows + public DialInstance(String name, JsonObject layoutSettings, int redisPort) { + this.name = name; + this.dataDir = FileUtil.resolveRes("layout-diff-" + name); + FileUtil.deleteDir(dataDir); + FileUtil.createDir(dataDir.resolve("test")); + + this.redis = RedisServer.newRedisServer() + .port(redisPort) + .bind("127.0.0.1") + .onShutdownForceStop(true) + .setting("maxmemory 16M") + .setting("maxmemory-policy volatile-lfu") + .build(); + + try { + redis.start(); + this.client = HttpClientBuilder.create().disableAutomaticRetries().build(); + this.dial = start(layoutSettings, redisPort); + this.port = dial.getServer().actualPort(); + } catch (Throwable e) { + closeQuietly(); + throw e; + } + } + + private AiDial start(JsonObject layoutSettings, int redisPort) throws Exception { + String overrides = """ + { + "client": { + "connectTimeout": 5000 + }, + "storage": { + "bucket": "test", + "provider": "filesystem", + "identity": "access-key", + "credential": "secret-key", + "prefix": "test-2", + "overrides": { + "jclouds.filesystem.basedir": %s + } + }, + "redis": { + "singleServerConfig": { + "address": "redis://localhost:%d" + } + }, + "resources": { + "syncPeriod": 1000, + "syncDelay": 1000, + "cacheExpiration": 1000, + "heartbeatPeriod": 1000 + }, + "applications": { + "controllerEndpoint": "http://localhost:17321", + "checkDelay": 1000, + "checkPeriod": 1000 + }, + "codeInterpreter" : { + "sessionImage": "fake.image" + } + } + """.formatted(Json.encode(dataDir.toString()), redisPort); + + JsonObject settings = AiDial.settings() + .mergeIn(new JsonObject(overrides), true) + .mergeIn(new JsonObject().put("storageLayout", layoutSettings), true); + + AiDial instance = new AiDial(); + instance.setSettings(settings); + instance.setGenerator(() -> ID_SEED + nextId.getAndIncrement()); + instance.setClock(() -> 0L); + instance.setAccessTokenValidator(claimsValidator()); + AiDialLifecycle.start(instance); + return instance; + } + + private static AccessTokenValidator claimsValidator() { + AccessTokenValidator validator = Mockito.mock(AccessTokenValidator.class); + Mockito.when(validator.extractClaims(Mockito.any())).thenAnswer(invocation -> { + String authorization = invocation.getArgument(0); + if (authorization == null) { + return Future.succeededFuture(); + } + + ObjectNode claims = ProxyUtil.MAPPER.createObjectNode(); + claims.put("title", "Manager"); + return Future.succeededFuture(new ExtractedClaims(authorization, List.of(authorization), + authorization, claims, null, authorization + " user")); + }); + return validator; + } + + /** + * The bucket the given api key writes into. Derived from the caller identity and the encryption secret, + * neither of which the layout touches, so the two instances are expected to report the same value — + * {@code LayoutReplayDiffTest} asserts that before it trusts any other comparison. + */ + public String bucket(String apiKey) { + RecordedResponse response = send("GET", "/v1/bucket", null, null, Map.of("api-key", apiKey), null); + if (response.status() != 200) { + throw new IllegalStateException("Cannot resolve bucket for " + apiKey + ": " + response.body()); + } + return new JsonObject(response.body()).getString("bucket"); + } + + @SneakyThrows + public RecordedResponse send(String method, String path, String query, String body, + Map headers, Multipart multipart) { + String uri = "http://127.0.0.1:" + port + path + (query == null ? "" : "?" + query); + HttpUriRequestBase request = new HttpUriRequestBase(method, URI.create(uri)); + + headers.forEach(request::setHeader); + if (!request.containsHeader("authorization") && !request.containsHeader("api-key")) { + request.setHeader("api-key", "proxyKey1"); + } + if (body != null) { + request.setEntity(multipart == null ? new StringEntity(body) : upload(body, multipart)); + } + + return client.execute(request, response -> { + Map responseHeaders = new LinkedHashMap<>(); + for (Header header : response.getHeaders()) { + responseHeaders.put(header.getName().toLowerCase(), header.getValue()); + } + String answer = response.getEntity() == null ? null : EntityUtils.toString(response.getEntity()); + return new RecordedResponse(response.getCode(), answer, responseHeaders); + }); + } + + private static HttpEntity upload(String body, Multipart multipart) { + return MultipartEntityBuilder.create() + .setMode(HttpMultipartMode.LEGACY) + .setCharset(StandardCharsets.UTF_8) + .addBinaryBody("attachment", body.getBytes(StandardCharsets.UTF_8), + ContentType.parse(multipart.contentType()), multipart.filename()) + .build(); + } + + @Override + public void close() { + closeQuietly(); + } + + private void closeQuietly() { + try { + if (client != null) { + client.close(); + } + } catch (Exception e) { + // the instance is going away; a client that will not close cannot affect the comparison + } + + try { + if (dial != null) { + AiDialLifecycle.stop(dial); + } + } catch (Exception e) { + throw new IllegalStateException("Cannot stop the " + name + " instance", e); + } finally { + try { + redis.stop(); + } catch (Exception e) { + throw new IllegalStateException("Cannot stop redis for the " + name + " instance", e); + } + FileUtil.deleteDir(dataDir); + } + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/Divergence.java b/server/src/test/java/com/epam/aidial/core/server/layout/Divergence.java new file mode 100644 index 000000000..c3bf120f2 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/Divergence.java @@ -0,0 +1,16 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.layout.CorpusRunner.StepKey; + +/** + * One observable difference between the two runs, addressed finely enough that an expectations entry can name + * it without covering anything else: {@code field} is {@code status}, {@code body}, or {@code header:}. + */ +public record Divergence(StepKey step, String field, String legacy, String tenantRooted) { + + public String describe() { + return step + " [" + field + "]\n" + + " legacy: " + legacy + "\n" + + " tenantRooted: " + tenantRooted; + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/ExpectedDivergences.java b/server/src/test/java/com/epam/aidial/core/server/layout/ExpectedDivergences.java new file mode 100644 index 000000000..7cb7119dc --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/ExpectedDivergences.java @@ -0,0 +1,81 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.core.type.TypeReference; +import lombok.SneakyThrows; + +import java.io.InputStream; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +/** + * The checked-in list of accepted divergences — the mechanism that turns "every difference is deliberate" + * into something reviewable. Anything not listed here fails the run. + * + *

A stale entry fails too. An expectations file that accumulates entries nobody can still justify is how a + * comparison suite becomes decorative, so an entry that matches nothing is treated as a defect in the file. + */ +public record ExpectedDivergences(List entries) { + + private static final String LOCATION = "layout-diff/expected-divergences.json"; + + /** + * @param reason why the difference is acceptable, in prose — this is what a reviewer reads + * @param issue where the difference is tracked; an accepted divergence with nothing tracking it is a + * divergence nobody has committed to resolving + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Entry(String scenario, String step, String field, String reason, String issue) { + + public boolean matches(Divergence divergence) { + return Objects.equals(scenario, divergence.step().scenario()) + && Objects.equals(step, divergence.step().step()) + && Objects.equals(field, divergence.field()); + } + + @Override + public String toString() { + return scenario + " / " + step + " [" + field + "]"; + } + } + + /** + * What the run has to answer for: divergences no entry accounts for, and entries that matched nothing. + */ + public record Verdict(List unexplained, List stale) { + public boolean clean() { + return unexplained.isEmpty() && stale.isEmpty(); + } + } + + @SneakyThrows + public static ExpectedDivergences load() { + try (InputStream in = ExpectedDivergences.class.getClassLoader().getResourceAsStream(LOCATION)) { + if (in == null) { + throw new IllegalStateException(LOCATION + " is missing; the run has no record of what is accepted"); + } + return new ExpectedDivergences(ProxyUtil.MAPPER.readValue(in, new TypeReference>() { })); + } + } + + public Verdict classify(List divergences) { + List unexplained = new ArrayList<>(); + Set matched = new LinkedHashSet<>(); + + for (Divergence divergence : divergences) { + Entry entry = entries.stream().filter(candidate -> candidate.matches(divergence)).findFirst().orElse(null); + if (entry == null) { + unexplained.add(divergence); + } else { + matched.add(entry); + } + } + + List stale = entries.stream().filter(entry -> !matched.contains(entry)).toList(); + return new Verdict(unexplained, stale); + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java new file mode 100644 index 000000000..bf67d5522 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java @@ -0,0 +1,108 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.epam.aidial.core.storage.resource.LegacyStorageLayout; +import com.epam.aidial.core.storage.resource.StorageLayouts; +import io.vertx.core.json.JsonObject; +import lombok.SneakyThrows; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Replays the corpus against both storage layouts and fails on any difference a caller could observe. + * + *

Runs under its own Gradle task ({@code ./gradlew :server:layoutDiffTest}) rather than in {@code :server:test}. + * The active layout is process-wide static state and this suite exists to flip it, which is exactly the kind of + * thing that makes unrelated classes fail elsewhere in the JVM. + */ +@Tag("layout-diff") +public class LayoutReplayDiffTest { + + private static final String TENANT = "layout-diff-tenant"; + + private static final int LEGACY_REDIS_PORT = 16371; + private static final int TENANT_ROOTED_REDIS_PORT = 16372; + + private static final Path REPORT_DIR = Paths.get("build", "reports", "layout-diff"); + + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + + @Test + public void testLayoutsAreIndistinguishableToCallers() { + List corpus = CorpusRunner.loadCorpus(CorpusRunner.REPLAY_CORPUS); + + CorpusRunner.Run legacy = run("legacy", new JsonObject().put("tenantRooted", false), + LEGACY_REDIS_PORT, corpus); + CorpusRunner.Run tenantRooted = run("tenant-rooted", new JsonObject() + .put("tenantRooted", true) + .put("defaultTenant", TENANT), + TENANT_ROOTED_REDIS_PORT, corpus); + + write("responses-legacy.json", legacy.responses()); + write("responses-tenant-rooted.json", tenantRooted.responses()); + + assertEquals(legacy.buckets(), tenantRooted.buckets(), + "Buckets differ between layouts; every url-bearing comparison below would be meaningless"); + + List divergences = ResponseDiffer.diff(legacy.responses(), tenantRooted.responses()); + write("divergences.txt", divergences.stream().map(Divergence::describe).collect(Collectors.joining("\n\n"))); + + ExpectedDivergences.Verdict verdict = ExpectedDivergences.load().classify(divergences); + if (!verdict.clean()) { + fail(report(verdict)); + } + } + + private static CorpusRunner.Run run(String name, JsonObject layoutSettings, int redisPort, List corpus) { + try (DialInstance instance = new DialInstance(name, layoutSettings, redisPort)) { + return CorpusRunner.replay(instance, corpus); + } finally { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + } + + private static String report(ExpectedDivergences.Verdict verdict) { + StringBuilder message = new StringBuilder(); + + if (!verdict.unexplained().isEmpty()) { + message.append(verdict.unexplained().size()) + .append(" divergence(s) between the layouts are not accounted for.\n") + .append("Fix them, or record each one in server/src/test/resources/") + .append("layout-diff/expected-divergences.json with a reason and an issue.\n\n") + .append(verdict.unexplained().stream().map(Divergence::describe) + .collect(Collectors.joining("\n\n"))); + } + + if (!verdict.stale().isEmpty()) { + message.append(message.isEmpty() ? "" : "\n\n") + .append(verdict.stale().size()) + .append(" accepted divergence(s) no longer happen and should be removed:\n") + .append(verdict.stale().stream().map(ExpectedDivergences.Entry::toString) + .collect(Collectors.joining("\n"))); + } + + return message.toString(); + } + + @SneakyThrows + private static void write(String file, Object content) { + Files.createDirectories(REPORT_DIR); + String text = content instanceof String plain + ? plain + : ProxyUtil.MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(content); + Files.writeString(REPORT_DIR.resolve(file), text); + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/Multipart.java b/server/src/test/java/com/epam/aidial/core/server/layout/Multipart.java new file mode 100644 index 000000000..878863a3b --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/Multipart.java @@ -0,0 +1,8 @@ +package com.epam.aidial.core.server.layout; + +/** + * Turns a step's body into a file upload. Files are the bulk of what customers actually store, so the corpus + * would be missing its largest content type without this. + */ +public record Multipart(String filename, String contentType) { +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/RecordedResponse.java b/server/src/test/java/com/epam/aidial/core/server/layout/RecordedResponse.java new file mode 100644 index 000000000..516023e12 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/RecordedResponse.java @@ -0,0 +1,11 @@ +package com.epam.aidial.core.server.layout; + +import java.util.Map; + +/** + * What a caller observes. The comparison is over these three fields and nothing else — physical paths are + * supposed to differ between layouts, so stored artifacts are not compared. {@code etag} travels in + * the headers and is derived from content, which is how content equality is checked without touching paths. + */ +public record RecordedResponse(int status, String body, Map headers) { +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/ResponseDiffer.java b/server/src/test/java/com/epam/aidial/core/server/layout/ResponseDiffer.java new file mode 100644 index 000000000..1b6a22353 --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/ResponseDiffer.java @@ -0,0 +1,51 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.layout.CorpusRunner.StepKey; +import lombok.experimental.UtilityClass; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.TreeSet; + +@UtilityClass +public class ResponseDiffer { + + public static List diff(Map legacy, + Map tenantRooted) { + if (!legacy.keySet().equals(tenantRooted.keySet())) { + throw new IllegalStateException("The two runs replayed different steps; the corpus is not deterministic"); + } + + List divergences = new ArrayList<>(); + legacy.forEach((step, left) -> compare(step, left, tenantRooted.get(step), divergences)); + return divergences; + } + + private static void compare(StepKey step, RecordedResponse legacy, RecordedResponse tenantRooted, + List divergences) { + if (legacy.status() != tenantRooted.status()) { + divergences.add(new Divergence(step, "status", + String.valueOf(legacy.status()), String.valueOf(tenantRooted.status()))); + } + + if (!Objects.equals(legacy.body(), tenantRooted.body())) { + divergences.add(new Divergence(step, "body", legacy.body(), tenantRooted.body())); + } + + for (String header : new TreeSet<>(union(legacy.headers(), tenantRooted.headers()))) { + String left = legacy.headers().get(header); + String right = tenantRooted.headers().get(header); + if (!Objects.equals(left, right)) { + divergences.add(new Divergence(step, "header:" + header, left, right)); + } + } + } + + private static List union(Map left, Map right) { + List names = new ArrayList<>(left.keySet()); + right.keySet().stream().filter(name -> !left.containsKey(name)).forEach(names::add); + return names; + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/ResponseNormalizer.java b/server/src/test/java/com/epam/aidial/core/server/layout/ResponseNormalizer.java new file mode 100644 index 000000000..d2e79e4ac --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/ResponseNormalizer.java @@ -0,0 +1,124 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; +import lombok.experimental.UtilityClass; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +/** + * Everything that is allowed to differ between the two runs, stated in one place: wall-clock stamps, randomly + * generated identifiers, and two headers. + * + *

The list is deliberately short, and each entry says why it is there. A normaliser that quietly ignores + * fields is worse than no comparison at all — the whole instrument is only worth what this list is. + */ +@UtilityClass +public class ResponseNormalizer { + + /** + * Headers dropped before comparison: + *

+ */ + private static final Set VOLATILE_HEADERS = Set.of("date", "content-length"); + + /** + * Fields stamped from the wall clock at write time. {@code AiDial.setClock} does not reach them — + * {@code ResourceService} calls {@link System#currentTimeMillis()} directly — so they differ between any + * two runs, of the same layout as much as of different ones. + */ + private static final Set WALL_CLOCK_FIELDS = Set.of("createdAt", "updatedAt", "expireAt"); + + private static final String ELIDED = ""; + + /** + * Values short enough that replacing them everywhere they appear would corrupt unrelated text. + */ + private static final int MIN_SUBSTITUTABLE_LENGTH = 8; + + /** + * @param variables what the scenario captured, by name. Their values are randomly generated — an + * invitation id is not derived from anything the two runs share — so they are replaced by + * their own name. That compares them by the role they play rather than by a value that + * was never going to match. + */ + public static RecordedResponse normalize(RecordedResponse response, Map variables) { + String body = detokenize(canonicalBody(response.body()), variables); + return new RecordedResponse(response.status(), body, stableHeaders(response.headers(), variables)); + } + + private static String detokenize(String text, Map variables) { + if (text == null) { + return null; + } + + String detokenized = text; + for (Map.Entry variable : variables.entrySet()) { + if (variable.getValue().length() >= MIN_SUBSTITUTABLE_LENGTH) { + detokenized = detokenized.replace(variable.getValue(), "${" + variable.getKey() + "}"); + } + } + return detokenized; + } + + private static Map stableHeaders(Map headers, Map variables) { + Map stable = new TreeMap<>(); + headers.forEach((name, value) -> { + if (!VOLATILE_HEADERS.contains(name)) { + stable.put(name, detokenize(value, variables)); + } + }); + return stable; + } + + /** + * JSON bodies are re-serialised with object keys sorted. Field order carries no meaning and some responses + * are built from hash-ordered maps, so raw text comparison would flag ordering as a divergence. Array order + * is left alone — listing order is observable behaviour and a difference there is a real finding. + */ + private static String canonicalBody(String body) { + if (body == null || body.isBlank()) { + return body; + } + + try { + return ProxyUtil.MAPPER.writeValueAsString(sortKeys(ProxyUtil.MAPPER.readTree(body))); + } catch (Exception e) { + return body; + } + } + + private static JsonNode sortKeys(JsonNode node) { + if (node.isObject()) { + List names = new ArrayList<>(); + node.fieldNames().forEachRemaining(names::add); + Collections.sort(names); + + ObjectNode sorted = ProxyUtil.MAPPER.createObjectNode(); + names.forEach(name -> sorted.set(name, WALL_CLOCK_FIELDS.contains(name) + ? TextNode.valueOf(ELIDED) + : sortKeys(node.get(name)))); + return sorted; + } + + if (node.isArray()) { + ArrayNode sorted = ProxyUtil.MAPPER.createArrayNode(); + node.forEach(element -> sorted.add(sortKeys(element))); + return sorted; + } + + return node; + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/Scenario.java b/server/src/test/java/com/epam/aidial/core/server/layout/Scenario.java new file mode 100644 index 000000000..a4399cbba --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/Scenario.java @@ -0,0 +1,60 @@ +package com.epam.aidial.core.server.layout; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +import java.util.List; +import java.util.Map; + +/** + * A checked-in, deterministic sequence of API calls. Scenarios are data rather than code so that one corpus + * feeds both runs by construction — a hand-written pair of tests could drift. + */ +@JsonIgnoreProperties(ignoreUnknown = false) +public record Scenario(String name, String description, List steps) { + + /** + * One request. {@code path}, {@code query}, {@code body} and header values go through placeholder + * substitution: {@code ${bucket1}} and {@code ${bucket2}} are the buckets of the two api keys, and any + * name captured by an earlier step is available under its own name. + * + * @param capture variable name to a place in this step's response body; lets a later step address + * something the server generated, such as a publication url + * @param captureHeaders variable name to response header name — how a conditional request gets the etag + * of the write that preceded it + * @param multipart set to upload the body as a file rather than send it as the request entity + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Step(String name, + String method, + String path, + String query, + Object body, + Map headers, + Map capture, + Map captureHeaders, + Multipart multipart) { + + public Map headersOrEmpty() { + return headers == null ? Map.of() : headers; + } + + public Map captureOrEmpty() { + return capture == null ? Map.of() : capture; + } + + public Map captureHeadersOrEmpty() { + return captureHeaders == null ? Map.of() : captureHeaders; + } + } + + /** + * Where a variable's value comes from. + * + * @param at JSON pointer into the response body + * @param extract optional regular expression applied to the pointed-at value; group 1 becomes the + * variable. An invitation link carries the id the response never returns on its own. + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Capture(String at, String extract) { + } +} diff --git a/server/src/test/resources/layout-diff/README.md b/server/src/test/resources/layout-diff/README.md new file mode 100644 index 000000000..8af850357 --- /dev/null +++ b/server/src/test/resources/layout-diff/README.md @@ -0,0 +1,85 @@ +# Layout comparison corpus + +Scenarios replayed against both storage layouts by `LayoutReplayDiffTest`. Anything a caller can observe — +status, body, response headers, etag — has to come back identical, or be listed in +[`expected-divergences.json`](expected-divergences.json). + +```bash +./gradlew :server:layoutDiffTest +``` + +The suite is not part of `:server:test`. The active layout is process-wide static state and this suite exists +to flip it, which is exactly what makes unrelated classes fail elsewhere in the same JVM. + +## Why responses and not stored paths + +The paths are *supposed* to differ — that is the change under test: + +``` +same request: PUT /v1/conversations//folder/chat1 + +on disk, legacy: Users/u1/conversations/folder/chat1 +on disk, new: .org/default/.users/u1/.conversations/folder/chat1 ← must differ +``` + +Etag is the bridge. It is derived from content, so matching etags say the stored bytes are the same without +comparing a single path. + +## Scenario format + +```jsonc +{ + "name": "conversations-crud", // must be unique across the corpus + "description": "…", // what this scenario is for; read by whoever triages a failure + "steps": [ + { + "name": "create", // must be unique within the scenario + "method": "PUT", + "path": "/v1/conversations/${bucket1}/crud/conversation", + "query": "recursive=true", // optional + "headers": {"api-key": "proxyKey2"}, // optional; defaults to proxyKey1 + "body": {"id": "…"}, // optional; object or string + "multipart": {"filename": "f.txt", "contentType": "text/plain"}, // optional; uploads the body as a file + "capture": {"publicationUrl": {"at": "/url"}}, // optional; JSON pointer into this response's body, + // with an optional "extract" regex taking group 1 + "captureHeaders": {"createdEtag": "etag"} // optional; a response header + } + ] +} +``` + +`${bucket1}` and `${bucket2}` are the buckets of `proxyKey1` and `proxyKey2`; anything captured by an earlier +step is available under its own name. An unresolved `${…}` fails the run rather than being sent literally. + +Captured values are also substituted back out of the recorded responses before comparison — a randomly +generated id such as an invitation id was never going to match across two runs. So an id a scenario asserts +on has to be captured: capturing is what makes it comparable. + +Both instances run on the same id generator sequence, so anything drawn from it matches across runs. The fixed +clock does not reach resource timestamps — `ResourceService` calls `System.currentTimeMillis()` directly — so +`createdAt`, `updatedAt` and `expireAt` are elided instead; see `ResponseNormalizer`. + +Scenarios share one instance per run, so **every scenario must address paths under its own prefix** — +`crud/`, `movediff/`, `sharediff/` — or it will see another scenario's writes. + +## Adding a scenario + +Cover something the corpus does not: an operation, a resource type, or a way access is granted. Coverage of +the access rules themselves belongs to the access-decision differ, not here. + +## Accepting a divergence + +Only when the difference is deliberate. Add an entry to `expected-divergences.json`: + +```json +{ + "scenario": "conversations-crud", + "step": "metadata-folder", + "field": "body", + "reason": "Why this difference is correct and acceptable.", + "issue": "https://github.com/epam/ai-dial-core/issues/…" +} +``` + +`field` is `status`, `body`, or `header:`. An entry that stops matching fails the run too — an +expectations file full of things nobody can still justify is how this suite would become decorative. diff --git a/server/src/test/resources/layout-diff/corpus/01-conversations-crud.json b/server/src/test/resources/layout-diff/corpus/01-conversations-crud.json new file mode 100644 index 000000000..1f5642f87 --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/01-conversations-crud.json @@ -0,0 +1,110 @@ +{ + "name": "conversations-crud", + "description": "Write, read, list, conditionally update and delete a conversation. Etags are content-derived, so matching etags across the two runs mean the stored bytes are the same even though the paths are not.", + "steps": [ + { + "name": "read-missing", + "method": "GET", + "path": "/v1/conversations/${bucket1}/crud/conversation" + }, + { + "name": "create", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/crud/conversation", + "body": { + "id": "conversation_id", + "name": "display_name", + "model": {"id": "model_id"}, + "prompt": "system prompt", + "temperature": 1, + "folderId": "folder1", + "messages": [], + "assistantModelId": "assistantId", + "lastActivityDate": 4848683153 + }, + "captureHeaders": {"createdEtag": "etag"} + }, + { + "name": "read-created", + "method": "GET", + "path": "/v1/conversations/${bucket1}/crud/conversation" + }, + { + "name": "metadata-item", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/crud/conversation", + "query": "permissions=true" + }, + { + "name": "metadata-folder", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/crud/" + }, + { + "name": "metadata-recursive", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/", + "query": "recursive=true" + }, + { + "name": "update-with-stale-etag", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/crud/conversation", + "headers": {"if-match": "\"not-the-current-etag\""}, + "body": { + "id": "conversation_id2", + "name": "display_name2", + "model": {"id": "model_id2"}, + "prompt": "system prompt2", + "temperature": 0, + "folderId": "folder1", + "messages": [], + "assistantModelId": "assistantId2", + "lastActivityDate": 98746886446 + } + }, + { + "name": "update-with-current-etag", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/crud/conversation", + "headers": {"if-match": "${createdEtag}"}, + "body": { + "id": "conversation_id2", + "name": "display_name2", + "model": {"id": "model_id2"}, + "prompt": "system prompt2", + "temperature": 0, + "folderId": "folder1", + "messages": [], + "assistantModelId": "assistantId2", + "lastActivityDate": 98746886446 + } + }, + { + "name": "read-updated", + "method": "GET", + "path": "/v1/conversations/${bucket1}/crud/conversation" + }, + { + "name": "read-other-bucket-forbidden", + "method": "GET", + "path": "/v1/conversations/${bucket1}/crud/conversation", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "delete", + "method": "DELETE", + "path": "/v1/conversations/${bucket1}/crud/conversation" + }, + { + "name": "read-deleted", + "method": "GET", + "path": "/v1/conversations/${bucket1}/crud/conversation" + }, + { + "name": "metadata-folder-after-delete", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/crud/" + } + ] +} diff --git a/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json b/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json new file mode 100644 index 000000000..a4cb9cd45 --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json @@ -0,0 +1,57 @@ +{ + "name": "prompts-and-folders", + "description": "A second resource type, nested folders and non-ascii names. The tenant-rooted layout moves the type into a reserved folder and re-roots the bucket, so a type other than conversations is what shows the transform is generic rather than special-cased.", + "steps": [ + { + "name": "create-top-level", + "method": "PUT", + "path": "/v1/prompts/${bucket1}/promptsdiff/prompt", + "body": {"id": "prompt_id", "name": "prompt", "folderId": "promptsdiff", "content": "content"} + }, + { + "name": "create-nested", + "method": "PUT", + "path": "/v1/prompts/${bucket1}/promptsdiff/nested/deeper/prompt2", + "body": {"id": "prompt_id2", "name": "prompt2", "folderId": "deeper", "content": "content2"} + }, + { + "name": "create-escaped-name", + "method": "PUT", + "path": "/v1/prompts/${bucket1}/promptsdiff/prompt%201%40", + "body": {"id": "prompt_id3", "name": "prompt 1@", "folderId": "promptsdiff", "content": "content3"} + }, + { + "name": "read-escaped-name", + "method": "GET", + "path": "/v1/prompts/${bucket1}/promptsdiff/prompt%201%40" + }, + { + "name": "list-folder", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/" + }, + { + "name": "list-recursive", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", + "query": "recursive=true" + }, + { + "name": "list-paged", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", + "query": "recursive=true&limit=2" + }, + { + "name": "delete-nested", + "method": "DELETE", + "path": "/v1/prompts/${bucket1}/promptsdiff/nested/deeper/prompt2" + }, + { + "name": "list-after-delete", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", + "query": "recursive=true" + } + ] +} diff --git a/server/src/test/resources/layout-diff/corpus/03-move-and-copy.json b/server/src/test/resources/layout-diff/corpus/03-move-and-copy.json new file mode 100644 index 000000000..4f2c98003 --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/03-move-and-copy.json @@ -0,0 +1,79 @@ +{ + "name": "move-and-copy", + "description": "Resource operations that rewrite paths on the server. These touch the physical layout most directly of anything in the API, which makes them the place a re-addressing mistake would surface first.", + "steps": [ + { + "name": "create-source", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/movediff/source", + "body": { + "id": "conversation_id", + "name": "display_name", + "model": {"id": "model_id"}, + "prompt": "system prompt", + "temperature": 1, + "folderId": "movediff", + "messages": [], + "assistantModelId": "assistantId", + "lastActivityDate": 4848683153 + } + }, + { + "name": "copy", + "method": "POST", + "path": "/v1/ops/resource/copy", + "body": { + "sourceUrl": "conversations/${bucket1}/movediff/source", + "destinationUrl": "conversations/${bucket1}/movediff/copied" + } + }, + { + "name": "read-copy", + "method": "GET", + "path": "/v1/conversations/${bucket1}/movediff/copied" + }, + { + "name": "copy-over-existing", + "method": "POST", + "path": "/v1/ops/resource/copy", + "body": { + "sourceUrl": "conversations/${bucket1}/movediff/source", + "destinationUrl": "conversations/${bucket1}/movediff/copied" + } + }, + { + "name": "move-into-new-folder", + "method": "POST", + "path": "/v1/ops/resource/move", + "body": { + "sourceUrl": "conversations/${bucket1}/movediff/source", + "destinationUrl": "conversations/${bucket1}/movediff/moved/target" + } + }, + { + "name": "read-move-source", + "method": "GET", + "path": "/v1/conversations/${bucket1}/movediff/source" + }, + { + "name": "read-move-target", + "method": "GET", + "path": "/v1/conversations/${bucket1}/movediff/moved/target" + }, + { + "name": "move-across-buckets-forbidden", + "method": "POST", + "path": "/v1/ops/resource/move", + "body": { + "sourceUrl": "conversations/${bucket1}/movediff/moved/target", + "destinationUrl": "conversations/${bucket2}/movediff/stolen" + } + }, + { + "name": "list-after-operations", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/movediff/", + "query": "recursive=true" + } + ] +} diff --git a/server/src/test/resources/layout-diff/corpus/04-files.json b/server/src/test/resources/layout-diff/corpus/04-files.json new file mode 100644 index 000000000..a193ec911 --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/04-files.json @@ -0,0 +1,53 @@ +{ + "name": "files", + "description": "File upload, download and listing. Files are the largest thing customers store and the only corpus entry that goes through the multipart path, where content type and content length are carried separately from the resource body.", + "steps": [ + { + "name": "upload", + "method": "PUT", + "path": "/v1/files/${bucket1}/filesdiff/file.txt", + "body": "content of the file under test", + "multipart": {"filename": "file.txt", "contentType": "text/custom"} + }, + { + "name": "download", + "method": "GET", + "path": "/v1/files/${bucket1}/filesdiff/file.txt" + }, + { + "name": "metadata", + "method": "GET", + "path": "/v1/metadata/files/${bucket1}/filesdiff/", + "query": "recursive=true" + }, + { + "name": "upload-nested", + "method": "PUT", + "path": "/v1/files/${bucket1}/filesdiff/nested/file2.txt", + "body": "another file", + "multipart": {"filename": "file2.txt", "contentType": "text/plain"} + }, + { + "name": "download-other-bucket-forbidden", + "method": "GET", + "path": "/v1/files/${bucket1}/filesdiff/file.txt", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "delete", + "method": "DELETE", + "path": "/v1/files/${bucket1}/filesdiff/file.txt" + }, + { + "name": "download-deleted", + "method": "GET", + "path": "/v1/files/${bucket1}/filesdiff/file.txt" + }, + { + "name": "metadata-after-delete", + "method": "GET", + "path": "/v1/metadata/files/${bucket1}/filesdiff/", + "query": "recursive=true" + } + ] +} diff --git a/server/src/test/resources/layout-diff/corpus/05-sharing.json b/server/src/test/resources/layout-diff/corpus/05-sharing.json new file mode 100644 index 000000000..29b100b24 --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/05-sharing.json @@ -0,0 +1,130 @@ +{ + "name": "sharing", + "description": "The full share lifecycle between two buckets — invite, accept, read as the recipient, list from both sides, revoke, discard. Shares key on logical urls, which the layout is not supposed to touch; this is where that claim gets exercised end to end rather than argued.", + "steps": [ + { + "name": "shared-with-me-empty", + "method": "POST", + "path": "/v1/ops/resource/share/list", + "body": {"resourceTypes": ["CONVERSATION"], "with": "me"}, + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "create-resource", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/sharediff/conversation%201%40", + "body": { + "id": "conversation_id", + "name": "display_name", + "model": {"id": "model_id"}, + "prompt": "system prompt", + "temperature": 1, + "folderId": "sharediff", + "messages": [], + "assistantModelId": "assistantId", + "lastActivityDate": 4848683153 + } + }, + { + "name": "read-before-share-forbidden", + "method": "GET", + "path": "/v1/conversations/${bucket1}/sharediff/conversation%201%40", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "create-invitation", + "method": "POST", + "path": "/v1/ops/resource/share/create", + "body": { + "invitationType": "link", + "resources": [{"url": "conversations/${bucket1}/sharediff/conversation%201%40"}] + }, + "capture": {"invitationId": {"at": "/invitationLink", "extract": "/v1/invitations/(.+)$"}} + }, + { + "name": "read-invitation", + "method": "GET", + "path": "/v1/invitations/${invitationId}" + }, + { + "name": "accept-invitation", + "method": "GET", + "path": "/v1/invitations/${invitationId}", + "query": "accept=true", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "read-after-share", + "method": "GET", + "path": "/v1/conversations/${bucket1}/sharediff/conversation%201%40", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "metadata-after-share", + "method": "GET", + "path": "/v1/metadata/conversations/${bucket1}/sharediff/conversation%201%40", + "query": "permissions=true", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "write-after-read-only-share-forbidden", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/sharediff/conversation%201%40", + "headers": {"api-key": "proxyKey2"}, + "body": { + "id": "conversation_id2", + "name": "display_name2", + "model": {"id": "model_id2"}, + "prompt": "system prompt2", + "temperature": 0, + "folderId": "sharediff", + "messages": [], + "assistantModelId": "assistantId2", + "lastActivityDate": 98746886446 + } + }, + { + "name": "shared-with-me", + "method": "POST", + "path": "/v1/ops/resource/share/list", + "body": {"resourceTypes": ["CONVERSATION"], "with": "me"}, + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "shared-by-me", + "method": "POST", + "path": "/v1/ops/resource/share/list", + "body": {"resourceTypes": ["CONVERSATION"], "with": "others"} + }, + { + "name": "list-invitations", + "method": "GET", + "path": "/v1/invitations" + }, + { + "name": "discard", + "method": "POST", + "path": "/v1/ops/resource/share/discard", + "body": {"resources": [{"url": "conversations/${bucket1}/sharediff/conversation%201%40"}]}, + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "read-after-discard-forbidden", + "method": "GET", + "path": "/v1/conversations/${bucket1}/sharediff/conversation%201%40", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "revoke", + "method": "POST", + "path": "/v1/ops/resource/share/revoke", + "body": {"resources": [{"url": "conversations/${bucket1}/sharediff/conversation%201%40"}]} + }, + { + "name": "shared-by-me-after-revoke", + "method": "POST", + "path": "/v1/ops/resource/share/list", + "body": {"resourceTypes": ["CONVERSATION"], "with": "others"} + } + ] +} diff --git a/server/src/test/resources/layout-diff/corpus/06-publication.json b/server/src/test/resources/layout-diff/corpus/06-publication.json new file mode 100644 index 000000000..d3be8bcdc --- /dev/null +++ b/server/src/test/resources/layout-diff/corpus/06-publication.json @@ -0,0 +1,109 @@ +{ + "name": "publication", + "description": "Publish a conversation into the public space and read it back as a different user. This is the only corpus entry that crosses into the review and public buckets, and the only one that writes the rules document that governs read access to everything under public/.", + "steps": [ + { + "name": "create-source", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/pubdiff/conversation", + "body": { + "id": "conversation_id", + "name": "display_name", + "model": {"id": "model_id"}, + "prompt": "system prompt", + "temperature": 1, + "folderId": "pubdiff", + "messages": [], + "assistantModelId": "assistantId", + "lastActivityDate": 4848683153 + } + }, + { + "name": "read-public-before-publish", + "method": "GET", + "path": "/v1/conversations/public/pubdiff/conversation", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "create-publication", + "method": "POST", + "path": "/v1/ops/publication/create", + "body": { + "name": "layout diff publication", + "targetFolder": "public/pubdiff/", + "resources": [ + { + "action": "ADD", + "sourceUrl": "conversations/${bucket1}/pubdiff/conversation", + "targetUrl": "conversations/public/pubdiff/conversation" + } + ], + "rules": [{"source": "roles", "function": "TRUE"}] + }, + "capture": {"publicationUrl": {"at": "/url"}} + }, + { + "name": "get-publication", + "method": "POST", + "path": "/v1/ops/publication/get", + "body": {"url": "${publicationUrl}"} + }, + { + "name": "list-own-publications", + "method": "POST", + "path": "/v1/ops/publication/list", + "body": {"url": "publications/${bucket1}/"} + }, + { + "name": "list-pending-as-admin", + "method": "POST", + "path": "/v1/ops/publication/list", + "body": {"url": "publications/public/"}, + "headers": {"authorization": "admin"} + }, + { + "name": "read-public-before-approval", + "method": "GET", + "path": "/v1/conversations/public/pubdiff/conversation", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "approve", + "method": "POST", + "path": "/v1/ops/publication/approve", + "body": {"url": "${publicationUrl}"}, + "headers": {"authorization": "admin"} + }, + { + "name": "read-public-after-approval", + "method": "GET", + "path": "/v1/conversations/public/pubdiff/conversation", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "list-public-folder", + "method": "GET", + "path": "/v1/metadata/conversations/public/pubdiff/", + "query": "recursive=true", + "headers": {"api-key": "proxyKey2"} + }, + { + "name": "list-published-resources", + "method": "POST", + "path": "/v1/ops/publication/resource/list", + "body": {"resourceTypes": ["CONVERSATION"]} + }, + { + "name": "list-rules", + "method": "POST", + "path": "/v1/ops/publication/rule/list", + "body": {"url": "public/pubdiff/"} + }, + { + "name": "get-publication-after-approval", + "method": "POST", + "path": "/v1/ops/publication/get", + "body": {"url": "${publicationUrl}"} + } + ] +} diff --git a/server/src/test/resources/layout-diff/expected-divergences.json b/server/src/test/resources/layout-diff/expected-divergences.json new file mode 100644 index 000000000..a4b5af482 --- /dev/null +++ b/server/src/test/resources/layout-diff/expected-divergences.json @@ -0,0 +1,9 @@ +[ + { + "scenario": "prompts-and-folders", + "step": "list-paged", + "field": "body", + "reason": "nextToken is the blob store's own continuation marker, passed through verbatim, so it carries the physical path and changes shape with the layout. Accepted: the token is opaque to clients and short-lived \u2014 it is handed straight back within one listing, and each layout accepts its own. The only case it bites is a token minted before a cutover and replayed after it, which fails one 'next page' click and succeeds on retry; that belongs to P2 migration, not to this change. Scope note: that this token exposes the storage prefix and the decrypted bucket location to callers is a separate, pre-existing defect on development \u2014 ResourceService is untouched by this stack and the legacy run leaks it identically. Tracked privately, not here.", + "issue": "https://github.com/epam/ai-dial-core/issues/1863" + } +] From c804b7d90454885a71683eb2ff10abfb320d20d3 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 18:46:02 +0300 Subject: [PATCH 2/3] test: close the replay differ's blind spots #1870 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ways the differ could report green while behaviour differed: - Normalisation replaces a captured value with its role name wherever it appears, so the value's own divergence was invisible — including the etag, the one signal that checks content without comparing paths. Captures both runs derive from shared inputs (etag, publication url) are now compared by value. - Pagination continuation was never replayed; the one accepted-divergence entry swallowed the whole first-page body. The corpus now walks all three pages, each layout consuming its own tokens, and the entry is gone — the token is compared by role instead. - The HTTP client decoded compressed responses transparently, hiding exactly the divergence a migration that drops blob metadata produces; and repeated response headers collapsed last-wins. Co-Authored-By: Claude Fable 5 --- .../core/server/layout/CorpusRunner.java | 33 ++++++++++++++++--- .../core/server/layout/DialInstance.java | 8 +++-- .../server/layout/LayoutReplayDiffTest.java | 19 +++++++++++ .../corpus/02-prompts-and-folders.json | 16 ++++++++- .../layout-diff/expected-divergences.json | 10 +----- 5 files changed, 70 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java index e0da45cc7..f079717ef 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java @@ -6,6 +6,8 @@ import java.io.InputStream; import java.net.URI; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -65,9 +67,13 @@ public static List loadCorpus(String dir) { /** * One instance's side of the comparison. {@code buckets} is carried alongside the responses because the two * runs must agree on it before any response comparison means anything — buckets thread through nearly every - * url in the corpus, so a difference there would make every other difference unreadable. + * url in the corpus, so a difference there would make every other difference unreadable. {@code captures} + * holds what each scenario captured, by scenario name: normalisation replaces a captured value with its + * role name wherever it appears, so the values themselves are only comparable here. */ - public record Run(Map buckets, Map responses) { + public record Run(Map buckets, + Map> captures, + Map responses) { } /** @@ -81,6 +87,7 @@ public static Run replay(DialInstance instance, List scenarios) { "bucket2", instance.bucket(API_KEY_2)); Map recorded = new LinkedHashMap<>(); + Map> captures = new LinkedHashMap<>(); for (Scenario scenario : scenarios) { Map variables = new HashMap<>(buckets); Map raw = new LinkedHashMap<>(); @@ -89,7 +96,7 @@ public static Run replay(DialInstance instance, List scenarios) { RecordedResponse response = instance.send( step.method(), substitute(step.path(), variables), - substitute(step.query(), variables), + substituteQuery(step.query(), variables), substitute(bodyText(step.body()), variables), substituteValues(step.headersOrEmpty(), variables), step.multipart()); @@ -105,8 +112,12 @@ public static Run replay(DialInstance instance, List scenarios) { // Normalisation waits for the end of the scenario: a value is only known to be a generated // identifier once some step has captured it, and the step that produced it ran before that. raw.forEach((key, response) -> recorded.put(key, ResponseNormalizer.normalize(response, variables))); + + Map captured = new LinkedHashMap<>(variables); + captured.keySet().removeAll(buckets.keySet()); + captures.put(scenario.name(), captured); } - return new Run(buckets, recorded); + return new Run(buckets, captures, recorded); } private static void capture(Scenario.Step step, RecordedResponse response, Map variables) { @@ -161,6 +172,20 @@ private static Map substituteValues(Map values, return resolved; } + /** + * Query values ride in a URI, so substituted values are percent-encoded first — a page token is a raw + * storage marker and can carry any character the blob store's keys can. + */ + private static String substituteQuery(String template, Map variables) { + if (template == null) { + return null; + } + + Map encoded = new LinkedHashMap<>(); + variables.forEach((name, value) -> encoded.put(name, URLEncoder.encode(value, StandardCharsets.UTF_8))); + return substitute(template, encoded); + } + private static String substitute(String template, Map variables) { if (template == null) { return null; diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java index 0e20a2630..178490b00 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java @@ -75,7 +75,10 @@ public DialInstance(String name, JsonObject layoutSettings, int redisPort) { try { redis.start(); - this.client = HttpClientBuilder.create().disableAutomaticRetries().build(); + // No transparent content decoding: it would decompress a response body and strip the + // content-encoding header before the comparison sees either, hiding exactly the class of + // divergence a migration that loses blob metadata produces. + this.client = HttpClientBuilder.create().disableAutomaticRetries().disableContentCompression().build(); this.dial = start(layoutSettings, redisPort); this.port = dial.getServer().actualPort(); } catch (Throwable e) { @@ -181,7 +184,8 @@ public RecordedResponse send(String method, String path, String query, String bo return client.execute(request, response -> { Map responseHeaders = new LinkedHashMap<>(); for (Header header : response.getHeaders()) { - responseHeaders.put(header.getName().toLowerCase(), header.getValue()); + // Joined rather than last-wins, or a duplicated header would be invisible to the diff. + responseHeaders.merge(header.getName().toLowerCase(), header.getValue(), (left, right) -> left + ", " + right); } String answer = response.getEntity() == null ? null : EntityUtils.toString(response.getEntity()); return new RecordedResponse(response.getCode(), answer, responseHeaders); diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java index bf67d5522..11469dc4d 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutReplayDiffTest.java @@ -13,6 +13,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -35,6 +37,18 @@ public class LayoutReplayDiffTest { private static final Path REPORT_DIR = Paths.get("build", "reports", "layout-diff"); + /** + * Captured values both runs derive from shared inputs — the same content bytes (an etag) or the same + * seeded id generator (a publication url) — so the values themselves must be equal across runs. This + * closes the hole normalisation opens: a captured value is replaced by its role name wherever it + * appears, so its own divergence is invisible in the body diff and only comparable here. Captures that + * are random by construction (an invitation id carries a random key, a page token carries the physical + * path) stay unlisted. + */ + private static final Map> CROSS_RUN_STABLE_CAPTURES = Map.of( + "conversations-crud", Set.of("createdEtag"), + "publication", Set.of("publicationUrl")); + @AfterEach public void restoreLegacyLayout() { StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); @@ -57,6 +71,11 @@ public void testLayoutsAreIndistinguishableToCallers() { assertEquals(legacy.buckets(), tenantRooted.buckets(), "Buckets differ between layouts; every url-bearing comparison below would be meaningless"); + CROSS_RUN_STABLE_CAPTURES.forEach((scenario, names) -> names.forEach(name -> { + assertEquals(legacy.captures().get(scenario).get(name), tenantRooted.captures().get(scenario).get(name), + "Captured value '" + name + "' of scenario '" + scenario + "' differs between the layouts"); + })); + List divergences = ResponseDiffer.diff(legacy.responses(), tenantRooted.responses()); write("divergences.txt", divergences.stream().map(Divergence::describe).collect(Collectors.joining("\n\n"))); diff --git a/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json b/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json index a4cb9cd45..af98df20b 100644 --- a/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json +++ b/server/src/test/resources/layout-diff/corpus/02-prompts-and-folders.json @@ -40,7 +40,21 @@ "name": "list-paged", "method": "GET", "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", - "query": "recursive=true&limit=2" + "query": "recursive=true&limit=2", + "capture": {"pageToken": {"at": "/nextToken"}} + }, + { + "name": "list-paged-continue", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", + "query": "recursive=true&limit=2&token=${pageToken}", + "capture": {"pageToken2": {"at": "/nextToken"}} + }, + { + "name": "list-paged-end", + "method": "GET", + "path": "/v1/metadata/prompts/${bucket1}/promptsdiff/", + "query": "recursive=true&limit=2&token=${pageToken2}" }, { "name": "delete-nested", diff --git a/server/src/test/resources/layout-diff/expected-divergences.json b/server/src/test/resources/layout-diff/expected-divergences.json index a4b5af482..fe51488c7 100644 --- a/server/src/test/resources/layout-diff/expected-divergences.json +++ b/server/src/test/resources/layout-diff/expected-divergences.json @@ -1,9 +1 @@ -[ - { - "scenario": "prompts-and-folders", - "step": "list-paged", - "field": "body", - "reason": "nextToken is the blob store's own continuation marker, passed through verbatim, so it carries the physical path and changes shape with the layout. Accepted: the token is opaque to clients and short-lived \u2014 it is handed straight back within one listing, and each layout accepts its own. The only case it bites is a token minted before a cutover and replayed after it, which fails one 'next page' click and succeeds on retry; that belongs to P2 migration, not to this change. Scope note: that this token exposes the storage prefix and the decrypted bucket location to callers is a separate, pre-existing defect on development \u2014 ResourceService is untouched by this stack and the legacy run leaks it identically. Tracked privately, not here.", - "issue": "https://github.com/epam/ai-dial-core/issues/1863" - } -] +[] From 722a89ccd1ab208e0e1f3313335aa884d6705890 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Mon, 7 Sep 2026 14:27:36 +0300 Subject: [PATCH 3/3] test: follow the layout settings under the storage block #1870 Co-Authored-By: Claude Fable 5 --- .../java/com/epam/aidial/core/server/layout/DialInstance.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java index 178490b00..dc7ab5d2a 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java @@ -127,7 +127,7 @@ private AiDial start(JsonObject layoutSettings, int redisPort) throws Exception JsonObject settings = AiDial.settings() .mergeIn(new JsonObject(overrides), true) - .mergeIn(new JsonObject().put("storageLayout", layoutSettings), true); + .mergeIn(new JsonObject().put("storage", new JsonObject().put("layout", layoutSettings)), true); AiDial instance = new AiDial(); instance.setSettings(settings);