fix(document-handling): refactor the Objectmapper supplier and make it private so only copy can be created #3787
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t private so only copy can be created
…t private so only copy can be created
johnBgood
reviewed
Dec 13, 2024
| /** Secrets used as fallback if SecretProvider is loaded via SPI */ | ||
| public static final String SECRETS_PROJECT_ENV_NAME = "SECRETS_PROJECT_ID"; | ||
|
|
||
| public static final String SECRETS_PREFIX_ENV_NAME = "SECRETS_PREFIX"; |
There was a problem hiding this comment.
is there any changes in this class? If not let's revert the changes (re ordering I guess)
There was a problem hiding this comment.
Yes thanks to spotless 🤔
| @@ -44,7 +45,13 @@ public class OutboundConnectorContextBuilder { | |||
| protected Map<String, Object> variables; | |||
| protected DocumentFactory documentFactory = | |||
| new DocumentFactoryImpl(InMemoryDocumentStore.INSTANCE); | |||
| private ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy(); | |||
| private ObjectMapper objectMapper = | |||
| ConnectorsObjectMapperSupplier.getCopy() | |||
There was a problem hiding this comment.
should be the other getCopy(.....) I guess
| @@ -51,7 +51,7 @@ public class InboundConnectorContextBuilder { | |||
| protected InboundConnectorDefinition definition; | |||
| protected ValidationProvider validationProvider; | |||
|
|
|||
| protected ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER; | |||
| protected ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy(); | |||
There was a problem hiding this comment.
should we also plug the document capability here?
| @@ -163,8 +163,7 @@ public ConsoleSecretApiClient consoleSecretApiClient(CamundaClientProperties cli | |||
| @Bean | |||
| @ConditionalOnMissingBean | |||
| public ObjectMapper objectMapper(DocumentFactory documentFactory) { | |||
| ConnectorsObjectMapperSupplier.registerDocumentModule( | |||
| documentFactory, DocumentModuleSettings.create()); | |||
| ConnectorsObjectMapperSupplier.getCopy(documentFactory, DocumentModuleSettings.create()); | |||
There was a problem hiding this comment.
we should return this line not the one below
johnBgood
approved these changes
Dec 16, 2024
sbuettner
reviewed
Dec 16, 2024
Comment on lines
+57
to
+63
| protected ObjectMapper objectMapper = | ||
| ConnectorsObjectMapperSupplier.getCopy() | ||
| .registerModule( | ||
| new JacksonModuleDocument( | ||
| this.documentFactory, | ||
| null, | ||
| JacksonModuleDocument.DocumentModuleSettings.create())); |
There was a problem hiding this comment.
Shouldnt we reuse the getCopy(DocumentFactory factory, DocumentModuleSettings settings) method here?
mathias-vandaele
deleted the
hacking-session-fixing-object-mapper-together
branch
sbuettner
reviewed
Dec 16, 2024
| ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readValue( | ||
| "{\"message\":{\"consumerTag\":\"myConsumerTag\",\"body\":{\"foo\": {\"bar\": \"barValue\"}},\"properties\":{}}}", | ||
| Object.class); | ||
| ConnectorsObjectMapperSupplier.getCopy() |
There was a problem hiding this comment.
Can we reuse it between the tests?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Done during the hacking session with the team