Skip to content

Add custom Jackson deserializer to handle empty plugin configs and re… #6598

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.

Sign up for GitHub

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

Open
Davidding4718 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add custom Jackson deserializer to handle empty plugin configs and re… #6598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4cee36f
Add custom Jackson deserializer to handle empty plugin configs and re…
Davidding4718 Mar 2, 2026
ff519f6
fix: update Jackson BOM to 2.20.2 and fix logstash expected YAML
Davidding4718 Mar 10, 2026
846416a
fix: update Jackson BOM to 2.20.2 and fix logstash expected YAML
Davidding4718 Mar 27, 2026
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
95 changes: 70 additions & 25 deletions ...prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/PluginModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
Expand All @@ -26,7 +26,6 @@

import java.io.IOException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -62,7 +61,6 @@ public class PluginModel {
*/
static class InternalJsonModel {
@JsonAnySetter
@JsonAnyGetter
private final Map<String, Object> pluginSettings;

@JsonCreator
Expand All @@ -73,6 +71,11 @@ static class InternalJsonModel {
InternalJsonModel(final Map<String, Object> pluginSettings) {
this.pluginSettings = pluginSettings;
}

@JsonAnyGetter
Map<String, Object> getPluginSettingsForSerialization() {
return pluginSettings != null ? pluginSettings : new HashMap<>();
}
}

public PluginModel(final String pluginName, final Map<String, Object> pluginSettings) {
Expand Down Expand Up @@ -117,10 +120,28 @@ public PluginModelSerializer(final Class<PluginModel> valueClass) {
public void serialize(
final PluginModel value, final JsonGenerator gen, final SerializerProvider provider) throws IOException {
gen.writeStartObject();
Map<String, Object> serializedInner = SERIALIZER_OBJECT_MAPPER.convertValue(value.innerModel, Map.class);
if(serializedInner != null && serializedInner.isEmpty())
serializedInner = null;
gen.writeObjectField(value.getPluginName(), serializedInner);

// Serialize innerModel to a Map via a JSON round-trip so that all Jackson-annotated
// fields on potential subclasses of InternalJsonModel (e.g. SinkInternalJsonModel with
// routes, tagsTargetKey, etc.) are included. Directly reading pluginSettings would miss
// those extra fields. The resulting Map is then inspected to distinguish between a
// truly empty/null inner model and one that has actual content to write.
final String jsonString = SERIALIZER_OBJECT_MAPPER.writeValueAsString(value.innerModel);
final Map<String, Object> serializedInner = SERIALIZER_OBJECT_MAPPER.readValue(jsonString, Map.class);
Copy link
Copy Markdown
Collaborator

@oeyh oeyh Mar 27, 2026

Choose a reason for hiding this comment

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

Can you clarify why this serialization and immediate deserialization is necessary here and add a comment above the code?

Copy link
Copy Markdown
Contributor Author

@Davidding4718 Davidding4718 Mar 27, 2026

Choose a reason for hiding this comment

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

Added a comment in the code. The round-trip is needed because innerModel can be a subclass (e.g. SinkInternalJsonModel) with extra Jackson-annotated fields. Serializing to JSON and reading back as a Map lets Jackson collect all those fields together, so we can check if the result is empty and write it out correctly. Directly accessing pluginSettings would miss any extra fields defined on subclasses.


if (serializedInner.isEmpty()) {
// Empty inner model: output null if pluginSettings was null, {} if it was an empty map
if (value.innerModel.pluginSettings == null) {
gen.writeObjectField(value.getPluginName(), null);
} else {
gen.writeFieldName(value.getPluginName());
gen.writeStartObject();
gen.writeEndObject();
}
} else {
gen.writeObjectField(value.getPluginName(), serializedInner);
}

gen.writeEndObject();
}
}
Expand All @@ -136,7 +157,7 @@ public void serialize(
static final class PluginModelDeserializer extends AbstractPluginModelDeserializer<PluginModel, InternalJsonModel> {

public PluginModelDeserializer() {
super(PluginModel.class, InternalJsonModel.class, PluginModel::new, InternalJsonModel::new);
super(PluginModel.class, InternalJsonModel.class, PluginModel::new, () -> new InternalJsonModel(null));
}
}

Expand All @@ -156,33 +177,57 @@ abstract static class AbstractPluginModelDeserializer<T extends PluginModel, M e

private final Class<M> innerModelClass;
private final BiFunction<String, M, T> constructorFunction;
private final Supplier<M> emptyInnerModelConstructor;
private final Supplier<M> nullSettingsModelSupplier;

protected AbstractPluginModelDeserializer(
final Class<T> valueClass,
final Class<M> innerModelClass,
final BiFunction<String, M, T> constructorFunction,
final Supplier<M> emptyInnerModelConstructor) {
final Supplier<M> nullSettingsModelSupplier) {
super(valueClass);
this.innerModelClass = innerModelClass;
this.constructorFunction = constructorFunction;
this.emptyInnerModelConstructor = emptyInnerModelConstructor;
this.nullSettingsModelSupplier = nullSettingsModelSupplier;
}

@Override
public PluginModel deserialize(final JsonParser jsonParser, final DeserializationContext context) throws IOException, JacksonException {
final JsonNode node = jsonParser.getCodec().readTree(jsonParser);

final Iterator<Map.Entry<String, JsonNode>> fields = node.fields();
final Map.Entry<String, JsonNode> onlyField = fields.next();

final String pluginName = onlyField.getKey();
final JsonNode value = onlyField.getValue();

M innerModel = SERIALIZER_OBJECT_MAPPER.convertValue(value, innerModelClass);
if(innerModel == null)
innerModel = emptyInnerModelConstructor.get();

public PluginModel deserialize(final JsonParser jsonParser, final DeserializationContext context) throws IOException {
final ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();

jsonParser.nextToken();

final String pluginName = jsonParser.currentName();
jsonParser.nextToken();

boolean isNull = false;
Map<String, Object> data = null;
if (jsonParser.currentToken() == JsonToken.START_OBJECT) {
data = mapper.readValue(jsonParser, Map.class);
// readValue consumed up to the inner END_OBJECT; advance to the outer END_OBJECT
jsonParser.nextToken();
} else if (jsonParser.currentToken() == JsonToken.VALUE_NULL) {
// null or no-value (stdout: null / stdout:) -> preserve as null settings
isNull = true;
// advance to the outer END_OBJECT
jsonParser.nextToken();
} else if (jsonParser.currentToken() == JsonToken.VALUE_STRING) {
final String value = jsonParser.getValueAsString();
if (value.isEmpty()) {
throw context.weirdStringException(value, Map.class,
"Empty string is not allowed for plugin '" + pluginName + "'. Use null, empty (no value), or {} instead.");
} else {
throw context.weirdStringException(value, Map.class,
"String values not allowed for plugin '" + pluginName + "'");
}
} else {
throw JsonMappingException.from(jsonParser,
"Unexpected value for plugin '" + pluginName + "': expected an object, null, or no value, but got " +
jsonParser.currentToken());
}

final M innerModel = isNull
? nullSettingsModelSupplier.get()
: SERIALIZER_OBJECT_MAPPER.convertValue(data, innerModelClass);
return constructorFunction.apply(pluginName, innerModel);
}
}
Expand Down
3 changes: 2 additions & 1 deletion data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ private static List<String> validateKeys(List<String> input, String tag) {

static class SinkModelDeserializer extends AbstractPluginModelDeserializer<SinkModel, SinkInternalJsonModel> {
SinkModelDeserializer() {
super(SinkModel.class, SinkInternalJsonModel.class, SinkModel::new, () -> new SinkInternalJsonModel(null, null, null, null, null));
super(SinkModel.class, SinkInternalJsonModel.class, SinkModel::new,
() -> new SinkInternalJsonModel(null, null, null, null, null, null));
}
}
}
138 changes: 135 additions & 3 deletions ...er-api/src/test/java/org/opensearch/dataprepper/model/configuration/PluginModelTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,17 @@
import java.io.Reader;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasKey;
import static org.junit.jupiter.api.Assertions.assertThrows;

class PluginModelTests {

Expand Down Expand Up @@ -75,14 +78,14 @@ final void testSerialization_empty_plugin_to_YAML() throws JsonGenerationExcepti

final String serialized = mapper.writeValueAsString(pluginModel);

InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_null.yaml");
InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_empty_object.yaml");

assertThat(serialized, notNullValue());
assertThat(serialized, equalTo(convertInputStreamToString(inputStream)));
assertThat(serialized, equalTo(stripComments(convertInputStreamToString(inputStream))));
}

@ParameterizedTest
@ValueSource(strings = {"plugin_model_empty.yaml", "plugin_model_not_present.yaml", "plugin_model_null.yaml"})
@ValueSource(strings = {"plugin_model_with_empty_object.yaml"})
final void deserialize_with_empty_inner(final String resourceName) throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream(resourceName);

Expand All @@ -94,6 +97,17 @@ final void deserialize_with_empty_inner(final String resourceName) throws IOExce
assertThat(pluginModel.getPluginSettings().size(), equalTo(0));
}

@Test
final void deserialize_with_no_value_returns_null_settings() throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_not_present.yaml");

final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final PluginModel pluginModel = mapper.readValue(inputStream, PluginModel.class);
assertThat(pluginModel.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel.getPluginSettings(), equalTo(null));
}

@Test
final void testUsingCustomSerializerWithPluginSettings_noExceptions() throws JsonGenerationException, JsonMappingException, IOException {
final PluginModel pluginModel = new PluginModel("customPlugin", validPluginSettings());
Expand Down Expand Up @@ -141,6 +155,116 @@ final void testUsingCustomDeserializer_with_array() throws JsonParseException, J
assertThat(readValue.listOfPlugins.get(1).getPluginSettings().get("key2"), equalTo("value2"));
}

@Test
final void testUsingCustomDeserializer_with_array_of_three_preserves_all_entries() throws JsonParseException, JsonMappingException, IOException {
InputStream inputStream = PluginModelTests.class.getResourceAsStream("/list_of_plugins_multiple_fields.yaml");

final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final PluginHolder readValue = mapper.readValue(inputStream, PluginHolder.class);
assertThat(readValue, notNullValue());
assertThat(readValue.listOfPlugins, notNullValue());
assertThat(readValue.listOfPlugins.size(), equalTo(3));
assertThat(readValue.listOfPlugins.get(0).getPluginName(), equalTo("customPluginA"));
assertThat(readValue.listOfPlugins.get(0).getPluginSettings().get("key1"), equalTo("value1"));
assertThat(readValue.listOfPlugins.get(1).getPluginName(), equalTo("customPluginB"));
assertThat(readValue.listOfPlugins.get(1).getPluginSettings().get("key2"), equalTo("value2"));
assertThat(readValue.listOfPlugins.get(2).getPluginName(), equalTo("customPluginC"));
assertThat(readValue.listOfPlugins.get(2).getPluginSettings().get("key3"), equalTo("value3"));
}

@Test
final void testRoundTrip_withEmptyObject() throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_empty_object.yaml");
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel1.getPluginSettings().size(), equalTo(0));

final String serialized = mapper.writeValueAsString(pluginModel1);
assertThat(serialized.contains("{}"), equalTo(true));

final PluginModel pluginModel2 = mapper.readValue(serialized, PluginModel.class);
assertThat(pluginModel2.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel2.getPluginSettings().size(), equalTo(0));
}

@Test
final void testRoundTrip_withNullValue() throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_null.yaml");
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

// explicit null input -> deserializes with null settings (preserves null)
final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel1.getPluginSettings(), equalTo(null));

// null settings -> serializes back as null (round-trip preserved)
final String serialized = mapper.writeValueAsString(pluginModel1);
assertThat(serialized.contains("null"), equalTo(true));

final PluginModel pluginModel2 = mapper.readValue(serialized, PluginModel.class);
assertThat(pluginModel2.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel2.getPluginSettings(), equalTo(null));
}

@Test
final void testRoundTrip_withEmptyValue() throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_empty_value.yaml");
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

// no-value (customPlugin:) -> same as null -> deserializes with null settings
final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel1.getPluginSettings(), equalTo(null));

// null settings -> serializes back as null (round-trip preserved)
final String serialized = mapper.writeValueAsString(pluginModel1);
assertThat(serialized.contains("null"), equalTo(true));

final PluginModel pluginModel2 = mapper.readValue(serialized, PluginModel.class);
assertThat(pluginModel2.getPluginName(), equalTo("customPlugin"));
assertThat(pluginModel2.getPluginSettings(), equalTo(null));
}

@Test
final void testDeserialize_emptyString_throwsException() throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_empty_string.yaml");
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final JsonMappingException exception = assertThrows(
JsonMappingException.class,
() -> mapper.readValue(inputStream, PluginModel.class)
);
assertThat(exception.getMessage(), containsString("Empty string is not allowed"));
}

@Test
final void testDeserialize_nonEmptyString_throwsException() throws IOException {
final String yaml = "customPlugin: someStringValue";
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final JsonMappingException exception = assertThrows(
JsonMappingException.class,
() -> mapper.readValue(yaml, PluginModel.class)
);
assertThat(exception.getMessage(), containsString("String values not allowed"));
}

@ParameterizedTest
@ValueSource(strings = {"plugin_model_number_value.yaml", "plugin_model_boolean_value.yaml", "plugin_model_array_value.yaml"})
final void testDeserialize_invalidTokenType_throwsException(final String resourceName) throws IOException {
final InputStream inputStream = PluginModelTests.class.getResourceAsStream(resourceName);
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());

final JsonMappingException exception = assertThrows(
JsonMappingException.class,
() -> mapper.readValue(inputStream, PluginModel.class)
);
assertThat(exception.getMessage(), containsString("Unexpected value for plugin"));
}

static Map<String, Object> validPluginSettings() {
final Map<String, Object> settings = new HashMap<>();
settings.put("key1", "value1");
Expand All @@ -160,4 +284,12 @@ static String convertInputStreamToString(InputStream inputStream) throws IOExcep
return stringBuilder.toString();
}

static String stripComments(final String content) {
final String stripped = Arrays.stream(content.split("\n"))
.filter(line -> !line.startsWith("#"))
.collect(java.util.stream.Collectors.joining("\n"))
.replaceAll("^\n+", "");
return content.endsWith("\n") ? stripped + "\n" : stripped;
}

}
Loading
Loading