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(jackson): replace ObjectMapper with lower-level APIs in FEEL deserializers #1724

Merged
merged 1 commit into from
Jan 16, 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
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import com.fasterxml.jackson.module.scala.DefaultScalaModule$;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import org.camunda.feel.FeelEngine;
Expand Down Expand Up @@ -143,13 +147,73 @@ public <T> T evaluate(final String expression, final Object... variables) {
}

public <T> T evaluate(final String expression, final Class<T> clazz, final Object... variables) {
Object result = evaluate(expression, variables);
return sanitizeScalaOutput(objectMapper.convertValue(result, clazz));
Function<JsonNode, T> converter =
(JsonNode jsonNode) -> {
try {
return objectMapper.treeToValue(jsonNode, clazz);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
};
var type = objectMapper.getTypeFactory().constructType(clazz);
return evaluateAndConvert(expression, type, converter, variables);
}

public <T> T evaluate(final String expression, final JavaType clazz, final Object... variables) {
Function<JsonNode, T> converter =
(JsonNode jsonNode) -> {
try {
return objectMapper.treeToValue(jsonNode, clazz);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
};
return evaluateAndConvert(expression, clazz, converter, variables);
}

/**
* For use in custom deserializers where there is a need to use the deserialization context. This
* allows to preserve custom settings and registered jackson modules when evaluating expressions.
*/
public <T> T evaluate(
final DeserializationContext ctx,
final String expression,
final JavaType clazz,
final Object... variables) {
Function<JsonNode, T> converter =
(JsonNode jsonNode) -> {
try {
if (jsonNode == null || jsonNode.isNull()) {
return null;
}
return ctx.readTreeAsValue(jsonNode, clazz);
} catch (IOException e) {
throw new RuntimeException(e);
}
};
return evaluateAndConvert(expression, clazz, converter, variables);
}

@SuppressWarnings("unchecked")
private <T> T evaluateAndConvert(
final String expression,
final JavaType clazz,
Function<JsonNode, T> converter,
final Object... variables) {

Object result = evaluate(expression, variables);
return sanitizeScalaOutput(objectMapper.convertValue(result, clazz));
JsonNode jsonNode = objectMapper.convertValue(result, JsonNode.class);

try {
if (clazz.getRawClass().equals(String.class) && jsonNode.isObject()) {
return (T) objectMapper.writeValueAsString(jsonNode);
} else {
return sanitizeScalaOutput(converter.apply(jsonNode));
}
} catch (Exception e) {
throw new FeelEngineWrapperException(
"Failed to convert FEEL evaluation result to the target type", expression, variables, e);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
package io.camunda.connector.feel.jackson;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.ObjectCodec;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -34,6 +32,16 @@ public abstract class AbstractFeelDeserializer<T> extends StdDeserializer<T>
protected FeelEngineWrapper feelEngineWrapper;
protected boolean relaxed;

/**
* A blank object mapper object for use in inheriting classes.
*
* <p>NOTE: This object mapper does not preserve the original deserialization context nor is it
* aware of any registered modules. It should not be used to deserialize the final result. For
* final results, use the {@link DeserializationContext} object passed to {@link
* #doDeserialize(JsonNode, JsonNode, DeserializationContext)} instead.
*/
protected static final ObjectMapper BLANK_OBJECT_MAPPER = new ObjectMapper();

protected AbstractFeelDeserializer(FeelEngineWrapper feelEngineWrapper, boolean relaxed) {
super(String.class);
this.feelEngineWrapper = feelEngineWrapper;
Expand All @@ -42,29 +50,20 @@ protected AbstractFeelDeserializer(FeelEngineWrapper feelEngineWrapper, boolean

@Override
public T deserialize(JsonParser parser, DeserializationContext context) throws IOException {
JsonNode node = parser.getCodec().readTree(parser);
JsonNode node = parser.readValueAsTree();
if (node == null || node.isNull()) {
return null;
}
ObjectCodec codec = parser.getCodec();
ObjectMapper mapper;
if (!(codec instanceof ObjectMapper)) {
DeserializationConfig config = context.getConfig();
mapper = new ObjectMapper();
mapper.setConfig(config);
} else {
mapper = (ObjectMapper) codec;
}

if (isFeelExpression(node.textValue()) || relaxed) {
var feelContextSupplier =
context.getAttribute(FeelContextAwareObjectReader.FEEL_CONTEXT_ATTRIBUTE);

if (feelContextSupplier == null) {
return doDeserialize(node, mapper, mapper.createObjectNode());
return doDeserialize(node, BLANK_OBJECT_MAPPER.createObjectNode(), context);
}
if (feelContextSupplier instanceof Supplier<?> supplier) {
return doDeserialize(node, mapper, mapper.valueToTree(supplier.get()));
return doDeserialize(node, BLANK_OBJECT_MAPPER.valueToTree(supplier.get()), context);
}
throw new IOException(
"Attribute "
Expand All @@ -82,6 +81,7 @@ protected boolean isFeelExpression(String value) {
return value != null && value.startsWith("=");
}

protected abstract T doDeserialize(JsonNode node, ObjectMapper mapper, JsonNode feelContext)
protected abstract T doDeserialize(
JsonNode node, JsonNode feelContext, DeserializationContext deserializationContext)
throws IOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
*/
package io.camunda.connector.feel.jackson;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.type.TypeFactory;
import io.camunda.connector.feel.FeelEngineWrapper;
import java.io.IOException;
Expand Down Expand Up @@ -51,11 +50,11 @@ protected FeelDeserializer(FeelEngineWrapper feelEngineWrapper, JavaType outputT
}

@Override
protected Object doDeserialize(JsonNode node, ObjectMapper mapper, JsonNode feelContext)
throws JsonProcessingException {
protected Object doDeserialize(
JsonNode node, JsonNode feelContext, DeserializationContext jacksonCtx) throws IOException {

if (isFeelExpression(node.textValue())) {
return handleFeelExpression(node, mapper, feelContext);
return feelEngineWrapper.evaluate(jacksonCtx, node.textValue(), outputType, feelContext);
}

if (node.isTextual()) {
Expand All @@ -66,36 +65,32 @@ protected Object doDeserialize(JsonNode node, ObjectMapper mapper, JsonNode feel
// Support legacy list like formats like: a,b,c | 1,2,3
return handleListLikeFormat(textValue);
} else {
try {
var jsonFactory = jacksonCtx.getParser().getCodec().getFactory();
try (JsonParser jsonParser = jsonFactory.createParser(textValue)) {
// check if this string contains a JSON object/array/etc inside (i.e. it's not just a
// string)
var convertedValue = mapper.readValue(textValue, JsonNode.class);
return handleNormalJsonNode(convertedValue, mapper);
JsonNode jsonNode = jsonParser.readValueAsTree();
if (jsonNode != null && !jsonNode.isNull()) {
return handleNormalJsonNode(jsonNode, jacksonCtx);
}
} catch (IOException e) {
// ignore, this is just a string, we will take care of it below
}
}
}
return handleNormalJsonNode(node, mapper);
return handleNormalJsonNode(node, jacksonCtx);
}

protected Object handleFeelExpression(JsonNode node, ObjectMapper mapper, JsonNode feelContext)
throws JsonProcessingException {
var jsonNode = feelEngineWrapper.evaluate(node.textValue(), JsonNode.class, feelContext);
if (outputType.getRawClass() == String.class && jsonNode.isObject()) {
return mapper.writeValueAsString(jsonNode);
} else {
return mapper.treeToValue(jsonNode, outputType);
}
}

protected Object handleNormalJsonNode(JsonNode node, ObjectMapper mapper)
throws JsonProcessingException {
protected Object handleNormalJsonNode(JsonNode node, DeserializationContext context)
throws IOException {

if (node == null || node.isNull()) {
return null;
}
if (outputType.getRawClass() == String.class && node.isObject()) {
return mapper.writeValueAsString(node);
return BLANK_OBJECT_MAPPER.writeValueAsString(node);
}
return mapper.treeToValue(node, outputType);
return context.readTreeAsValue(node, outputType);
}

protected Object handleListLikeFormat(String textValue) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@
package io.camunda.connector.feel.jackson;

import com.fasterxml.jackson.annotation.JsonMerge;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.type.TypeFactory;
import io.camunda.connector.feel.FeelEngineWrapper;
import java.io.IOException;
Expand All @@ -45,32 +43,42 @@ public FeelFunctionDeserializer(JavaType outputType, FeelEngineWrapper feelEngin
private final FeelEngineWrapper feelEngineWrapper = new FeelEngineWrapper();

@Override
@SuppressWarnings("unchecked")
protected Function<IN, OUT> doDeserialize(
JsonNode node, ObjectMapper mapper, JsonNode feelContext) {
JsonNode node, JsonNode feelContext, DeserializationContext deserializationContext) {
return (input) -> {
var jsonNode =
JsonNode jsonNode =
feelEngineWrapper.evaluate(
node.textValue(), JsonNode.class, mergeContexts(input, feelContext, mapper));
deserializationContext,
node.textValue(),
deserializationContext.getTypeFactory().constructType(JsonNode.class),
mergeContexts(input, feelContext));
try {
if (jsonNode == null || jsonNode.isNull()) {
return null;
}
if (outputType.getRawClass() == String.class && jsonNode.isObject()) {
return (OUT) mapper.writeValueAsString(jsonNode);
return (OUT) BLANK_OBJECT_MAPPER.writeValueAsString(jsonNode);
} else {
return mapper.treeToValue(jsonNode, outputType);
return deserializationContext.readTreeAsValue(jsonNode, outputType);
}
} catch (JsonProcessingException e) {
} catch (IOException e) {
throw new RuntimeException(e);
}
};
}

private Object mergeContexts(Object inputContext, Object feelContext, ObjectMapper mapper) {
private Object mergeContexts(Object inputContext, Object feelContext) {
try {
var wrappedInput = new MergedContext(mapper.convertValue(inputContext, MAP_TYPE_REF));
var wrappedFeelContext = new MergedContext(mapper.convertValue(feelContext, MAP_TYPE_REF));
var wrappedInput =
new MergedContext(BLANK_OBJECT_MAPPER.convertValue(inputContext, MAP_TYPE_REF));
var wrappedFeelContext =
new MergedContext(BLANK_OBJECT_MAPPER.convertValue(feelContext, MAP_TYPE_REF));
var merged =
mapper
BLANK_OBJECT_MAPPER
.readerForUpdating(wrappedInput)
.treeToValue(mapper.valueToTree(wrappedFeelContext), MergedContext.class);
.treeToValue(
BLANK_OBJECT_MAPPER.valueToTree(wrappedFeelContext), MergedContext.class);
return merged.context;
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.type.TypeFactory;
import io.camunda.connector.feel.FeelEngineWrapper;
import java.util.function.Supplier;
Expand All @@ -35,8 +34,11 @@ protected FeelSupplierDeserializer(JavaType outputType, FeelEngineWrapper feelEn
}

@Override
protected Supplier<OUT> doDeserialize(JsonNode node, ObjectMapper mapper, JsonNode context) {
return () -> feelEngineWrapper.evaluate(node.textValue(), outputType, context);
protected Supplier<OUT> doDeserialize(
JsonNode node, JsonNode feelContext, DeserializationContext deserializationContext) {
return () ->
feelEngineWrapper.evaluate(
deserializationContext, node.textValue(), outputType, feelContext);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import io.camunda.connector.feel.annotation.FEEL;
import java.time.LocalDate;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
Expand All @@ -33,7 +35,9 @@
public class FeelDeserializerTest {

private final ObjectMapper mapper =
new ObjectMapper().registerModule(new JacksonModuleFeelFunction());
new ObjectMapper()
.registerModule(new JacksonModuleFeelFunction())
.registerModule(new JavaTimeModule());

@Test
void feelDeserializer_deserializeMap() throws JsonProcessingException {
Expand Down Expand Up @@ -219,6 +223,32 @@ void feelDeserializer_contextSupplied_invalidObjectProvided() {
assertThat(e.getMessage()).contains("Attribute FEEL_CONTEXT must be a Supplier");
}

@Test
void feelDeserializer_notFeel_java8Time_parsed() {
// this test is to ensure that deserialization takes active jackson modules into account

// given
String json = """
{ "props": "2019-01-01" }
""";

// when && then
var targetType = assertDoesNotThrow(() -> mapper.readValue(json, TargetTypeJava8Time.class));
assertThat(targetType.props).isEqualTo(LocalDate.of(2019, 1, 1));
}

@Test
void feelDeserializer_notFeel_null_parsed() {
// given
String json = """
{ "props": null }
""";

// when && then
var targetType = assertDoesNotThrow(() -> mapper.readValue(json, TargetTypeString.class));
assertThat(targetType.props).isNull();
}

private record TargetTypeMap(@FEEL Map<String, String> props) {}

private record TargetTypeObject(@FEEL StubObject stubObject) {}
Expand All @@ -234,4 +264,6 @@ private record TargetTypeList(@FEEL List<String> props) {}
private record TargetTypeListLong(@FEEL List<Long> props) {}

private record TargetTypeListInteger(@FEEL List<Integer> props) {}

private record TargetTypeJava8Time(@FEEL LocalDate props) {}
}
Loading