Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing of flat object fields with dots in keys #11425

Merged
merged 1 commit into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

### Fixed
- Fix failure in dissect ingest processor parsing empty brackets ([#9225](https://github.com/opensearch-project/OpenSearch/pull/9255))
- Fix class_cast_exception when passing int to _version and other metadata fields in ingest simulate API ([#10101](https://github.com/opensearch-project/OpenSearch/pull/10101))
- Fix `class_cast_exception` when passing int to `_version` and other metadata fields in ingest simulate API ([#10101](https://github.com/opensearch-project/OpenSearch/pull/10101))
- Fix Segment Replication ShardLockObtainFailedException bug during index corruption ([10370](https://github.com/opensearch-project/OpenSearch/pull/10370))
- Fix some test methods in SimulatePipelineRequestParsingTests never run and fix test failure ([#10496](https://github.com/opensearch-project/OpenSearch/pull/10496))
- Fix passing wrong parameter when calling newConfigurationException() in DotExpanderProcessor ([#10737](https://github.com/opensearch-project/OpenSearch/pull/10737))
Expand All @@ -200,6 +200,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix the issue with DefaultSpanScope restoring wrong span in the TracerContextStorage upon detach ([#11316](https://github.com/opensearch-project/OpenSearch/issues/11316))
- Remove shadowJar from `lang-painless` module publication ([#11369](https://github.com/opensearch-project/OpenSearch/issues/11369))
- Fix remote shards balancer and remove unused variables ([#11167](https://github.com/opensearch-project/OpenSearch/pull/11167))
- Fix parsing of flat object fields with dots in keys ([#11425](https://github.com/opensearch-project/OpenSearch/pull/11425))
- Fix bug where replication lag grows post primary relocation ([#11238](https://github.com/opensearch-project/OpenSearch/pull/11238))
- Fix for stuck update action in a bulk with `retry_on_conflict` property ([#11152](https://github.com/opensearch-project/OpenSearch/issues/11152))
- Fix template setting override for replication type ([#11417](https://github.com/opensearch-project/OpenSearch/pull/11417))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentLocation;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.index.mapper.ParseContext;

import java.io.IOException;
import java.math.BigInteger;
Expand All @@ -40,7 +39,6 @@
private ArrayList<String> keyList = new ArrayList<>();

private XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent);
private ParseContext parseContext;

private NamedXContentRegistry xContentRegistry;

Expand All @@ -54,14 +52,13 @@
public JsonToStringXContentParser(
NamedXContentRegistry xContentRegistry,
DeprecationHandler deprecationHandler,
ParseContext parseContext,
XContentParser parser,
String fieldTypeName
) throws IOException {
super(xContentRegistry, deprecationHandler);
this.parseContext = parseContext;
this.deprecationHandler = deprecationHandler;
this.xContentRegistry = xContentRegistry;
this.parser = parseContext.parser();
this.parser = parser;
this.fieldTypeName = fieldTypeName;
}

Expand All @@ -86,8 +83,22 @@
StringBuilder parsedFields = new StringBuilder();

if (this.parser.currentToken() == Token.FIELD_NAME) {
path.append(DOT_SYMBOL + currentFieldName);
this.keyList.add(currentFieldName);
path.append(DOT_SYMBOL).append(currentFieldName);
int dotIndex = currentFieldName.indexOf(DOT_SYMBOL);
String fieldNameSuffix = currentFieldName;
// The field name may be of the form foo.bar.baz
// If that's the case, each "part" is a key.
while (dotIndex >= 0) {
msfroh marked this conversation as resolved.
Show resolved Hide resolved
String fieldNamePrefix = fieldNameSuffix.substring(0, dotIndex);
if (!fieldNamePrefix.isEmpty()) {
this.keyList.add(fieldNamePrefix);
}
fieldNameSuffix = fieldNameSuffix.substring(dotIndex + 1);
dotIndex = fieldNameSuffix.indexOf(DOT_SYMBOL);
}
if (!fieldNameSuffix.isEmpty()) {
this.keyList.add(fieldNameSuffix);
}
} else if (this.parser.currentToken() == Token.START_ARRAY) {
parseToken(path, currentFieldName);
break;
Expand All @@ -97,18 +108,18 @@
parseToken(path, currentFieldName);
int dotIndex = path.lastIndexOf(DOT_SYMBOL);
if (dotIndex != -1) {
path.delete(dotIndex, path.length());
path.setLength(path.length() - currentFieldName.length() - 1);
}
} else {
if (!path.toString().contains(currentFieldName)) {
path.append(DOT_SYMBOL + currentFieldName);
path.append(DOT_SYMBOL).append(currentFieldName);

Check warning on line 115 in server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java#L115

Added line #L115 was not covered by tests
}
parseValue(parsedFields);
this.valueList.add(parsedFields.toString());
this.valueAndPathList.add(path + EQUAL_SYMBOL + parsedFields);
int dotIndex = path.lastIndexOf(DOT_SYMBOL);
if (dotIndex != -1) {
path.delete(dotIndex, path.length());
path.setLength(path.length() - currentFieldName.length() - 1);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ protected void parseCreateField(ParseContext context) throws IOException {
JsonToStringXContentParser JsonToStringParser = new JsonToStringXContentParser(
NamedXContentRegistry.EMPTY,
DeprecationHandler.IGNORE_DEPRECATIONS,
context,
context.parser(),
fieldType().name()
);
/*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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 {
String transformed;
try (
XContentParser parser = JsonXContent.jsonXContent.createParser(
xContentRegistry(),
DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
in
)
) {
JsonToStringXContentParser jsonToStringXContentParser = new JsonToStringXContentParser(
xContentRegistry(),
DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
parser,
fieldName
);
// Skip the START_OBJECT token:
jsonToStringXContentParser.nextToken();

XContentParser transformedParser = jsonToStringXContentParser.parseObject();
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\":[\"first\",\"second\",\"inner\",\"third\"],"
+ "\"flat._value\":[\"1\",\"2.0\",\"three\"],"
+ "\"flat._valueAndPath\":[\"flat.first=1\",\"flat.second.inner=2.0\",\"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\":[\"first\",\"second\",\"inner\",\"third\"],"
+ "\"flat._value\":[\"1\",\"2.0\",\"three\"],"
+ "\"flat._valueAndPath\":[\"flat.first=1\",\"flat.second.inner=2.0\",\"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\":[\"first\",\"second\",\"inner\",\"really_inner\",\"third\"],"
+ "\"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\":[\"first\",\"second\",\"inner\",\"totally\",\"absolutely\",\"inner\",\"third\"],"
msfroh marked this conversation as resolved.
Show resolved Hide resolved
+ "\"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)
);
}

}
Loading