From 7c39caaf287fec3741307ec1772d9f8c7ad32b4a Mon Sep 17 00:00:00 2001 From: Kiryl_Kurnosenka Date: Tue, 9 Jun 2026 22:01:15 +0300 Subject: [PATCH 1/3] fix: return ETag header on GET for config resource endpoints Co-Authored-By: Claude Sonnet 4.6 --- .../controller/ConfigResourceController.java | 71 +++++++++++++------ 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java index 53429902a..dad1fe233 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java @@ -207,9 +207,14 @@ private Future handleSingleGet(Map source, // client-supplied tag matches. final T matchedItem = item; final String matched = canonicalId(); - return respondNotModifiedIfMatched(matched) - .onSuccess(notModified -> { - if (!notModified) { + return evaluateConditionalGet() + .onSuccess(result -> { + if (result.respondNotModified()) { + context.respond(result.notModified()); + } else { + if (result.etag() != null) { + context.putHeader(HttpHeaders.ETAG, result.etag()).exposeHeaders(); + } context.respond(HttpStatus.OK, projector.apply(matched, matchedItem)); } }) @@ -225,38 +230,32 @@ private Future handleSingleGet(Map source, } /** - * Emits a {@code 304 Not Modified} if the matched entity's stored blob has an ETag - * that matches the client's {@code If-None-Match} header. The blob-metadata fetch - * runs on the blocking executor to keep the Vert.x event loop unblocked; the response is - * written back on the event loop via the future-chain completion. + * Fetches blob metadata and evaluates RFC 7232 conditional headers, returning a + * {@link GetResult} that carries the outcome: a 304 exception (when {@code If-None-Match} + * matches) or the blob-backed ETag for the caller to include on a 200 response. + * The metadata fetch runs on the blocking executor; response dispatch stays on the event loop. */ - private Future respondNotModifiedIfMatched(String matchedKey) { + private Future evaluateConditionalGet() { ResourceDescriptor descriptor = singleEntityDescriptor(); if (descriptor == null) { - return Future.succeededFuture(false); + return Future.succeededFuture(GetResult.withoutEtag()); } EtagHeader etag = ProxyUtil.etag(context.getRequest()); - return taskExecutor.submit(() -> { + return taskExecutor.submit(() -> { ResourceItemMetadata meta = resourceService.getResourceMetadata(descriptor); if (meta == null || meta.getEtag() == null) { - return null; + return GetResult.withoutEtag(); } try { etag.validate(meta.getEtag()); } catch (HttpException e) { if (e.getStatus() == HttpStatus.NOT_MODIFIED) { - return e; + return GetResult.withNotModified(e); } // Other status codes (e.g. If-Match failure) are not surfaced here — GET ignores // If-Match per RFC 7232; only If-None-Match drives 304/200 on GET. } - return null; - }).map(notModified -> { - if (notModified != null) { - context.respond(notModified); - return true; - } - return false; + return GetResult.withEtag(meta.getEtag()); }); } @@ -299,7 +298,7 @@ private String canonicalId() { return entityType + "/" + bucket + "/" + path; } - private Future handleSchemaGet(Config config, boolean admin) throws JsonProcessingException { + private Future handleSchemaGet(Config config, boolean admin) { Map schemas = config.getApplicationTypeSchemas(); Map invalid = mergedConfigStore.getInvalidEntities() .getOrDefault(ResourceTypes.APP_TYPE_SCHEMA, Map.of()); @@ -313,9 +312,14 @@ private Future handleSchemaGet(Config config, boolean admin) throws JsonProce if (schemaJson != null) { final String matched = canonicalId(); final String json = schemaJson; - return respondNotModifiedIfMatched(matched) - .onSuccess(notModified -> { - if (!notModified) { + return evaluateConditionalGet() + .onSuccess(result -> { + if (result.respondNotModified()) { + context.respond(result.notModified()); + } else { + if (result.etag() != null) { + context.putHeader(HttpHeaders.ETAG, result.etag()).exposeHeaders(); + } try { context.respond(HttpStatus.OK, projectSchemaItem(matched, json)); } catch (JsonProcessingException e) { @@ -835,4 +839,25 @@ private record WriteSpec( boolean isKey ) {} + private record GetResult(HttpException notModified, String etag) { + static GetResult withNotModified(HttpException e) { + if (e.getStatus() != HttpStatus.NOT_MODIFIED) { + throw new IllegalArgumentException("Expected 304 NOT_MODIFIED, got: " + e.getStatus()); + } + return new GetResult(e, null); + } + + static GetResult withEtag(String etag) { + return new GetResult(null, etag); + } + + static GetResult withoutEtag() { + return new GetResult(null, null); + } + + boolean respondNotModified() { + return notModified != null; + } + } + } From a0355b482087a865a835e35d8bc305278b7c5124 Mon Sep 17 00:00:00 2001 From: Kiryl_Kurnosenka Date: Tue, 9 Jun 2026 22:25:32 +0300 Subject: [PATCH 2/3] test: assert ETag header on GET 200 responses in conditional-header tests Co-Authored-By: Claude Sonnet 4.6 --- .../ConfigResourceConditionalHeaderTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java b/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java index 6090afd97..a1e0f2634 100644 --- a/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java @@ -65,6 +65,20 @@ void testGet200OnStaleIfNoneMatch() { verify(stale, 200); assertTrue(stale.body().contains("\"name\":\"models/public/cond-stale\""), () -> "Expected full body on stale If-None-Match: " + stale.body()); + assertNotNull(stale.headers().get("etag"), + () -> "GET 200 must include ETag header even on stale If-None-Match: " + stale.headers()); + } + + @Test + void testGet200IncludesEtagHeader() { + verify(send(HttpMethod.PUT, "/v1/models/public/cond-etag-get", null, MODEL_BODY, + "authorization", "admin", "If-None-Match", "*"), 200); + + Response get = send(HttpMethod.GET, "/v1/models/public/cond-etag-get", null, "", + "authorization", "admin"); + verify(get, 200); + assertNotNull(get.headers().get("etag"), + () -> "Plain GET 200 must include ETag header: " + get.headers()); } @Test From b74172f1386089e1781901aa2b483149fbd312e1 Mon Sep 17 00:00:00 2001 From: Kiryl_Kurnosenka Date: Fri, 12 Jun 2026 14:14:35 +0300 Subject: [PATCH 3/3] fix: honor If-Match on GET for config resource endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed the try-catch in evaluateConditionalGet() that was silently swallowing If-Match failures (412) on GET requests. RFC 7232 does not exempt GET from If-Match evaluation; only If-None-Match has special GET/HEAD semantics (304). Both 304 and 412 now propagate as HttpException through the Future failure path to handleWriteError, consistent with how ResourceController handles conditional headers. Also simplified GetResult away — evaluateConditionalGet now returns Future (the ETag or null). Co-Authored-By: Claude Sonnet 4.6 --- .../controller/ConfigResourceController.java | 80 +++++-------------- .../ConfigResourceConditionalHeaderTest.java | 24 ++++++ 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java index dad1fe233..c4bfedd77 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java @@ -156,7 +156,7 @@ public Future handle() throws Exception { return respondMethodNotAllowed(); } - private Future handleGet() throws JsonProcessingException { + private Future handleGet() { Config config = context.getConfig(); boolean admin = authorizationService.isAdmin(context); // Per-entity GET is blob-only (slice U.1): only canonical-ID lookups resolve here. @@ -208,15 +208,11 @@ private Future handleSingleGet(Map source, final T matchedItem = item; final String matched = canonicalId(); return evaluateConditionalGet() - .onSuccess(result -> { - if (result.respondNotModified()) { - context.respond(result.notModified()); - } else { - if (result.etag() != null) { - context.putHeader(HttpHeaders.ETAG, result.etag()).exposeHeaders(); - } - context.respond(HttpStatus.OK, projector.apply(matched, matchedItem)); + .onSuccess(etag -> { + if (etag != null) { + context.putHeader(HttpHeaders.ETAG, etag).exposeHeaders(); } + context.respond(HttpStatus.OK, projector.apply(matched, matchedItem)); }) .onFailure(this::handleWriteError); } @@ -230,32 +226,25 @@ private Future handleSingleGet(Map source, } /** - * Fetches blob metadata and evaluates RFC 7232 conditional headers, returning a - * {@link GetResult} that carries the outcome: a 304 exception (when {@code If-None-Match} - * matches) or the blob-backed ETag for the caller to include on a 200 response. + * Fetches blob metadata and evaluates RFC 7232 conditional headers, returning the blob-backed + * ETag (or {@code null} when no blob exists) for the caller to include on a 200 response. + * Throws {@link HttpException} (304 or 412) when a conditional header fires — the caller's + * {@code .onFailure} handler dispatches it via {@link #handleWriteError}. * The metadata fetch runs on the blocking executor; response dispatch stays on the event loop. */ - private Future evaluateConditionalGet() { + private Future evaluateConditionalGet() { ResourceDescriptor descriptor = singleEntityDescriptor(); if (descriptor == null) { - return Future.succeededFuture(GetResult.withoutEtag()); + return Future.succeededFuture(null); } EtagHeader etag = ProxyUtil.etag(context.getRequest()); return taskExecutor.submit(() -> { ResourceItemMetadata meta = resourceService.getResourceMetadata(descriptor); if (meta == null || meta.getEtag() == null) { - return GetResult.withoutEtag(); - } - try { - etag.validate(meta.getEtag()); - } catch (HttpException e) { - if (e.getStatus() == HttpStatus.NOT_MODIFIED) { - return GetResult.withNotModified(e); - } - // Other status codes (e.g. If-Match failure) are not surfaced here — GET ignores - // If-Match per RFC 7232; only If-None-Match drives 304/200 on GET. + return null; } - return GetResult.withEtag(meta.getEtag()); + etag.validate(meta.getEtag()); + return meta.getEtag(); }); } @@ -313,18 +302,14 @@ private Future handleSchemaGet(Config config, boolean admin) { final String matched = canonicalId(); final String json = schemaJson; return evaluateConditionalGet() - .onSuccess(result -> { - if (result.respondNotModified()) { - context.respond(result.notModified()); - } else { - if (result.etag() != null) { - context.putHeader(HttpHeaders.ETAG, result.etag()).exposeHeaders(); - } - try { - context.respond(HttpStatus.OK, projectSchemaItem(matched, json)); - } catch (JsonProcessingException e) { - context.respond(HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage()); - } + .onSuccess(etag -> { + if (etag != null) { + context.putHeader(HttpHeaders.ETAG, etag).exposeHeaders(); + } + try { + context.respond(HttpStatus.OK, projectSchemaItem(matched, json)); + } catch (JsonProcessingException e) { + context.respond(HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage()); } }) .onFailure(this::handleWriteError); @@ -839,25 +824,4 @@ private record WriteSpec( boolean isKey ) {} - private record GetResult(HttpException notModified, String etag) { - static GetResult withNotModified(HttpException e) { - if (e.getStatus() != HttpStatus.NOT_MODIFIED) { - throw new IllegalArgumentException("Expected 304 NOT_MODIFIED, got: " + e.getStatus()); - } - return new GetResult(e, null); - } - - static GetResult withEtag(String etag) { - return new GetResult(null, etag); - } - - static GetResult withoutEtag() { - return new GetResult(null, null); - } - - boolean respondNotModified() { - return notModified != null; - } - } - } diff --git a/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java b/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java index a1e0f2634..c309df658 100644 --- a/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/ConfigResourceConditionalHeaderTest.java @@ -69,6 +69,30 @@ void testGet200OnStaleIfNoneMatch() { () -> "GET 200 must include ETag header even on stale If-None-Match: " + stale.headers()); } + @Test + void testGet200OnMatchingIfMatch() { + Response put = send(HttpMethod.PUT, "/v1/models/public/cond-get-ifmatch-ok", null, MODEL_BODY, + "authorization", "admin", "If-None-Match", "*"); + verify(put, 200); + String etag = put.headers().get("etag"); + assertNotNull(etag, () -> "PUT must emit an ETag header: " + put.headers()); + + Response get = send(HttpMethod.GET, "/v1/models/public/cond-get-ifmatch-ok", null, "", + "authorization", "admin", "If-Match", etag); + verify(get, 200); + assertNotNull(get.headers().get("etag"), "GET 200 must include ETag header"); + } + + @Test + void testGet412OnStaleIfMatch() { + verify(send(HttpMethod.PUT, "/v1/models/public/cond-get-ifmatch-fail", null, MODEL_BODY, + "authorization", "admin", "If-None-Match", "*"), 200); + + Response get = send(HttpMethod.GET, "/v1/models/public/cond-get-ifmatch-fail", null, "", + "authorization", "admin", "If-Match", "\"wrong-etag\""); + verify(get, 412); + } + @Test void testGet200IncludesEtagHeader() { verify(send(HttpMethod.PUT, "/v1/models/public/cond-etag-get", null, MODEL_BODY,