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

Preserve JSON types in flatten_json function #13947

Merged
merged 9 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/issue-13888.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Entry type according to https://keepachangelog.com/en/1.0.0/
# One of: a(dded), c(hanged), d(eprecated), r(emoved), f(ixed), s(ecurity)
type = "fixed"
message = "Pipeline function flatten_json respects the original JSON types. An optional parameter is provided for back-compat."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
message = "Pipeline function flatten_json respects the original JSON types. An optional parameter is provided for back-compat."
message = "The pipeline function flatten_json now respects the original JSON types. An optional parameter is provided for backwards compatibility"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should note that also in UPGRADING.md because it's a breaking change.
I do agree that we should change the default though, it's better to do it now, rather than to have the wrong default forever in our product.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickmann did you see my note about UPGRADING.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, missed that - fixed it


issues = ["13888"]
pulls = ["13947"]
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,33 @@ public class JsonFlatten extends AbstractFunction<JsonNode> {
private final ObjectMapper objectMapper;
private final ParameterDescriptor<String, String> valueParam;
private final ParameterDescriptor<String, String> arrayHandlerParam;
private final ParameterDescriptor<Boolean, Boolean> stringifyParam;

@Inject
public JsonFlatten(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
valueParam = ParameterDescriptor.string("value").description("The string to parse as a JSON tree").build();
arrayHandlerParam = ParameterDescriptor.string("array_handler").description("Determines how arrays are processed").build();
stringifyParam = ParameterDescriptor.bool("stringify").optional().description("Convert all extracted values to strings").build();
}

@Override
public JsonNode evaluate(FunctionArgs args, EvaluationContext context) {
final String value = valueParam.required(args, context);
final String arrayHandler = arrayHandlerParam.required(args, context);
final boolean stringify = stringifyParam.optional(args, context).orElse(false);

try {
switch (arrayHandler) {
case OPTION_IGNORE:
// ignore all top-level arrays
return JsonUtils.extractJson(value, objectMapper, FLAGS_IGNORE);
return JsonUtils.extractJson(value, objectMapper, FLAGS_IGNORE, stringify);
case OPTION_JSON:
// return top-level arrays as valid JSON strings
return JsonUtils.extractJson(value, objectMapper, FLAGS_JSON);
return JsonUtils.extractJson(value, objectMapper, FLAGS_JSON, stringify);
case OPTION_FLATTEN:
// explode all arrays and objects into top-level key/values
return JsonUtils.extractJson(value, objectMapper, FLAGS_FLATTEN);
return JsonUtils.extractJson(value, objectMapper, FLAGS_FLATTEN, stringify);
default:
LOG.warn("Unknown parameter array_handler: {}", arrayHandler);
}
Expand All @@ -96,7 +99,7 @@ public FunctionDescriptor<JsonNode> descriptor() {
.name(NAME)
.returnType(JsonNode.class)
.params(of(
valueParam, arrayHandlerParam
valueParam, arrayHandlerParam, stringifyParam
))
.description("Parses a string as a JSON tree, while flattening all containers to a single level")
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

import javax.annotation.Nullable;
import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -51,7 +53,7 @@ private JsonUtils() {
}

public static JsonNode extractJson(
String value, ObjectMapper mapper, ExtractFlags extractFlags)
String value, ObjectMapper mapper, ExtractFlags extractFlags, Boolean stringify)
mpfz0r marked this conversation as resolved.
Show resolved Hide resolved
throws IOException {
if (isNullOrEmpty(value)) {
throw new IOException("null result");
Expand All @@ -61,12 +63,41 @@ public static JsonNode extractJson(
ObjectNode resultRoot = mapper.createObjectNode();
for (Map.Entry<String, Object> mapEntry : json.entrySet()) {
for (Entry entry : parseValue(mapEntry.getKey(), mapEntry.getValue(), mapper, extractFlags)) {
resultRoot.put(entry.key(), entry.value().toString());
if (stringify) {
resultRoot.put(entry.key(), entry.value().toString());
} else {
putNodeWithType(resultRoot, entry.key(), entry.value());
mpfz0r marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return resultRoot;
}

private static void putNodeWithType(ObjectNode node, String key, Object value) {
if (value instanceof Short) {
node.put(key, (Short) value);
} else if (value instanceof Integer) {
node.put(key, (Integer) value);
} else if (value instanceof Long) {
node.put(key, (Long) value);
} else if (value instanceof Float) {
node.put(key, (Float) value);
} else if (value instanceof Double) {
node.put(key, (Double) value);
} else if (value instanceof BigDecimal) {
node.put(key, (BigDecimal) value);
} else if (value instanceof BigInteger) {
node.put(key, (BigInteger) value);
} else if (value instanceof String) {
node.put(key, (String) value);
} else if (value instanceof Boolean) {
node.put(key, (Boolean) value);
} else {
LOG.debug("Unknown type \"{}\" in key \"{}\"", value.getClass(), key);
node.put(key, value.toString());
}
}

private static Collection<Entry> parseValue(
String key, Object value, ObjectMapper mapper, ExtractFlags extractFlags)
throws JsonProcessingException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ public void extractArrays() throws IOException {

// delete all top-level arrays
String expected = "{\"k0\":\"v0\"}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_IGNORE);
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_IGNORE, false);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);

// serialize arrays into valid JSON with escaping; retain empty arrays
expected = "{\"k0\":\"v0\",\"arr1\":\"[\\\"a1\\\",[\\\"a21\\\",\\\"a22\\\"],[],\\\"a2\\\"]\"}";
result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_JSON);
result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_JSON, false);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);
}

Expand All @@ -50,17 +50,27 @@ public void extractObjects() throws IOException {

// flatten objects by concatenating keys
String expected = "{\"k0\":\"v0\",\"obj1_k1\":\"v1\",\"obj1_obj11_k11\":\"v11\",\"obj1_obj11_obj111_k111\":\"v111\"}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_IGNORE);
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_IGNORE, false);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);
}

@Test
public void extractArrayOfObjects() throws IOException {
String jsonString = "{\"k0\":\"v0\",\"arr1\":[{\"k11\":\"v11\",\"k12\":\"v12\"},{\"k21\":\"v21\"}]}";
String jsonString = "{\"k0\":\"v0\",\"arr1\":[{\"k11\":\"v11\",\"k12\":12},{\"k21\":\"v21\"}]}";

// flatten objects by concatenating keys
String expected = "{\"k0\":\"v0\",\"arr1_0_k11\":\"v11\",\"arr1_0_k12\":\"v12\",\"arr1_1_k21\":\"v21\"}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_FLATTEN);
String expected = "{\"k0\":\"v0\",\"arr1_0_k11\":\"v11\",\"arr1_0_k12\":12,\"arr1_1_k21\":\"v21\"}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_FLATTEN, false);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);
}

@Test
public void preserveTypes() throws IOException {
String jsonString = "{\"string\":\"s0\",\"arr1\":[{\"int\":1,\"float\":1.2},{\"bool\":true}]}";

// flatten objects by concatenating keys
String expected = "{\"string\":\"s0\",\"arr1_0_int\":1,\"arr1_0_float\":1.2,\"arr1_1_bool\":true}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_FLATTEN, false);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);
}

Expand Down Expand Up @@ -169,4 +179,13 @@ public void deleteNestedObject() throws IOException {
JsonUtils.deleteBelow(node, 3);
assertThat(mapper.writeValueAsString(node)).isEqualTo(expected);
}

@Test
public void testStringify() throws IOException {
String jsonString = "{\"bool\":true,\"arr1\":[{\"int\":1,\"float\":1.2},{\"bool\":true}]}";

String expected = "{\"bool\":\"true\",\"arr1_0_int\":\"1\",\"arr1_0_float\":\"1.2\",\"arr1_1_bool\":\"true\"}";
JsonNode result = JsonUtils.extractJson(jsonString, mapper, JsonFlatten.FLAGS_FLATTEN, true);
assertThat(mapper.writeValueAsString(result)).isEqualTo(expected);
}
}