Skip to content

Commit af4964f

Browse files
quux00quux00
committed
Cleanup for PR review - ready for next round review
1 parent 63c4549 commit af4964f

File tree

5 files changed

+26
-42
lines changed

5 files changed

+26
-42
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/MappingParserContext.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ public MappingParserContext(
7474
this.namespaceValidator = namespaceValidator;
7575
}
7676

77-
// MP TODO: only used by tests, so remove this after tests are updated?
7877
public MappingParserContext(
7978
Function<String, SimilarityProvider> similarityLookupService,
8079
Function<String, Mapper.TypeParser> typeParsers,
@@ -207,7 +206,8 @@ private static class MultiFieldParserContext extends MappingParserContext {
207206
in.indexAnalyzers,
208207
in.indexSettings,
209208
in.idFieldMapper,
210-
in.bitSetProducer
209+
in.bitSetProducer,
210+
in.namespaceValidator
211211
);
212212
}
213213

@@ -237,7 +237,8 @@ private static class DynamicTemplateParserContext extends MappingParserContext {
237237
in.indexAnalyzers,
238238
in.indexSettings,
239239
in.idFieldMapper,
240-
in.bitSetProducer
240+
in.bitSetProducer,
241+
in.namespaceValidator
241242
);
242243
this.dateFormatter = dateFormatter;
243244
}

‎server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapperNamespaceValidator.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,17 @@
99

1010
package org.elasticsearch.index.mapper;
1111

12+
import org.elasticsearch.core.Nullable;
13+
1214
/**
13-
* SPI to inject additional rules around namespaces that are prohibited
15+
* SPI to inject additional rules around namespaces (top level fields) that are prohibited
1416
* in Elasticsearch mappings.
1517
*/
1618
public interface RootObjectMapperNamespaceValidator {
1719
/**
1820
* If the namespace in the mapper is not allowed, an Exception should be thrown.
21+
* @param subobjects Whether subobjects are enabled. Null is allowed
22+
* @param name namespace (field name) to validate
1923
*/
20-
void validateNamespace(ObjectMapper.Subobjects subobjects, String name);
24+
void validateNamespace(@Nullable ObjectMapper.Subobjects subobjects, String name);
2125
}

‎server/src/main/java/org/elasticsearch/indices/IndicesModule.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -96,28 +96,12 @@ public class IndicesModule extends AbstractModule {
9696
private final MapperRegistry mapperRegistry;
9797

9898
public IndicesModule(List<MapperPlugin> mapperPlugins, RootObjectMapperNamespaceValidator namespaceValidator) {
99-
// this is the only place that the MapperRegistry is created
10099
this.mapperRegistry = new MapperRegistry(
101100
getMappers(mapperPlugins),
102101
getRuntimeFields(mapperPlugins),
103102
getMetadataMappers(mapperPlugins),
104103
getFieldFilter(mapperPlugins),
105104
namespaceValidator
106-
// new RootObjectMapperNamespaceValidator() {
107-
// @Override
108-
// public void validateNamespace(ObjectMapper.Subobjects subobjects, Mapper mapper) {
109-
// // TODO: in the future, this will be a no-op on stateful and loaded somehow dynamically in serverless
110-
// if (subobjects == ObjectMapper.Subobjects.ENABLED) {
111-
// if (mapper.leafName().equals(RESERVED_NAMESPACE)) {
112-
// throw new IllegalArgumentException("xx reserved namespace: [" + RESERVED_NAMESPACE + ']');
113-
// }
114-
// } else {
115-
// if (mapper.leafName().startsWith(RESERVED_NAMESPACE)) {
116-
// throw new IllegalArgumentException("xx reserved namespace: [" + RESERVED_NAMESPACE + ']');
117-
// }
118-
// }
119-
// }
120-
// }
121105
);
122106
}
123107

‎server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -373,16 +373,14 @@ public void testEmptyType() throws Exception {
373373

374374
public void testWithRootObjectMapperNamespaceValidator() throws Exception {
375375
final String errorMessage = "error 1234";
376+
final String disallowed = "_project";
376377

377378
RootObjectMapperNamespaceValidator validator = new RootObjectMapperNamespaceValidator() {
378379
@Override
379380
public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
380-
System.err.println(">>> XXX subobjects: " + subobjects.toString());
381-
String disallowed = "_project";
382381
if (name.equals(disallowed)) {
383382
throw new IllegalArgumentException(errorMessage);
384383
} else if (subobjects != ObjectMapper.Subobjects.ENABLED) {
385-
System.err.println(">>> YYYYYYYYYYYYYYYYY subobjects: " + subobjects.toString());
386384
// name here will be something like _project.my_field, rather than just _project
387385
if (name.startsWith(disallowed + ".")) {
388386
throw new IllegalArgumentException(errorMessage);
@@ -404,7 +402,7 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
404402

405403
// _project should fail, regardless of type
406404
{
407-
String json = notNested.replace("<FIELD_NAME>", "_project");
405+
String json = notNested.replace("<FIELD_NAME>", disallowed);
408406

409407
String keyword = json.replace("<TYPE>", "keyword");
410408
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperServiceWithNamespaceValidator(keyword, validator));
@@ -421,29 +419,30 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
421419

422420
// _project.subfield should fail
423421
{
424-
String json = notNested.replace("<FIELD_NAME>", "_project.subfield").replace("<TYPE>", randomFrom("text", "keyword", "object"));
422+
String json = notNested.replace("<FIELD_NAME>", disallowed + ".subfield")
423+
.replace("<TYPE>", randomFrom("text", "keyword", "object"));
425424
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperServiceWithNamespaceValidator(json, validator));
426425
assertThat(e.getMessage(), equalTo(errorMessage));
427426
}
428427

429428
// _projectx should pass
430429
{
431-
String json = notNested.replace("<FIELD_NAME>", "_projectx").replace("<TYPE>", randomFrom("text", "keyword", "object"));
430+
String json = notNested.replace("<FIELD_NAME>", disallowed + "x").replace("<TYPE>", randomFrom("text", "keyword", "object"));
432431
MapperService mapperService = createMapperServiceWithNamespaceValidator(json, validator);
433432
assertNotNull(mapperService);
434433
}
435434

436435
// _project_subfield should pass
437436
{
438-
String json = notNested.replace("<FIELD_NAME>", "_project_subfield");
437+
String json = notNested.replace("<FIELD_NAME>", disallowed + "_subfield");
439438
json = json.replace("<TYPE>", randomFrom("text", "keyword", "object"));
440439
MapperService mapperService = createMapperServiceWithNamespaceValidator(json, validator);
441440
assertNotNull(mapperService);
442441
}
443442

444443
// _projectx.subfield should pass
445444
{
446-
String json = notNested.replace("<FIELD_NAME>", "_projectx.subfield");
445+
String json = notNested.replace("<FIELD_NAME>", disallowed + "x.subfield");
447446
json = json.replace("<TYPE>", randomFrom("text", "keyword", "object"));
448447
MapperService mapperService = createMapperServiceWithNamespaceValidator(json, validator);
449448
assertNotNull(mapperService);
@@ -484,21 +483,21 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
484483

485484
// nested _project { my_field } should fail
486485
{
487-
String json = nested.replace("<FIELD_NAME1>", "_project").replace("<FIELD_NAME2>", "my_field");
486+
String json = nested.replace("<FIELD_NAME1>", disallowed).replace("<FIELD_NAME2>", "my_field");
488487
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperServiceWithNamespaceValidator(json, validator));
489488
assertThat(e.getMessage(), equalTo(errorMessage));
490489
}
491490

492491
// nested my_field { _project } should succeed
493492
{
494-
String json = nested.replace("<FIELD_NAME1>", "my_field").replace("<FIELD_NAME2>", "_project");
493+
String json = nested.replace("<FIELD_NAME1>", "my_field").replace("<FIELD_NAME2>", disallowed);
495494
MapperService mapperService = createMapperServiceWithNamespaceValidator(json, validator);
496495
assertNotNull(mapperService);
497496
}
498497

499498
// nested _projectx { _project } should succeed
500499
{
501-
String json = nested.replace("<FIELD_NAME1>", "_projectx").replace("<FIELD_NAME2>", "_project");
500+
String json = nested.replace("<FIELD_NAME1>", disallowed + "x").replace("<FIELD_NAME2>", disallowed);
502501
MapperService mapperService = createMapperServiceWithNamespaceValidator(json, validator);
503502
assertNotNull(mapperService);
504503
}
@@ -558,16 +557,14 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
558557

559558
public void testRuntimeFieldInMappingWithNamespaceValidator() throws IOException {
560559
final String errorMessage = "error 1234";
560+
final String disallowed = "_project";
561561

562562
RootObjectMapperNamespaceValidator validator = new RootObjectMapperNamespaceValidator() {
563563
@Override
564564
public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
565-
System.err.println(">>> XXX subobjects: " + subobjects);
566-
String disallowed = "_project";
567565
if (name.equals(disallowed)) {
568566
throw new IllegalArgumentException(errorMessage);
569567
} else if (subobjects != ObjectMapper.Subobjects.ENABLED) {
570-
System.err.println(">>> YYYYYYYYYYYYYYYYY subobjects: " + subobjects);
571568
// name here will be something like _project.my_field, rather than just _project
572569
if (name.startsWith(disallowed + ".")) {
573570
throw new IllegalArgumentException(errorMessage);
@@ -579,9 +576,9 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
579576
// ensure that things close to the disallowed fields that are allowed
580577
{
581578
String mapping = Strings.toString(runtimeMapping(builder -> {
582-
builder.startObject("_project_x").field("type", "ip").endObject();
583-
builder.startObject("_projectx").field("type", "date").endObject();
584-
builder.startObject("field1._project").field("type", "double").endObject();
579+
builder.startObject(disallowed + "_x").field("type", "ip").endObject();
580+
builder.startObject(disallowed + "x").field("type", "date").endObject();
581+
builder.startObject("field1." + disallowed).field("type", "double").endObject();
585582
}));
586583
MapperService mapperService = createMapperServiceWithNamespaceValidator(mapping, validator);
587584
assertEquals(mapping, mapperService.documentMapper().mappingSource().toString());
@@ -592,7 +589,7 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
592589
{
593590
String mapping = Strings.toString(runtimeMapping(builder -> {
594591
builder.startObject("field1").field("type", "double").endObject();
595-
builder.startObject("_project").field("type", "date").endObject();
592+
builder.startObject(disallowed).field("type", "date").endObject();
596593
builder.startObject("field3").field("type", "ip").endObject();
597594
}));
598595
Exception e = expectThrows(MapperParsingException.class, () -> createMapperServiceWithNamespaceValidator(mapping, validator));
@@ -605,7 +602,7 @@ public void validateNamespace(ObjectMapper.Subobjects subobjects, String name) {
605602
{
606603
String mapping = Strings.toString(runtimeMapping(builder -> {
607604
builder.startObject("field1").field("type", "double").endObject();
608-
builder.startObject("_project.my_sub_field").field("type", "keyword").endObject();
605+
builder.startObject(disallowed + ".my_sub_field").field("type", "keyword").endObject();
609606
builder.startObject("field3").field("type", "ip").endObject();
610607
}));
611608
Exception e = expectThrows(MapperParsingException.class, () -> createMapperServiceWithNamespaceValidator(mapping, validator));

‎test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,6 @@ public MapperService createMapperServiceWithNamespaceValidator(String mappings,
242242
.namespaceValidator(validator)
243243
.build();
244244

245-
// TODO: is this step necessary?
246-
mapperService = withMapping(mapperService, mapping(b -> {}));
247245
merge(mapperService, mappings);
248246
return mapperService;
249247
}

0 commit comments

Comments
 (0)