Accept more ingest simulate params as ints or strings (elastic#66197)

_version, _if_seq_no and _if_primary_term params in the simulate pipeline API cannot be parsed correctly when the value is integer orstring type. This PR fix the bug and modify the test method to test it. # Conflicts: # server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java
probakowski · Jan 11, 2021 · f9ca8b1 · f9ca8b1
1 parent 6605bc8
commit f9ca8b1
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java b/server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java
@@ -194,7 +194,13 @@ private static List<IngestDocument> parseDocs(Map<String, Object> config) {
                 dataMap, Metadata.ROUTING.getFieldName());
             Long version = null;
             if (dataMap.containsKey(Metadata.VERSION.getFieldName())) {
-                version = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.VERSION.getFieldName());
+                String versionValue = ConfigurationUtils.readOptionalStringOrIntProperty(null, null,
+                    dataMap, Metadata.VERSION.getFieldName());
+                if (versionValue != null) {
+                    version = Long.valueOf(versionValue);
+                } else {
+                    throw new IllegalArgumentException("[_version] cannot be null");
+                }
             }
             VersionType versionType = null;
             if (dataMap.containsKey(Metadata.VERSION_TYPE.getFieldName())) {
@@ -204,12 +210,24 @@ private static List<IngestDocument> parseDocs(Map<String, Object> config) {
             IngestDocument ingestDocument =
                 new IngestDocument(index, type, id, routing, version, versionType, document);
             if (dataMap.containsKey(Metadata.IF_SEQ_NO.getFieldName())) {
-                Long ifSeqNo = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.IF_SEQ_NO.getFieldName());
-                ingestDocument.setFieldValue(Metadata.IF_SEQ_NO.getFieldName(), ifSeqNo);
+                String ifSeqNoValue = ConfigurationUtils.readOptionalStringOrIntProperty(null, null,
+                    dataMap, Metadata.IF_SEQ_NO.getFieldName());
+                if (ifSeqNoValue != null) {
+                    Long ifSeqNo = Long.valueOf(ifSeqNoValue);
+                    ingestDocument.setFieldValue(Metadata.IF_SEQ_NO.getFieldName(), ifSeqNo);
+                } else {
+                    throw new IllegalArgumentException("[_if_seq_no] cannot be null");
+                }
             }
             if (dataMap.containsKey(Metadata.IF_PRIMARY_TERM.getFieldName())) {
-                Long ifPrimaryTerm = (Long) ConfigurationUtils.readObject(null, null, dataMap, Metadata.IF_PRIMARY_TERM.getFieldName());
-                ingestDocument.setFieldValue(Metadata.IF_PRIMARY_TERM.getFieldName(), ifPrimaryTerm);
+                String ifPrimaryTermValue = ConfigurationUtils.readOptionalStringOrIntProperty(null, null,
+                    dataMap, Metadata.IF_PRIMARY_TERM.getFieldName());
+                if (ifPrimaryTermValue != null) {
+                    Long ifPrimaryTerm = Long.valueOf(ifPrimaryTermValue);
+                    ingestDocument.setFieldValue(Metadata.IF_PRIMARY_TERM.getFieldName(), ifPrimaryTerm);
+                } else {
+                    throw new IllegalArgumentException("[_if_primary_term] cannot be null");
+                }
             }
             ingestDocumentList.add(ingestDocument);
         }

diff --git a/...er/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java b/...er/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java
@@ -152,23 +152,15 @@ private void innerTestParseWithProvidedPipeline(boolean useExplicitType) throws
         for (int i = 0; i < numDocs; i++) {
             Map<String, Object> doc = new HashMap<>();
             Map<String, Object> expectedDoc = new HashMap<>();
-            List<IngestDocument.Metadata> fields = Arrays.asList(INDEX, TYPE, ID, ROUTING, VERSION, VERSION_TYPE, IF_SEQ_NO, 
+            List<IngestDocument.Metadata> fields = Arrays.asList(INDEX, TYPE, ID, ROUTING, VERSION, VERSION_TYPE, IF_SEQ_NO,
                 IF_PRIMARY_TERM);
             for(IngestDocument.Metadata field : fields) {
-                if (field == VERSION) {
-                    Long value = randomLong();
-                    doc.put(field.getFieldName(), value);
-                    expectedDoc.put(field.getFieldName(), value);
-                } else if (field == VERSION_TYPE) {
+                if (field == VERSION_TYPE) {
                     String value = VersionType.toString(
                         randomFrom(VersionType.INTERNAL, VersionType.EXTERNAL, VersionType.EXTERNAL_GTE)
                     );
                     doc.put(field.getFieldName(), value);
                     expectedDoc.put(field.getFieldName(), value);
-                } else if (field == IF_SEQ_NO || field == IF_PRIMARY_TERM) {
-                    Long value = randomNonNegativeLong();
-                    doc.put(field.getFieldName(), value);
-                    expectedDoc.put(field.getFieldName(), value);
                 } else if (field == TYPE) {
                     if (useExplicitType) {
                         String value = randomAlphaOfLengthBetween(1, 10);