Skip to content

Commit c16ef04

Browse files
martijnvgmartijnvg
committed
added unit tests
1 parent 6b556d0 commit c16ef04

File tree

3 files changed

+103
-31
lines changed

3 files changed

+103
-31
lines changed

‎server/src/main/java/org/elasticsearch/search/fetch/StoredFieldsSpec.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,15 @@ public boolean onlyRequiresIgnoredFields() {
5454
*/
5555
public static final StoredFieldsSpec NEEDS_SOURCE = new StoredFieldsSpec(true, false, Set.of());
5656

57+
/**
58+
* @return a stored field spec that requires source only for the specified source paths.
59+
* This is more efficient than using {@link #NEEDS_SOURCE}.
60+
*/
5761
public static StoredFieldsSpec withSourcePaths(IgnoredSourceFormat ignoredSourceFormat, Set<String> sourcePaths) {
5862
// The fields in source paths might also be in ignored source, so include source paths there as well.
5963
IgnoredFieldsSpec ignoredFieldsSpec = ignoredSourceFormat == IgnoredSourceFormat.NO_IGNORED_SOURCE
60-
? new IgnoredFieldsSpec(sourcePaths, ignoredSourceFormat)
61-
: IgnoredFieldsSpec.NONE;
64+
? IgnoredFieldsSpec.NONE
65+
: new IgnoredFieldsSpec(sourcePaths, ignoredSourceFormat);
6266
return new StoredFieldsSpec(true, false, Set.of(), ignoredFieldsSpec, sourcePaths);
6367
}
6468

‎server/src/test/java/org/elasticsearch/index/mapper/BlockSourceReaderTests.java‎

Lines changed: 60 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@
1111

1212
import org.apache.lucene.index.LeafReaderContext;
1313
import org.apache.lucene.util.BytesRef;
14-
import org.elasticsearch.cluster.metadata.IndexMetadata;
14+
import org.elasticsearch.common.CheckedBiConsumer;
1515
import org.elasticsearch.common.settings.Settings;
1616
import org.elasticsearch.core.CheckedConsumer;
17-
import org.elasticsearch.index.IndexSettings;
18-
import org.elasticsearch.index.IndexVersion;
1917
import org.elasticsearch.index.fieldvisitor.StoredFieldLoader;
2018
import org.elasticsearch.index.mapper.IgnoredSourceFieldMapper.IgnoredSourceFormat;
2119
import org.elasticsearch.search.fetch.StoredFieldsSpec;
22-
import org.elasticsearch.test.ESTestCase;
20+
import org.elasticsearch.search.lookup.SourceFilter;
2321
import org.elasticsearch.xcontent.XContentBuilder;
2422

2523
import java.io.IOException;
@@ -35,40 +33,46 @@ public class BlockSourceReaderTests extends MapperServiceTestCase {
3533
public void testSingle() throws IOException {
3634
withIndex(
3735
source -> source.field("field", "foo"),
38-
ctx -> loadBlock(ctx, block -> assertThat(block.get(0), equalTo(new BytesRef("foo"))))
36+
(mapperService, ctx) -> loadBlock(mapperService, ctx, block -> assertThat(block.get(0), equalTo(new BytesRef("foo"))))
3937
);
4038
}
4139

4240
public void testMissing() throws IOException {
43-
withIndex(source -> {}, ctx -> loadBlock(ctx, block -> assertThat(block.get(0), nullValue())));
41+
withIndex(source -> {}, (mapperService, ctx) -> loadBlock(mapperService, ctx, block -> assertThat(block.get(0), nullValue())));
4442
}
4543

4644
public void testArray() throws IOException {
4745
withIndex(
4846
source -> source.startArray("field").value("foo").value("bar").endArray(),
49-
ctx -> loadBlock(ctx, block -> assertThat(block.get(0), equalTo(List.of(new BytesRef("foo"), new BytesRef("bar")))))
47+
(mapperService, ctx) -> loadBlock(
48+
mapperService,
49+
ctx,
50+
block -> assertThat(block.get(0), equalTo(List.of(new BytesRef("foo"), new BytesRef("bar"))))
51+
)
5052
);
5153
}
5254

5355
public void testEmptyArray() throws IOException {
54-
withIndex(source -> source.startArray("field").endArray(), ctx -> loadBlock(ctx, block -> assertThat(block.get(0), nullValue())));
56+
withIndex(
57+
source -> source.startArray("field").endArray(),
58+
(mapperService, ctx) -> loadBlock(mapperService, ctx, block -> assertThat(block.get(0), nullValue()))
59+
);
5560
}
5661

57-
private void loadBlock(LeafReaderContext ctx, Consumer<TestBlock> test) throws IOException {
58-
boolean syntheticSource = randomBoolean();
59-
IndexMetadata indexMetadata = IndexMetadata.builder("index")
60-
.settings(
61-
Settings.builder()
62-
.put(
63-
ESTestCase.indexSettings(IndexVersion.current(), 1, 1)
64-
.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), syntheticSource ? "synthetic" : "stored")
65-
.build()
66-
)
67-
)
68-
.build();
69-
IndexSettings indexSettings = new IndexSettings(indexMetadata, Settings.EMPTY);
70-
var valueFetcher = SourceValueFetcher.toString(Set.of("field"), indexSettings);
71-
BlockSourceReader.LeafIteratorLookup lookup = BlockSourceReader.lookupFromNorms("field");
62+
public void testMoreFields() throws IOException {
63+
withIndex(
64+
source -> source.field("field", "foo").field("other_field", "bar").field("other_field_2", 1L),
65+
(mapperService, ctx) -> loadBlock(mapperService, ctx, block -> assertThat(block.get(0), equalTo(new BytesRef("foo"))))
66+
);
67+
}
68+
69+
private void loadBlock(MapperService mapperService, LeafReaderContext ctx, Consumer<TestBlock> test) throws IOException {
70+
boolean syntheticSource = mapperService.mappingLookup().isSourceSynthetic();
71+
var valueFetcher = SourceValueFetcher.toString(Set.of("field"), mapperService.getIndexSettings());
72+
boolean hasNorms = mapperService.mappingLookup().getFieldType("field").getTextSearchInfo().hasNorms();
73+
BlockSourceReader.LeafIteratorLookup lookup = hasNorms
74+
? BlockSourceReader.lookupFromNorms("field")
75+
: BlockSourceReader.lookupMatchingAll();
7276
BlockLoader loader = new BlockSourceReader.BytesRefsBlockLoader(valueFetcher, lookup);
7377
assertThat(loader.columnAtATimeReader(ctx), nullValue());
7478
BlockLoader.RowStrideReader reader = loader.rowStrideReader(ctx);
@@ -81,9 +85,14 @@ private void loadBlock(LeafReaderContext ctx, Consumer<TestBlock> test) throws I
8185
)
8286
)
8387
);
84-
BlockLoaderStoredFieldsFromLeafLoader storedFields = new BlockLoaderStoredFieldsFromLeafLoader(
88+
var sourceLoader = mapperService.mappingLookup()
89+
.newSourceLoader(new SourceFilter(new String[] { "field" }, null), SourceFieldMetrics.NOOP);
90+
var sourceLoaderLeaf = sourceLoader.leaf(ctx.reader(), null);
91+
92+
assertThat(loader.rowStrideStoredFieldSpec().requiresSource(), equalTo(true));
93+
var storedFields = new BlockLoaderStoredFieldsFromLeafLoader(
8594
StoredFieldLoader.fromSpec(loader.rowStrideStoredFieldSpec()).getLoader(ctx, null),
86-
loader.rowStrideStoredFieldSpec().requiresSource() ? SourceLoader.FROM_STORED_SOURCE.leaf(ctx.reader(), null) : null
95+
sourceLoaderLeaf
8796
);
8897
BlockLoader.Builder builder = loader.builder(TestBlock.factory(), 1);
8998
storedFields.advanceTo(0);
@@ -93,15 +102,37 @@ private void loadBlock(LeafReaderContext ctx, Consumer<TestBlock> test) throws I
93102
test.accept(block);
94103
}
95104

96-
private void withIndex(CheckedConsumer<XContentBuilder, IOException> buildSource, CheckedConsumer<LeafReaderContext, IOException> test)
97-
throws IOException {
98-
MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "text")));
105+
private void withIndex(
106+
CheckedConsumer<XContentBuilder, IOException> buildSource,
107+
CheckedBiConsumer<MapperService, LeafReaderContext, IOException> test
108+
) throws IOException {
109+
boolean syntheticSource = randomBoolean();
110+
String fieldType = randomFrom("text", "keyword");
111+
MapperService mapperService = syntheticSource
112+
? createMapperServiceWithSyntheticSource(mapping(fieldType))
113+
: createMapperService(mapping(fieldType));
99114
withLuceneIndex(mapperService, writer -> {
100115
ParsedDocument parsed = mapperService.documentParser().parseDocument(source(buildSource), mapperService.mappingLookup());
101116
writer.addDocuments(parsed.docs());
102117
}, reader -> {
103118
assertThat(reader.leaves(), hasSize(1));
104-
test.accept(reader.leaves().get(0));
119+
test.accept(mapperService, reader.leaves().get(0));
120+
});
121+
}
122+
123+
MapperService createMapperServiceWithSyntheticSource(XContentBuilder mappings) throws IOException {
124+
var settings = Settings.builder()
125+
.put("index.mapping.source.mode", "synthetic")
126+
.put("index.mapping.synthetic_source_keep", "arrays")
127+
.build();
128+
return createMapperService(getVersion(), settings, () -> true, mappings);
129+
}
130+
131+
static XContentBuilder mapping(String type) throws IOException {
132+
return mapping(b -> {
133+
b.startObject("field").field("type", type).endObject();
134+
b.startObject("other_field").field("type", "keyword").endObject();
135+
b.startObject("other_field_2").field("type", "long").endObject();
105136
});
106137
}
107138
}

‎server/src/test/java/org/elasticsearch/search/fetch/StoredFieldsSpecTests.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.search.fetch;
1111

12+
import org.elasticsearch.index.mapper.IgnoredFieldsSpec;
1213
import org.elasticsearch.script.Script;
1314
import org.elasticsearch.search.builder.SearchSourceBuilder;
1415
import org.elasticsearch.search.fetch.subphase.FetchSourcePhase;
@@ -20,6 +21,9 @@
2021

2122
import java.util.Set;
2223

24+
import static org.hamcrest.Matchers.containsInAnyOrder;
25+
import static org.hamcrest.Matchers.empty;
26+
import static org.hamcrest.Matchers.equalTo;
2327
import static org.hamcrest.Matchers.hasSize;
2428
import static org.hamcrest.Matchers.sameInstance;
2529
import static org.mockito.Mockito.mock;
@@ -84,6 +88,39 @@ public void testNoCloneOnMerge() {
8488
assertThat(spec.requiredStoredFields(), sameInstance(withCat.requiredStoredFields()));
8589
}
8690

91+
public void testMergeSourcePaths() {
92+
StoredFieldsSpec spec = StoredFieldsSpec.NO_REQUIREMENTS;
93+
spec = spec.merge(new StoredFieldsSpec(true, false, Set.of(), IgnoredFieldsSpec.NONE, Set.of("cat")));
94+
assertThat(spec.ignoredFieldsSpec(), equalTo(IgnoredFieldsSpec.NONE));
95+
assertThat(spec.requiresSource(), equalTo(true));
96+
assertThat(spec.requiresMetadata(), equalTo(false));
97+
assertThat(spec.requiredStoredFields(), empty());
98+
assertThat(spec.sourcePaths(), containsInAnyOrder("cat"));
99+
100+
spec = spec.merge(new StoredFieldsSpec(true, false, Set.of(), IgnoredFieldsSpec.NONE, Set.of("dog")));
101+
assertThat(spec.ignoredFieldsSpec(), equalTo(IgnoredFieldsSpec.NONE));
102+
assertThat(spec.requiresSource(), equalTo(true));
103+
assertThat(spec.requiresMetadata(), equalTo(false));
104+
assertThat(spec.requiredStoredFields(), empty());
105+
assertThat(spec.sourcePaths(), containsInAnyOrder("cat", "dog"));
106+
107+
spec = spec.merge(new StoredFieldsSpec(true, false, Set.of(), IgnoredFieldsSpec.NONE, Set.of("hamster")));
108+
assertThat(spec.ignoredFieldsSpec(), equalTo(IgnoredFieldsSpec.NONE));
109+
assertThat(spec.requiresSource(), equalTo(true));
110+
assertThat(spec.requiresMetadata(), equalTo(false));
111+
assertThat(spec.requiredStoredFields(), empty());
112+
assertThat(spec.sourcePaths(), containsInAnyOrder("cat", "dog", "hamster"));
113+
var pref = spec.sourcePaths();
114+
115+
spec = spec.merge(new StoredFieldsSpec(true, false, Set.of("other_field"), IgnoredFieldsSpec.NONE, null));
116+
assertThat(spec.ignoredFieldsSpec(), equalTo(IgnoredFieldsSpec.NONE));
117+
assertThat(spec.requiresSource(), equalTo(true));
118+
assertThat(spec.requiresMetadata(), equalTo(false));
119+
assertThat(spec.requiredStoredFields(), containsInAnyOrder("other_field"));
120+
assertThat(spec.sourcePaths(), containsInAnyOrder("cat", "dog", "hamster"));
121+
assertThat(spec.sourcePaths(), sameInstance(pref));
122+
}
123+
87124
private static SearchContext searchContext(SearchSourceBuilder sourceBuilder) {
88125
SearchContext sc = mock(SearchContext.class);
89126
when(sc.fetchSourceContext()).thenReturn(sourceBuilder.fetchSource());

0 commit comments

Comments
 (0)