Skip to content

Commit b8f219e

Browse files
javannajavanna
committed
SearchLookup to not require MapperService as a constructor argument (elastic#64017)
SearchLookup currently takes the whole MapperService as an argument, and it passes it over to DocLookup and FieldsLookup. In practice, what they all need is only a way to lookup field types given a field name. With this commit, SearchLookup requires a Function<String,MappedFieldType> to be created rather than the whole MapperService. This helps clarifying the dependency between MapperService and SearchLookup, especially as we introduce runtime fields defined in the search request: they can be exposed at QueryShardContext level, through its lookup methods, rather than at the MapperService level which QueryShardContext depends on.
1 parent 8c73755 commit b8f219e

File tree

25 files changed

+82
-119
lines changed

25 files changed

+82
-119
lines changed

‎modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ private static DoubleValuesSource getDocValueSource(String variable, SearchLooku
458458
}
459459

460460
String fieldname = parts[1].text;
461-
MappedFieldType fieldType = lookup.doc().mapperService().fieldType(fieldname);
461+
MappedFieldType fieldType = lookup.doc().fieldType(fieldname);
462462

463463
if (fieldType == null) {
464464
throw new ParseException("Field [" + fieldname + "] does not exist in mappings", 5);

‎modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionFieldScriptTests.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.index.fielddata.IndexNumericFieldData;
2323
import org.elasticsearch.index.fielddata.LeafNumericFieldData;
2424
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
25-
import org.elasticsearch.index.mapper.MapperService;
2625
import org.elasticsearch.index.mapper.NumberFieldMapper;
2726
import org.elasticsearch.script.FieldScript;
2827
import org.elasticsearch.script.ScriptException;
@@ -48,9 +47,6 @@ public void setUp() throws Exception {
4847
super.setUp();
4948

5049
NumberFieldMapper.NumberFieldType fieldType = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.DOUBLE);
51-
MapperService mapperService = mock(MapperService.class);
52-
when(mapperService.fieldType("field")).thenReturn(fieldType);
53-
when(mapperService.fieldType("alias")).thenReturn(fieldType);
5450

5551
SortedNumericDoubleValues doubleValues = mock(SortedNumericDoubleValues.class);
5652
when(doubleValues.advanceExact(anyInt())).thenReturn(true);
@@ -64,7 +60,8 @@ public void setUp() throws Exception {
6460
when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
6561

6662
service = new ExpressionScriptEngine();
67-
lookup = new SearchLookup(mapperService, (ignored, lookup) -> fieldData, null);
63+
lookup = new SearchLookup(field -> field.equals("field") || field.equals("alias") ? fieldType : null,
64+
(ignored, lookup) -> fieldData, null);
6865
}
6966

7067
private FieldScript.LeafFactory compile(String expression) {

‎modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionNumberSortScriptTests.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.index.fielddata.LeafNumericFieldData;
2626
import org.elasticsearch.index.fielddata.IndexNumericFieldData;
2727
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
28-
import org.elasticsearch.index.mapper.MapperService;
2928
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType;
3029
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
3130
import org.elasticsearch.script.NumberSortScript;
@@ -47,9 +46,6 @@ public void setUp() throws Exception {
4746
super.setUp();
4847

4948
NumberFieldType fieldType = new NumberFieldType("field", NumberType.DOUBLE);
50-
MapperService mapperService = mock(MapperService.class);
51-
when(mapperService.fieldType("field")).thenReturn(fieldType);
52-
when(mapperService.fieldType("alias")).thenReturn(fieldType);
5349

5450
SortedNumericDoubleValues doubleValues = mock(SortedNumericDoubleValues.class);
5551
when(doubleValues.advanceExact(anyInt())).thenReturn(true);
@@ -63,7 +59,8 @@ public void setUp() throws Exception {
6359
when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
6460

6561
service = new ExpressionScriptEngine();
66-
lookup = new SearchLookup(mapperService, (ignored, lookup) -> fieldData, null);
62+
lookup = new SearchLookup(field -> field.equals("field") || field.equals("alias") ? fieldType : null,
63+
(ignored, lookup) -> fieldData, null);
6764
}
6865

6966
private NumberSortScript.LeafFactory compile(String expression) {

‎modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionTermsSetQueryTests.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.index.fielddata.LeafNumericFieldData;
2626
import org.elasticsearch.index.fielddata.IndexNumericFieldData;
2727
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
28-
import org.elasticsearch.index.mapper.MapperService;
2928
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType;
3029
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
3130
import org.elasticsearch.script.ScriptException;
@@ -47,9 +46,6 @@ public void setUp() throws Exception {
4746
super.setUp();
4847

4948
NumberFieldType fieldType = new NumberFieldType("field", NumberType.DOUBLE);
50-
MapperService mapperService = mock(MapperService.class);
51-
when(mapperService.fieldType("field")).thenReturn(fieldType);
52-
when(mapperService.fieldType("alias")).thenReturn(fieldType);
5349

5450
SortedNumericDoubleValues doubleValues = mock(SortedNumericDoubleValues.class);
5551
when(doubleValues.advanceExact(anyInt())).thenReturn(true);
@@ -63,7 +59,8 @@ public void setUp() throws Exception {
6359
when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
6460

6561
service = new ExpressionScriptEngine();
66-
lookup = new SearchLookup(mapperService, (ignored, lookup) -> fieldData, null);
62+
lookup = new SearchLookup(field -> field.equals("field") || field.equals("alias") ? fieldType : null,
63+
(ignored, lookup) -> fieldData, null);
6764
}
6865

6966
private TermsSetQueryScript.LeafFactory compile(String expression) {

‎server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ public TypeFieldType(String type) {
8181
this.type = type;
8282
}
8383

84+
/**
85+
* Returns the name of the current type
86+
*/
87+
public String getType() {
88+
return type;
89+
}
90+
8491
@Override
8592
public String typeName() {
8693
return CONTENT_TYPE;

‎server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public Collection<String> queryTypes() {
371371
public SearchLookup lookup() {
372372
if (this.lookup == null) {
373373
this.lookup = new SearchLookup(
374-
mapperService,
374+
this::getFieldType,
375375
(fieldType, searchLookup) -> indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), searchLookup),
376376
types
377377
);
@@ -388,7 +388,7 @@ public SearchLookup newFetchLookup() {
388388
* Real customization coming soon, I promise!
389389
*/
390390
return new SearchLookup(
391-
mapperService,
391+
this::getFieldType,
392392
(fieldType, searchLookup) -> indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), searchLookup),
393393
types
394394
);

‎server/src/main/java/org/elasticsearch/search/lookup/DocLookup.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,33 +22,33 @@
2222
import org.elasticsearch.common.Nullable;
2323
import org.elasticsearch.index.fielddata.IndexFieldData;
2424
import org.elasticsearch.index.mapper.MappedFieldType;
25-
import org.elasticsearch.index.mapper.MapperService;
2625

2726
import java.util.function.Function;
2827

29-
public class DocLookup {
28+
public final class DocLookup {
3029

31-
private final MapperService mapperService;
30+
private final Function<String, MappedFieldType> fieldTypeLookup;
3231
private final Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup;
3332
@Nullable
3433
private final String[] types;
3534

36-
DocLookup(MapperService mapperService, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup, @Nullable String[] types) {
37-
this.mapperService = mapperService;
35+
DocLookup(Function<String, MappedFieldType> fieldTypeLookup, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup,
36+
@Nullable String[] types) {
37+
this.fieldTypeLookup = fieldTypeLookup;
3838
this.fieldDataLookup = fieldDataLookup;
3939
this.types = types;
4040
}
4141

42-
public MapperService mapperService() {
43-
return this.mapperService;
42+
public MappedFieldType fieldType(String fieldName) {
43+
return fieldTypeLookup.apply(fieldName);
4444
}
4545

4646
public IndexFieldData<?> getForField(MappedFieldType fieldType) {
4747
return fieldDataLookup.apply(fieldType);
4848
}
4949

5050
public LeafDocLookup getLeafDocLookup(LeafReaderContext context) {
51-
return new LeafDocLookup(mapperService, fieldDataLookup, types, context);
51+
return new LeafDocLookup(fieldTypeLookup, fieldDataLookup, types, context);
5252
}
5353

5454
public String[] getTypes() {

‎server/src/main/java/org/elasticsearch/search/lookup/FieldLookup.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public class FieldLookup {
4444
this.fieldType = fieldType;
4545
}
4646

47-
public MappedFieldType fieldType() {
47+
MappedFieldType fieldType() {
4848
return fieldType;
4949
}
5050

‎server/src/main/java/org/elasticsearch/search/lookup/FieldsLookup.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,22 @@
2020

2121
import org.apache.lucene.index.LeafReaderContext;
2222
import org.elasticsearch.common.Nullable;
23+
import org.elasticsearch.index.mapper.MappedFieldType;
2324
import org.elasticsearch.index.mapper.MapperService;
25+
import java.util.function.Function;
2426

2527
public class FieldsLookup {
2628

27-
private final MapperService mapperService;
29+
private final Function<String, MappedFieldType> fieldTypeLookup;
2830
@Nullable
2931
private final String[] types;
3032

31-
FieldsLookup(MapperService mapperService, @Nullable String[] types) {
32-
this.mapperService = mapperService;
33+
FieldsLookup(Function<String, MappedFieldType> fieldTypeLookup, @Nullable String[] types) {
34+
this.fieldTypeLookup = fieldTypeLookup;
3335
this.types = types;
3436
}
3537

3638
public LeafFieldsLookup getLeafFieldsLookup(LeafReaderContext context) {
37-
return new LeafFieldsLookup(mapperService, types, context.reader());
39+
return new LeafFieldsLookup(fieldTypeLookup, types, context.reader());
3840
}
39-
4041
}

‎server/src/main/java/org/elasticsearch/search/lookup/LeafDocLookup.java

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.index.fielddata.IndexFieldData;
2626
import org.elasticsearch.index.fielddata.ScriptDocValues;
2727
import org.elasticsearch.index.mapper.MappedFieldType;
28-
import org.elasticsearch.index.mapper.MapperService;
2928

3029
import java.io.IOException;
3130
import java.security.AccessController;
@@ -45,8 +44,7 @@ public class LeafDocLookup implements Map<String, ScriptDocValues<?>> {
4544
"[types removal] Looking up doc types [_type] in scripts is deprecated.";
4645

4746
private final Map<String, ScriptDocValues<?>> localCacheFieldData = new HashMap<>(4);
48-
49-
private final MapperService mapperService;
47+
private final Function<String, MappedFieldType> fieldTypeLookup;
5048
private final Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup;
5149

5250
@Nullable
@@ -56,18 +54,14 @@ public class LeafDocLookup implements Map<String, ScriptDocValues<?>> {
5654

5755
private int docId = -1;
5856

59-
LeafDocLookup(MapperService mapperService, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup, @Nullable String[] types,
60-
LeafReaderContext reader) {
61-
this.mapperService = mapperService;
57+
LeafDocLookup(Function<String, MappedFieldType> fieldTypeLookup, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup,
58+
@Nullable String[] types, LeafReaderContext reader) {
59+
this.fieldTypeLookup = fieldTypeLookup;
6260
this.fieldDataLookup = fieldDataLookup;
6361
this.types = types;
6462
this.reader = reader;
6563
}
6664

67-
public MapperService mapperService() {
68-
return this.mapperService;
69-
}
70-
7165
public void setDocument(int docId) {
7266
this.docId = docId;
7367
}
@@ -82,7 +76,7 @@ public ScriptDocValues<?> get(Object key) {
8276
String fieldName = key.toString();
8377
ScriptDocValues<?> scriptValues = localCacheFieldData.get(fieldName);
8478
if (scriptValues == null) {
85-
final MappedFieldType fieldType = mapperService.fieldType(fieldName);
79+
final MappedFieldType fieldType = fieldTypeLookup.apply(fieldName);
8680
if (fieldType == null) {
8781
throw new IllegalArgumentException("No field found for [" + fieldName + "] in mapping with types " +
8882
Arrays.toString(types));
@@ -110,13 +104,7 @@ public boolean containsKey(Object key) {
110104
// assume its a string...
111105
String fieldName = key.toString();
112106
ScriptDocValues<?> scriptValues = localCacheFieldData.get(fieldName);
113-
if (scriptValues == null) {
114-
MappedFieldType fieldType = mapperService.fieldType(fieldName);
115-
if (fieldType == null) {
116-
return false;
117-
}
118-
}
119-
return true;
107+
return scriptValues != null || fieldTypeLookup.apply(fieldName) != null;
120108
}
121109

122110
@Override

‎server/src/main/java/org/elasticsearch/search/lookup/LeafFieldsLookup.java

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,25 @@
2222
import org.elasticsearch.ElasticsearchParseException;
2323
import org.elasticsearch.common.Nullable;
2424
import org.elasticsearch.index.fieldvisitor.SingleFieldsVisitor;
25-
import org.elasticsearch.index.mapper.DocumentMapper;
2625
import org.elasticsearch.index.mapper.MappedFieldType;
27-
import org.elasticsearch.index.mapper.MapperService;
2826
import org.elasticsearch.index.mapper.TypeFieldMapper;
2927

3028
import java.io.IOException;
3129
import java.util.ArrayList;
3230
import java.util.Arrays;
3331
import java.util.Collection;
32+
import java.util.Collections;
3433
import java.util.HashMap;
3534
import java.util.List;
3635
import java.util.Map;
3736
import java.util.Set;
37+
import java.util.function.Function;
3838

3939
import static java.util.Collections.singletonMap;
4040

4141
public class LeafFieldsLookup implements Map {
4242

43-
private final MapperService mapperService;
43+
private final Function<String, MappedFieldType> fieldTypeLookup;
4444

4545
@Nullable
4646
private final String[] types;
@@ -51,8 +51,8 @@ public class LeafFieldsLookup implements Map {
5151

5252
private final Map<String, FieldLookup> cachedFieldData = new HashMap<>();
5353

54-
LeafFieldsLookup(MapperService mapperService, @Nullable String[] types, LeafReader reader) {
55-
this.mapperService = mapperService;
54+
LeafFieldsLookup(Function<String, MappedFieldType> fieldTypeLookup, @Nullable String[] types, LeafReader reader) {
55+
this.fieldTypeLookup = fieldTypeLookup;
5656
this.types = types;
5757
this.reader = reader;
5858
}
@@ -65,7 +65,6 @@ public void setDocument(int docId) {
6565
clearCache();
6666
}
6767

68-
6968
@Override
7069
public Object get(Object key) {
7170
return loadFieldData(key.toString());
@@ -134,32 +133,29 @@ public boolean containsValue(Object value) {
134133
private FieldLookup loadFieldData(String name) {
135134
FieldLookup data = cachedFieldData.get(name);
136135
if (data == null) {
137-
MappedFieldType fieldType = mapperService.fieldType(name);
136+
MappedFieldType fieldType = fieldTypeLookup.apply(name);
138137
if (fieldType == null) {
139138
throw new IllegalArgumentException("No field found for [" + name + "] in mapping with types " + Arrays.toString(types));
140139
}
141140
data = new FieldLookup(fieldType);
142141
cachedFieldData.put(name, data);
143142
}
144143
if (data.fields() == null) {
144+
MappedFieldType fieldType = data.fieldType();
145145
List<Object> values;
146-
if (TypeFieldMapper.NAME.equals(data.fieldType().name())) {
146+
if (TypeFieldMapper.NAME.equals(fieldType.name())) {
147147
TypeFieldMapper.emitTypesDeprecationWarning();
148-
values = new ArrayList<>(1);
149-
final DocumentMapper mapper = mapperService.documentMapper();
150-
if (mapper != null) {
151-
values.add(mapper.type());
152-
}
148+
values = Collections.singletonList(((TypeFieldMapper.TypeFieldType)fieldType).getType());
153149
} else {
154-
values = new ArrayList<Object>(2);
155-
SingleFieldsVisitor visitor = new SingleFieldsVisitor(data.fieldType(), values);
150+
values = new ArrayList<>(2);
151+
SingleFieldsVisitor visitor = new SingleFieldsVisitor(fieldType, values);
156152
try {
157153
reader.document(docId, visitor);
158154
} catch (IOException e) {
159155
throw new ElasticsearchParseException("failed to load field [{}]", e, name);
160156
}
161157
}
162-
data.fields(singletonMap(data.fieldType().name(), values));
158+
data.fields(singletonMap(fieldType.name(), values));
163159
}
164160
return data;
165161
}

0 commit comments

Comments
 (0)