Skip to content

Commit c7ad1f8

Browse files
committed
Fix tests
Signed-off-by: Mohammad Qureshi <qreshi@amazon.com>
1 parent 69a6b30 commit c7ad1f8

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

server/src/test/java/org/opensearch/index/mapper/DerivedFieldMapperTests.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ protected boolean supportsOrIgnoresBoost() {
2222
return false;
2323
}
2424

25+
protected boolean supportsMeta() {
26+
return false;
27+
}
28+
2529
// Overriding fieldMapping to make it create derived mappings by default.
2630
// This way, the parent tests are checking the right behavior for this Mapper.
2731
@Override
@@ -43,8 +47,11 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
4347
// supported for DerivedFieldMapper (we explicitly set these values on initialization)
4448
}
4549

46-
// TESTCASE: testDefaults() (no script provided and assuming field is not in the source either)
47-
// TODO: Might not need this (no default that really deviates it from testSerialization()
50+
// TODO: Can update this once the query implementation is completed
51+
// This is also being left blank because the super assertExistsQuery is trying to parse
52+
// an empty source and fails.
53+
@Override
54+
protected void assertExistsQuery(MapperService mapperService) {}
4855

4956
public void testSerialization() throws IOException {
5057

@@ -60,10 +67,12 @@ public void testSerialization() throws IOException {
6067

6168
public void testParsesScript() throws IOException {
6269

63-
String scriptString = "{\"source\": \"doc['test'].value\"}";
70+
String scriptString = "doc['test'].value";
6471
DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
6572
b.field("type", "keyword");
66-
b.field("script", scriptString);
73+
b.startObject("script");
74+
b.field("source", scriptString);
75+
b.endObject();
6776
}));
6877
Mapper mapper = defaultMapper.mappers().getMapper("field");
6978
assertTrue(mapper instanceof DerivedFieldMapper);
@@ -73,11 +82,11 @@ public void testParsesScript() throws IOException {
7382
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
7483
mapper.toXContent(builder, ToXContent.EMPTY_PARAMS);
7584
builder.endObject();
76-
assertEquals("{\"type\": \"keyword\",\"script\": {\"source\": \"emit(doc['test'].value)\"}}", builder.toString());
85+
assertEquals("{\"field\":{\"type\":\"keyword\",\"script\":{\"source\":\"doc['test'].value\",\"lang\":\"painless\"}}}", builder.toString());
7786
}
7887

7988
// TODO: Is there a case where we want to allow the field to be defined in both 'derived' and 'properties'?
80-
// If the user wants to move a field they were testing as derived to be indexed they should be able to update
89+
// If the user wants to move a field they were testing as derived to be indexed, they should be able to update
8190
// the mappings in index template to move the field from 'derived' to 'properties' and it should take affect
8291
// during the next index rollover (so even in this case, the field is only defined in one or the other).
8392
public void testFieldInDerivedAndProperties() throws IOException {
@@ -96,8 +105,11 @@ public void testFieldInDerivedAndProperties() throws IOException {
96105
b.endObject();
97106
}))
98107
);
108+
// TODO: Do we want to handle this as a different error? As it stands, it fails as a merge conflict which makes sense.
109+
// If it didn't fail here, it would hit the MapperParsingException for the field being defined more than once
110+
// when MappingLookup is initialized
99111
assertEquals(
100-
"Field [field] is defined more than once",
112+
"Failed to parse mapping [_doc]: mapper [field] cannot be changed from type [derived_field] to [keyword]",
101113
ex.getMessage()
102114
);
103115
}

server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,9 @@ public void testDerivedFields() throws Exception {
451451
.endObject()
452452
.startObject("derived_field_name2")
453453
.field("type", "keyword")
454-
.field("script", "{\"source\": \"doc['test'].value\"}")
454+
.startObject("script")
455+
.field("source", "doc['test'].value")
456+
.endObject()
455457
.endObject()
456458
.endObject()
457459
.startObject("properties")
@@ -478,7 +480,7 @@ public void testDerivedFields() throws Exception {
478480
assertTrue(mapper instanceof DerivedFieldMapper);
479481
derivedFieldMapper = (DerivedFieldMapper) mapper;
480482
assertEquals("keyword", derivedFieldMapper.getType());
481-
assertEquals(Script.parse("{\"source\": \"doc['test'].value\"}"), derivedFieldMapper.getScript());
483+
assertEquals(Script.parse("doc['test'].value"), derivedFieldMapper.getScript());
482484

483485
// Check that field in properties was parsed correctly as well
484486
mapper = documentMapper.root().getMapper("field_name");

0 commit comments

Comments
 (0)