Skip to content

Commit 9fd97de

Browse files
Davidding4718Davidding4718
committed
fix: update Jackson BOM to 2.20.2 and fix logstash expected YAML
Signed-off-by: Siqi Ding <dingdd@amazon.com>
1 parent 4cee36f commit 9fd97de

File tree

8 files changed

+83
-59
lines changed

8 files changed

+83
-59
lines changed

‎data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/PluginModel.java‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Map;
2929
import java.util.Objects;
3030
import java.util.function.BiFunction;
31+
import java.util.function.Supplier;
3132

3233
/**
3334
* Model class for a Plugin in Configuration YAML containing name of the Plugin and its associated settings
@@ -38,13 +39,7 @@
3839
@JsonDeserialize(using = PluginModel.PluginModelDeserializer.class)
3940
public class PluginModel {
4041

41-
private static final ObjectMapper SERIALIZER_OBJECT_MAPPER;
42-
43-
static {
44-
SERIALIZER_OBJECT_MAPPER = new ObjectMapper();
45-
// Note: We don't configure coercion here because our custom deserializer
46-
// handles all the cases (null, empty, {}, and rejects empty strings)
47-
}
42+
private static final ObjectMapper SERIALIZER_OBJECT_MAPPER = new ObjectMapper();
4843

4944
private final String pluginName;
5045
private final InternalJsonModel innerModel;
@@ -65,7 +60,6 @@ public class PluginModel {
6560
*/
6661
static class InternalJsonModel {
6762
@JsonAnySetter
68-
@JsonAnyGetter
6963
private final Map<String, Object> pluginSettings;
7064

7165
@JsonCreator
@@ -76,6 +70,11 @@ static class InternalJsonModel {
7670
InternalJsonModel(final Map<String, Object> pluginSettings) {
7771
this.pluginSettings = pluginSettings;
7872
}
73+
74+
@JsonAnyGetter
75+
Map<String, Object> getPluginSettingsForSerialization() {
76+
return pluginSettings != null ? pluginSettings : new HashMap<>();
77+
}
7978
}
8079

8180
public PluginModel(final String pluginName, final Map<String, Object> pluginSettings) {
@@ -121,27 +120,22 @@ public void serialize(
121120
final PluginModel value, final JsonGenerator gen, final SerializerProvider provider) throws IOException {
122121
gen.writeStartObject();
123122

124-
// Serialize the inner model to JSON string then back to Map
125-
// This properly respects all Jackson annotations including @JsonInclude on subclasses
126-
String jsonString = SERIALIZER_OBJECT_MAPPER.writeValueAsString(value.innerModel);
127-
Map<String, Object> serializedInner = SERIALIZER_OBJECT_MAPPER.readValue(jsonString, Map.class);
123+
final String jsonString = SERIALIZER_OBJECT_MAPPER.writeValueAsString(value.innerModel);
124+
final Map<String, Object> serializedInner = SERIALIZER_OBJECT_MAPPER.readValue(jsonString, Map.class);
128125

129126
if (serializedInner.isEmpty()) {
130-
// Inner model has no content - check if pluginSettings was explicitly null
131-
// to decide between null and {}
127+
// Empty inner model: output null if pluginSettings was null, {} if it was an empty map
132128
if (value.innerModel.pluginSettings == null) {
133-
// Explicitly null settings -> serialize as null
134129
gen.writeObjectField(value.getPluginName(), null);
135130
} else {
136-
// Empty (non-null) settings -> serialize as {}
137131
gen.writeFieldName(value.getPluginName());
138132
gen.writeStartObject();
139133
gen.writeEndObject();
140134
}
141135
} else {
142-
// Inner model has content (plugin settings or subclass fields like routes)
143136
gen.writeObjectField(value.getPluginName(), serializedInner);
144137
}
138+
145139
gen.writeEndObject();
146140
}
147141
}
@@ -157,7 +151,7 @@ public void serialize(
157151
static final class PluginModelDeserializer extends AbstractPluginModelDeserializer<PluginModel, InternalJsonModel> {
158152

159153
public PluginModelDeserializer() {
160-
super(PluginModel.class, InternalJsonModel.class, PluginModel::new);
154+
super(PluginModel.class, InternalJsonModel.class, PluginModel::new, () -> new InternalJsonModel(null));
161155
}
162156
}
163157

@@ -177,35 +171,37 @@ abstract static class AbstractPluginModelDeserializer<T extends PluginModel, M e
177171

178172
private final Class<M> innerModelClass;
179173
private final BiFunction<String, M, T> constructorFunction;
174+
private final Supplier<M> nullSettingsModelSupplier;
180175

181176
protected AbstractPluginModelDeserializer(
182177
final Class<T> valueClass,
183178
final Class<M> innerModelClass,
184-
final BiFunction<String, M, T> constructorFunction) {
179+
final BiFunction<String, M, T> constructorFunction,
180+
final Supplier<M> nullSettingsModelSupplier) {
185181
super(valueClass);
186182
this.innerModelClass = innerModelClass;
187183
this.constructorFunction = constructorFunction;
184+
this.nullSettingsModelSupplier = nullSettingsModelSupplier;
188185
}
189186

190187
@Override
191188
public PluginModel deserialize(final JsonParser jsonParser, final DeserializationContext context) throws IOException {
192-
ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
189+
final ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
193190

194191
jsonParser.nextToken();
195192

196193
final String pluginName = jsonParser.currentName();
197194
jsonParser.nextToken();
198195

199-
Map<String, Object> data = new HashMap<>();
196+
boolean isNull = false;
197+
Map<String, Object> data = null;
200198
if (jsonParser.currentToken() == JsonToken.START_OBJECT) {
201199
data = mapper.readValue(jsonParser, Map.class);
202200
} else if (jsonParser.currentToken() == JsonToken.VALUE_NULL) {
203-
// null value -> treat as empty object (acceptable format)
204-
data = new HashMap<>();
201+
// null or no-value (stdout: null / stdout:) -> preserve as null settings
202+
isNull = true;
205203
} else if (jsonParser.currentToken() == JsonToken.VALUE_STRING) {
206-
String value = jsonParser.getValueAsString();
207-
// Empty string "" is NOT allowed - throw exception
208-
// Any other string value is also not allowed
204+
final String value = jsonParser.getValueAsString();
209205
if (value.isEmpty()) {
210206
throw context.weirdStringException(value, Map.class,
211207
"Empty string is not allowed for plugin '" + pluginName + "'. Use null, empty (no value), or {} instead.");
@@ -214,12 +210,14 @@ public PluginModel deserialize(final JsonParser jsonParser, final Deserializatio
214210
"String values not allowed for plugin '" + pluginName + "'");
215211
}
216212
}
213+
217214
while (jsonParser.currentToken() != JsonToken.END_OBJECT) {
218215
jsonParser.nextToken();
219216
}
220217

221-
final M innerModel = SERIALIZER_OBJECT_MAPPER.convertValue(data, innerModelClass);
222-
218+
final M innerModel = isNull
219+
? nullSettingsModelSupplier.get()
220+
: SERIALIZER_OBJECT_MAPPER.convertValue(data, innerModelClass);
223221
return constructorFunction.apply(pluginName, innerModel);
224222
}
225223
}

‎data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ private static List<String> validateKeys(List<String> input, String tag) {
164164

165165
static class SinkModelDeserializer extends AbstractPluginModelDeserializer<SinkModel, SinkInternalJsonModel> {
166166
SinkModelDeserializer() {
167-
super(SinkModel.class, SinkInternalJsonModel.class, SinkModel::new);
167+
super(SinkModel.class, SinkInternalJsonModel.class, SinkModel::new,
168+
() -> new SinkInternalJsonModel(null, null, null, null, null, null));
168169
}
169170
}
170171
}

‎data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/PluginModelTests.java‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import static org.hamcrest.CoreMatchers.equalTo;
3434
import static org.hamcrest.CoreMatchers.notNullValue;
3535
import static org.hamcrest.MatcherAssert.assertThat;
36+
import static org.hamcrest.Matchers.containsString;
3637
import static org.hamcrest.Matchers.hasKey;
38+
import static org.junit.jupiter.api.Assertions.assertThrows;
3739

3840
class PluginModelTests {
3941

@@ -82,7 +84,7 @@ final void testSerialization_empty_plugin_to_YAML() throws JsonGenerationExcepti
8284
}
8385

8486
@ParameterizedTest
85-
@ValueSource(strings = {"plugin_model_empty.yaml", "plugin_model_not_present.yaml", "plugin_model_null.yaml"})
87+
@ValueSource(strings = {"plugin_model_empty.yaml", "plugin_model_null.yaml"})
8688
final void deserialize_with_empty_inner(final String resourceName) throws IOException {
8789
final InputStream inputStream = PluginModelTests.class.getResourceAsStream(resourceName);
8890

@@ -94,6 +96,17 @@ final void deserialize_with_empty_inner(final String resourceName) throws IOExce
9496
assertThat(pluginModel.getPluginSettings().size(), equalTo(0));
9597
}
9698

99+
@Test
100+
final void deserialize_with_no_value_returns_null_settings() throws IOException {
101+
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_not_present.yaml");
102+
103+
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
104+
105+
final PluginModel pluginModel = mapper.readValue(inputStream, PluginModel.class);
106+
assertThat(pluginModel.getPluginName(), equalTo("customPlugin"));
107+
assertThat(pluginModel.getPluginSettings(), equalTo(null));
108+
}
109+
97110
@Test
98111
final void testUsingCustomSerializerWithPluginSettings_noExceptions() throws JsonGenerationException, JsonMappingException, IOException {
99112
final PluginModel pluginModel = new PluginModel("customPlugin", validPluginSettings());
@@ -144,7 +157,7 @@ final void testUsingCustomDeserializer_with_array() throws JsonParseException, J
144157
@Test
145158
final void testRoundTrip_withEmptyObject() throws IOException {
146159
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_empty_object.yaml");
147-
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory().enable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS));
160+
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
148161

149162
final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
150163
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
@@ -161,63 +174,63 @@ final void testRoundTrip_withEmptyObject() throws IOException {
161174
@Test
162175
final void testRoundTrip_withNullValue() throws IOException {
163176
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_null.yaml");
164-
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory().enable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS));
177+
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
165178

166-
// null input -> deserializes to empty settings
179+
// explicit null input -> deserializes with null settings (preserves null)
167180
final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
168181
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
169-
assertThat(pluginModel1.getPluginSettings().size(), equalTo(0));
182+
assertThat(pluginModel1.getPluginSettings(), equalTo(null));
170183

171-
// empty settings -> serializes as {}
184+
// null settings -> serializes back as null (round-trip preserved)
172185
final String serialized = mapper.writeValueAsString(pluginModel1);
173-
assertThat(serialized.contains("{}"), equalTo(true));
186+
assertThat(serialized.contains("null"), equalTo(true));
174187

175188
final PluginModel pluginModel2 = mapper.readValue(serialized, PluginModel.class);
176189
assertThat(pluginModel2.getPluginName(), equalTo("customPlugin"));
177-
assertThat(pluginModel2.getPluginSettings().size(), equalTo(0));
190+
assertThat(pluginModel2.getPluginSettings(), equalTo(null));
178191
}
179192

180193
@Test
181194
final void testRoundTrip_withEmptyValue() throws IOException {
182195
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_with_empty_value.yaml");
183-
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory().enable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS));
196+
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
184197

185-
// empty value (no value after colon) -> deserializes to empty settings
198+
// no-value (customPlugin:) -> same as null -> deserializes with null settings
186199
final PluginModel pluginModel1 = mapper.readValue(inputStream, PluginModel.class);
187200
assertThat(pluginModel1.getPluginName(), equalTo("customPlugin"));
188-
assertThat(pluginModel1.getPluginSettings().size(), equalTo(0));
201+
assertThat(pluginModel1.getPluginSettings(), equalTo(null));
189202

190-
// empty settings -> serializes as {}
203+
// null settings -> serializes back as null (round-trip preserved)
191204
final String serialized = mapper.writeValueAsString(pluginModel1);
192-
assertThat(serialized.contains("{}"), equalTo(true));
205+
assertThat(serialized.contains("null"), equalTo(true));
193206

194207
final PluginModel pluginModel2 = mapper.readValue(serialized, PluginModel.class);
195208
assertThat(pluginModel2.getPluginName(), equalTo("customPlugin"));
196-
assertThat(pluginModel2.getPluginSettings().size(), equalTo(0));
209+
assertThat(pluginModel2.getPluginSettings(), equalTo(null));
197210
}
198211

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

204-
final JsonMappingException exception = org.junit.jupiter.api.Assertions.assertThrows(
217+
final JsonMappingException exception = assertThrows(
205218
JsonMappingException.class,
206219
() -> mapper.readValue(inputStream, PluginModel.class)
207220
);
208-
assertThat(exception.getMessage(), org.hamcrest.Matchers.containsString("Empty string is not allowed"));
221+
assertThat(exception.getMessage(), containsString("Empty string is not allowed"));
209222
}
210223

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

216-
final JsonMappingException exception = org.junit.jupiter.api.Assertions.assertThrows(
229+
final JsonMappingException exception = assertThrows(
217230
JsonMappingException.class,
218231
() -> mapper.readValue(yaml, PluginModel.class)
219232
);
220-
assertThat(exception.getMessage(), org.hamcrest.Matchers.containsString("String values not allowed"));
233+
assertThat(exception.getMessage(), containsString("String values not allowed"));
221234
}
222235

223236
static Map<String, Object> validPluginSettings() {

‎data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SamplePipelineConfigurationTest.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919

2020
import java.io.IOException;
2121
import java.io.InputStream;
22+
import java.util.Map;
2223

2324
import static org.hamcrest.CoreMatchers.equalTo;
2425
import static org.hamcrest.CoreMatchers.notNullValue;
2526
import static org.hamcrest.MatcherAssert.assertThat;
27+
import static org.hamcrest.Matchers.containsString;
2628
import static org.junit.jupiter.api.Assertions.assertThrows;
2729

2830
/**
@@ -43,8 +45,6 @@ void setup() {
4345
objectMapper = new ObjectMapper(new YAMLFactory());
4446
}
4547

46-
// --- Valid pipeline scenarios ---
47-
4848
@ParameterizedTest(name = "pipeline with plugin config [{0}] should deserialize successfully")
4949
@ValueSource(strings = {
5050
"sample_pipelines/sample_pipeline_plugin_null.yaml",
@@ -100,13 +100,14 @@ void deserialize_pipeline_withEmptyPluginConfig_hasEmptySettings(final String re
100100
final PipelinesDataFlowModel model = objectMapper.readValue(inputStream, PipelinesDataFlowModel.class);
101101
final PipelineModel pipeline = model.getPipelines().get(PIPELINE_NAME);
102102

103-
assertThat(pipeline.getSource().getPluginSettings().size(), equalTo(0));
104-
assertThat(pipeline.getProcessors().get(0).getPluginSettings().size(), equalTo(0));
105-
assertThat(pipeline.getSinks().get(0).getPluginSettings().size(), equalTo(0));
103+
final Map<String, Object> sourceSettings = pipeline.getSource().getPluginSettings();
104+
assertThat(sourceSettings == null || sourceSettings.isEmpty(), equalTo(true));
105+
final Map<String, Object> processorSettings = pipeline.getProcessors().get(0).getPluginSettings();
106+
assertThat(processorSettings == null || processorSettings.isEmpty(), equalTo(true));
107+
final Map<String, Object> sinkSettings = pipeline.getSinks().get(0).getPluginSettings();
108+
assertThat(sinkSettings == null || sinkSettings.isEmpty(), equalTo(true));
106109
}
107110

108-
// --- Invalid pipeline scenario ---
109-
110111
@Test
111112
void deserialize_pipeline_withEmptyStringPluginConfig_throwsException() {
112113
final InputStream inputStream = getClass().getResourceAsStream("sample_pipelines/sample_pipeline_plugin_empty_string.yaml");
@@ -115,6 +116,6 @@ void deserialize_pipeline_withEmptyStringPluginConfig_throwsException() {
115116
JsonMappingException.class,
116117
() -> objectMapper.readValue(inputStream, PipelinesDataFlowModel.class)
117118
);
118-
assertThat(exception.getMessage(), org.hamcrest.Matchers.containsString("Empty string is not allowed"));
119+
assertThat(exception.getMessage(), containsString("Empty string is not allowed"));
119120
}
120121
}

‎data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SinkModelTest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ void deserialize_with_any_pluginModel() throws IOException {
121121
}
122122

123123
@ParameterizedTest
124-
@ValueSource(strings = {"plugin_model_empty.yaml", "plugin_model_not_present.yaml", "plugin_model_null.yaml"})
124+
@ValueSource(strings = {"plugin_model_empty.yaml", "plugin_model_null.yaml"})
125125
final void deserialize_with_empty_inner(final String resourceName) throws IOException {
126126
final InputStream inputStream = PluginModelTests.class.getResourceAsStream(resourceName);
127127

@@ -133,6 +133,17 @@ final void deserialize_with_empty_inner(final String resourceName) throws IOExce
133133
assertThat(sinkModel.getPluginSettings().size(), equalTo(0));
134134
}
135135

136+
@Test
137+
final void deserialize_with_no_value_returns_null_settings() throws IOException {
138+
final InputStream inputStream = PluginModelTests.class.getResourceAsStream("plugin_model_not_present.yaml");
139+
140+
final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
141+
142+
final SinkModel sinkModel = mapper.readValue(inputStream, SinkModel.class);
143+
assertThat(sinkModel.getPluginName(), equalTo("customPlugin"));
144+
assertThat(sinkModel.getPluginSettings(), equalTo(null));
145+
}
146+
136147
@Test
137148
void serialize_with_just_pluginModel() throws IOException {
138149
final Map<String, Object> pluginSettings = new LinkedHashMap<>();

‎data-prepper-api/src/test/resources/org/opensearch/dataprepper/model/configuration/plugin_model_not_present.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
---
2-
customPlugin: {}
2+
customPlugin:

‎data-prepper-api/src/test/resources/pipeline_with_extension.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ extension:
22
test_extension:
33
test-pipeline:
44
source:
5-
testSource: {}
5+
testSource: null
66
processor:
77
- testProcessor: {}
88
sink:

‎data-prepper-logstash-configuration/src/test/resources/org/opensearch/dataprepper/logstash/log-ingest-to-opensearch.expected.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ logstash-converted-pipeline:
1212
match:
1313
log:
1414
- "%{COMBINEDAPACHELOG}"
15-
- drop_events: null
15+
- drop_events: {}
1616
- key_value:
1717
source: "message"
1818
destination: "test"

0 commit comments

Comments
 (0)
⚡