Skip to content

Commit

Permalink
Explicitly say if stored fields aren't supported in MapperTestCase (#…
Browse files Browse the repository at this point in the history
…72474)

MapperTestCase has a check that if a field mapper supports stored fields,
those stored fields are available to index time scripts. Many of our mappers
do not support stored fields, and we try and catch this with an assumeFalse
so that those mappers do not run this test. However, this test is fragile - it
does not work for mappers created with an index version below 8.0, and it
misses mappers that always store their values, e.g. match_only_text.

This commit adds a new supportsStoredField method to MapperTestCase,
and overrides it for those mappers that do not support storing values. It
also adds a minimalStoredMapping method that defaults to the minimal
mapping plus a store parameter, which is overridden by match_only_text
because storing is not configurable and always available on this mapper.
  • Loading branch information
romseygeek committed Apr 30, 2021
1 parent c6aab5f commit 54da625
Show file tree
Hide file tree
Showing 17 changed files with 55 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
b.field("type", "match_only_text");
}

@Override
protected void minimalStoreMapping(XContentBuilder b) throws IOException {
// 'store' is always true
minimalMapping(b);
}

public void testDefaults() throws IOException {
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
assertEquals(Strings.toString(fieldMapping(this::minimalMapping)), mapper.mappingSource().toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ protected void assertSearchable(MappedFieldType fieldType) {
}

@Override
protected Collection<? extends Plugin> getPlugins() {
return List.of(new MapperExtrasPlugin());
protected boolean supportsStoredFields() {
return false;
}

@Override
protected boolean allowsStore() {
return false;
protected Collection<? extends Plugin> getPlugins() {
return List.of(new MapperExtrasPlugin());
}

static int getFrequency(TokenStream tk) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,17 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
}

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false));
protected boolean supportsStoredFields() {
return false;
}

@Override
protected boolean supportsMeta() {
return false;
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false));
}

@Override
protected boolean allowsStore() {
protected boolean supportsMeta() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ protected void metaMapping(XContentBuilder b) throws IOException {
b.field("max_input_length", 50);
}

@Override
protected boolean supportsStoredFields() {
return false;
}

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("analyzer", b -> b.field("analyzer", "standard"));
Expand Down Expand Up @@ -108,11 +113,6 @@ protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {
return new IndexAnalyzers(analyzers, Collections.emptyMap(), Collections.emptyMap());
}

@Override
protected boolean allowsStore() {
return false;
}

public void testDefaultConfiguration() throws IOException {

DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
}

@Override
protected Object getSampleValueForDocument() {
return "POINT (14.0 15.0)";
protected boolean supportsStoredFields() {
return false;
}

@Override
protected boolean allowsStore() {
return false;
protected Object getSampleValueForDocument() {
return "POINT (14.0 15.0)";
}

public void testDefaultConfiguration() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
b.field("type", "geo_shape").field("strategy", "recursive");
}

@Override
protected boolean supportsStoredFields() {
return false;
}

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {

Expand Down Expand Up @@ -98,11 +103,6 @@ protected boolean supportsMeta() {
return false;
}

@Override
protected boolean allowsStore() {
return false;
}

public void testLegacySwitches() throws IOException {
// if one of the legacy parameters is added to a 'type':'geo_shape' config then
// that will select the legacy field mapper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,29 +631,24 @@ public final void testIndexTimeFieldData() throws IOException {
});
}

protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return true;
}

protected void minimalStoreMapping(XContentBuilder b) throws IOException {
minimalMapping(b);
b.field("store", true);
}

/**
* Checks that loading stored fields for this field produces the same set of values
* for query time scripts and index time scripts
*/
public final void testIndexTimeStoredFieldsAccess() throws IOException {

assumeTrue("FieldMapper implementation does not allow stored fields", allowsStore());
MapperService mapperService;
try {
mapperService = createMapperService(fieldMapping(b -> {
minimalMapping(b);
b.field("store", true);
}));
assertParseMinimalWarnings();
} catch (MapperParsingException e) {
assertParseMinimalWarnings();
assumeFalse("Field type does not support stored fields", true);
return;
}
assumeTrue("Field type does not support stored fields", supportsStoredFields());
MapperService mapperService = createMapperService(fieldMapping(this::minimalStoreMapping));
assertParseMinimalWarnings();

MappedFieldType fieldType = mapperService.fieldType("field");
SourceToParse source = source(this::writeField);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ protected boolean supportsSearchLookup() {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ protected Object getSampleValueForQuery() {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ protected Collection<Plugin> getPlugins() {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ protected boolean supportsSearchLookup() {
return false;
}

@Override
protected boolean supportsStoredFields() {
return false;
}

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false));
Expand All @@ -81,11 +86,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
});
}

@Override
protected boolean allowsStore() {
return false;
}

@Override
protected Collection<Plugin> getPlugins() {
return Collections.singletonList(new LocalStateSpatialPlugin());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
}

@Override
protected boolean allowsStore() {
protected boolean supportsStoredFields() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ static SearchExecutionContext createMockSearchExecutionContext(boolean allowExpe
static KeywordFieldMapper keywordFieldType;

@Override
protected boolean allowsStore() {
return false;
protected Collection<? extends Plugin> getPlugins() {
return Collections.singleton(new Wildcard());
}

@Override
protected Collection<? extends Plugin> getPlugins() {
return Collections.singleton(new Wildcard());
protected boolean supportsStoredFields() {
return false;
}

@Override
Expand Down

0 comments on commit 54da625

Please sign in to comment.