Skip to content

Commit bf0edbb

Browse files
committed
remove unused etag
Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent abeb08d commit bf0edbb

2 files changed

Lines changed: 3 additions & 115 deletions

File tree

plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLGrammarAction.java

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -34,31 +34,6 @@
3434
* <li>Channel and mode names
3535
* </ul>
3636
*
37-
* <p>The bundle is cached in memory and versioned via ETag (grammar hash). Clients should cache
38-
* locally and only refetch when ETag changes.
39-
*
40-
* <p>Example request:
41-
*
42-
* <pre>
43-
* GET /_plugins/_ppl/_grammar
44-
* If-None-Match: "sha256:abc123..."
45-
* </pre>
46-
*
47-
* <p>Response headers:
48-
*
49-
* <pre>
50-
* HTTP/1.1 200 OK
51-
* ETag: "sha256:abc123..."
52-
* Cache-Control: public, max-age=3600
53-
* Content-Type: application/json
54-
* </pre>
55-
*
56-
* <p>Or if client has latest:
57-
*
58-
* <pre>
59-
* HTTP/1.1 304 Not Modified
60-
* ETag: "sha256:abc123..."
61-
* </pre>
6237
*/
6338
@Log4j2
6439
public class RestPPLGrammarAction extends BaseRestHandler {
@@ -90,37 +65,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
9065
// Get or build artifact (lazy initialization)
9166
AutocompleteArtifact artifact = getOrBuildArtifact();
9267

93-
String grammarHash = artifact.getGrammarHash();
94-
String ifNoneMatch = request.header("If-None-Match");
95-
96-
// Strip quotes from ETag if present ("sha256:..." → sha256:...)
97-
if (ifNoneMatch != null && ifNoneMatch.startsWith("\"") && ifNoneMatch.endsWith("\"")) {
98-
ifNoneMatch = ifNoneMatch.substring(1, ifNoneMatch.length() - 1);
99-
}
100-
101-
// Return 304 Not Modified if client has latest version
102-
if (grammarHash.equals(ifNoneMatch)) {
103-
log.debug("Client has latest grammar (ETag match), returning 304");
104-
BytesRestResponse response =
105-
new BytesRestResponse(RestStatus.NOT_MODIFIED, "application/json", "");
106-
response.addHeader("ETag", "\"" + grammarHash + "\"");
107-
response.addHeader("Cache-Control", "public, max-age=3600");
108-
channel.sendResponse(response);
109-
return;
110-
}
111-
112-
// Serialize artifact to JSON
113-
log.debug("Serializing grammar to JSON (hash: {})", grammarHash);
11468
XContentBuilder builder = channel.newBuilder();
11569
serializeArtifact(builder, artifact);
11670

11771
BytesRestResponse response = new BytesRestResponse(RestStatus.OK, builder);
118-
119-
// Add caching headers
120-
response.addHeader("ETag", "\"" + grammarHash + "\"");
121-
response.addHeader("Cache-Control", "public, max-age=3600");
122-
response.addHeader("Content-Type", "application/json; charset=UTF-8");
123-
12472
log.info("Returning PPL grammar (size: {} bytes)", response.content().length());
12573
channel.sendResponse(response);
12674

plugin/src/test/java/org/opensearch/sql/plugin/rest/RestPPLGrammarActionTest.java

Lines changed: 3 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import java.io.ByteArrayOutputStream;
1414
import java.nio.charset.StandardCharsets;
15-
import java.util.Collections;
1615
import org.junit.Before;
1716
import org.junit.Test;
1817
import org.opensearch.core.rest.RestStatus;
@@ -31,7 +30,6 @@
3130
*
3231
* <ul>
3332
* <li>200 OK response with grammar bundle
34-
* <li>304 Not Modified when ETag matches
3533
* <li>Grammar structure validation
3634
* <li>Cache behavior
3735
* </ul>
@@ -62,36 +60,19 @@ public void testRoutes() {
6260

6361
@Test
6462
public void testGetGrammar_ReturnsBundle() throws Exception {
65-
// Create request without ETag
6663
FakeRestRequest request =
6764
new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
6865
.withMethod(RestRequest.Method.GET)
6966
.withPath("/_plugins/_ppl/_grammar")
70-
.withHeaders(Collections.emptyMap())
7167
.build();
7268

73-
// Create mock channel to capture response
7469
MockRestChannel channel = new MockRestChannel(request, true);
75-
76-
// Execute request
7770
action.prepareRequest(request, client).accept(channel);
7871

79-
// Verify response
8072
RestResponse response = channel.getResponse();
8173
assertNotNull("Response should not be null", response);
8274
assertEquals("Should return 200 OK", RestStatus.OK, response.status());
8375

84-
// Verify ETag header present
85-
String etag = response.getHeaders().get("ETag").get(0);
86-
assertNotNull("ETag header should be present", etag);
87-
assertTrue("ETag should start with sha256:", etag.contains("sha256:"));
88-
89-
// Verify Cache-Control header
90-
String cacheControl = response.getHeaders().get("Cache-Control").get(0);
91-
assertNotNull("Cache-Control header should be present", cacheControl);
92-
assertTrue("Should allow caching", cacheControl.contains("max-age=3600"));
93-
94-
// Verify response content
9576
String content = new String(response.content().array(), StandardCharsets.UTF_8);
9677
assertTrue("Should contain bundleVersion", content.contains("\"bundleVersion\":"));
9778
assertTrue("Should contain grammarHash", content.contains("\"grammarHash\":"));
@@ -101,39 +82,6 @@ public void testGetGrammar_ReturnsBundle() throws Exception {
10182
assertTrue("Should contain symbolicNames", content.contains("\"symbolicNames\":"));
10283
}
10384

104-
@Test
105-
public void testGetGrammar_Returns304WhenETagMatches() throws Exception {
106-
// First request to get ETag
107-
FakeRestRequest firstRequest =
108-
new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
109-
.withMethod(RestRequest.Method.GET)
110-
.withPath("/_plugins/_ppl/_grammar")
111-
.build();
112-
113-
MockRestChannel firstChannel = new MockRestChannel(firstRequest, true);
114-
action.prepareRequest(firstRequest, client).accept(firstChannel);
115-
116-
RestResponse firstResponse = firstChannel.getResponse();
117-
String etag = firstResponse.getHeaders().get("ETag").get(0);
118-
119-
// Second request with matching ETag
120-
FakeRestRequest secondRequest =
121-
new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
122-
.withMethod(RestRequest.Method.GET)
123-
.withPath("/_plugins/_ppl/_grammar")
124-
.withHeaders(Collections.singletonMap("If-None-Match", Collections.singletonList(etag)))
125-
.build();
126-
127-
MockRestChannel secondChannel = new MockRestChannel(secondRequest, true);
128-
action.prepareRequest(secondRequest, client).accept(secondChannel);
129-
130-
RestResponse secondResponse = secondChannel.getResponse();
131-
assertEquals(
132-
"Should return 304 Not Modified when ETag matches",
133-
RestStatus.NOT_MODIFIED,
134-
secondResponse.status());
135-
}
136-
13785
@Test
13886
public void testGetGrammar_ArtifactIsCached() throws Exception {
13987
// Make two requests
@@ -160,11 +108,6 @@ public void testGetGrammar_ArtifactIsCached() throws Exception {
160108
action.prepareRequest(request2, client).accept(channel2);
161109
long elapsed2 = System.currentTimeMillis() - startTime2;
162110

163-
// Both should return same ETag
164-
String etag1 = channel1.getResponse().getHeaders().get("ETag").get(0);
165-
String etag2 = channel2.getResponse().getHeaders().get("ETag").get(0);
166-
assertEquals("ETags should match", etag1, etag2);
167-
168111
// Second request should be faster (artifact cached)
169112
assertTrue(
170113
"Second request should be faster due to caching (elapsed1="
@@ -186,12 +129,12 @@ public void testInvalidateCache() throws Exception {
186129

187130
MockRestChannel channel1 = new MockRestChannel(request1, true);
188131
action.prepareRequest(request1, client).accept(channel1);
189-
String etag1 = channel1.getResponse().getHeaders().get("ETag").get(0);
132+
assertEquals(RestStatus.OK, channel1.getResponse().status());
190133

191134
// Invalidate cache
192135
action.invalidateCache();
193136

194-
// Get grammar again
137+
// Get grammar again — should still return 200 OK with same content
195138
FakeRestRequest request2 =
196139
new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
197140
.withMethod(RestRequest.Method.GET)
@@ -200,10 +143,7 @@ public void testInvalidateCache() throws Exception {
200143

201144
MockRestChannel channel2 = new MockRestChannel(request2, true);
202145
action.prepareRequest(request2, client).accept(channel2);
203-
String etag2 = channel2.getResponse().getHeaders().get("ETag").get(0);
204-
205-
// ETags should still match (same grammar)
206-
assertEquals("ETags should match after cache invalidation", etag1, etag2);
146+
assertEquals(RestStatus.OK, channel2.getResponse().status());
207147
}
208148

209149
/** Mock RestChannel to capture responses */

0 commit comments

Comments
 (0)