Skip to content

Commit b7edbe7

Browse files
gaobinlongdblock
gaobinlong
and
dblock
committed
Fix class_cast_exception when passing int to _version and other metadata fields in ingest simulate API (opensearch-project#10101)
* Fix class_cast_exception when passing int to _version and other metadata fields in ingest simulate API Signed-off-by: Gao Binlong <gbinlong@amazon.com> * modify change log Signed-off-by: Gao Binlong <gbinlong@amazon.com> * Add more tests Signed-off-by: Gao Binlong <gbinlong@amazon.com> --------- Signed-off-by: Gao Binlong <gbinlong@amazon.com> Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com> Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com> (cherry picked from commit fdaa438)
1 parent 4cfcedd commit b7edbe7

File tree

4 files changed

+215
-11
lines changed

4 files changed

+215
-11
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1010
### Deprecated
1111
### Removed
1212
### Fixed
13+
- Fix class_cast_exception when passing int to _version and other metadata fields in ingest simulate API ([#10101](https://github.com/opensearch-project/OpenSearch/pull/10101))
14+
1315
### Security
1416

1517
[Unreleased]: https://github.com/opensearch-project/OpenSearch/compare/1.3.12...HEAD

‎modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/90_simulate.yml‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,3 +992,140 @@ teardown:
992992
}
993993
- match: { error.root_cause.0.type: "illegal_argument_exception" }
994994
- match: { error.root_cause.0.reason: "Pipeline processor configured for non-existent pipeline [____pipeline_doesnot_exist___]" }
995+
996+
---
997+
"Test simulate with docs containing metadata fields":
998+
- do:
999+
ingest.simulate:
1000+
body: >
1001+
{
1002+
"pipeline": {
1003+
"description": "_description",
1004+
"processors": [
1005+
{
1006+
"set" : {
1007+
"field": "field2",
1008+
"value": "foo"
1009+
}
1010+
}
1011+
]
1012+
},
1013+
"docs": [
1014+
{
1015+
"_index": "index",
1016+
"_id": "id",
1017+
"_routing": "foo",
1018+
"_version": 100,
1019+
"_if_seq_no": 12333333333333333,
1020+
"_if_primary_term": 1,
1021+
"_source": {
1022+
"foo": "bar"
1023+
}
1024+
}
1025+
]
1026+
}
1027+
1028+
- length: { docs: 1 }
1029+
- match: { docs.0.doc._index: "index" }
1030+
- match: { docs.0.doc._id: "id" }
1031+
- match: { docs.0.doc._routing: "foo" }
1032+
- match: { docs.0.doc._version: "100" }
1033+
- match: { docs.0.doc._if_seq_no: "12333333333333333" }
1034+
- match: { docs.0.doc._if_primary_term: "1" }
1035+
- match: { docs.0.doc._source.foo: "bar" }
1036+
1037+
- do:
1038+
catch: bad_request
1039+
ingest.simulate:
1040+
body: >
1041+
{
1042+
"pipeline": {
1043+
"description": "_description",
1044+
"processors": [
1045+
{
1046+
"set" : {
1047+
"field" : "field2",
1048+
"value": "foo"
1049+
}
1050+
}
1051+
]
1052+
},
1053+
"docs": [
1054+
{
1055+
"_index": "index",
1056+
"_id": "id",
1057+
"_routing": "foo",
1058+
"_version": "bar",
1059+
"_source": {
1060+
"foo": "bar"
1061+
}
1062+
}
1063+
]
1064+
}
1065+
- match: { status: 400 }
1066+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
1067+
- match: { error.root_cause.0.reason: "Failed to parse parameter [_version], only int or long is accepted" }
1068+
1069+
- do:
1070+
catch: bad_request
1071+
ingest.simulate:
1072+
body: >
1073+
{
1074+
"pipeline": {
1075+
"description": "_description",
1076+
"processors": [
1077+
{
1078+
"set" : {
1079+
"field" : "field2",
1080+
"value": "foo"
1081+
}
1082+
}
1083+
]
1084+
},
1085+
"docs": [
1086+
{
1087+
"_index": "index",
1088+
"_id": "id",
1089+
"_routing": "foo",
1090+
"_if_seq_no": "123",
1091+
"_source": {
1092+
"foo": "bar"
1093+
}
1094+
}
1095+
]
1096+
}
1097+
- match: { status: 400 }
1098+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
1099+
- match: { error.root_cause.0.reason: "Failed to parse parameter [_if_seq_no], only int or long is accepted" }
1100+
1101+
- do:
1102+
catch: bad_request
1103+
ingest.simulate:
1104+
body: >
1105+
{
1106+
"pipeline": {
1107+
"description": "_description",
1108+
"processors": [
1109+
{
1110+
"set" : {
1111+
"field" : "field2",
1112+
"value": "foo"
1113+
}
1114+
}
1115+
]
1116+
},
1117+
"docs": [
1118+
{
1119+
"_index": "index",
1120+
"_id": "id",
1121+
"_routing": "foo",
1122+
"_if_primary_term": "1",
1123+
"_source": {
1124+
"foo": "bar"
1125+
}
1126+
}
1127+
]
1128+
}
1129+
- match: { status: 400 }
1130+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
1131+
- match: { error.root_cause.0.reason: "Failed to parse parameter [_if_primary_term], only int or long is accepted" }

‎server/src/main/java/org/opensearch/action/ingest/SimulatePipelineRequest.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,12 @@ private static List<IngestDocument> parseDocs(Map<String, Object> config) {
205205
String routing = ConfigurationUtils.readOptionalStringOrIntProperty(null, null, dataMap, Metadata.ROUTING.getFieldName());
206206
Long version = null;
207207
if (dataMap.containsKey(Metadata.VERSION.getFieldName())) {
208-
version = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.VERSION.getFieldName());
208+
Object versionFieldValue = ConfigurationUtils.readObject(null, null, dataMap, Metadata.VERSION.getFieldName());
209+
if (versionFieldValue instanceof Integer || versionFieldValue instanceof Long) {
210+
version = ((Number) versionFieldValue).longValue();
211+
} else {
212+
throw new IllegalArgumentException("Failed to parse parameter [_version], only int or long is accepted");
213+
}
209214
}
210215
VersionType versionType = null;
211216
if (dataMap.containsKey(Metadata.VERSION_TYPE.getFieldName())) {
@@ -215,12 +220,25 @@ private static List<IngestDocument> parseDocs(Map<String, Object> config) {
215220
}
216221
IngestDocument ingestDocument = new IngestDocument(index, type, id, routing, version, versionType, document);
217222
if (dataMap.containsKey(Metadata.IF_SEQ_NO.getFieldName())) {
218-
Long ifSeqNo = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.IF_SEQ_NO.getFieldName());
219-
ingestDocument.setFieldValue(Metadata.IF_SEQ_NO.getFieldName(), ifSeqNo);
223+
Object ifSeqNoFieldValue = ConfigurationUtils.readObject(null, null, dataMap, Metadata.IF_SEQ_NO.getFieldName());
224+
if (ifSeqNoFieldValue instanceof Integer || ifSeqNoFieldValue instanceof Long) {
225+
ingestDocument.setFieldValue(Metadata.IF_SEQ_NO.getFieldName(), ((Number) ifSeqNoFieldValue).longValue());
226+
} else {
227+
throw new IllegalArgumentException("Failed to parse parameter [_if_seq_no], only int or long is accepted");
228+
}
220229
}
221230
if (dataMap.containsKey(Metadata.IF_PRIMARY_TERM.getFieldName())) {
222-
Long ifPrimaryTerm = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.IF_PRIMARY_TERM.getFieldName());
223-
ingestDocument.setFieldValue(Metadata.IF_PRIMARY_TERM.getFieldName(), ifPrimaryTerm);
231+
Object ifPrimaryTermFieldValue = ConfigurationUtils.readObject(
232+
null,
233+
null,
234+
dataMap,
235+
Metadata.IF_PRIMARY_TERM.getFieldName()
236+
);
237+
if (ifPrimaryTermFieldValue instanceof Integer || ifPrimaryTermFieldValue instanceof Long) {
238+
ingestDocument.setFieldValue(Metadata.IF_PRIMARY_TERM.getFieldName(), ((Number) ifPrimaryTermFieldValue).longValue());
239+
} else {
240+
throw new IllegalArgumentException("Failed to parse parameter [_if_primary_term], only int or long is accepted");
241+
}
224242
}
225243
ingestDocumentList.add(ingestDocument);
226244
}

‎server/src/test/java/org/opensearch/action/ingest/SimulatePipelineRequestParsingTests.java‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,17 +183,28 @@ private void innerTestParseWithProvidedPipeline(boolean useExplicitType) throws
183183
);
184184
for (IngestDocument.Metadata field : fields) {
185185
if (field == VERSION) {
186-
Long value = randomLong();
187-
doc.put(field.getFieldName(), value);
188-
expectedDoc.put(field.getFieldName(), value);
186+
if (randomBoolean()) {
187+
Long value = randomLong();
188+
doc.put(field.getFieldName(), value);
189+
expectedDoc.put(field.getFieldName(), value);
190+
} else {
191+
Integer value = randomIntBetween(1, 1000000);
192+
doc.put(field.getFieldName(), value);
193+
expectedDoc.put(field.getFieldName(), value);
194+
}
189195
} else if (field == VERSION_TYPE) {
190196
String value = VersionType.toString(randomFrom(VersionType.INTERNAL, VersionType.EXTERNAL, VersionType.EXTERNAL_GTE));
191197
doc.put(field.getFieldName(), value);
192198
expectedDoc.put(field.getFieldName(), value);
193199
} else if (field == IF_SEQ_NO || field == IF_PRIMARY_TERM) {
194-
Long value = randomNonNegativeLong();
195-
doc.put(field.getFieldName(), value);
196-
expectedDoc.put(field.getFieldName(), value);
200+
if (randomBoolean()) {
201+
Long value = randomNonNegativeLong();
202+
doc.put(field.getFieldName(), value);
203+
expectedDoc.put(field.getFieldName(), value);
204+
} else {
205+
Integer value = randomIntBetween(1, 1000000);
206+
doc.put(field.getFieldName(), value);
207+
expectedDoc.put(field.getFieldName(), value);
197208
} else if (field == TYPE) {
198209
if (useExplicitType) {
199210
String value = randomAlphaOfLengthBetween(1, 10);
@@ -332,4 +343,40 @@ public void testNotValidDocs() {
332343
);
333344
assertThat(e3.getMessage(), containsString("required property is missing"));
334345
}
346+
347+
public void testNotValidMetadataFields() {
348+
List<IngestDocument.Metadata> fields = Arrays.asList(VERSION, IF_SEQ_NO, IF_PRIMARY_TERM);
349+
for (IngestDocument.Metadata field : fields) {
350+
String metadataFieldName = field.getFieldName();
351+
Map<String, Object> requestContent = new HashMap<>();
352+
List<Map<String, Object>> docs = new ArrayList<>();
353+
requestContent.put(Fields.DOCS, docs);
354+
Map<String, Object> doc = new HashMap<>();
355+
doc.put(metadataFieldName, randomAlphaOfLengthBetween(1, 10));
356+
doc.put(Fields.SOURCE, Collections.singletonMap(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 10)));
357+
docs.add(doc);
358+
359+
Map<String, Object> pipelineConfig = new HashMap<>();
360+
List<Map<String, Object>> processors = new ArrayList<>();
361+
Map<String, Object> processorConfig = new HashMap<>();
362+
List<Map<String, Object>> onFailureProcessors = new ArrayList<>();
363+
int numOnFailureProcessors = randomIntBetween(0, 1);
364+
for (int j = 0; j < numOnFailureProcessors; j++) {
365+
onFailureProcessors.add(Collections.singletonMap("mock_processor", Collections.emptyMap()));
366+
}
367+
if (numOnFailureProcessors > 0) {
368+
processorConfig.put("on_failure", onFailureProcessors);
369+
}
370+
processors.add(Collections.singletonMap("mock_processor", processorConfig));
371+
pipelineConfig.put("processors", processors);
372+
373+
requestContent.put(Fields.PIPELINE, pipelineConfig);
374+
375+
assertThrows(
376+
"Failed to parse parameter [" + metadataFieldName + "], only int or long is accepted",
377+
IllegalArgumentException.class,
378+
() -> SimulatePipelineRequest.parse(requestContent, false, ingestService)
379+
);
380+
}
381+
}
335382
}

0 commit comments

Comments
 (0)