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

Prevent failing on unknown JSON properties in CloudtrailSNSNotificationParser #47

Merged
merged 2 commits into from Oct 16, 2017

Conversation

Projects
None yet
3 participants
@joschi
Contributor

joschi commented Oct 4, 2017

The Jackson ObjectMapper used in CloudtrailSNSNotificationParser was configured to fail on unknown properties (default) and thus parsing the SNS notifications failed if the format was changed in AWS.

Fixes #44

Prevent failing on unknown JSON properties in CloudtrailSNSNotificati…
…onParser

The Jackson ObjectMapper used in CloudtrailSNSNotificationParser was configured to
fail on unknown properties (default) and thus parsing the SNS notifications failed
if the format was changed in AWS.

Fixes #44

@joschi joschi added this to the 3.0.0 milestone Oct 4, 2017

@joschi joschi requested review from bernd and kroepke Oct 10, 2017

@bernd bernd modified the milestones: 3.0.0, 2.4.0 Oct 10, 2017

@joschi joschi modified the milestones: 2.4.0, 3.0.0 Oct 11, 2017


if (envelope.message == null) {
return Collections.emptyList();
}

CloudtrailWriteNotification notification = om.readValue(envelope.message, CloudtrailWriteNotification.class);
final CloudtrailWriteNotification notification = objectMapper.readValue(envelope.message, CloudtrailWriteNotification.class);

This comment has been minimized.

@kroepke

kroepke Oct 11, 2017

Member

Does this need to be done in TreeReader.java as well, it is using a custom ObjectMapper instance as well.

I'm wondering why the @JsonIgnoreProperties(ignoreUnknown = true) that the DTOs have on them don't work as expected?

This comment has been minimized.

@joschi

joschi Oct 11, 2017

Contributor

The parsing failed with CloudtrailWriteNotification (which doesn't have this annotation), not with SQSMessage (which does).

public CloudtrailSNSNotificationParser() {
om = new ObjectMapper();
}
private final ObjectMapper objectMapper = new ObjectMapper().disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);

This comment has been minimized.

@kroepke

kroepke Oct 11, 2017

Member

Can we inject a single instance across the project to avoid having different configurations?

This comment has been minimized.

@joschi

joschi Oct 11, 2017

Contributor

@bernd

bernd approved these changes Oct 16, 2017

@bernd bernd self-assigned this Oct 16, 2017

@bernd bernd merged commit 92d7cb3 into master Oct 16, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 594 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@bernd bernd deleted the issue-44 branch Oct 16, 2017

bernd added a commit that referenced this pull request Oct 16, 2017

Prevent failing on unknown JSON properties in CloudtrailSNSNotificati…
…onParser (#47)

* Prevent failing on unknown JSON properties in CloudtrailSNSNotificationParser

The Jackson ObjectMapper used in CloudtrailSNSNotificationParser was configured to
fail on unknown properties (default) and thus parsing the SNS notifications failed
if the format was changed in AWS.

Fixes #44

* Create lenient object mapper once and inject it where required

(cherry picked from commit 92d7cb3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment