Skip to content

Scripted keyword field type: update family type and test field caps output #59672

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 7 commits into from
Jul 17, 2020
Merged

Scripted keyword field type: update family type and test field caps output #59672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
69340e7
Scripted keyword field type: update family type and test field caps o…
javanna Jul 15, 2020
37e44f2
Merge branch 'feature/runtime_fields' into enhancement/scripted_field…
javanna Jul 17, 2020
aa771b2
remove assertion
javanna Jul 17, 2020
1ac336c
Merge branch 'feature/runtime_fields' into enhancement/scripted_field…
javanna Jul 17, 2020
da24f12
Merge branch 'feature/runtime_fields' into enhancement/scripted_field…
javanna Jul 17, 2020
b586ca0
expand field caps testing
javanna Jul 17, 2020
06875dc
spotless
javanna Jul 17, 2020
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
18 changes: 16 additions & 2 deletions ...main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.ToXContent.Params;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.TextSearchInfo;
import org.elasticsearch.index.query.QueryShardContext;
Expand Down Expand Up @@ -64,11 +65,24 @@ public Object valueForDisplay(Object value) {

@Override
public String typeName() {
// TODO not sure what we should return here: the runtime type or the field type?
// why is the same string returned from three different methods?
return ScriptFieldMapper.CONTENT_TYPE;
}

@Override
public String familyTypeName() {
return KeywordFieldMapper.CONTENT_TYPE;
}

@Override
public boolean isSearchable() {
return true;
}

@Override
public boolean isAggregatable() {
return true;
}

@Override
public ScriptBinaryFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
// TODO once we get SearchLookup as an argument, we can already call scriptFactory.newFactory here and pass through the result
Expand Down
28 changes: 23 additions & 5 deletions ...-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperParsingException;
Expand All @@ -21,6 +22,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.function.BiFunction;

public final class ScriptFieldMapper extends ParametrizedFieldMapper {

Expand Down Expand Up @@ -69,6 +71,22 @@ protected String contentType() {

public static class Builder extends ParametrizedFieldMapper.Builder {

static final Map<String, BiFunction<Builder, BuilderContext, MappedFieldType>> FIELD_TYPE_RESOLVER = Map.of(
KeywordFieldMapper.CONTENT_TYPE,
(builder, context) -> {
StringScriptFieldScript.Factory factory = builder.scriptCompiler.compile(
builder.script.getValue(),
StringScriptFieldScript.CONTEXT
);
return new RuntimeKeywordMappedFieldType(
builder.buildFullName(context),
builder.script.getValue(),
factory,
builder.meta.getValue()
);
}
);

private static ScriptFieldMapper toType(FieldMapper in) {
return (ScriptFieldMapper) in;
}
Expand Down Expand Up @@ -112,13 +130,13 @@ protected List<Parameter<?>> getParameters() {

@Override
public ScriptFieldMapper build(BuilderContext context) {
MappedFieldType mappedFieldType;
if (runtimeType.getValue().equals("keyword")) {
StringScriptFieldScript.Factory factory = scriptCompiler.compile(script.getValue(), StringScriptFieldScript.CONTEXT);
mappedFieldType = new RuntimeKeywordMappedFieldType(buildFullName(context), script.getValue(), factory, meta.getValue());
} else {
BiFunction<Builder, BuilderContext, MappedFieldType> fieldTypeResolver = Builder.FIELD_TYPE_RESOLVER.get(
Copy link
Member Author

Choose a reason for hiding this comment

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

@nik9000 you may have opinions on this. I added it because I did not want to have tests to maintain a list of supported runtime types and rather rely on the truth which cannot get outdated. Downside is we have no if but rather a map with a bifunction, maybe a bit cryptic.

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty used to maps to functions at this point. I'm happy with this. I'm not 100% this is better than the switch statement that I had in #59721, but it does the job just as well. If you like it, I'm happy with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

it does not do a better job than the switch, but it allows to expose all the keys for testing :)

runtimeType.getValue()
);
if (fieldTypeResolver == null) {
throw new IllegalArgumentException("runtime_type [" + runtimeType.getValue() + "] not supported");
}
MappedFieldType mappedFieldType = fieldTypeResolver.apply(this, context);
// TODO copy to and multi_fields should not be supported, parametrized field mapper needs to be adapted
return new ScriptFieldMapper(
name,
Expand Down
73 changes: 65 additions & 8 deletions ...ds/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@

package org.elasticsearch.xpack.runtimefields.mapper;

import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.plugins.Plugin;
Expand All @@ -22,15 +25,22 @@
import org.elasticsearch.xpack.runtimefields.RuntimeFields;
import org.elasticsearch.xpack.runtimefields.StringScriptFieldScript;

import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
import java.util.Set;

import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.instanceOf;

public class ScriptFieldMapperTests extends ESSingleNodeTestCase {

private static final String[] SUPPORTED_RUNTIME_TYPES = new String[] { "keyword" };
private final String[] runtimeTypes;

public ScriptFieldMapperTests() {
this.runtimeTypes = ScriptFieldMapper.Builder.FIELD_TYPE_RESOLVER.keySet().toArray(new String[0]);
Arrays.sort(runtimeTypes);
}

@Override
protected Collection<Class<? extends Plugin>> getPlugins() {
Expand All @@ -45,7 +55,7 @@ public void testRuntimeTypeIsRequired() throws Exception {
.startObject("properties")
.startObject("my_field")
.field("type", "script")
.field("script", "value('test')")
.field("script", "keyword('test')")
.endObject()
.endObject()
.endObject()
Expand All @@ -63,7 +73,7 @@ public void testScriptIsRequired() throws Exception {
.startObject("properties")
.startObject("my_field")
.field("type", "script")
.field("runtime_type", randomFrom(SUPPORTED_RUNTIME_TYPES))
.field("runtime_type", randomFrom(runtimeTypes))
.endObject()
.endObject()
.endObject()
Expand All @@ -80,7 +90,7 @@ public void testStoredScriptsAreNotSupported() throws Exception {
.startObject("properties")
.startObject("my_field")
.field("type", "script")
.field("runtime_type", randomFrom(SUPPORTED_RUNTIME_TYPES))
.field("runtime_type", randomFrom(runtimeTypes))
.startObject("script")
.field("id", "test")
.endObject()
Expand All @@ -104,7 +114,7 @@ public void testUnsupportedRuntimeType() throws Exception {
.field("type", "script")
.field("runtime_type", "unsupported")
.startObject("script")
.field("source", "value('test')")
.field("source", "keyword('test')")
.field("lang", "test")
.endObject()
.endObject()
Expand All @@ -116,16 +126,63 @@ public void testUnsupportedRuntimeType() throws Exception {
assertEquals("Failed to parse mapping: runtime_type [unsupported] not supported", exc.getMessage());
}

public void testFieldCaps() throws Exception {
for (String runtimeType : runtimeTypes) {
{
XContentBuilder mapping = XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("field")
.field("type", "script")
.field("runtime_type", runtimeType)
.startObject("script")
.field("source", runtimeType + "('test')")
.field("lang", "test")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
createIndex("test_script", Settings.EMPTY, mapping);
}
{
XContentBuilder mapping = XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("field")
.field("type", runtimeType)
.endObject()
.endObject()
.endObject()
.endObject();
createIndex("test_concrete", Settings.EMPTY, mapping);
}
FieldCapabilitiesResponse response = client().prepareFieldCaps("test_*").setFields("field").get();
assertThat(response.getIndices(), arrayContainingInAnyOrder("test_script", "test_concrete"));
Map<String, FieldCapabilities> field = response.getField("field");
assertEquals(1, field.size());
FieldCapabilities fieldCapabilities = field.get(KeywordFieldMapper.CONTENT_TYPE);
assertTrue(fieldCapabilities.isSearchable());
assertTrue(fieldCapabilities.isAggregatable());
assertEquals(runtimeType, fieldCapabilities.getType());
assertNull(fieldCapabilities.nonAggregatableIndices());
assertNull(fieldCapabilities.nonSearchableIndices());
assertEquals("field", fieldCapabilities.getName());
}
}

public void testDefaultMapping() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder()
.startObject()
.startObject("_doc")
.startObject("properties")
.startObject("field")
.field("type", "script")
.field("runtime_type", randomFrom(SUPPORTED_RUNTIME_TYPES))
.field("runtime_type", randomFrom(runtimeTypes))
.startObject("script")
.field("source", "value('test')")
.field("source", "keyword('test')")
.field("lang", "test")
.endObject()
.endObject()
Expand Down Expand Up @@ -155,7 +212,7 @@ public <FactoryType> FactoryType compile(
ScriptContext<FactoryType> context,
Map<String, String> paramsMap
) {
if ("value('test')".equals(code)) {
if ("keyword('test')".equals(code)) {
StringScriptFieldScript.Factory factory = (params, lookup) -> ctx -> new StringScriptFieldScript(
params,
lookup,
Expand Down