Mapping: Fix possibility of losing meta configuration on field mapping update
The TTL, size, timestamp and index meta properties could be lost on an update of a single field mapping due to a wrong comparison in the merge method (which was caused by a wrong initialization, which marked an update as explicitely disabled instead of unset. Closes #5053
This commit is contained in:
parent
d18fb8bfbd
commit
9569166f94
|
@ -67,7 +67,7 @@ public class IndexFieldMapper extends AbstractFieldMapper<String> implements Int
|
|||
FIELD_TYPE.freeze();
|
||||
}
|
||||
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.DISABLED;
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.UNSET_DISABLED;
|
||||
}
|
||||
|
||||
public static class Builder extends AbstractFieldMapper.Builder<Builder, IndexFieldMapper> {
|
||||
|
@ -202,7 +202,7 @@ public class IndexFieldMapper extends AbstractFieldMapper<String> implements Int
|
|||
if (includeDefaults || fieldType().stored() != Defaults.FIELD_TYPE.stored() && enabledState.enabled) {
|
||||
builder.field("store", fieldType().stored());
|
||||
}
|
||||
if (includeDefaults || enabledState != Defaults.ENABLED_STATE) {
|
||||
if (includeDefaults || enabledState.enabled != Defaults.ENABLED_STATE.enabled) {
|
||||
builder.field("enabled", enabledState.enabled);
|
||||
}
|
||||
|
||||
|
|
|
@ -47,7 +47,7 @@ public class SizeFieldMapper extends IntegerFieldMapper implements RootMapper {
|
|||
|
||||
public static class Defaults extends IntegerFieldMapper.Defaults {
|
||||
public static final String NAME = CONTENT_TYPE;
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.DISABLED;
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.UNSET_DISABLED;
|
||||
|
||||
public static final FieldType SIZE_FIELD_TYPE = new FieldType(IntegerFieldMapper.Defaults.FIELD_TYPE);
|
||||
|
||||
|
@ -161,7 +161,7 @@ public class SizeFieldMapper extends IntegerFieldMapper implements RootMapper {
|
|||
return builder;
|
||||
}
|
||||
builder.startObject(contentType());
|
||||
if (includeDefaults || enabledState != Defaults.ENABLED_STATE) {
|
||||
if (includeDefaults || enabledState.enabled != Defaults.ENABLED_STATE.enabled) {
|
||||
builder.field("enabled", enabledState.enabled);
|
||||
}
|
||||
if (includeDefaults || fieldType().stored() != Defaults.SIZE_FIELD_TYPE.stored() && enabledState.enabled) {
|
||||
|
|
|
@ -64,7 +64,7 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
|
|||
TTL_FIELD_TYPE.freeze();
|
||||
}
|
||||
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.DISABLED;
|
||||
public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.UNSET_DISABLED;
|
||||
public static final long DEFAULT = -1;
|
||||
}
|
||||
|
||||
|
@ -225,7 +225,7 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
|
|||
return builder;
|
||||
}
|
||||
builder.startObject(CONTENT_TYPE);
|
||||
if (includeDefaults || enabledState != Defaults.ENABLED_STATE) {
|
||||
if (includeDefaults || enabledState.enabled != Defaults.ENABLED_STATE.enabled) {
|
||||
builder.field("enabled", enabledState.enabled);
|
||||
}
|
||||
if (includeDefaults || defaultTTL != Defaults.DEFAULT && enabledState.enabled) {
|
||||
|
|
|
@ -67,7 +67,7 @@ public class TimestampFieldMapper extends DateFieldMapper implements InternalMap
|
|||
FIELD_TYPE.freeze();
|
||||
}
|
||||
|
||||
public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.DISABLED;
|
||||
public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_DISABLED;
|
||||
public static final String PATH = null;
|
||||
public static final FormatDateTimeFormatter DATE_TIME_FORMATTER = Joda.forPattern(DEFAULT_DATE_TIME_FORMAT);
|
||||
}
|
||||
|
@ -230,7 +230,7 @@ public class TimestampFieldMapper extends DateFieldMapper implements InternalMap
|
|||
return builder;
|
||||
}
|
||||
builder.startObject(CONTENT_TYPE);
|
||||
if (includeDefaults || enabledState != Defaults.ENABLED) {
|
||||
if (includeDefaults || enabledState.enabled != Defaults.ENABLED.enabled) {
|
||||
builder.field("enabled", enabledState.enabled);
|
||||
}
|
||||
if (enabledState.enabled) {
|
||||
|
|
|
@ -0,0 +1,71 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
package org.elasticsearch.index.mapper.index;
|
||||
|
||||
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
|
||||
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
public class IndexTypeMapperIntegrationTests extends ElasticsearchIntegrationTest {
|
||||
|
||||
@Test // issue 5053
|
||||
public void testThatUpdatingMappingShouldNotRemoveSizeMappingConfiguration() throws Exception {
|
||||
String index = "foo";
|
||||
String type = "mytype";
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startObject("_index").field("enabled", true).endObject().endObject();
|
||||
assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
|
||||
|
||||
// check mapping again
|
||||
assertIndexMappingEnabled(index, type);
|
||||
|
||||
// update some field in the mapping
|
||||
XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "string").endObject().endObject();
|
||||
PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
|
||||
assertAcked(putMappingResponse);
|
||||
|
||||
// make sure timestamp field is still in mapping
|
||||
assertIndexMappingEnabled(index, type);
|
||||
}
|
||||
|
||||
private void assertIndexMappingEnabled(String index, String type) throws IOException {
|
||||
String errMsg = String.format(Locale.ROOT, "Expected index field mapping to be enabled for %s/%s", index, type);
|
||||
|
||||
GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
|
||||
Map<String, Object> mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap();
|
||||
assertThat(errMsg, mappingSource, hasKey("_index"));
|
||||
String ttlAsString = mappingSource.get("_index").toString();
|
||||
assertThat(ttlAsString, is(notNullValue()));
|
||||
assertThat(errMsg, ttlAsString, is("{enabled=true}"));
|
||||
}
|
||||
|
||||
}
|
|
@ -0,0 +1,67 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
package org.elasticsearch.index.mapper.size;
|
||||
|
||||
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
|
||||
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
||||
public class SizeMappingIntegrationTests extends ElasticsearchIntegrationTest {
|
||||
|
||||
@Test // issue 5053
|
||||
public void testThatUpdatingMappingShouldNotRemoveSizeMappingConfiguration() throws Exception {
|
||||
String index = "foo";
|
||||
String type = "mytype";
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
|
||||
assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
|
||||
|
||||
// check mapping again
|
||||
assertSizeMappingEnabled(index, type);
|
||||
|
||||
// update some field in the mapping
|
||||
XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "string").endObject().endObject();
|
||||
PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
|
||||
assertAcked(putMappingResponse);
|
||||
|
||||
// make sure timestamp field is still in mapping
|
||||
assertSizeMappingEnabled(index, type);
|
||||
}
|
||||
|
||||
private void assertSizeMappingEnabled(String index, String type) throws IOException {
|
||||
String errMsg = String.format(Locale.ROOT, "Expected size field mapping to be enabled for %s/%s", index, type);
|
||||
|
||||
GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
|
||||
Map<String, Object> mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap();
|
||||
assertThat(errMsg, mappingSource, hasKey("_size"));
|
||||
String ttlAsString = mappingSource.get("_size").toString();
|
||||
assertThat(ttlAsString, is(notNullValue()));
|
||||
assertThat(errMsg, ttlAsString, is("{enabled=true}"));
|
||||
}
|
||||
}
|
|
@ -19,12 +19,19 @@
|
|||
|
||||
package org.elasticsearch.timestamp;
|
||||
|
||||
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
|
||||
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
|
||||
import org.elasticsearch.action.get.GetResponse;
|
||||
import org.elasticsearch.cluster.metadata.MappingMetaData;
|
||||
import org.elasticsearch.common.Priority;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.Locale;
|
||||
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
||||
/**
|
||||
|
@ -35,7 +42,7 @@ public class SimpleTimestampTests extends ElasticsearchIntegrationTest {
|
|||
public void testSimpleTimestamp() throws Exception {
|
||||
|
||||
client().admin().indices().prepareCreate("test")
|
||||
.addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("_timestamp").field("enabled", true).field("store", "yes").endObject().endObject().endObject())
|
||||
.addMapping("type1", jsonBuilder().startObject().startObject("type1").startObject("_timestamp").field("enabled", true).field("store", "yes").endObject().endObject().endObject())
|
||||
.execute().actionGet();
|
||||
client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
|
||||
|
||||
|
@ -82,4 +89,32 @@ public class SimpleTimestampTests extends ElasticsearchIntegrationTest {
|
|||
getResponse = client().prepareGet("test", "type1", "1").setFields("_timestamp").setRealtime(false).execute().actionGet();
|
||||
assertThat(((Number) getResponse.getField("_timestamp").getValue()).longValue(), equalTo(timestamp));
|
||||
}
|
||||
|
||||
@Test // issue 5053
|
||||
public void testThatUpdatingMappingShouldNotRemoveTimestampConfiguration() throws Exception {
|
||||
String index = "foo";
|
||||
String type = "mytype";
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startObject("_timestamp").field("enabled", true).endObject().endObject();
|
||||
assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
|
||||
|
||||
// check mapping again
|
||||
assertTimestampMappingEnabled(index, type);
|
||||
|
||||
// update some field in the mapping
|
||||
XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "string").endObject().endObject();
|
||||
PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
|
||||
assertAcked(putMappingResponse);
|
||||
|
||||
// make sure timestamp field is still in mapping
|
||||
assertTimestampMappingEnabled(index, type);
|
||||
}
|
||||
|
||||
private void assertTimestampMappingEnabled(String index, String type) {
|
||||
GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
|
||||
MappingMetaData.Timestamp timestamp = getMappingsResponse.getMappings().get(index).get(type).timestamp();
|
||||
assertThat(timestamp, is(notNullValue()));
|
||||
String errMsg = String.format(Locale.ROOT, "Expected timestamp field mapping to be enabled for %s/%s", index, type);
|
||||
assertThat(errMsg, timestamp.enabled(), is(true));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -20,18 +20,26 @@
|
|||
package org.elasticsearch.ttl;
|
||||
|
||||
import com.google.common.base.Predicate;
|
||||
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
|
||||
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
|
||||
import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
|
||||
import org.elasticsearch.action.get.GetResponse;
|
||||
import org.elasticsearch.action.index.IndexResponse;
|
||||
import org.elasticsearch.cluster.metadata.MappingMetaData;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.test.ElasticsearchIntegrationTest.*;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
@ -192,4 +200,35 @@ public class SimpleTTLTests extends ElasticsearchIntegrationTest {
|
|||
getResponse = client().prepareGet("test", "type1", "with_routing").setRouting("routing").setFields("_ttl").setRealtime(false).execute().actionGet();
|
||||
assertThat(getResponse.isExists(), equalTo(false));
|
||||
}
|
||||
|
||||
@Test // issue 5053
|
||||
public void testThatUpdatingMappingShouldNotRemoveTTLConfiguration() throws Exception {
|
||||
String index = "foo";
|
||||
String type = "mytype";
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startObject("_ttl").field("enabled", true).endObject().endObject();
|
||||
assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
|
||||
|
||||
// check mapping again
|
||||
assertTTLMappingEnabled(index, type);
|
||||
|
||||
// update some field in the mapping
|
||||
XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "string").endObject().endObject();
|
||||
PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
|
||||
assertAcked(putMappingResponse);
|
||||
|
||||
// make sure timestamp field is still in mapping
|
||||
assertTTLMappingEnabled(index, type);
|
||||
}
|
||||
|
||||
private void assertTTLMappingEnabled(String index, String type) throws IOException {
|
||||
String errMsg = String.format(Locale.ROOT, "Expected ttl field mapping to be enabled for %s/%s", index, type);
|
||||
|
||||
GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
|
||||
Map<String, Object> mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap();
|
||||
assertThat(errMsg, mappingSource, hasKey("_ttl"));
|
||||
String ttlAsString = mappingSource.get("_ttl").toString();
|
||||
assertThat(ttlAsString, is(notNullValue()));
|
||||
assertThat(errMsg, ttlAsString, is("{enabled=true}"));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue