Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Backport 2.x] Add missing data types to IngestDocument deep copy #14413

Merged
merged 1 commit into from
Jun 18, 2024
Merged

[Backport 2.x] Add missing data types to IngestDocument deep copy #14413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Removed

### Fixed
- Fix handling of Short and Byte data types in ScriptProcessor ingest pipeline ([#14379](https://github.com/opensearch-project/OpenSearch/issues/14379))

### Security

Expand Down
75 changes: 75 additions & 0 deletions ...gest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/190_script_processor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,78 @@ teardown:
body: {source_field: "fooBar", foo: {foo: "bar"}}
- match: { error.root_cause.0.type: "illegal_argument_exception" }
- match: { error.root_cause.0.reason: "Iterable object is self-referencing itself (ingest script)" }

---
"Test painless data types":
- do:
ingest.put_pipeline:
id: "my_pipeline"
body: >
{
"description": "_description",
"processors": [
{
"script" : {
"source" : "ctx.byte = (byte)127;ctx.short = (short)32767;ctx.int = (int)2147483647;ctx.long = (long)9223372036854775807L;ctx.float = (float)0.1;ctx.double = (double)0.1;ctx.boolean = (boolean)true"
}
},
{
"script" : {
"source" : "ctx.other_field = 'other_field'"
}
}
]
}
- match: { acknowledged: true }

- do:
index:
index: test
id: 1
pipeline: "my_pipeline"
body: {source_field: "FooBar"}

- do:
get:
index: test
id: 1
- match: { _source.byte: 127 }
- match: { _source.int: 2147483647 }
- match: { _source.long: 9223372036854775807 }
- gt: { _source.float: 0.0 }
- lt: { _source.float: 0.2 }
- gt: { _source.double: 0.0 }
- lt: { _source.double: 0.2 }
- match: { _source.boolean: true }

---
"Test char type fails":
- do:
ingest.put_pipeline:
id: "my_pipeline"
body: >
{
"description": "_description",
"processors": [
{
"script" : {
"source" : "ctx.char = (char)'a'"
}
},
{
"script" : {
"source" : "ctx.other_field = 'other_field'"
}
}
]
}
- match: { acknowledged: true }

- do:
catch: bad_request
index:
index: test
id: 1
pipeline: "my_pipeline"
body: {source_field: "FooBar"}
- match: { error.root_cause.0.type: "illegal_argument_exception" }
3 changes: 3 additions & 0 deletions server/src/main/java/org/opensearch/ingest/IngestDocument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,9 @@ public static Object deepCopy(Object value) {
byte[] bytes = (byte[]) value;
return Arrays.copyOf(bytes, bytes.length);
} else if (value == null
|| value instanceof Byte
|| value instanceof Character
|| value instanceof Short
|| value instanceof String
|| value instanceof Integer
|| value instanceof Long
Expand Down
47 changes: 32 additions & 15 deletions server/src/test/java/org/opensearch/ingest/IngestDocumentTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public class IngestDocumentTests extends OpenSearchTestCase {

private static final ZonedDateTime BOGUS_TIMESTAMP = ZonedDateTime.of(2016, 10, 23, 0, 0, 0, 0, ZoneOffset.UTC);
private IngestDocument ingestDocument;
private int initialSourceAndMetadataSize;

@Before
public void setTestIngestDocument() {
Expand All @@ -70,7 +71,6 @@ public void setTestIngestDocument() {
ingestMap.put("timestamp", BOGUS_TIMESTAMP);
document.put("_ingest", ingestMap);
document.put("foo", "bar");
document.put("int", 123);
Map<String, Object> innerObject = new HashMap<>();
innerObject.put("buzz", "hello world");
innerObject.put("foo_null", null);
Expand All @@ -92,7 +92,17 @@ public void setTestIngestDocument() {
list2.add("bar");
list2.add("baz");
document.put("list2", list2);
document.put("byte", (byte) 1);
document.put("short", (short) 2);
document.put("int", Integer.MAX_VALUE);
document.put("long", Long.MAX_VALUE);
document.put("float", 0.1f);
document.put("double", 0.1d);
document.put("char", 'a');
document.put("string", "A test string \uD83C\uDF89");
document.put("datetime", ZonedDateTime.parse("2007-12-03T10:15:30+01:00[Europe/Paris]"));
ingestDocument = new IngestDocument("index", "id", null, null, null, document);
initialSourceAndMetadataSize = 16; // i.e. ingestDocument.getSourceAndMetadata().size()
}

public void testSelfReferencingSource() {
Expand All @@ -101,11 +111,18 @@ public void testSelfReferencingSource() {
expectThrows(IllegalArgumentException.class, () -> IngestDocument.deepCopyMap(value));
}

public void testCopy() {
final IngestDocument copy = new IngestDocument(ingestDocument);
assertThat(copy, equalTo(ingestDocument));
assertThat(copy.getSourceAndMetadata(), not(sameInstance(ingestDocument.getSourceAndMetadata())));
assertThat(copy.getIngestMetadata(), not(sameInstance(ingestDocument.getIngestMetadata())));
}

public void testSimpleGetFieldValue() {
assertThat(ingestDocument.getFieldValue("foo", String.class), equalTo("bar"));
assertThat(ingestDocument.getFieldValue("int", Integer.class), equalTo(123));
assertThat(ingestDocument.getFieldValue("int", Integer.class), equalTo(Integer.MAX_VALUE));
assertThat(ingestDocument.getFieldValue("_source.foo", String.class), equalTo("bar"));
assertThat(ingestDocument.getFieldValue("_source.int", Integer.class), equalTo(123));
assertThat(ingestDocument.getFieldValue("_source.int", Integer.class), equalTo(Integer.MAX_VALUE));
assertThat(ingestDocument.getFieldValue("_index", String.class), equalTo("index"));
assertThat(ingestDocument.getFieldValue("_id", String.class), equalTo("id"));
assertThat(
Expand Down Expand Up @@ -578,7 +595,7 @@ public void testAppendFieldValueConvertIntegerToList() {
@SuppressWarnings("unchecked")
List<Object> list = (List<Object>) object;
assertThat(list.size(), equalTo(2));
assertThat(list.get(0), equalTo(123));
assertThat(list.get(0), equalTo(Integer.MAX_VALUE));
assertThat(list.get(1), equalTo(456));
}

Expand All @@ -589,7 +606,7 @@ public void testAppendFieldValuesConvertIntegerToList() {
@SuppressWarnings("unchecked")
List<Object> list = (List<Object>) object;
assertThat(list.size(), equalTo(3));
assertThat(list.get(0), equalTo(123));
assertThat(list.get(0), equalTo(Integer.MAX_VALUE));
assertThat(list.get(1), equalTo(456));
assertThat(list.get(2), equalTo(789));
}
Expand Down Expand Up @@ -812,23 +829,23 @@ public void testSetFieldValueEmptyName() {

public void testRemoveField() {
ingestDocument.removeField("foo");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(7));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize - 1));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("foo"), equalTo(false));
ingestDocument.removeField("_index");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(6));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize - 2));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("_index"), equalTo(false));
ingestDocument.removeField("_source.fizz");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(5));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize - 3));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("fizz"), equalTo(false));
assertThat(ingestDocument.getIngestMetadata().size(), equalTo(1));
ingestDocument.removeField("_ingest.timestamp");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(5));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize - 3));
assertThat(ingestDocument.getIngestMetadata().size(), equalTo(0));
}

public void testRemoveInnerField() {
ingestDocument.removeField("fizz.buzz");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(8));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize));
assertThat(ingestDocument.getSourceAndMetadata().get("fizz"), instanceOf(Map.class));
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) ingestDocument.getSourceAndMetadata().get("fizz");
Expand All @@ -837,17 +854,17 @@ public void testRemoveInnerField() {

ingestDocument.removeField("fizz.foo_null");
assertThat(map.size(), equalTo(2));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(8));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("fizz"), equalTo(true));

ingestDocument.removeField("fizz.1");
assertThat(map.size(), equalTo(1));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(8));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("fizz"), equalTo(true));

ingestDocument.removeField("fizz.list");
assertThat(map.size(), equalTo(0));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(8));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("fizz"), equalTo(true));
}

Expand Down Expand Up @@ -883,7 +900,7 @@ public void testRemoveSourceObject() {

public void testRemoveIngestObject() {
ingestDocument.removeField("_ingest");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(7));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize - 1));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("_ingest"), equalTo(false));
}

Expand All @@ -905,7 +922,7 @@ public void testRemoveEmptyPathAfterStrippingOutPrefix() {

public void testListRemoveField() {
ingestDocument.removeField("list.0.field");
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(8));
assertThat(ingestDocument.getSourceAndMetadata().size(), equalTo(initialSourceAndMetadataSize));
assertThat(ingestDocument.getSourceAndMetadata().containsKey("list"), equalTo(true));
Object object = ingestDocument.getSourceAndMetadata().get("list");
assertThat(object, instanceOf(List.class));
Expand Down
Loading