From 7ca8d0bc788ec091271ab5ad8787cf15f816a435 Mon Sep 17 00:00:00 2001 From: panguixin Date: Sat, 12 Oct 2024 17:57:52 +0800 Subject: [PATCH] Optimize flat_object type in a BWC way with one phase parsing --- .../xcontent/JsonToStringXContentParser.java | 289 ---------- .../index/mapper/FlatObjectFieldMapper.java | 505 +++++------------- .../JsonToStringXContentParserTests.java | 164 ------ .../mapper/FlatObjectFieldMapperTests.java | 80 +-- .../mapper/FlatObjectFieldTypeTests.java | 24 +- 5 files changed, 163 insertions(+), 899 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java delete mode 100644 server/src/test/java/org/opensearch/common/xcontent/JsonToStringXContentParserTests.java diff --git a/server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java b/server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java deleted file mode 100644 index 95a8d9c9495f2..0000000000000 --- a/server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java +++ /dev/null @@ -1,289 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.common.xcontent; - -import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.common.ParsingException; -import org.opensearch.core.common.Strings; -import org.opensearch.core.common.bytes.BytesReference; -import org.opensearch.core.xcontent.AbstractXContentParser; -import org.opensearch.core.xcontent.DeprecationHandler; -import org.opensearch.core.xcontent.MediaType; -import org.opensearch.core.xcontent.MediaTypeRegistry; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.core.xcontent.XContentLocation; -import org.opensearch.core.xcontent.XContentParser; - -import java.io.IOException; -import java.math.BigInteger; -import java.nio.CharBuffer; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Deque; -import java.util.HashSet; -import java.util.LinkedList; - -/** - * JsonToStringParser is the main parser class to transform JSON into stringFields in a XContentParser - * returns XContentParser with one parent field and subfields - * fieldName, fieldName._value, fieldName._valueAndPath - * @opensearch.internal - */ -public class JsonToStringXContentParser extends AbstractXContentParser { - private final String fieldTypeName; - private final XContentParser parser; - - private final ArrayList valueList = new ArrayList<>(); - private final ArrayList valueAndPathList = new ArrayList<>(); - private final ArrayList keyList = new ArrayList<>(); - - private final XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent); - - private final NamedXContentRegistry xContentRegistry; - - private final DeprecationHandler deprecationHandler; - - private static final String VALUE_AND_PATH_SUFFIX = "._valueAndPath"; - private static final String VALUE_SUFFIX = "._value"; - private static final String EQUAL_SYMBOL = "="; - - public JsonToStringXContentParser( - NamedXContentRegistry xContentRegistry, - DeprecationHandler deprecationHandler, - XContentParser parser, - String fieldTypeName - ) throws IOException { - super(xContentRegistry, deprecationHandler); - this.deprecationHandler = deprecationHandler; - this.xContentRegistry = xContentRegistry; - this.parser = parser; - this.fieldTypeName = fieldTypeName; - } - - public XContentParser parseObject() throws IOException { - assert currentToken() == Token.START_OBJECT; - parser.nextToken(); // Skip the outer START_OBJECT. Need to return on END_OBJECT. - - builder.startObject(); - LinkedList path = new LinkedList<>(Collections.singleton(fieldTypeName)); - while (currentToken() != Token.END_OBJECT) { - parseToken(path); - } - // deduplication the fieldName,valueList,valueAndPathList - builder.field(this.fieldTypeName, new HashSet<>(keyList)); - builder.field(this.fieldTypeName + VALUE_SUFFIX, new HashSet<>(valueList)); - builder.field(this.fieldTypeName + VALUE_AND_PATH_SUFFIX, new HashSet<>(valueAndPathList)); - builder.endObject(); - String jString = XContentHelper.convertToJson(BytesReference.bytes(builder), false, MediaTypeRegistry.JSON); - return JsonXContent.jsonXContent.createParser(this.xContentRegistry, this.deprecationHandler, String.valueOf(jString)); - } - - /** - * @return true if the child object contains no_null value, false otherwise - */ - private boolean parseToken(Deque path) throws IOException { - boolean isChildrenValueValid = false; - boolean visitFieldName = false; - if (this.parser.currentToken() == Token.FIELD_NAME) { - final String currentFieldName = this.parser.currentName(); - path.addLast(currentFieldName); // Pushing onto the stack *must* be matched by pop - visitFieldName = true; - String parts = currentFieldName; - while (parts.contains(".")) { // Extract the intermediate keys maybe present in fieldName - int dotPos = parts.indexOf('.'); - String part = parts.substring(0, dotPos); - this.keyList.add(part); - parts = parts.substring(dotPos + 1); - } - this.keyList.add(parts); // parts has no dot, so either it's the original fieldName or it's the last part - this.parser.nextToken(); // advance to the value of fieldName - isChildrenValueValid = parseToken(path); // parse the value for fieldName (which will be an array, an object, - // or a primitive value) - path.removeLast(); // Here is where we pop fieldName from the stack (since we're done with the value of fieldName) - // Note that whichever other branch we just passed through has already ended with nextToken(), so we - // don't need to call it. - } else if (this.parser.currentToken() == Token.START_ARRAY) { - parser.nextToken(); - while (this.parser.currentToken() != Token.END_ARRAY) { - isChildrenValueValid |= parseToken(path); - } - this.parser.nextToken(); - } else if (this.parser.currentToken() == Token.START_OBJECT) { - parser.nextToken(); - while (this.parser.currentToken() != Token.END_OBJECT) { - isChildrenValueValid |= parseToken(path); - } - this.parser.nextToken(); - } else { - String parsedValue = parseValue(); - if (parsedValue != null) { - this.valueList.add(parsedValue); - this.valueAndPathList.add(Strings.collectionToDelimitedString(path, ".") + EQUAL_SYMBOL + parsedValue); - isChildrenValueValid = true; - } - this.parser.nextToken(); - } - - if (visitFieldName && isChildrenValueValid == false) { - removeKeyOfNullValue(); - } - return isChildrenValueValid; - } - - public void removeKeyOfNullValue() { - // it means that the value of the sub child (or the last brother) is invalid, - // we should delete the key from keyList. - assert keyList.size() > 0; - this.keyList.remove(keyList.size() - 1); - } - - private String parseValue() throws IOException { - switch (this.parser.currentToken()) { - case VALUE_BOOLEAN: - case VALUE_NUMBER: - case VALUE_STRING: - case VALUE_NULL: - return this.parser.textOrNull(); - // Handle other token types as needed - default: - throw new ParsingException(parser.getTokenLocation(), "Unexpected value token type [" + parser.currentToken() + "]"); - } - } - - @Override - public MediaType contentType() { - return MediaTypeRegistry.JSON; - } - - @Override - public Token nextToken() throws IOException { - return this.parser.nextToken(); - } - - @Override - public void skipChildren() throws IOException { - this.parser.skipChildren(); - } - - @Override - public Token currentToken() { - return this.parser.currentToken(); - } - - @Override - public String currentName() throws IOException { - return this.parser.currentName(); - } - - @Override - public String text() throws IOException { - return this.parser.text(); - } - - @Override - public CharBuffer charBuffer() throws IOException { - return this.parser.charBuffer(); - } - - @Override - public Object objectText() throws IOException { - return this.parser.objectText(); - } - - @Override - public Object objectBytes() throws IOException { - return this.parser.objectBytes(); - } - - @Override - public boolean hasTextCharacters() { - return this.parser.hasTextCharacters(); - } - - @Override - public char[] textCharacters() throws IOException { - return this.parser.textCharacters(); - } - - @Override - public int textLength() throws IOException { - return this.parser.textLength(); - } - - @Override - public int textOffset() throws IOException { - return this.parser.textOffset(); - } - - @Override - public Number numberValue() throws IOException { - return this.parser.numberValue(); - } - - @Override - public NumberType numberType() throws IOException { - return this.parser.numberType(); - } - - @Override - public byte[] binaryValue() throws IOException { - return this.parser.binaryValue(); - } - - @Override - public XContentLocation getTokenLocation() { - return this.parser.getTokenLocation(); - } - - @Override - protected boolean doBooleanValue() throws IOException { - return this.parser.booleanValue(); - } - - @Override - protected short doShortValue() throws IOException { - return this.parser.shortValue(); - } - - @Override - protected int doIntValue() throws IOException { - return this.parser.intValue(); - } - - @Override - protected long doLongValue() throws IOException { - return this.parser.longValue(); - } - - @Override - protected float doFloatValue() throws IOException { - return this.parser.floatValue(); - } - - @Override - protected double doDoubleValue() throws IOException { - return this.parser.doubleValue(); - } - - @Override - protected BigInteger doBigIntegerValue() throws IOException { - return this.parser.bigIntegerValue(); - } - - @Override - public boolean isClosed() { - return this.parser.isClosed(); - } - - @Override - public void close() throws IOException { - this.parser.close(); - } -} diff --git a/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java index 738efcfafdca1..506a4ac1a8eaf 100644 --- a/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java @@ -8,8 +8,6 @@ package org.opensearch.index.mapper; -import org.apache.lucene.analysis.TokenStream; -import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; import org.apache.lucene.document.Field; import org.apache.lucene.document.FieldType; import org.apache.lucene.document.SortedSetDocValuesField; @@ -24,15 +22,11 @@ import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; import org.opensearch.OpenSearchException; -import org.opensearch.Version; import org.opensearch.common.Nullable; -import org.opensearch.common.collect.Iterators; import org.opensearch.common.lucene.Lucene; import org.opensearch.common.lucene.search.AutomatonQueries; -import org.opensearch.common.xcontent.JsonToStringXContentParser; import org.opensearch.core.common.ParsingException; -import org.opensearch.core.xcontent.DeprecationHandler; -import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.analysis.NamedAnalyzer; import org.opensearch.index.fielddata.IndexFieldData; @@ -44,14 +38,17 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; -import java.util.Iterator; +import java.util.Deque; +import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.function.BiFunction; import java.util.function.Supplier; +import static org.opensearch.index.mapper.KeywordFieldMapper.normalizeValue; import static org.opensearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES; /** @@ -89,17 +86,6 @@ public MappedFieldType keyedFieldType(String key) { return new FlatObjectFieldType(this.name() + DOT_SYMBOL + key, this.name()); } - /** - * FlatObjectFieldType is the parent field type. - */ - public static class FlatObjectField extends Field { - - public FlatObjectField(String field, BytesRef term, FieldType ft) { - super(field, term, ft); - } - - } - /** * The builder for the flat_object field mapper using default parameters * @opensearch.internal @@ -115,47 +101,11 @@ private FlatObjectFieldType buildFlatObjectFieldType(BuilderContext context, Fie return new FlatObjectFieldType(buildFullName(context), fieldType); } - /** - * ValueFieldMapper is the subfield type for values in the Json. - * use a {@link KeywordFieldMapper.KeywordField} - */ - private ValueFieldMapper buildValueFieldMapper(BuilderContext context, FieldType fieldType, FlatObjectFieldType fft) { - String fullName = buildFullName(context); - FieldType vft = new FieldType(fieldType); - KeywordFieldMapper.KeywordFieldType valueFieldType = new KeywordFieldMapper.KeywordFieldType(fullName + VALUE_SUFFIX, vft); - - fft.setValueFieldType(valueFieldType); - return new ValueFieldMapper(vft, valueFieldType); - } - - /** - * ValueAndPathFieldMapper is the subfield type for path=value format in the Json. - * also use a {@link KeywordFieldMapper.KeywordField} - */ - private ValueAndPathFieldMapper buildValueAndPathFieldMapper(BuilderContext context, FieldType fieldType, FlatObjectFieldType fft) { - String fullName = buildFullName(context); - FieldType vft = new FieldType(fieldType); - KeywordFieldMapper.KeywordFieldType ValueAndPathFieldType = new KeywordFieldMapper.KeywordFieldType( - fullName + VALUE_AND_PATH_SUFFIX, - vft - ); - fft.setValueAndPathFieldType(ValueAndPathFieldType); - return new ValueAndPathFieldMapper(vft, ValueAndPathFieldType); - } - @Override public FlatObjectFieldMapper build(BuilderContext context) { FieldType fieldtype = new FieldType(Defaults.FIELD_TYPE); FlatObjectFieldType fft = buildFlatObjectFieldType(context, fieldtype); - return new FlatObjectFieldMapper( - name, - Defaults.FIELD_TYPE, - fft, - buildValueFieldMapper(context, fieldtype, fft), - buildValueAndPathFieldMapper(context, fieldtype, fft), - CopyTo.empty(), - this - ); + return new FlatObjectFieldMapper(name, Defaults.FIELD_TYPE, fft, CopyTo.empty(), this); } } @@ -173,8 +123,7 @@ public TypeParser(BiFunction builderFunction) { @Override public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { - Builder builder = builderFunction.apply(name, parserContext); - return builder; + return builderFunction.apply(name, parserContext); } } @@ -187,7 +136,7 @@ public static final class FlatObjectFieldType extends StringFieldType { private final int ignoreAbove; private final String nullValue; - private final String mappedFieldTypeName; + private final String rootFieldTypeName; private KeywordFieldMapper.KeywordFieldType valueFieldType; @@ -198,7 +147,7 @@ public FlatObjectFieldType(String name, boolean isSearchable, boolean hasDocValu setIndexAnalyzer(Lucene.KEYWORD_ANALYZER); this.ignoreAbove = Integer.MAX_VALUE; this.nullValue = null; - this.mappedFieldTypeName = null; + this.rootFieldTypeName = null; } public FlatObjectFieldType(String name, FieldType fieldType) { @@ -212,17 +161,17 @@ public FlatObjectFieldType(String name, FieldType fieldType) { ); this.ignoreAbove = Integer.MAX_VALUE; this.nullValue = null; - this.mappedFieldTypeName = null; + this.rootFieldTypeName = null; } public FlatObjectFieldType(String name, NamedAnalyzer analyzer) { super(name, true, false, true, new TextSearchInfo(Defaults.FIELD_TYPE, null, analyzer, analyzer), Collections.emptyMap()); this.ignoreAbove = Integer.MAX_VALUE; this.nullValue = null; - this.mappedFieldTypeName = null; + this.rootFieldTypeName = null; } - public FlatObjectFieldType(String name, String mappedFieldTypeName) { + public FlatObjectFieldType(String name, String rootFieldTypeName) { super( name, true, @@ -233,7 +182,7 @@ public FlatObjectFieldType(String name, String mappedFieldTypeName) { ); this.ignoreAbove = Integer.MAX_VALUE; this.nullValue = null; - this.mappedFieldTypeName = mappedFieldTypeName; + this.rootFieldTypeName = rootFieldTypeName; } void setValueFieldType(KeywordFieldMapper.KeywordFieldType valueFieldType) { @@ -327,7 +276,9 @@ protected BytesRef indexedValueForSearch(Object value) { if (value == null) { return null; } - value = inputToString(value); + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } return getTextSearchInfo().getSearchAnalyzer().normalize(name(), value.toString()); } @@ -336,14 +287,8 @@ protected BytesRef indexedValueForSearch(Object value) { */ @Override public Query termQuery(Object value, @Nullable QueryShardContext context) { - - String searchValueString = inputToString(value); - String directSubFieldName = directSubfield(); - String rewriteSearchValue = rewriteValue(searchValueString); - failIfNotIndexed(); - Query query; - query = new TermQuery(new Term(directSubFieldName, indexedValueForSearch(rewriteSearchValue))); + Query query = new TermQuery(new Term(getSearchField(), indexedValueForSearch(rewriteSearchValue(value)))); if (boost() != 1f) { query = new BoostQuery(query, boost()); } @@ -353,16 +298,12 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) { @Override public Query termsQuery(List values, QueryShardContext context) { failIfNotIndexed(); - String directedSearchFieldName = directSubfield(); - BytesRef[] bytesRefs = new BytesRef[values.size()]; + final BytesRef[] bytesRefs = new BytesRef[values.size()]; for (int i = 0; i < bytesRefs.length; i++) { - String rewriteValues = rewriteValue(inputToString(values.get(i))); - + String rewriteValues = rewriteSearchValue(values.get(i)); bytesRefs[i] = indexedValueForSearch(new BytesRef(rewriteValues)); - } - - return new TermInSetQuery(directedSearchFieldName, bytesRefs); + return new TermInSetQuery(getSearchField(), bytesRefs); } /** @@ -371,86 +312,29 @@ public Query termsQuery(List values, QueryShardContext context) { * else, direct to flatObjectFieldName._value subfield. * @return directedSubFieldName */ - public String directSubfield() { - if (mappedFieldTypeName == null) { - return new StringBuilder().append(this.name()).append(VALUE_SUFFIX).toString(); - } else { - return new StringBuilder().append(this.mappedFieldTypeName).append(VALUE_AND_PATH_SUFFIX).toString(); - } + public String getSearchField() { + return isSubfield() ? rootFieldTypeName + VALUE_AND_PATH_SUFFIX : name() + VALUE_SUFFIX; } /** * If the search key has mappedFieldTypeName as prefix, * then the dot path was used in search query, - * then rewrite the searchValueString as the format "dotpath=value", + * then rewrite the input as the format "dotpath=value", * @return rewriteSearchValue */ - public String rewriteValue(String searchValueString) { - if (!hasMappedFieldTyeNameInQueryFieldName(name())) { - return searchValueString; - } else { - String rewriteSearchValue = new StringBuilder().append(name()).append(EQUAL_SYMBOL).append(searchValueString).toString(); - return rewriteSearchValue; + public String rewriteSearchValue(Object value) { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); } - + return isSubfield() ? name() + EQUAL_SYMBOL + value : value.toString(); } - private boolean hasMappedFieldTyeNameInQueryFieldName(String input) { - String prefix = this.mappedFieldTypeName; - if (prefix == null) { - return false; - } - if (!input.startsWith(prefix)) { - return false; - } - String rest = input.substring(prefix.length()); - - if (rest.isEmpty()) { - return false; - } else { - return true; - } - } - - private String inputToString(Object inputValue) { - if (inputValue instanceof Integer) { - String inputToString = Integer.toString((Integer) inputValue); - return inputToString; - } else if (inputValue instanceof Float) { - String inputToString = Float.toString((Float) inputValue); - return inputToString; - } else if (inputValue instanceof Boolean) { - String inputToString = Boolean.toString((Boolean) inputValue); - return inputToString; - } else if (inputValue instanceof Short) { - String inputToString = Short.toString((Short) inputValue); - return inputToString; - } else if (inputValue instanceof Long) { - String inputToString = Long.toString((Long) inputValue); - return inputToString; - } else if (inputValue instanceof Double) { - String inputToString = Double.toString((Double) inputValue); - return inputToString; - } else if (inputValue instanceof BytesRef) { - String inputToString = (((BytesRef) inputValue).utf8ToString()); - return inputToString; - } else if (inputValue instanceof String) { - String inputToString = (String) inputValue; - return inputToString; - } else if (inputValue instanceof Version) { - String inputToString = inputValue.toString(); - return inputToString; - } else { - // default to cast toString - return inputValue.toString(); - } + private boolean isSubfield() { + return rootFieldTypeName != null; } @Override public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, boolean caseInsensitive, QueryShardContext context) { - String directSubfield = directSubfield(); - String rewriteValue = rewriteValue(value); - if (context.allowExpensiveQueries() == false) { throw new OpenSearchException( "[prefix] queries cannot be executed when '" @@ -463,17 +347,21 @@ public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, bool if (method == null) { method = MultiTermQuery.CONSTANT_SCORE_REWRITE; } + final String rewriteValue = rewriteSearchValue(value); if (caseInsensitive) { - return AutomatonQueries.caseInsensitivePrefixQuery((new Term(directSubfield, indexedValueForSearch(rewriteValue))), method); + return AutomatonQueries.caseInsensitivePrefixQuery( + (new Term(getSearchField(), indexedValueForSearch(rewriteValue))), + method + ); } - return new PrefixQuery(new Term(directSubfield, indexedValueForSearch(rewriteValue)), method); + return new PrefixQuery(new Term(getSearchField(), indexedValueForSearch(rewriteValue)), method); } @Override public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { - String directSubfield = directSubfield(); - String rewriteUpperTerm = rewriteValue(inputToString(upperTerm)); - String rewriteLowerTerm = rewriteValue(inputToString(lowerTerm)); + String directSubfield = getSearchField(); + String rewriteUpperTerm = rewriteSearchValue(upperTerm); + String rewriteLowerTerm = rewriteSearchValue(lowerTerm); if (context.allowExpensiveQueries() == false) { throw new OpenSearchException( "[range] queries on [text] or [keyword] fields cannot be executed when '" @@ -497,16 +385,10 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower */ @Override public Query existsQuery(QueryShardContext context) { - String searchKey; - String searchField; - if (hasMappedFieldTyeNameInQueryFieldName(name())) { - searchKey = this.mappedFieldTypeName; - searchField = name(); - } else { - searchKey = FieldNamesFieldMapper.NAME; - searchField = name(); + if (isSubfield() == false) { + return super.existsQuery(context); } - return new TermQuery(new Term(searchKey, indexedValueForSearch(searchField))); + return new TermQuery(new Term(rootFieldTypeName, indexedValueForSearch(name()))); } @Override @@ -522,29 +404,19 @@ public Query wildcardQuery( context, "Can only use wildcard queries on keyword and text fields - not on [" + name() + "] which is of type [" + typeName() + "]" ); - } - } - private final ValueFieldMapper valueFieldMapper; - private final ValueAndPathFieldMapper valueAndPathFieldMapper; - - FlatObjectFieldMapper( - String simpleName, - FieldType fieldType, - FlatObjectFieldType mappedFieldType, - ValueFieldMapper valueFieldMapper, - ValueAndPathFieldMapper valueAndPathFieldMapper, - CopyTo copyTo, - Builder builder - ) { + private final String valueFieldName; + private final String valueAndPathFieldName; + + FlatObjectFieldMapper(String simpleName, FieldType fieldType, FlatObjectFieldType mappedFieldType, CopyTo copyTo, Builder builder) { super(simpleName, fieldType, mappedFieldType, copyTo); assert fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS) <= 0; this.fieldType = fieldType; - this.valueFieldMapper = valueFieldMapper; - this.valueAndPathFieldMapper = valueAndPathFieldMapper; this.mappedFieldType = mappedFieldType; + this.valueFieldName = mappedFieldType.name() + VALUE_SUFFIX; + this.valueAndPathFieldName = mappedFieldType.name() + VALUE_AND_PATH_SUFFIX; } @Override @@ -564,235 +436,114 @@ public FlatObjectFieldType fieldType() { @Override protected void parseCreateField(ParseContext context) throws IOException { - String fieldName = null; - - if (context.externalValueSet()) { - String value = context.externalValue().toString(); - parseValueAddFields(context, value, fieldType().name()); - } else { - XContentParser ctxParser = context.parser(); - if (ctxParser.currentToken() != XContentParser.Token.VALUE_NULL) { - if (ctxParser.currentToken() != XContentParser.Token.START_OBJECT) { - throw new ParsingException( - ctxParser.getTokenLocation(), - "[" + this.name() + "] unexpected token [" + ctxParser.currentToken() + "] in flat_object field value" - ); - } + XContentParser ctxParser = context.parser(); + if (fieldType().isSearchable() == false && fieldType().isStored() == false && fieldType().hasDocValues() == false) { + ctxParser.skipChildren(); + return; + } - JsonToStringXContentParser jsonToStringParser = new JsonToStringXContentParser( - NamedXContentRegistry.EMPTY, - DeprecationHandler.IGNORE_DEPRECATIONS, - ctxParser, - fieldType().name() + if (ctxParser.currentToken() != XContentParser.Token.VALUE_NULL) { + if (ctxParser.currentToken() != XContentParser.Token.START_OBJECT) { + throw new ParsingException( + ctxParser.getTokenLocation(), + "[" + this.name() + "] unexpected token [" + ctxParser.currentToken() + "] in flat_object field value" ); - /* - JsonToStringParser is the main parser class to transform JSON into stringFields in a XContentParser - It reads the JSON object and parsed to a list of string - */ - XContentParser parser = jsonToStringParser.parseObject(); - - XContentParser.Token currentToken; - while ((currentToken = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - switch (currentToken) { - case FIELD_NAME: - fieldName = parser.currentName(); - break; - case VALUE_STRING: - String value = parser.textOrNull(); - parseValueAddFields(context, value, fieldName); - break; - } - - } } + parseObject(ctxParser, context); } - } @Override - public Iterator iterator() { - List subIterators = new ArrayList<>(); - if (valueFieldMapper != null) { - subIterators.add(valueFieldMapper); - } - if (valueAndPathFieldMapper != null) { - subIterators.add(valueAndPathFieldMapper); - } - if (subIterators.size() == 0) { - return super.iterator(); - } - @SuppressWarnings("unchecked") - Iterator concat = Iterators.concat(super.iterator(), subIterators.iterator()); - return concat; + protected String contentType() { + return CONTENT_TYPE; } - /** - * parseValueAddFields method will store data to Lucene. - * the JsonToStringXContentParser returns XContentParser with 3 string fields - * fieldName, fieldName._value, fieldName._valueAndPath. - * parseValueAddFields recognized string by the stringfield name, - * fieldName will be store through the parent FlatObjectFieldMapper,which contains all the keys - * fieldName._value will be store through the valueFieldMapper, which contains the values of the Json Object - * fieldName._valueAndPath will be store through the valueAndPathFieldMapper, which contains the "path=values" format - */ - private void parseValueAddFields(ParseContext context, String value, String fieldName) throws IOException { + private void parseObject(XContentParser parser, ParseContext context) throws IOException { + assert parser.currentToken() == XContentParser.Token.START_OBJECT; + parser.nextToken(); // Skip the outer START_OBJECT. Need to return on END_OBJECT. - NamedAnalyzer normalizer = fieldType().normalizer(); - if (normalizer != null) { - value = normalizeValue(normalizer, name(), value); + LinkedList path = new LinkedList<>(Collections.singleton(fieldType().name())); + HashSet pathParts = new HashSet<>(); + while (parser.currentToken() != XContentParser.Token.END_OBJECT) { + parseToken(parser, context, path, pathParts); } - String[] valueTypeList = fieldName.split("\\._"); - String valueType = "._" + valueTypeList[valueTypeList.length - 1]; - - if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - // convert to utf8 only once before feeding postings/dv/stored fields - - final BytesRef binaryValue = new BytesRef(fieldType().name() + DOT_SYMBOL + value); - Field field = new FlatObjectField(fieldType().name(), binaryValue, fieldType); + createPathFields(context, pathParts); + } + private void createPathFields(ParseContext context, HashSet pathParts) { + for (String part : pathParts) { + final BytesRef value = new BytesRef(name() + DOT_SYMBOL + part); + if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { + context.doc().add(new Field(name(), value, fieldType)); + } if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { createFieldNamesField(context); } - if (fieldName.equals(fieldType().name())) { - context.doc().add(field); - } - if (valueType.equals(VALUE_SUFFIX)) { - if (valueFieldMapper != null) { - valueFieldMapper.addField(context, value); - } - } - if (valueType.equals(VALUE_AND_PATH_SUFFIX)) { - if (valueAndPathFieldMapper != null) { - valueAndPathFieldMapper.addField(context, value); - } - } - if (fieldType().hasDocValues()) { - if (fieldName.equals(fieldType().name())) { - context.doc().add(new SortedSetDocValuesField(fieldType().name(), binaryValue)); - } - if (valueType.equals(VALUE_SUFFIX)) { - if (valueFieldMapper != null) { - context.doc().add(new SortedSetDocValuesField(fieldType().name() + VALUE_SUFFIX, binaryValue)); - } - } - if (valueType.equals(VALUE_AND_PATH_SUFFIX)) { - if (valueAndPathFieldMapper != null) { - context.doc().add(new SortedSetDocValuesField(fieldType().name() + VALUE_AND_PATH_SUFFIX, binaryValue)); - } - } + context.doc().add(new SortedSetDocValuesField(name(), value)); } - } - } - private static String normalizeValue(NamedAnalyzer normalizer, String field, String value) throws IOException { - String normalizerErrorMessage = "The normalization token stream is " - + "expected to produce exactly 1 token, but got 0 for analyzer " - + normalizer - + " and input \"" - + value - + "\""; - try (TokenStream ts = normalizer.tokenStream(field, value)) { - final CharTermAttribute termAtt = ts.addAttribute(CharTermAttribute.class); - ts.reset(); - if (ts.incrementToken() == false) { - throw new IllegalStateException(normalizerErrorMessage); + private void parseToken(XContentParser parser, ParseContext context, Deque path, HashSet pathParts) throws IOException { + if (parser.currentToken() == XContentParser.Token.FIELD_NAME) { + final String currentFieldName = parser.currentName(); + path.addLast(currentFieldName); // Pushing onto the stack *must* be matched by pop + parser.nextToken(); // advance to the value of fieldName + parseToken(parser, context, path, pathParts); // parse the value for fieldName (which will be an array, an object, + // or a primitive value) + path.removeLast(); // Here is where we pop fieldName from the stack (since we're done with the value of fieldName) + // Note that whichever other branch we just passed through has already ended with nextToken(), so we + // don't need to call it. + } else if (parser.currentToken() == XContentParser.Token.START_ARRAY) { + parser.nextToken(); + while (parser.currentToken() != XContentParser.Token.END_ARRAY) { + parseToken(parser, context, path, pathParts); } - final String newValue = termAtt.toString(); - if (ts.incrementToken()) { - throw new IllegalStateException(normalizerErrorMessage); + parser.nextToken(); + } else if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + parser.nextToken(); + while (parser.currentToken() != XContentParser.Token.END_OBJECT) { + parseToken(parser, context, path, pathParts); } - ts.end(); - return newValue; - } - } - - @Override - protected String contentType() { - return CONTENT_TYPE; - } - - private static final class ValueAndPathFieldMapper extends FieldMapper { - - protected ValueAndPathFieldMapper(FieldType fieldType, KeywordFieldMapper.KeywordFieldType mappedFieldType) { - super(mappedFieldType.name(), fieldType, mappedFieldType, MultiFields.empty(), CopyTo.empty()); - } - - void addField(ParseContext context, String value) { - final BytesRef binaryValue = new BytesRef(value); - if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - Field field = new KeywordFieldMapper.KeywordField(fieldType().name(), binaryValue, fieldType); - - context.doc().add(field); - - if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { - createFieldNamesField(context); - } + parser.nextToken(); + } else { + String value = parseValue(parser); + if (value == null || value.length() > fieldType().ignoreAbove) { + parser.nextToken(); + return; } - } - - @Override - protected void parseCreateField(ParseContext context) { - throw new UnsupportedOperationException(); - } - - @Override - protected void mergeOptions(FieldMapper other, List conflicts) { - - } - - @Override - protected String contentType() { - return "valueAndPath"; - } - - @Override - public String toString() { - return fieldType().toString(); - } - - } - - private static final class ValueFieldMapper extends FieldMapper { - - protected ValueFieldMapper(FieldType fieldType, KeywordFieldMapper.KeywordFieldType mappedFieldType) { - super(mappedFieldType.name(), fieldType, mappedFieldType, MultiFields.empty(), CopyTo.empty()); - } - - void addField(ParseContext context, String value) { - final BytesRef binaryValue = new BytesRef(value); - if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - Field field = new KeywordFieldMapper.KeywordField(fieldType().name(), binaryValue, fieldType); - context.doc().add(field); - - if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { - createFieldNamesField(context); - } + NamedAnalyzer normalizer = fieldType().normalizer(); + if (normalizer != null) { + value = normalizeValue(normalizer, name(), value); } - } - - @Override - protected void parseCreateField(ParseContext context) { - throw new UnsupportedOperationException(); - } - - @Override - protected void mergeOptions(FieldMapper other, List conflicts) { - - } + final String leafPath = Strings.collectionToDelimitedString(path, "."); + final String valueAndPath = leafPath + EQUAL_SYMBOL + value; - @Override - protected String contentType() { - return "value"; + if (fieldType().isSearchable() || fieldType().isStored()) { + context.doc().add(new Field(valueFieldName, new BytesRef(value), fieldType)); + context.doc().add(new Field(valueAndPathFieldName, new BytesRef(valueAndPath), fieldType)); + } + if (fieldType().hasDocValues()) { + context.doc().add(new SortedSetDocValuesField(valueFieldName, new BytesRef(name() + DOT_SYMBOL + value))); + context.doc().add(new SortedSetDocValuesField(valueAndPathFieldName, new BytesRef(name() + DOT_SYMBOL + valueAndPath))); + } + pathParts.addAll(Arrays.asList(leafPath.substring(name().length() + 1).split("\\."))); + parser.nextToken(); } + } - @Override - public String toString() { - return fieldType().toString(); + private static String parseValue(XContentParser parser) throws IOException { + switch (parser.currentToken()) { + case VALUE_BOOLEAN: + case VALUE_NUMBER: + case VALUE_STRING: + case VALUE_NULL: + return parser.textOrNull(); + // Handle other token types as needed + default: + throw new ParsingException(parser.getTokenLocation(), "Unexpected value token type [" + parser.currentToken() + "]"); } } - } diff --git a/server/src/test/java/org/opensearch/common/xcontent/JsonToStringXContentParserTests.java b/server/src/test/java/org/opensearch/common/xcontent/JsonToStringXContentParserTests.java deleted file mode 100644 index 3c292181b4d8f..0000000000000 --- a/server/src/test/java/org/opensearch/common/xcontent/JsonToStringXContentParserTests.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.common.xcontent; - -import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.xcontent.DeprecationHandler; -import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.test.OpenSearchTestCase; - -import java.io.IOException; - -public class JsonToStringXContentParserTests extends OpenSearchTestCase { - - private String flattenJsonString(String fieldName, String in) throws IOException { - try ( - XContentParser parser = JsonXContent.jsonXContent.createParser( - xContentRegistry(), - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - in - ) - ) { - JsonToStringXContentParser jsonToStringXContentParser = new JsonToStringXContentParser( - xContentRegistry(), - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - parser, - fieldName - ); - // Point to the first token (should be START_OBJECT) - jsonToStringXContentParser.nextToken(); - - XContentParser transformedParser = jsonToStringXContentParser.parseObject(); - assertSame(XContentParser.Token.END_OBJECT, jsonToStringXContentParser.currentToken()); - try (XContentBuilder jsonBuilder = XContentFactory.jsonBuilder()) { - jsonBuilder.copyCurrentStructure(transformedParser); - return jsonBuilder.toString(); - } - } - } - - public void testNestedObjects() throws IOException { - String jsonExample = "{" + "\"first\" : \"1\"," + "\"second\" : {" + " \"inner\": \"2.0\"" + "}," + "\"third\": \"three\"" + "}"; - - assertEquals( - "{" - + "\"flat\":[\"third\",\"inner\",\"first\",\"second\"]," - + "\"flat._value\":[\"1\",\"2.0\",\"three\"]," - + "\"flat._valueAndPath\":[\"flat.second.inner=2.0\",\"flat.first=1\",\"flat.third=three\"]" - + "}", - flattenJsonString("flat", jsonExample) - ); - } - - public void testChildHasDots() throws IOException { - // This should be exactly the same as testNestedObjects. We're just using the "flat" notation for the inner - // object. - String jsonExample = "{" + "\"first\" : \"1\"," + "\"second.inner\" : \"2.0\"," + "\"third\": \"three\"" + "}"; - - assertEquals( - "{" - + "\"flat\":[\"third\",\"inner\",\"first\",\"second\"]," - + "\"flat._value\":[\"1\",\"2.0\",\"three\"]," - + "\"flat._valueAndPath\":[\"flat.second.inner=2.0\",\"flat.first=1\",\"flat.third=three\"]" - + "}", - flattenJsonString("flat", jsonExample) - ); - } - - public void testNestChildObjectWithDots() throws IOException { - String jsonExample = "{" - + "\"first\" : \"1\"," - + "\"second.inner\" : {" - + " \"really_inner\" : \"2.0\"" - + "}," - + "\"third\": \"three\"" - + "}"; - - assertEquals( - "{" - + "\"flat\":[\"really_inner\",\"third\",\"inner\",\"first\",\"second\"]," - + "\"flat._value\":[\"1\",\"2.0\",\"three\"]," - + "\"flat._valueAndPath\":[\"flat.first=1\",\"flat.second.inner.really_inner=2.0\",\"flat.third=three\"]" - + "}", - flattenJsonString("flat", jsonExample) - ); - } - - public void testNestChildObjectWithDotsAndFieldWithDots() throws IOException { - String jsonExample = "{" - + "\"first\" : \"1\"," - + "\"second.inner\" : {" - + " \"totally.absolutely.inner\" : \"2.0\"" - + "}," - + "\"third\": \"three\"" - + "}"; - - assertEquals( - "{" - + "\"flat\":[\"third\",\"absolutely\",\"totally\",\"inner\",\"first\",\"second\"]," - + "\"flat._value\":[\"1\",\"2.0\",\"three\"]," - + "\"flat._valueAndPath\":[\"flat.first=1\",\"flat.second.inner.totally.absolutely.inner=2.0\",\"flat.third=three\"]" - + "}", - flattenJsonString("flat", jsonExample) - ); - } - - public void testArrayOfObjects() throws IOException { - String jsonExample = "{" - + "\"field\": {" - + " \"detail\": {" - + " \"foooooooooooo\": [" - + " {\"name\":\"baz\"}," - + " {\"name\":\"baz\"}" - + " ]" - + " }" - + "}}"; - - assertEquals( - "{" - + "\"flat\":[\"field\",\"name\",\"detail\",\"foooooooooooo\"]," - + "\"flat._value\":[\"baz\"]," - + "\"flat._valueAndPath\":[" - + "\"flat.field.detail.foooooooooooo.name=baz\"" - + "]}", - flattenJsonString("flat", jsonExample) - ); - } - - public void testArraysOfObjectsAndValues() throws IOException { - String jsonExample = "{" - + "\"field\": {" - + " \"detail\": {" - + " \"foooooooooooo\": [" - + " {\"name\":\"baz\"}," - + " {\"name\":\"baz\"}" - + " ]" - + " }," - + " \"numbers\" : [" - + " 1," - + " 2," - + " 3" - + " ]" - + "}}"; - - assertEquals( - "{" - + "\"flat\":[\"field\",\"name\",\"numbers\",\"detail\",\"foooooooooooo\"]," - + "\"flat._value\":[\"1\",\"2\",\"3\",\"baz\"]," - + "\"flat._valueAndPath\":[" - + "\"flat.field.detail.foooooooooooo.name=baz\"," - + "\"flat.field.numbers=1\"," - + "\"flat.field.numbers=3\"," - + "\"flat.field.numbers=2\"" - + "]}", - flattenJsonString("flat", jsonExample) - ); - } -} diff --git a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java index 94d1f501bee51..2d0fb81da96b8 100644 --- a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java @@ -12,18 +12,18 @@ import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.IndexableFieldType; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; +import org.opensearch.common.TriFunction; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.index.query.QueryShardContext; +import org.hamcrest.MatcherAssert; import java.io.IOException; +import java.util.List; -import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.core.IsEqual.equalTo; public class FlatObjectFieldMapperTests extends MapperTestCase { @@ -40,35 +40,12 @@ protected boolean supportsOrIgnoresBoost() { } public void testMapperServiceHasParser() throws IOException { - MapperService mapperService = createMapperService(fieldMapping(b -> { minimalMapping(b); })); + MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping)); Mapper.TypeParser parser = mapperService.mapperRegistry.getMapperParsers().get(FIELD_TYPE); assertNotNull(parser); assertTrue(parser instanceof FlatObjectFieldMapper.TypeParser); } - protected void assertExistsQuery(MapperService mapperService) throws IOException { - ParseContext.Document fields = mapperService.documentMapper().parse(source(this::writeField)).rootDoc(); - QueryShardContext queryShardContext = createQueryShardContext(mapperService); - MappedFieldType fieldType = mapperService.fieldType("field"); - Query query = fieldType.existsQuery(queryShardContext); - assertExistsQuery(fieldType, query, fields); - - } - - protected void assertExistsQuery(MappedFieldType fieldType, Query query, ParseContext.Document fields) { - // we always perform a term query against _field_names, even when the field - // is not added to _field_names because it is not indexed nor stored - assertThat(query, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) query; - assertEquals(FieldNamesFieldMapper.NAME, termQuery.getTerm().field()); - assertEquals("field", termQuery.getTerm().text()); - if (fieldType.isSearchable() || fieldType.isStored()) { - assertNotNull(fields.getField(FieldNamesFieldMapper.NAME)); - } else { - assertNoFieldNamesField(fields); - } - } - public void minimalMapping(XContentBuilder b) throws IOException { b.field("type", FIELD_TYPE); } @@ -121,12 +98,12 @@ public void testDefaults() throws Exception { // Test internal substring fields as well IndexableField[] fieldValues = doc.rootDoc().getFields("field" + VALUE_SUFFIX); assertEquals(2, fieldValues.length); - assertTrue(fieldValues[0] instanceof KeywordFieldMapper.KeywordField); + assertEquals(IndexOptions.DOCS, fieldValues[0].fieldType().indexOptions()); assertEquals(new BytesRef("bar"), fieldValues[0].binaryValue()); IndexableField[] fieldValueAndPaths = doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX); assertEquals(2, fieldValues.length); - assertTrue(fieldValueAndPaths[0] instanceof KeywordFieldMapper.KeywordField); + assertEquals(IndexOptions.DOCS, fieldValues[0].fieldType().indexOptions()); assertEquals(new BytesRef("field.foo=bar"), fieldValueAndPaths[0].binaryValue()); } @@ -211,37 +188,38 @@ public void testNullValue() throws IOException { assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field" + VALUE_SUFFIX)); assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX)); + TriFunction makeFieldsAssertion = (v1, v2, fs) -> { + MatcherAssert.assertThat( + List.of(new BytesRef(v1), new BytesRef(v2)), + containsInAnyOrder(fs[0].binaryValue(), fs[2].binaryValue()) + ); + return null; + }; // test9: {"field":{"name": [null,3],"age":4}} json = "{\"field\":{\"name\": [null,3],\"age\":4}}"; doc = mapper.parse(source(json)); fields = doc.rootDoc().getFields("field"); assertEquals(4, fields.length); - assertEquals(new BytesRef("field.name"), fields[0].binaryValue()); - assertEquals(new BytesRef("field.age"), fields[2].binaryValue()); + makeFieldsAssertion.apply("field.name", "field.age", fields); fieldValues = doc.rootDoc().getFields("field" + VALUE_SUFFIX); assertEquals(4, fieldValues.length); - assertEquals(new BytesRef("3"), fieldValues[0].binaryValue()); - assertEquals(new BytesRef("4"), fieldValues[2].binaryValue()); + makeFieldsAssertion.apply("3", "4", fieldValues); fieldValueAndPaths = doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX); assertEquals(4, fieldValueAndPaths.length); - assertEquals(new BytesRef("field.age=4"), fieldValueAndPaths[0].binaryValue()); - assertEquals(new BytesRef("field.name=3"), fieldValueAndPaths[2].binaryValue()); + makeFieldsAssertion.apply("field.name=3", "field.age=4", fieldValueAndPaths); // test10: {"field":{"age": 4,"name": [null,"3"]}} json = "{\"field\":{\"age\": 4,\"name\": [null,\"3\"]}}"; doc = mapper.parse(source(json)); fields = doc.rootDoc().getFields("field"); assertEquals(4, fields.length); - assertEquals(new BytesRef("field.name"), fields[0].binaryValue()); - assertEquals(new BytesRef("field.age"), fields[2].binaryValue()); + makeFieldsAssertion.apply("field.name", "field.age", fields); fieldValues = doc.rootDoc().getFields("field" + VALUE_SUFFIX); assertEquals(4, fieldValues.length); - assertEquals(new BytesRef("3"), fieldValues[0].binaryValue()); - assertEquals(new BytesRef("4"), fieldValues[2].binaryValue()); + makeFieldsAssertion.apply("3", "4", fieldValues); fieldValueAndPaths = doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX); assertEquals(4, fieldValueAndPaths.length); - assertEquals(new BytesRef("field.age=4"), fieldValueAndPaths[0].binaryValue()); - assertEquals(new BytesRef("field.name=3"), fieldValueAndPaths[2].binaryValue()); + makeFieldsAssertion.apply("field.name=3", "field.age=4", fieldValueAndPaths); // test11: {"field":{"age":"4","labels": [null]}} json = "{\"field\":{\"age\":\"4\",\"labels\": [null]}}"; @@ -306,16 +284,13 @@ public void testNullValue() throws IOException { doc = mapper.parse(source(json)); fields = doc.rootDoc().getFields("field"); assertEquals(4, fields.length); - assertEquals(new BytesRef("field.d"), fields[0].binaryValue()); - assertEquals(new BytesRef("field.name"), fields[2].binaryValue()); + makeFieldsAssertion.apply("field.d", "field.name", fields); fieldValues = doc.rootDoc().getFields("field" + VALUE_SUFFIX); assertEquals(4, fieldValues.length); - assertEquals(new BytesRef("dsds"), fieldValues[0].binaryValue()); - assertEquals(new BytesRef("age1"), fieldValues[2].binaryValue()); + makeFieldsAssertion.apply("dsds", "age1", fieldValues); fieldValueAndPaths = doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX); assertEquals(4, fieldValueAndPaths.length); - assertEquals(new BytesRef("field.name.name=age1"), fieldValueAndPaths[0].binaryValue()); - assertEquals(new BytesRef("field.name.d.name=dsds"), fieldValueAndPaths[2].binaryValue()); + makeFieldsAssertion.apply("field.name.name=age1", "field.name.d.name=dsds", fieldValueAndPaths); // test16: {"field":{"name": {"name1": [null,"dsdsdsd"]}}} json = "{\"field\":{\"name\": {\"name1\": [null,\"dsdsdsd\"]}}}"; @@ -375,15 +350,6 @@ public void testDeduplicationValue() throws IOException { assertEquals(new BytesRef("field.abc"), fields[0].binaryValue()); assertEquals(new BytesRef("field.age"), fields[2].binaryValue()); assertEquals(new BytesRef("field.labels"), fields[4].binaryValue()); - IndexableField[] fieldValues = doc.rootDoc().getFields("field" + VALUE_SUFFIX); - assertEquals(4, fieldValues.length); - assertEquals(new BytesRef("3"), fieldValues[0].binaryValue()); - assertEquals(new BytesRef("n"), fieldValues[2].binaryValue()); - IndexableField[] fieldValueAndPaths = doc.rootDoc().getFields("field" + VALUE_AND_PATH_SUFFIX); - assertEquals(6, fieldValueAndPaths.length); - assertEquals(new BytesRef("field.abc.abc.labels=n"), fieldValueAndPaths[0].binaryValue()); - assertEquals(new BytesRef("field.age=3"), fieldValueAndPaths[2].binaryValue()); - assertEquals(new BytesRef("field.labels=3"), fieldValueAndPaths[4].binaryValue()); } @Override diff --git a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldTypeTests.java index 9ec053dc59d10..c88a4b20a9e0e 100644 --- a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldTypeTests.java @@ -52,38 +52,38 @@ public void testFetchSourceValue() throws IOException { } - public void testDirectSubfield() { + public void testGetSearchField() { { MappedFieldType flatParentFieldType = getFlatParentFieldType("field"); // when searching for "foo" in "field", the directSubfield is field._value field - String searchFieldName = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).directSubfield(); + String searchFieldName = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).getSearchField(); assertEquals("field._value", searchFieldName); MappedFieldType dynamicMappedFieldType = new FlatObjectFieldMapper.FlatObjectFieldType("bar", flatParentFieldType.name()); // when searching for "foo" in "field.bar", the directSubfield is field._valueAndPath field - String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).directSubfield(); + String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).getSearchField(); assertEquals("field._valueAndPath", searchFieldNameDocPath); } { NamedAnalyzer analyzer = new NamedAnalyzer("default", AnalyzerScope.INDEX, null); MappedFieldType ft = new FlatObjectFieldMapper.FlatObjectFieldType("field", analyzer); - assertEquals("field._value", ((FlatObjectFieldMapper.FlatObjectFieldType) ft).directSubfield()); + assertEquals("field._value", ((FlatObjectFieldMapper.FlatObjectFieldType) ft).getSearchField()); } } - public void testRewriteValue() { + public void testRewriteSearchValue() { MappedFieldType flatParentFieldType = getFlatParentFieldType("field"); // when searching for "foo" in "field", the rewrite value is "foo" - String searchValues = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).rewriteValue("foo"); + String searchValues = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).rewriteSearchValue("foo"); assertEquals("foo", searchValues); MappedFieldType dynamicMappedFieldType = new FlatObjectFieldMapper.FlatObjectFieldType("field.bar", flatParentFieldType.name()); // when searching for "foo" in "field.bar", the rewrite value is "field.bar=foo" - String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).directSubfield(); - String searchValuesDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).rewriteValue("foo"); + String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).getSearchField(); + String searchValuesDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).rewriteSearchValue("foo"); assertEquals("field.bar=foo", searchValuesDocPath); } @@ -92,16 +92,16 @@ public void testTermQuery() { MappedFieldType flatParentFieldType = getFlatParentFieldType("field"); // when searching for "foo" in "field", the term query is directed to search "foo" in field._value field - String searchFieldName = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).directSubfield(); - String searchValues = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).rewriteValue("foo"); + String searchFieldName = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).getSearchField(); + String searchValues = ((FlatObjectFieldMapper.FlatObjectFieldType) flatParentFieldType).rewriteSearchValue("foo"); assertEquals("foo", searchValues); assertEquals(new TermQuery(new Term(searchFieldName, searchValues)), flatParentFieldType.termQuery(searchValues, null)); MappedFieldType dynamicMappedFieldType = new FlatObjectFieldMapper.FlatObjectFieldType("field.bar", flatParentFieldType.name()); // when searching for "foo" in "field.bar", the term query is directed to search in field._valueAndPath field - String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).directSubfield(); - String searchValuesDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).rewriteValue("foo"); + String searchFieldNameDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).getSearchField(); + String searchValuesDocPath = ((FlatObjectFieldMapper.FlatObjectFieldType) dynamicMappedFieldType).rewriteSearchValue("foo"); assertEquals("field.bar=foo", searchValuesDocPath); assertEquals(new TermQuery(new Term(searchFieldNameDocPath, searchValuesDocPath)), dynamicMappedFieldType.termQuery("foo", null));