Skip to content

Commit

Permalink
Fix template equals when mappings are wrapped (#77008)
Browse files Browse the repository at this point in the history 
When create template v2. mappings will add _doc. This will cause the created template to be inconsistent with the queried template.

In template class, add mappingsEquals method, to deal with this case.

reproduced:
MetadataIndexTemplateServiceTests.testAddComponentTemplate
when mappings are not null, the case will failed.

```
        Template template = new Template(
            Settings.builder().build(),
            new CompressedXContent("{\"properties\":{\"@timestamp\":{\"type\":\"date\"}}}"),
            ComponentTemplateTests.randomAliases()
        );
        ComponentTemplate componentTemplate = new ComponentTemplate(template, 1L, new HashMap<>());
```

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
@weizijun @elasticmachine
weizijun and elasticmachine authored Sep 3, 2021
1 parent 096b8cc commit e321fb0
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 55 deletions.
27 changes: 25 additions & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Template.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.cluster.AbstractDiffable;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.xcontent.ParseField;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -141,7 +142,7 @@ public boolean equals(Object obj) {
}
Template other = (Template) obj;
return Objects.equals(settings, other.settings) &&
Objects.equals(mappings, other.mappings) &&
mappingsEquals(this.mappings, other.mappings) &&
Objects.equals(aliases, other.aliases);
}

Expand Down Expand Up @@ -178,11 +179,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
}

@SuppressWarnings("unchecked")
private static Map<String, Object> reduceMapping(Map<String, Object> mapping) {
static Map<String, Object> reduceMapping(Map<String, Object> mapping) {
if (mapping.size() == 1 && MapperService.SINGLE_MAPPING_NAME.equals(mapping.keySet().iterator().next())) {
return (Map<String, Object>) mapping.values().iterator().next();
} else {
return mapping;
}
}

static boolean mappingsEquals(CompressedXContent m1, CompressedXContent m2) {
if (m1 == m2) {
return true;
}

if (m1 == null || m2 == null) {
return false;
}

if (m1.equals(m2)) {
return true;
}

Map<String, Object> thisUncompressedMapping = reduceMapping(
XContentHelper.convertToMap(m1.uncompressed(), true, XContentType.JSON).v2()
);
Map<String, Object> otherUncompressedMapping = reduceMapping(
XContentHelper.convertToMap(m2.uncompressed(), true, XContentType.JSON).v2()
);
return Maps.deepEquals(thisUncompressedMapping, otherUncompressedMapping);
}
}
56 changes: 56 additions & 0 deletions server/src/test/java/org/elasticsearch/cluster/metadata/ComponentTemplateTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,25 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.cluster.Diff;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.test.AbstractDiffableSerializationTestCase;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;

public class ComponentTemplateTests extends AbstractDiffableSerializationTestCase<ComponentTemplate> {
@Override
protected ComponentTemplate makeTestChanges(ComponentTemplate testInstance) {
Expand Down Expand Up @@ -154,4 +162,52 @@ public static ComponentTemplate mutateTemplate(ComponentTemplate orig) {
throw new IllegalStateException("illegal randomization branch");
}
}

public void testMappingsEquals() throws IOException {
{
CompressedXContent mappings = randomMappings();
assertThat(Template.mappingsEquals(mappings, mappings), equalTo(true));
}

{
assertThat(Template.mappingsEquals(null, null), equalTo(true));
}

{
CompressedXContent mappings = randomMappings();
assertThat(Template.mappingsEquals(mappings, null), equalTo(false));
assertThat(Template.mappingsEquals(null, mappings), equalTo(false));
}

{
String randomString = randomAlphaOfLength(10);
CompressedXContent m1 = new CompressedXContent("{\"properties\":{\"" + randomString + "\":{\"type\":\"keyword\"}}}");
CompressedXContent m2 = new CompressedXContent("{\"properties\":{\"" + randomString + "\":{\"type\":\"keyword\"}}}");
assertThat(Template.mappingsEquals(m1, m2), equalTo(true));
}

{
CompressedXContent m1 = randomMappings();
CompressedXContent m2 = new CompressedXContent("{\"properties\":{\"" + randomAlphaOfLength(10) + "\":{\"type\":\"keyword\"}}}");
assertThat(Template.mappingsEquals(m1, m2), equalTo(false));
}

{
Map<String, Object> map = XContentHelper.convertToMap(
new BytesArray(
"{\""
+ MapperService.SINGLE_MAPPING_NAME
+ "\":{\"properties\":{\""
+ randomAlphaOfLength(10)
+ "\":{\"type\":\"keyword\"}}}}"
),
true,
XContentType.JSON
).v2();
Map<String, Object> reduceMap = Template.reduceMapping(map);
CompressedXContent m1 = new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(map)));
CompressedXContent m2 = new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(reduceMap)));
assertThat(Template.mappingsEquals(m1, m2), equalTo(true));
}
}
}
59 changes: 6 additions & 53 deletions ...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.mapper.DataStreamTimestampFieldMapper;
Expand All @@ -42,7 +39,6 @@
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESSingleNodeTestCase;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -341,7 +337,11 @@ public void testPutGlobalTemplateWithIndexHiddenSetting() throws Exception {
public void testAddComponentTemplate() throws Exception{
MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
ClusterState state = ClusterState.EMPTY_STATE;
Template template = new Template(Settings.builder().build(), null, ComponentTemplateTests.randomAliases());
Template template = new Template(
Settings.builder().build(),
new CompressedXContent("{\"properties\":{\"@timestamp\":{\"type\":\"date\"}}}"),
ComponentTemplateTests.randomAliases()
);
ComponentTemplate componentTemplate = new ComponentTemplate(template, 1L, new HashMap<>());
state = metadataIndexTemplateService.addComponentTemplate(state, false, "foo", componentTemplate);

Expand Down Expand Up @@ -1587,55 +1587,8 @@ clusterService, createIndexService, new AliasValidator(), indicesService,
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS), xContentRegistry());
}

@SuppressWarnings("unchecked")
public static void assertTemplatesEqual(ComposableIndexTemplate actual, ComposableIndexTemplate expected) {
ComposableIndexTemplate actualNoTemplate = new ComposableIndexTemplate(actual.indexPatterns(), null,
actual.composedOf(), actual.priority(), actual.version(), actual.metadata(), actual.getDataStreamTemplate(), null);
ComposableIndexTemplate expectedNoTemplate = new ComposableIndexTemplate(expected.indexPatterns(), null,
expected.composedOf(), expected.priority(), expected.version(), expected.metadata(), expected.getDataStreamTemplate(), null);

assertThat(actualNoTemplate, equalTo(expectedNoTemplate));
Template actualTemplate = actual.template();
Template expectedTemplate = expected.template();

assertThat("expected both templates to have either a template or no template",
Objects.nonNull(actualTemplate), equalTo(Objects.nonNull(expectedTemplate)));

if (actualTemplate != null) {
assertThat(actualTemplate.settings(), equalTo(expectedTemplate.settings()));
assertThat(actualTemplate.aliases(), equalTo(expectedTemplate.aliases()));
assertThat("expected both templates to have either mappings or no mappings",
Objects.nonNull(actualTemplate.mappings()), equalTo(Objects.nonNull(expectedTemplate.mappings())));

if (actualTemplate.mappings() != null) {
Map<String, Object> actualMappings;
Map<String, Object> expectedMappings;
try (XContentParser parser = XContentType.JSON.xContent()
.createParser(new NamedXContentRegistry(List.of()), LoggingDeprecationHandler.INSTANCE,
actualTemplate.mappings().string())) {
actualMappings = parser.map();
} catch (IOException e) {
throw new AssertionError(e);
}
try (XContentParser parser = XContentType.JSON.xContent()
.createParser(new NamedXContentRegistry(List.of()), LoggingDeprecationHandler.INSTANCE,
expectedTemplate.mappings().string())) {
expectedMappings = parser.map();
} catch (IOException e) {
throw new AssertionError(e);
}

if (actualMappings.size() == 1 && actualMappings.containsKey(MapperService.SINGLE_MAPPING_NAME)) {
actualMappings = (Map<String, Object>) actualMappings.get(MapperService.SINGLE_MAPPING_NAME);
}

if (expectedMappings.size() == 1 && expectedMappings.containsKey(MapperService.SINGLE_MAPPING_NAME)) {
expectedMappings = (Map<String, Object>) expectedMappings.get(MapperService.SINGLE_MAPPING_NAME);
}

assertThat(actualMappings, equalTo(expectedMappings));
}
}
assertTrue(Objects.equals(actual, expected));
}

// Composable index template with data_stream definition need _timestamp meta field mapper,
Expand Down

0 comments on commit e321fb0

Please sign in to comment.