Skip to content

Commit

Permalink
[Remove] Multiple Types from IndexTemplateMetadata (#2400)
Browse files Browse the repository at this point in the history
Removes multi-type support from IndexTemplateMetadata so that instead of holding
a map of multiple types to mappings, it only returns a single mapping for a
single type. Also removes type from documentMapper() method to avoid any
accidental NullPointerExceptions in the internal mapping retrieval.

Signed-off-by: Nicholas Walter Knize <[email protected]>
  • Loading branch information
nknize authored Mar 11, 2022
1 parent 9cfa395 commit b00b3ce
Show file tree
Hide file tree
Showing 56 changed files with 530 additions and 919 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void testFromXContent() throws IOException {
.test();
}

public void testParsingFromEsResponse() throws IOException {
public void testParsingFromOpenSearchResponse() throws IOException {
for (int runs = 0; runs < 20; runs++) {
org.opensearch.action.admin.indices.template.get.GetIndexTemplatesResponse esResponse =
new org.opensearch.action.admin.indices.template.get.GetIndexTemplatesResponse(new ArrayList<>());
Expand Down Expand Up @@ -131,8 +131,7 @@ public void testParsingFromEsResponse() throws IOException {
assertThat(result.order(), equalTo(esIMD.order()));
assertThat(result.version(), equalTo(esIMD.version()));

assertThat(esIMD.mappings().size(), equalTo(1));
BytesReference mappingSource = esIMD.mappings().valuesIt().next().uncompressed();
BytesReference mappingSource = esIMD.mappings().uncompressed();
Map<String, Object> expectedMapping = XContentHelper.convertToMap(mappingSource, true, xContentBuilder.contentType())
.v2();
assertThat(result.mappings().sourceAsMap(), equalTo(expectedMapping.get("_doc")));
Expand Down Expand Up @@ -224,7 +223,10 @@ static void toXContent(GetIndexTemplatesResponse response, XContentBuilder build
serverTemplateBuilder.order(clientITMD.order());
serverTemplateBuilder.version(clientITMD.version());
if (clientITMD.mappings() != null) {
serverTemplateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, clientITMD.mappings().source());
// The client-side mappings never include a wrapping type, but server-side mappings
// for index templates still do so we need to wrap things here
String mappings = "{\"" + MapperService.SINGLE_MAPPING_NAME + "\": " + clientITMD.mappings().source().string() + "}";
serverTemplateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, mappings);
}
serverIndexTemplates.add(serverTemplateBuilder.build());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
);
}
}
docMapper = mapperService.documentMapper(type);
docMapper = mapperService.documentMapper();
for (BytesReference document : documents) {
docs.add(docMapper.parse(new SourceToParse(context.index().getName(), type, "_temp_id", document, documentXContentType)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public void init() throws Exception {
.endObject()
);
mapperService.merge("type", new CompressedXContent(percolatorMapper), MapperService.MergeReason.MAPPING_UPDATE);
fieldMapper = (PercolatorFieldMapper) mapperService.documentMapper("type").mappers().getMapper(queryField);
fieldMapper = (PercolatorFieldMapper) mapperService.documentMapper().mappers().getMapper(queryField);
fieldType = (PercolatorFieldMapper.PercolatorFieldType) fieldMapper.fieldType();

queries = new ArrayList<>();
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ protected Collection<Class<? extends Plugin>> getPlugins() {

public void testSizeEnabled() throws Exception {
IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=true");
DocumentMapper docMapper = service.mapperService().documentMapper("type");
DocumentMapper docMapper = service.mapperService().documentMapper();

BytesReference source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "value").endObject());
ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1", source, XContentType.JSON));
ParsedDocument doc = docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", source, XContentType.JSON));

boolean stored = false;
boolean points = false;
Expand All @@ -80,27 +80,27 @@ public void testSizeEnabled() throws Exception {

public void testSizeDisabled() throws Exception {
IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=false");
DocumentMapper docMapper = service.mapperService().documentMapper("type");
DocumentMapper docMapper = service.mapperService().documentMapper();

BytesReference source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "value").endObject());
ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1", source, XContentType.JSON));
ParsedDocument doc = docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", source, XContentType.JSON));

assertThat(doc.rootDoc().getField("_size"), nullValue());
}

public void testSizeNotSet() throws Exception {
IndexService service = createIndex("test", Settings.EMPTY, "type");
DocumentMapper docMapper = service.mapperService().documentMapper("type");
IndexService service = createIndex("test", Settings.EMPTY, MapperService.SINGLE_MAPPING_NAME);
DocumentMapper docMapper = service.mapperService().documentMapper();

BytesReference source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "value").endObject());
ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1", source, XContentType.JSON));
ParsedDocument doc = docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", source, XContentType.JSON));

assertThat(doc.rootDoc().getField("_size"), nullValue());
}

public void testThatDisablingWorksWhenMerging() throws Exception {
IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=true");
DocumentMapper docMapper = service.mapperService().documentMapper("type");
DocumentMapper docMapper = service.mapperService().documentMapper();
assertThat(docMapper.metadataMapper(SizeFieldMapper.class).enabled(), is(true));

String disabledMapping = Strings.toString(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,9 +664,6 @@ public void testEmptyShard() throws IOException {
// before timing out
.put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "100ms")
.put(SETTING_ALLOCATION_MAX_RETRY.getKey(), "0"); // fail faster
if (randomBoolean()) {
settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(), "-1");
}
createIndex(index, settings.build());
}
ensureGreen(index);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
"Return empty object if field doesn't exist, but type and index do":
"Return empty object if field doesn't exist, but index does":
- do:
indices.create:
index: test_index
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,18 @@
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.index.IndexService;
import org.opensearch.index.mapper.MapperParsingException;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.index.query.RangeQueryBuilder;
import org.opensearch.indices.IndicesService;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope;
import org.opensearch.test.OpenSearchIntegTestCase.Scope;

import java.util.HashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiFunction;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_WAIT_FOR_ACTIVE_SHARDS;
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertBlocked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertRequestBuilderThrows;
Expand Down Expand Up @@ -109,28 +108,6 @@ public void testCreationDateGenerated() {
assertThat(index.getCreationDate(), allOf(lessThanOrEqualTo(timeAfterRequest), greaterThanOrEqualTo(timeBeforeRequest)));
}

public void testDoubleAddMapping() throws Exception {
try {
prepareCreate("test").addMapping("type1", "date", "type=date").addMapping("type1", "num", "type=integer");
fail("did not hit expected exception");
} catch (IllegalStateException ise) {
// expected
}
try {
prepareCreate("test").addMapping("type1", new HashMap<String, Object>()).addMapping("type1", new HashMap<String, Object>());
fail("did not hit expected exception");
} catch (IllegalStateException ise) {
// expected
}
try {
prepareCreate("test").addMapping("type1", jsonBuilder().startObject().endObject())
.addMapping("type1", jsonBuilder().startObject().endObject());
fail("did not hit expected exception");
} catch (IllegalStateException ise) {
// expected
}
}

public void testNonNestedMappings() throws Exception {
assertAcked(
prepareCreate("test").addMapping(
Expand Down Expand Up @@ -168,11 +145,16 @@ public void testMappingParamAndNestedMismatch() throws Exception {
MapperParsingException e = expectThrows(
MapperParsingException.class,
() -> prepareCreate("test").addMapping(
"type1",
MapperService.SINGLE_MAPPING_NAME,
XContentFactory.jsonBuilder().startObject().startObject("type2").endObject().endObject()
).get()
);
assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: Root mapping definition has unsupported parameters"));
assertThat(
e.getMessage(),
startsWith(
"Failed to parse mapping [" + MapperService.SINGLE_MAPPING_NAME + "]: Root mapping definition has unsupported parameters"
)
);
}

public void testEmptyMappings() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ public void testDelayedMappingPropagationOnReplica() throws Exception {
final IndexService indexService = indicesService.indexServiceSafe(index);
assertNotNull(indexService);
final MapperService mapperService = indexService.mapperService();
DocumentMapper mapper = mapperService.documentMapper(MapperService.SINGLE_MAPPING_NAME);
DocumentMapper mapper = mapperService.documentMapper();
assertNotNull(mapper);
assertNotNull(mapper.mappers().getMapper("field"));
});
Expand All @@ -389,7 +389,7 @@ public void testDelayedMappingPropagationOnReplica() throws Exception {
final IndexService indexService = indicesService.indexServiceSafe(index);
assertNotNull(indexService);
final MapperService mapperService = indexService.mapperService();
DocumentMapper mapper = mapperService.documentMapper(MapperService.SINGLE_MAPPING_NAME);
DocumentMapper mapper = mapperService.documentMapper();
assertNotNull(mapper);
assertNotNull(mapper.mappers().getMapper("field2"));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.hamcrest.OpenSearchAssertions;

Expand Down Expand Up @@ -182,7 +183,7 @@ public void testIndexActions() throws Exception {
// test successful
SearchResponse countResponse = client().prepareSearch("test")
.setSize(0)
.setQuery(termQuery("_type", "type1"))
.setQuery(termQuery("_type", MapperService.SINGLE_MAPPING_NAME))
.execute()
.actionGet();
assertNoFailures(countResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.env.NodeEnvironment;
import org.opensearch.index.mapper.MapperParsingException;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.indices.IndexClosedException;
import org.opensearch.indices.ShardLimitValidator;
import org.opensearch.test.OpenSearchIntegTestCase;
Expand Down Expand Up @@ -108,14 +109,7 @@ public void testMappingMetadataParsed() throws Exception {
.prepareCreate("test")
.addMapping(
"type1",
XContentFactory.jsonBuilder()
.startObject()
.startObject("type1")
.startObject("_routing")
.field("required", true)
.endObject()
.endObject()
.endObject()
XContentFactory.jsonBuilder().startObject().startObject("_routing").field("required", true).endObject().endObject()
)
.execute()
.actionGet();
Expand All @@ -130,7 +124,7 @@ public void testMappingMetadataParsed() throws Exception {
.metadata()
.index("test")
.getMappings()
.get("type1");
.get(MapperService.SINGLE_MAPPING_NAME);
assertThat(mappingMd.routing().required(), equalTo(true));

logger.info("--> restarting nodes...");
Expand All @@ -149,7 +143,7 @@ public void testMappingMetadataParsed() throws Exception {
.metadata()
.index("test")
.getMappings()
.get("type1");
.get(MapperService.SINGLE_MAPPING_NAME);
assertThat(mappingMd.routing().required(), equalTo(true));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ private XContentBuilder createTypeSource() throws IOException {
private XContentBuilder createPutMappingSource() throws IOException {
return XContentFactory.jsonBuilder()
.startObject()
.startObject("my-type")
.startObject("properties")
.startObject("title")
.field("type", "text")
Expand All @@ -220,7 +219,6 @@ private XContentBuilder createPutMappingSource() throws IOException {
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.opensearch.index.engine.Engine;
import org.opensearch.index.engine.NoOpEngine;
import org.opensearch.index.flush.FlushStats;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.index.mapper.SourceToParse;
import org.opensearch.index.seqno.RetentionLeaseSyncer;
import org.opensearch.index.seqno.SequenceNumbers;
Expand Down Expand Up @@ -445,7 +446,7 @@ public void testMaybeRollTranslogGeneration() throws Exception {
.put("index.number_of_shards", 1)
.put("index.translog.generation_threshold_size", generationThreshold + "b")
.build();
createIndex("test", settings, "test");
createIndex("test", settings, MapperService.SINGLE_MAPPING_NAME);
ensureGreen("test");
final IndicesService indicesService = getInstanceFromNode(IndicesService.class);
final IndexService test = indicesService.indexService(resolveIndex("test"));
Expand All @@ -459,7 +460,7 @@ public void testMaybeRollTranslogGeneration() throws Exception {
final Engine.IndexResult result = shard.applyIndexOperationOnPrimary(
Versions.MATCH_ANY,
VersionType.INTERNAL,
new SourceToParse("test", "test", "1", new BytesArray("{}"), XContentType.JSON),
new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", new BytesArray("{}"), XContentType.JSON),
SequenceNumbers.UNASSIGNED_SEQ_NO,
0,
IndexRequest.UNSET_AUTO_GENERATED_TIMESTAMP,
Expand Down
Loading

0 comments on commit b00b3ce

Please sign in to comment.