Simplify runtime fields factory function (elastic#71027)

javanna · web-flow · commit 7bf37806b42e · 2021-03-31T12:00:31.000+02:00
We currently use a TriFunction that takes the field name, the script params and the search lookup and returns a LeafFactory. Name and script params never change though and are available when the function is created, so the tri function can become a function search lookup -&gt; leaf factory which is much easier to reason about.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java
@@ -14,7 +14,6 @@
 import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
 import org.apache.lucene.search.spans.SpanQuery;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.unit.Fuzziness;
@@ -42,12 +41,12 @@
  */
 abstract class AbstractScriptFieldType<LeafFactory> extends MappedFieldType implements RuntimeField {
     protected final Script script;
-    private final TriFunction<String, Map<String, Object>, SearchLookup, LeafFactory> factory;
+    private final Function<SearchLookup, LeafFactory> factory;
     private final ToXContent toXContent;
 
     AbstractScriptFieldType(
         String name,
-        TriFunction<String, Map<String, Object>, SearchLookup, LeafFactory> factory,
+        Function<SearchLookup, LeafFactory> factory,
         Script script,
         Map<String, String> meta,
         ToXContent toXContent
@@ -188,7 +187,7 @@ public final ValueFetcher valueFetcher(SearchExecutionContext context, String fo
      * Create a script leaf factory.
      */
     protected final LeafFactory leafFactory(SearchLookup searchLookup) {
-        return factory.apply(name(), script.getParams(), searchLookup);
+        return factory.apply(searchLookup);
     }
 
     /**
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BooleanScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/BooleanScriptFieldType.java
@@ -16,14 +16,14 @@
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.fielddata.BooleanScriptFieldData;
+import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.script.BooleanFieldScript;
-import org.elasticsearch.search.runtime.BooleanScriptFieldExistsQuery;
-import org.elasticsearch.search.runtime.BooleanScriptFieldTermQuery;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.runtime.BooleanScriptFieldExistsQuery;
+import org.elasticsearch.search.runtime.BooleanScriptFieldTermQuery;
 
 import java.time.ZoneId;
 import java.util.Collection;
@@ -76,7 +76,7 @@ public BooleanScriptFieldType(String name) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java
@@ -127,7 +127,8 @@ public DateScriptFieldType(String name, DateFormatter dateTimeFormatter) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, (n, params, ctx) -> scriptFactory.newFactory(n, params, ctx, dateTimeFormatter), script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup, dateTimeFormatter),
+            script, meta, toXContent);
         this.dateTimeFormatter = dateTimeFormatter;
     }
 
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java
@@ -14,17 +14,17 @@
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.index.fielddata.DoubleScriptFieldData;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
 import org.elasticsearch.index.query.SearchExecutionContext;
-import org.elasticsearch.index.fielddata.DoubleScriptFieldData;
 import org.elasticsearch.script.DoubleFieldScript;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.runtime.DoubleScriptFieldExistsQuery;
 import org.elasticsearch.search.runtime.DoubleScriptFieldRangeQuery;
 import org.elasticsearch.search.runtime.DoubleScriptFieldTermQuery;
 import org.elasticsearch.search.runtime.DoubleScriptFieldTermsQuery;
-import org.elasticsearch.script.Script;
-import org.elasticsearch.search.DocValueFormat;
-import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.time.ZoneId;
 import java.util.Collection;
@@ -77,7 +77,7 @@ public DoubleScriptFieldType(String name) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java
@@ -92,7 +92,7 @@ RuntimeField newRuntimeField(GeoPointFieldScript.Factory scriptFactory) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/IpScriptFieldType.java
@@ -20,16 +20,16 @@
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.BytesRefHash;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.fielddata.IpScriptFieldData;
+import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.script.IpFieldScript;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.runtime.IpScriptFieldExistsQuery;
 import org.elasticsearch.search.runtime.IpScriptFieldRangeQuery;
 import org.elasticsearch.search.runtime.IpScriptFieldTermQuery;
 import org.elasticsearch.search.runtime.IpScriptFieldTermsQuery;
-import org.elasticsearch.script.Script;
-import org.elasticsearch.search.DocValueFormat;
-import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.net.InetAddress;
 import java.time.ZoneId;
@@ -79,7 +79,7 @@ RuntimeField newRuntimeField(IpFieldScript.Factory scriptFactory) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordScriptFieldType.java
@@ -15,9 +15,11 @@
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.fielddata.StringScriptFieldData;
+import org.elasticsearch.index.query.SearchExecutionContext;
+import org.elasticsearch.script.Script;
 import org.elasticsearch.script.StringFieldScript;
+import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.runtime.StringScriptFieldExistsQuery;
 import org.elasticsearch.search.runtime.StringScriptFieldFuzzyQuery;
 import org.elasticsearch.search.runtime.StringScriptFieldPrefixQuery;
@@ -26,8 +28,6 @@
 import org.elasticsearch.search.runtime.StringScriptFieldTermQuery;
 import org.elasticsearch.search.runtime.StringScriptFieldTermsQuery;
 import org.elasticsearch.search.runtime.StringScriptFieldWildcardQuery;
-import org.elasticsearch.script.Script;
-import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.time.ZoneId;
 import java.util.Collection;
@@ -78,7 +78,7 @@ public KeywordScriptFieldType(String name) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java
@@ -14,17 +14,17 @@
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.index.fielddata.LongScriptFieldData;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
 import org.elasticsearch.index.query.SearchExecutionContext;
-import org.elasticsearch.index.fielddata.LongScriptFieldData;
 import org.elasticsearch.script.LongFieldScript;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.runtime.LongScriptFieldExistsQuery;
 import org.elasticsearch.search.runtime.LongScriptFieldRangeQuery;
 import org.elasticsearch.search.runtime.LongScriptFieldTermQuery;
 import org.elasticsearch.search.runtime.LongScriptFieldTermsQuery;
-import org.elasticsearch.script.Script;
-import org.elasticsearch.search.DocValueFormat;
-import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.time.ZoneId;
 import java.util.Collection;
@@ -73,7 +73,7 @@ public LongScriptFieldType(String name) {
         Map<String, String> meta,
         ToXContent toXContent
     ) {
-        super(name, scriptFactory::newFactory, script, meta, toXContent);
+        super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,8 @@ public DateScriptFieldType(String name, DateFormatter dateTimeFormatter) {`
`127`	`127`	`Map<String, String> meta,`
`128`	`128`	`ToXContent toXContent`
`129`	`129`	`) {`
`130`		`- super(name, (n, params, ctx) -> scriptFactory.newFactory(n, params, ctx, dateTimeFormatter), script, meta, toXContent);`
	`130`	`+ super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup, dateTimeFormatter),`
	`131`	`+ script, meta, toXContent);`
`131`	`132`	`this.dateTimeFormatter = dateTimeFormatter;`
`132`	`133`	`}`
`133`	`134`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ RuntimeField newRuntimeField(GeoPointFieldScript.Factory scriptFactory) {`
`92`	`92`	`Map<String, String> meta,`
`93`	`93`	`ToXContent toXContent`
`94`	`94`	`) {`
`95`		`- super(name, scriptFactory::newFactory, script, meta, toXContent);`
	`95`	`+ super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta, toXContent);`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`@Override`