Skip to content

Conversation

@xtermi2
Copy link
Contributor

@xtermi2 xtermi2 commented Feb 9, 2023

Currently ObjectMapper#findAndRegisterModules is not skipable by a subclass of JacksonSnapshotSerializer. This means all Modules in the classpath which supports registering via service loader will be registered. This is not always what is needed, like in our case:

We use in our porject production code Afterburner (if JRE is <= 8) and Blackbird (if JRE is > 8). We have a switch when our ObjectMapper is configured, like this:

        if (SystemUtils.isJavaVersionAtMost(JavaVersion.JAVA_1_8)) {
            log.info("Use Afterburner in Java 8 environment");
            objectMapper.registerModule(new AfterburnerModule());
        } else {
            log.info("Use Blackbird in Java 9+ environment");
            objectMapper.registerModule(new BlackbirdModule());
        }

With this change a subclass of JacksonSnapshotSerializer can decide in its configure override if ObjectMapper#findAndRegisterModules should be executed (just call super) or if (like in our case) the impl should register needed modules manually (don't call super).

jackmatt2
jackmatt2 previously approved these changes Feb 11, 2023
@jackmatt2
Copy link
Member

@xtermi2 can you run

./gradlew spotlessApply

This will format the code according to specifications.

@xtermi2
Copy link
Contributor Author

xtermi2 commented Feb 11, 2023

@xtermi2 can you run

./gradlew spotlessApply

This will format the code according to specifications.

Done.
Sorry, haven't read carefully your contribution guide 🙈

jackmatt2
jackmatt2 previously approved these changes Feb 12, 2023
}
};
}
;
Copy link
Member

Choose a reason for hiding this comment

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

This is failing format check ./gradlew spotlessApply

Copy link
Contributor Author

Choose a reason for hiding this comment

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

./gradlew spotlessApply has done this. I was also surprised while commiting.

@jackmatt2 jackmatt2 merged commit 9797800 into origin-energy:master Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants