Skip to content

Commit

Permalink
Correctly enable _all for older 5.x indices
Browse files Browse the repository at this point in the history
When we disabled `_all` by default for indices created in 6.0, we missed adding
a layer that would handle the situation where `_all` was not enabled in 5.x and
then the cluster was updated to 6.0, this means that when the cluster was
updated the `_all` field would be disabled for 5.x indices and field values
would not be added to the `_all` field.

This adds a compatibility layer for 5.x indices where we treat the default
enabled value for the `_all` field to be `true` if unset on 5.x indices.

Resolves elastic#25068
  • Loading branch information
dakrone committed Jun 8, 2017
1 parent 3f6d80a commit 119f8ed
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,38 @@ public MetadataFieldMapper.Builder<?,?> parse(String name, Map<String, Object> n
}

parseTextField(builder, builder.name, node, parserContext);
boolean enabledSet = false;
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String fieldName = entry.getKey();
Object fieldNode = entry.getValue();
if (fieldName.equals("enabled")) {
boolean enabled = TypeParsers.nodeBooleanValueLenient(name, "enabled", fieldNode);
builder.enabled(enabled ? EnabledAttributeMapper.ENABLED : EnabledAttributeMapper.DISABLED);
enabledSet = true;
iterator.remove();
}
}
if (enabledSet == false && parserContext.indexVersionCreated().before(Version.V_6_0_0_alpha1)) {
// So there is no "enabled" field, however, the index was created prior to 6.0,
// and therefore the default for this particular index should be "true" for
// enabling _all
builder.enabled(EnabledAttributeMapper.ENABLED);
}
return builder;
}

@Override
public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext context) {
final Settings indexSettings = context.mapperService().getIndexSettings().getSettings();
if (fieldType != null) {
return new AllFieldMapper(indexSettings, fieldType);
if (context.indexVersionCreated().before(Version.V_6_0_0_alpha1)) {
// The index was created prior to 6.0, and therefore the default for this
// particular index should be "true" for enabling _all
return new AllFieldMapper(fieldType.clone(), EnabledAttributeMapper.ENABLED, indexSettings);
} else {
return new AllFieldMapper(indexSettings, fieldType);
}
} else {
return parse(NAME, Collections.emptyMap(), context)
.build(new BuilderContext(indexSettings, new ContentPath(1)));
Expand Down Expand Up @@ -197,7 +211,6 @@ private AllFieldMapper(Settings indexSettings, MappedFieldType existing) {
private AllFieldMapper(MappedFieldType fieldType, EnabledAttributeMapper enabled, Settings indexSettings) {
super(NAME, fieldType, Defaults.FIELD_TYPE, indexSettings);
this.enabledState = enabled;

}

public boolean enabled() {
Expand Down
109 changes: 109 additions & 0 deletions core/src/test/java/org/elasticsearch/index/mapper/AllFieldIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* 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;

import org.elasticsearch.Version;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalSettingsPlugin;

import java.util.Arrays;
import java.util.Collection;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;

public class AllFieldIT extends ESIntegTestCase {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(InternalSettingsPlugin.class); // uses index.version.created
}

public void test5xIndicesContinueToUseAll() throws Exception {
// Default 5.x settings
assertAcked(prepareCreate("test").setSettings("index.version.created", Version.V_5_1_1.id));
client().prepareIndex("test", "type", "1").setSource("body", "foo").get();
refresh();
SearchResponse resp = client().prepareSearch("test").setQuery(QueryBuilders.matchQuery("_all", "foo")).get();
assertHitCount(resp, 1);
assertSearchHits(resp, "1");

// _all explicitly enabled
assertAcked(prepareCreate("test2")
.setSource(jsonBuilder()
.startObject()
.startObject("mappings")
.startObject("type")
.startObject("_all")
.field("enabled", true)
.endObject() // _all
.endObject() // type
.endObject() // mappings
.endObject())
.setSettings("index.version.created", Version.V_5_4_0_ID));
client().prepareIndex("test2", "type", "1").setSource("foo", "bar").get();
refresh();
resp = client().prepareSearch("test2").setQuery(QueryBuilders.matchQuery("_all", "bar")).get();
assertHitCount(resp, 1);
assertSearchHits(resp, "1");

// _all explicitly disabled
assertAcked(prepareCreate("test3")
.setSource(jsonBuilder()
.startObject()
.startObject("mappings")
.startObject("type")
.startObject("_all")
.field("enabled", false)
.endObject() // _all
.endObject() // type
.endObject() // mappings
.endObject())
.setSettings("index.version.created", Version.V_5_4_0_ID));
client().prepareIndex("test3", "type", "1").setSource("foo", "baz").get();
refresh();
resp = client().prepareSearch("test3").setQuery(QueryBuilders.matchQuery("_all", "baz")).get();
assertHitCount(resp, 0);

// _all present, but not enabled or disabled (default settings)
assertAcked(prepareCreate("test4")
.setSource(jsonBuilder()
.startObject()
.startObject("mappings")
.startObject("type")
.startObject("_all")
.endObject() // _all
.endObject() // type
.endObject() // mappings
.endObject())
.setSettings("index.version.created", Version.V_5_4_0_ID));
client().prepareIndex("test4", "type", "1").setSource("foo", "eggplant").get();
refresh();
resp = client().prepareSearch("test4").setQuery(QueryBuilders.matchQuery("_all", "eggplant")).get();
assertHitCount(resp, 1);
assertSearchHits(resp, "1");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.apache.lucene.search.spans.SpanTermQuery;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.compress.CompressedXContent;
Expand Down Expand Up @@ -833,6 +834,9 @@ public void testToQuerySplitOnWhitespace() throws IOException {

public void testExistsFieldQuery() throws Exception {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
assumeTrue("5.x behaves differently, so skip on non-6.x indices",
indexVersionCreated.onOrAfter(Version.V_6_0_0_alpha1));

QueryShardContext context = createShardContext();
QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*");
Query query = queryBuilder.toQuery(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.TestUtil;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.index.mapper.MapperService;
Expand Down Expand Up @@ -198,6 +199,9 @@ public void testFieldsCannotBeSetToNull() {
}

public void testDefaultFieldParsing() throws IOException {
assumeTrue("5.x behaves differently, so skip on non-6.x indices",
indexVersionCreated.onOrAfter(Version.V_6_0_0_alpha1));

String query = randomAlphaOfLengthBetween(1, 10).toLowerCase(Locale.ROOT);
String contentString = "{\n" +
" \"simple_query_string\" : {\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
indices.get_mapping:
index: test_index

- match: { test_index.mappings.type_1: {}}
- is_true: test_index.mappings.type_1

---
"Create index with settings":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ setup:
- do:
indices.get_mapping: {}

- match: { test_1.mappings.type_1: {}}
- match: { test_1.mappings.type_2: {}}
- match: { test_2.mappings.type_2: {}}
- match: { test_2.mappings.type_3: {}}
- is_true: test_1.mappings.type_1
- is_true: test_1.mappings.type_2
- is_true: test_2.mappings.type_2
- is_true: test_2.mappings.type_3

---
"Get /{index}/_mapping":
Expand All @@ -58,8 +58,8 @@ setup:
indices.get_mapping:
index: test_1

- match: { test_1.mappings.type_1: {}}
- match: { test_1.mappings.type_2: {}}
- is_true: test_1.mappings.type_1
- is_true: test_1.mappings.type_2
- is_false: test_2


Expand All @@ -75,8 +75,8 @@ setup:
index: test_1
type: _all

- match: { test_1.mappings.type_1: {}}
- match: { test_1.mappings.type_2: {}}
- is_true: test_1.mappings.type_1
- is_true: test_1.mappings.type_2
- is_false: test_2

---
Expand All @@ -91,8 +91,8 @@ setup:
index: test_1
type: '*'

- match: { test_1.mappings.type_1: {}}
- match: { test_1.mappings.type_2: {}}
- is_true: test_1.mappings.type_1
- is_true: test_1.mappings.type_2
- is_false: test_2

---
Expand All @@ -107,7 +107,6 @@ setup:
index: test_1
type: type_1

- match: { test_1.mappings.type_1: {}}
- is_false: test_1.mappings.type_2
- is_false: test_2

Expand All @@ -123,8 +122,8 @@ setup:
index: test_1
type: type_1,type_2

- match: { test_1.mappings.type_1: {}}
- match: { test_1.mappings.type_2: {}}
- is_true: test_1.mappings.type_1
- is_true: test_1.mappings.type_2
- is_false: test_2

---
Expand All @@ -139,7 +138,7 @@ setup:
index: test_1
type: '*2'

- match: { test_1.mappings.type_2: {}}
- is_true: test_1.mappings.type_2
- is_false: test_1.mappings.type_1
- is_false: test_2

Expand All @@ -154,8 +153,8 @@ setup:
indices.get_mapping:
type: type_2

- match: { test_1.mappings.type_2: {}}
- match: { test_2.mappings.type_2: {}}
- is_true: test_1.mappings.type_2
- is_true: test_2.mappings.type_2
- is_false: test_1.mappings.type_1
- is_false: test_2.mappings.type_3

Expand All @@ -171,8 +170,8 @@ setup:
index: _all
type: type_2

- match: { test_1.mappings.type_2: {}}
- match: { test_2.mappings.type_2: {}}
- is_true: test_1.mappings.type_2
- is_true: test_2.mappings.type_2
- is_false: test_1.mappings.type_1
- is_false: test_2.mappings.type_3

Expand All @@ -188,8 +187,8 @@ setup:
index: '*'
type: type_2

- match: { test_1.mappings.type_2: {}}
- match: { test_2.mappings.type_2: {}}
- is_true: test_1.mappings.type_2
- is_true: test_2.mappings.type_2
- is_false: test_1.mappings.type_1
- is_false: test_2.mappings.type_3

Expand All @@ -205,8 +204,8 @@ setup:
index: test_1,test_2
type: type_2

- match: { test_1.mappings.type_2: {}}
- match: { test_2.mappings.type_2: {}}
- is_true: test_1.mappings.type_2
- is_true: test_2.mappings.type_2
- is_false: test_2.mappings.type_3

---
Expand All @@ -221,6 +220,6 @@ setup:
index: '*2'
type: type_2

- match: { test_2.mappings.type_2: {}}
- is_true: test_2.mappings.type_2
- is_false: test_1
- is_false: test_2.mappings.type_3
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ setup:
indices.get_mapping:
index: test-x*

- match: { test-xxx.mappings.type_1: {}}
- match: { test-xxy.mappings.type_2: {}}
- is_true: test-xxx.mappings.type_1
- is_true: test-xxy.mappings.type_2

---
"Get test-* with wildcard_expansion=all":
Expand All @@ -67,9 +67,9 @@ setup:
index: test-x*
expand_wildcards: all

- match: { test-xxx.mappings.type_1: {}}
- match: { test-xxy.mappings.type_2: {}}
- match: { test-xyy.mappings.type_3: {}}
- is_true: test-xxx.mappings.type_1
- is_true: test-xxy.mappings.type_2
- is_true: test-xyy.mappings.type_3

---
"Get test-* with wildcard_expansion=open":
Expand All @@ -79,8 +79,8 @@ setup:
index: test-x*
expand_wildcards: open

- match: { test-xxx.mappings.type_1: {}}
- match: { test-xxy.mappings.type_2: {}}
- is_true: test-xxx.mappings.type_1
- is_true: test-xxy.mappings.type_2

---
"Get test-* with wildcard_expansion=closed":
Expand All @@ -90,7 +90,7 @@ setup:
index: test-x*
expand_wildcards: closed

- match: { test-xyy.mappings.type_3: {}}
- is_true: test-xyy.mappings.type_3

---
"Get test-* with wildcard_expansion=none":
Expand All @@ -112,8 +112,6 @@ setup:
index: test-x*
expand_wildcards: open,closed

- match: { test-xxx.mappings.type_1: {}}
- match: { test-xxy.mappings.type_2: {}}
- match: { test-xyy.mappings.type_3: {}}


- is_true: test-xxx.mappings.type_1
- is_true: test-xxy.mappings.type_2
- is_true: test-xyy.mappings.type_3
Loading

0 comments on commit 119f8ed

Please sign in to comment.