Skip to content

Commit 08faca3

Browse files
committed
Remove redundant Jackson type polymorphism for DataSourceResult
The direct-query transport already carries the concrete result type as a separate 'dataSourceType' string and dispatches on it via a switch in both writeTo() and the StreamInput constructor. The Jackson polymorphism on DataSourceResult (@JsonTypeInfo/@JsonSubTypes) was therefore never exercised through the interface, and the earlier @JsonTypeInfo(Id.NONE) workaround plus the addTypeFieldToJson()/contains("\"type\":") guard existed only to satisfy that unused machinery -- which is what let a Prometheus metric label named 'type' break deserialization (#5684). Remove the polymorphism scaffolding entirely: drop the discriminator annotations, read PrometheusResult directly, and delete the type injection. Add a writeTo()->StreamInput round-trip test that carries a metric label named 'type' to cover the transport path. Note: this changes the serialized shape (no root 'type' in the result JSON). Dispatch is unaffected (the separate dataSourceType string), and the API is @opensearch.experimental, so the only impact is a bounded, one-directional mixed-version window during a rolling upgrade. Signed-off-by: Nagendra Mohan <nagendramohan1990@gmail.com>
1 parent 2b5e994 commit 08faca3

4 files changed

Lines changed: 43 additions & 29 deletions

File tree

direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/ExecuteDirectQueryActionResponse.java

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -131,19 +131,11 @@ private Map<String, DataSourceResult> parseResult(
131131
Map<String, DataSourceResult> parsedResults = new HashMap<>();
132132

133133
try {
134-
// Add type to JSON if it doesn't already have it
135-
final String resultWithType;
136-
if (!rawResult.contains("\"type\":")) {
137-
resultWithType = addTypeFieldToJson(rawResult, dataSourceType);
138-
} else {
139-
resultWithType = rawResult;
140-
}
141-
142134
DataSourceResult result;
143135
// Parse based on the determined data source type
144136
switch (dataSourceType.toLowerCase()) {
145137
case "prometheus":
146-
result = OBJECT_MAPPER.readValue(resultWithType, PrometheusResult.class);
138+
result = OBJECT_MAPPER.readValue(rawResult, PrometheusResult.class);
147139
break;
148140
// Add cases for other data source types as they're implemented
149141
default:
@@ -163,15 +155,4 @@ private Map<String, DataSourceResult> parseResult(
163155

164156
return parsedResults;
165157
}
166-
167-
/**
168-
* Adds a type field to the JSON string for proper polymorphic deserialization.
169-
*
170-
* @param rawJson The raw JSON string without a type field
171-
* @param type The type to add
172-
* @return Modified JSON string with type field
173-
*/
174-
private String addTypeFieldToJson(String rawJson, String type) {
175-
return rawJson.replaceFirst("\\{", "{\"type\":\"" + type + "\",");
176-
}
177158
}

direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/datasource/DataSourceResult.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@
55

66
package org.opensearch.sql.directquery.transport.model.datasource;
77

8-
import com.fasterxml.jackson.annotation.JsonSubTypes;
9-
import com.fasterxml.jackson.annotation.JsonTypeInfo;
10-
118
/**
129
*
1310
* @opensearch.experimental
1411
*
1512
* Interface for results from various data sources.
13+
*
14+
* <p>Concrete result types are dispatched by the {@code dataSourceType} string carried alongside
15+
* the serialized payload in the transport protocol (see {@code ExecuteDirectQueryActionResponse}),
16+
* so no in-JSON Jackson type discriminator is used here.
1617
*/
17-
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")
18-
@JsonSubTypes({@JsonSubTypes.Type(value = PrometheusResult.class, name = "prometheus")})
1918
public interface DataSourceResult {}

direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/datasource/PrometheusResult.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77

88
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
99
import com.fasterxml.jackson.annotation.JsonProperty;
10-
import com.fasterxml.jackson.annotation.JsonTypeInfo;
11-
import com.fasterxml.jackson.annotation.JsonTypeName;
1210
import java.util.List;
1311
import java.util.Map;
1412
import lombok.Getter;
@@ -22,8 +20,6 @@
2220
*/
2321
@Getter
2422
@Setter
25-
@JsonTypeName("prometheus")
26-
@JsonTypeInfo(use = JsonTypeInfo.Id.NONE)
2723
@JsonIgnoreProperties(ignoreUnknown = true)
2824
public class PrometheusResult implements DataSourceResult {
2925

direct-query/src/test/java/org/opensearch/sql/directquery/transport/model/ExecuteDirectQueryActionResponseTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.ByteArrayOutputStream;
1616
import java.io.IOException;
1717
import java.util.HashMap;
18+
import java.util.List;
1819
import java.util.Map;
1920
import org.junit.jupiter.api.Test;
2021
import org.opensearch.core.common.io.stream.InputStreamStreamInput;
@@ -147,6 +148,43 @@ public void testStreamSerializationWithPrometheusResult() throws IOException {
147148
assertInstanceOf(PrometheusResult.class, deserializedResponse.getResults().get("prom-ds-1"));
148149
}
149150

151+
@Test
152+
public void testStreamSerializationPreservesTypeMetricLabel() throws IOException {
153+
// Regression for #5684: a metric label literally named "type" must survive the
154+
// writeTo() -> StreamInput round-trip (the transport path), not only parseResult().
155+
Map<String, DataSourceResult> results = new HashMap<>();
156+
PrometheusResult prometheusResult = new PrometheusResult();
157+
prometheusResult.setResultType("vector");
158+
PrometheusResult.PrometheusResultItem item = new PrometheusResult.PrometheusResultItem();
159+
Map<String, String> metric = new HashMap<>();
160+
metric.put("type", "counter");
161+
metric.put("__name__", "http_requests_total");
162+
item.setMetric(metric);
163+
prometheusResult.setResult(List.of(item));
164+
results.put("prom-ds-1", prometheusResult);
165+
166+
ExecuteDirectQueryActionResponse response =
167+
new ExecuteDirectQueryActionResponse("query-type-label", results, "session-type-label");
168+
169+
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
170+
StreamOutput streamOutput = new OutputStreamStreamOutput(outputStream);
171+
response.writeTo(streamOutput);
172+
streamOutput.close();
173+
174+
StreamInput streamInput =
175+
new InputStreamStreamInput(new ByteArrayInputStream(outputStream.toByteArray()));
176+
ExecuteDirectQueryActionResponse deserialized =
177+
new ExecuteDirectQueryActionResponse(streamInput);
178+
streamInput.close();
179+
180+
PrometheusResult out = (PrometheusResult) deserialized.getResults().get("prom-ds-1");
181+
assertInstanceOf(PrometheusResult.class, out);
182+
assertEquals("vector", out.getResultType());
183+
assertEquals(1, out.getResult().size());
184+
assertEquals("counter", out.getResult().get(0).getMetric().get("type"));
185+
assertEquals("http_requests_total", out.getResult().get(0).getMetric().get("__name__"));
186+
}
187+
150188
@Test
151189
public void testStreamSerializationWithNullSessionId() throws IOException {
152190
String queryId = "query-no-session";

0 commit comments

Comments
 (0)