Skip to content

Add back support for enabled/includes/excludes in _source field #11171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 14, 2015
Merged

Add back support for enabled/includes/excludes in _source field #11171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d31ce43
Mappings: Add back support for enabled/includes/excludes in _source
rjernst May 14, 2015
0e14c6d
Fix includes/excludes to be handled on merge conflict checking when they
rjernst May 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.elasticsearch.index.mapper.core.AbstractFieldMapper;

import java.io.IOException;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -150,7 +151,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
Map.Entry<String, Object> entry = iterator.next();
String fieldName = Strings.toUnderscoreCase(entry.getKey());
Object fieldNode = entry.getValue();
if (fieldName.equals("enabled") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
if (fieldName.equals("enabled")) {
builder.enabled(nodeBooleanValue(fieldNode));
iterator.remove();
} else if (fieldName.equals("compress") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
Expand All @@ -172,15 +173,15 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
} else if ("format".equals(fieldName)) {
builder.format(nodeStringValue(fieldNode, null));
iterator.remove();
} else if (fieldName.equals("includes") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
} else if (fieldName.equals("includes")) {
List<Object> values = (List<Object>) fieldNode;
String[] includes = new String[values.size()];
for (int i = 0; i < includes.length; i++) {
includes[i] = values.get(i).toString();
}
builder.includes(includes);
iterator.remove();
} else if (fieldName.equals("excludes") && parserContext.indexVersionCreated().before(Version.V_2_0_0)) {
} else if (fieldName.equals("excludes")) {
List<Object> values = (List<Object>) fieldNode;
String[] excludes = new String[values.size()];
for (int i = 0; i < excludes.length; i++) {
Expand All @@ -197,11 +198,14 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext

private final boolean enabled;

/** indicates whether the source will always exist and be complete, for use by features like the update API */
private final boolean complete;

private Boolean compress;
private long compressThreshold;

private String[] includes;
private String[] excludes;
private final String[] includes;
private final String[] excludes;

private String format;

Expand All @@ -218,23 +222,28 @@ protected SourceFieldMapper(String name, boolean enabled, String format, Boolean
this.enabled = enabled;
this.compress = compress;
this.compressThreshold = compressThreshold;
this.includes = includes;
this.excludes = excludes;
this.includes = includes == null ? Strings.EMPTY_ARRAY : includes;
this.excludes = excludes == null ? Strings.EMPTY_ARRAY : excludes;
this.format = format;
this.formatContentType = format == null ? null : XContentType.fromRestContentType(format);
this.complete = enabled && includes == null && excludes == null;
}

public boolean enabled() {
return this.enabled;
return enabled;
}

public String[] excludes() {
return this.excludes != null ? this.excludes : Strings.EMPTY_ARRAY;
return excludes;

}

public String[] includes() {
return this.includes != null ? this.includes : Strings.EMPTY_ARRAY;
return includes;
}

public boolean isComplete() {
return complete;
}

@Override
Expand Down Expand Up @@ -420,19 +429,23 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
SourceFieldMapper sourceMergeWith = (SourceFieldMapper) mergeWith;
if (!mergeResult.simulate()) {
if (mergeResult.simulate()) {
if (this.enabled != sourceMergeWith.enabled) {
mergeResult.addConflict("Cannot update enabled setting for [_source]");
}
if (Arrays.equals(includes, sourceMergeWith.includes) == false) {
mergeResult.addConflict("Cannot update includes setting for [_source]");
}
if (Arrays.equals(excludes, sourceMergeWith.excludes) == false) {
mergeResult.addConflict("Cannot update excludes setting for [_source]");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also emit conflicts if simulate is false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just following what I saw other mappers doing. Really I think merge should be split into two separate methods, one for validating updates, and the other for applying them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my dreams, merging would either throw an exception or return a new independent mapping so that we wouldn't need this validation phase :)

} else {
if (sourceMergeWith.compress != null) {
this.compress = sourceMergeWith.compress;
}
if (sourceMergeWith.compressThreshold != -1) {
this.compressThreshold = sourceMergeWith.compressThreshold;
}
if (sourceMergeWith.includes != null) {
this.includes = sourceMergeWith.includes;
}
if (sourceMergeWith.excludes != null) {
this.excludes = sourceMergeWith.excludes;
}
}
}
}
4 changes: 2 additions & 2 deletions src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public void hitExecute(SearchContext context, HitContext hitContext) {

if (context.highlight().forceSource(field)) {
SourceFieldMapper sourceFieldMapper = context.mapperService().documentMapper(hitContext.hit().type()).sourceMapper();
if (!sourceFieldMapper.enabled()) {
throw new IllegalArgumentException("source is forced for fields " + fieldNamesToHighlight + " but type [" + hitContext.hit().type() + "] has disabled _source");
if (!sourceFieldMapper.isComplete()) {
throw new IllegalArgumentException("source is forced for fields " + fieldNamesToHighlight + " but type [" + hitContext.hit().type() + "] has incomplete _source");
}
}

Expand Down
132 changes: 103 additions & 29 deletions src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@
import org.elasticsearch.test.ElasticsearchSingleNodeTest;
import org.junit.Test;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.*;

public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest {
Settings backcompatSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build();

public void testNoFormat() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
Expand Down Expand Up @@ -80,8 +81,8 @@ public void testJsonFormat() throws Exception {

documentMapper = parser.parse(mapping);
doc = documentMapper.parse("type", "1", XContentFactory.smileBuilder().startObject()
.field("field", "value")
.endObject().bytes());
.field("field", "value")
.endObject().bytes());

assertThat(XContentFactory.xContentType(doc.source()), equalTo(XContentType.JSON));
}
Expand All @@ -91,6 +92,7 @@ public void testJsonFormatCompressedBackcompat() throws Exception {
.startObject("_source").field("format", "json").field("compress", true).endObject()
.endObject().endObject().string();

Settings backcompatSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build();
DocumentMapperParser parser = createIndex("test", backcompatSettings).mapperService().documentMapperParser();
DocumentMapper documentMapper = parser.parse(mapping);
ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
Expand All @@ -111,19 +113,12 @@ public void testJsonFormatCompressedBackcompat() throws Exception {
assertThat(XContentFactory.xContentType(uncompressed), equalTo(XContentType.JSON));
}

public void testIncludesBackcompat() throws Exception {
public void testIncludes() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").field("includes", new String[]{"path1*"}).endObject()
.endObject().endObject().string();

try {
createIndex("testbad").mapperService().documentMapperParser().parse(mapping);
fail("includes should not be allowed");
} catch (MapperParsingException e) {
assertTrue(e.getMessage().contains("unsupported parameters"));
}

DocumentMapper documentMapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse(mapping);
DocumentMapper documentMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
.startObject("path1").field("field1", "value1").endObject()
Expand All @@ -136,19 +131,12 @@ public void testIncludesBackcompat() throws Exception {
assertThat(sourceAsMap.containsKey("path2"), equalTo(false));
}

public void testExcludesBackcompat() throws Exception {
public void testExcludes() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").field("excludes", new String[]{"path1*"}).endObject()
.endObject().endObject().string();

try {
createIndex("testbad").mapperService().documentMapperParser().parse(mapping);
fail("excludes should not be allowed");
} catch (MapperParsingException e) {
assertTrue(e.getMessage().contains("unsupported parameters"));
}

DocumentMapper documentMapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse(mapping);
DocumentMapper documentMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);

ParsedDocument doc = documentMapper.parse("type", "1", XContentFactory.jsonBuilder().startObject()
.startObject("path1").field("field1", "value1").endObject()
Expand All @@ -161,12 +149,12 @@ public void testExcludesBackcompat() throws Exception {
assertThat(sourceAsMap.containsKey("path2"), equalTo(true));
}

public void testDefaultMappingAndNoMappingBackcompat() throws Exception {
public void testDefaultMappingAndNoMapping() throws Exception {
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();

DocumentMapperParser parser = createIndex("test", backcompatSettings).mapperService().documentMapperParser();
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
DocumentMapper mapper = parser.parse("my_type", null, defaultMapping);
assertThat(mapper.type(), equalTo("my_type"));
assertThat(mapper.sourceMapper().enabled(), equalTo(false));
Expand All @@ -189,7 +177,7 @@ public void testDefaultMappingAndNoMappingBackcompat() throws Exception {
}
}

public void testDefaultMappingAndWithMappingOverrideBackcompat() throws Exception {
public void testDefaultMappingAndWithMappingOverride() throws Exception {
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();
Expand All @@ -198,30 +186,30 @@ public void testDefaultMappingAndWithMappingOverrideBackcompat() throws Exceptio
.startObject("_source").field("enabled", true).endObject()
.endObject().endObject().string();

DocumentMapper mapper = createIndex("test", backcompatSettings).mapperService().documentMapperParser().parse("my_type", mapping, defaultMapping);
DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser().parse("my_type", mapping, defaultMapping);
assertThat(mapper.type(), equalTo("my_type"));
assertThat(mapper.sourceMapper().enabled(), equalTo(true));
}

public void testDefaultMappingAndNoMappingWithMapperServiceBackcompat() throws Exception {
public void testDefaultMappingAndNoMappingWithMapperService() throws Exception {
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();

MapperService mapperService = createIndex("test", backcompatSettings).mapperService();
MapperService mapperService = createIndex("test").mapperService();
mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedString(defaultMapping), true);

DocumentMapper mapper = mapperService.documentMapperWithAutoCreate("my_type").v1();
assertThat(mapper.type(), equalTo("my_type"));
assertThat(mapper.sourceMapper().enabled(), equalTo(false));
}

public void testDefaultMappingAndWithMappingOverrideWithMapperServiceBackcompat() throws Exception {
public void testDefaultMappingAndWithMappingOverrideWithMapperService() throws Exception {
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();

MapperService mapperService = createIndex("test", backcompatSettings).mapperService();
MapperService mapperService = createIndex("test").mapperService();
mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedString(defaultMapping), true);

String mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
Expand All @@ -233,4 +221,90 @@ public void testDefaultMappingAndWithMappingOverrideWithMapperServiceBackcompat(
assertThat(mapper.type(), equalTo("my_type"));
assertThat(mapper.sourceMapper().enabled(), equalTo(true));
}

void assertConflicts(String mapping1, String mapping2, DocumentMapperParser parser, String... conflicts) throws IOException {
DocumentMapper docMapper = parser.parse(mapping1);
docMapper.refreshSource();
docMapper = parser.parse(docMapper.mappingSource().string());
MergeResult mergeResult = docMapper.merge(parser.parse(mapping2).mapping(), true);

List<String> expectedConflicts = new ArrayList<>(Arrays.asList(conflicts));
for (String conflict : mergeResult.buildConflicts()) {
assertTrue("found unexpected conflict [" + conflict + "]", expectedConflicts.remove(conflict));
}
assertTrue("missing conflicts: " + Arrays.toString(expectedConflicts.toArray()), expectedConflicts.isEmpty());
}

public void testEnabledNotUpdateable() throws Exception {
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
// using default of true
String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();
assertConflicts(mapping1, mapping2, parser, "Cannot update enabled setting for [_source]");

// not changing is ok
String mapping3 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").field("enabled", true).endObject()
.endObject().endObject().string();
assertConflicts(mapping1, mapping3, parser);
}

public void testIncludesNotUpdateable() throws Exception {
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("includes", "foo.*").endObject()
.endObject().endObject().string();
assertConflicts(defaultMapping, mapping1, parser, "Cannot update includes setting for [_source]");
assertConflicts(mapping1, defaultMapping, parser, "Cannot update includes setting for [_source]");

String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("includes", "foo.*", "bar.*").endObject()
.endObject().endObject().string();
assertConflicts(mapping1, mapping2, parser, "Cannot update includes setting for [_source]");

// not changing is ok
assertConflicts(mapping1, mapping1, parser);
}

public void testExcludesNotUpdateable() throws Exception {
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
String mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("excludes", "foo.*").endObject()
.endObject().endObject().string();
assertConflicts(defaultMapping, mapping1, parser, "Cannot update excludes setting for [_source]");
assertConflicts(mapping1, defaultMapping, parser, "Cannot update excludes setting for [_source]");

String mapping2 = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("excludes", "foo.*", "bar.*").endObject()
.endObject().endObject().string();
assertConflicts(mapping1, mapping2, parser, "Cannot update excludes setting for [_source]");

// not changing is ok
assertConflicts(mapping1, mapping1, parser);
}

public void testComplete() throws Exception {
DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
assertTrue(parser.parse(mapping).sourceMapper().isComplete());

mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").field("enabled", false).endObject()
.endObject().endObject().string();
assertFalse(parser.parse(mapping).sourceMapper().isComplete());

mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("includes", "foo.*").endObject()
.endObject().endObject().string();
assertFalse(parser.parse(mapping).sourceMapper().isComplete());

mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("_source").array("excludes", "foo.*").endObject()
.endObject().endObject().string();
assertFalse(parser.parse(mapping).sourceMapper().isComplete());
}
}
Loading