Skip to content
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

fix(document-handling): refactor the Objectmapper supplier and make it private so only copy can be created #3787

Merged
merged 3 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ConnectorHelper {

private static final String ERROR_CANNOT_PARSE_VARIABLES = "Cannot parse '%s' as '%s'.";
public static FeelEngineWrapper FEEL_ENGINE_WRAPPER = new FeelEngineWrapper();
public static ObjectMapper OBJECT_MAPPER = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
public static ObjectMapper OBJECT_MAPPER = ConnectorsObjectMapperSupplier.getCopy();

/**
* @return a map with output process variables for a given response from an {@link
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

class InboundConnectorContextImplTest {
private final SecretProvider secretProvider = new FooBarSecretProvider();
private final ObjectMapper mapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private final ObjectMapper mapper = ConnectorsObjectMapperSupplier.getCopy();

@Test
void bindProperties_shouldThrowExceptionWhenWrongFormat() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void rawProperties_notSerializedAsJson() throws JsonProcessingException {
new ProcessElement("myProcess", 0, 0, "element1", "<default>"));

// when
var result = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(testObj);
var result = ConnectorsObjectMapperSupplier.getCopy().writeValueAsString(testObj);

// then
assertThat(result).doesNotContain("auth", "abc");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ public class ConsoleSecretApiClient {

private final Authentication authentication;

private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy();

private static final TypeReference<Map<String, String>> mapTypeReference =
new TypeReference<>() {};

public ConsoleSecretApiClient(String secretsEndpoint, JwtCredential jwt) {
TokenResponseMapper tokenResponseMapper =
new JacksonTokenResponseMapper(ConnectorsObjectMapperSupplier.DEFAULT_MAPPER);
new JacksonTokenResponseMapper(ConnectorsObjectMapperSupplier.getCopy());
this.authentication = new JwtAuthentication(jwt, tokenResponseMapper);
this.secretsEndpoint = secretsEndpoint;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
@EnableConfigurationProperties(ConnectorProperties.class)
public class OutboundConnectorsAutoConfiguration {

private static final Logger LOG =
LoggerFactory.getLogger(OutboundConnectorsAutoConfiguration.class);

@Value("${camunda.connector.secretprovider.discovery.enabled:true}")
Boolean secretProviderLookupEnabled;

Expand All @@ -72,9 +75,6 @@ public class OutboundConnectorsAutoConfiguration {
@Value("${camunda.connector.secretprovider.console.audience:secrets.camunda.io}")
String consoleSecretsApiAudience;

private static final Logger LOG =
LoggerFactory.getLogger(OutboundConnectorsAutoConfiguration.class);

/** Provides a {@link FeelEngineWrapper} unless already present in the Spring Context */
@Bean
@ConditionalOnMissingBean(FeelEngineWrapper.class)
Expand All @@ -99,7 +99,7 @@ public SecretProviderAggregator springSecretProviderAggregator(
@Bean(name = "zeebeJsonMapper")
@ConditionalOnMissingBean
public JsonMapper jsonMapper() {
return new ZeebeObjectMapper(ConnectorsObjectMapperSupplier.DEFAULT_MAPPER);
return new ZeebeObjectMapper(ConnectorsObjectMapperSupplier.getCopy());
}

@Bean(name = "commonJsonMapper")
Expand Down Expand Up @@ -163,8 +163,6 @@ public ConsoleSecretApiClient consoleSecretApiClient(CamundaClientProperties cli
@Bean
@ConditionalOnMissingBean
public ObjectMapper objectMapper(DocumentFactory documentFactory) {
ConnectorsObjectMapperSupplier.registerDocumentModule(
documentFactory, DocumentModuleSettings.create());
return ConnectorsObjectMapperSupplier.getCopy();
return ConnectorsObjectMapperSupplier.getCopy(documentFactory, DocumentModuleSettings.create());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
@MockitoSettings(strictness = Strictness.LENIENT)
public class WebhookControllerPlainJavaTests {

private static final ObjectMapper mapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private static final ObjectMapper mapper = ConnectorsObjectMapperSupplier.getCopy();

@Test
public void multipleWebhooksOnSameContextPathAreNotSupported() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
/** Default ObjectMapper supplier to be used by the connector runtime. */
public class ConnectorsObjectMapperSupplier {

public static final ObjectMapper DEFAULT_MAPPER =
private static final ObjectMapper DEFAULT_MAPPER =
JsonMapper.builder()
.addModules(new JacksonModuleFeelFunction(), new Jdk8Module(), new JavaTimeModule())
.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)
Expand All @@ -47,8 +47,7 @@ public static ObjectMapper getCopy() {
return DEFAULT_MAPPER.copy();
}

public static void registerDocumentModule(
DocumentFactory factory, DocumentModuleSettings settings) {
DEFAULT_MAPPER.registerModule(new JacksonModuleDocument(factory, null, settings));
public static ObjectMapper getCopy(DocumentFactory factory, DocumentModuleSettings settings) {
return DEFAULT_MAPPER.copy().registerModule(new JacksonModuleDocument(factory, null, settings));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.camunda.connector.api.json.ConnectorsObjectMapperSupplier;
import io.camunda.connector.api.secret.SecretProvider;
import io.camunda.connector.api.validation.ValidationProvider;
import io.camunda.connector.document.annotation.jackson.JacksonModuleDocument;
import io.camunda.connector.runtime.core.AbstractConnectorContext;
import io.camunda.connector.runtime.core.inbound.InboundConnectorElement;
import io.camunda.connector.runtime.core.inbound.InboundConnectorReportingContext;
Expand All @@ -50,13 +51,16 @@ public class InboundConnectorContextBuilder {
protected Map<String, Object> properties;
protected InboundConnectorDefinition definition;
protected ValidationProvider validationProvider;

protected ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;

protected CorrelationResult result;

protected DocumentFactory documentFactory =
new DocumentFactoryImpl(InMemoryDocumentStore.INSTANCE);
protected ObjectMapper objectMapper =
ConnectorsObjectMapperSupplier.getCopy()
.registerModule(
new JacksonModuleDocument(
this.documentFactory,
null,
JacksonModuleDocument.DocumentModuleSettings.create()));
Comment on lines +57 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt we reuse the getCopy(DocumentFactory factory, DocumentModuleSettings settings) method here?


public static InboundConnectorContextBuilder create() {
return new InboundConnectorContextBuilder();
Expand Down Expand Up @@ -167,6 +171,13 @@ public InboundConnectorContextBuilder validation(ValidationProvider validationPr
return this;
}

public InboundConnectorContextBuilder documentFactory(DocumentFactory documentFactory) {
this.objectMapper =
ConnectorsObjectMapperSupplier.getCopy(
documentFactory, JacksonModuleDocument.DocumentModuleSettings.create());
return this;
}

/**
* Sets a custom {@link ObjectMapper} that is used to serialize and deserialize the properties. If
* not provided, default mapper will be used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.camunda.connector.api.outbound.OutboundConnectorContext;
import io.camunda.connector.api.secret.SecretProvider;
import io.camunda.connector.api.validation.ValidationProvider;
import io.camunda.connector.document.annotation.jackson.JacksonModuleDocument;
import io.camunda.connector.runtime.core.AbstractConnectorContext;
import io.camunda.connector.test.ConnectorContextTestUtil;
import io.camunda.document.Document;
Expand All @@ -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()
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be the other getCopy(.....) I guess

.registerModule(
new JacksonModuleDocument(
this.documentFactory,
null,
JacksonModuleDocument.DocumentModuleSettings.create()));

/**
* @return a new instance of the {@link OutboundConnectorContextBuilder}
Expand Down Expand Up @@ -168,6 +175,13 @@ public OutboundConnectorContextBuilder objectMapper(ObjectMapper objectMapper) {
return this;
}

public OutboundConnectorContextBuilder documentFactory(DocumentFactory documentFactory) {
this.objectMapper =
ConnectorsObjectMapperSupplier.getCopy(
documentFactory, JacksonModuleDocument.DocumentModuleSettings.create());
return this;
}

/**
* @return the {@link OutboundConnectorContext} including all previously defined properties
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
*/
package io.camunda.connector.e2e.app;

import io.camunda.connector.api.json.ConnectorsObjectMapperSupplier;
import io.camunda.connector.document.annotation.jackson.JacksonModuleDocument.DocumentModuleSettings;
import io.camunda.connector.runtime.inbound.search.SearchQueryClient;
import io.camunda.document.factory.DocumentFactory;
import io.camunda.document.factory.DocumentFactoryImpl;
import io.camunda.document.store.InMemoryDocumentStore;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Primary;

@SpringBootApplication
@ImportAutoConfiguration({
Expand All @@ -36,9 +37,12 @@
public class TestConnectorRuntimeApplication {

public static void main(String[] args) {
ConnectorsObjectMapperSupplier.registerDocumentModule(
new DocumentFactoryImpl(InMemoryDocumentStore.INSTANCE), DocumentModuleSettings.create());

SpringApplication.run(TestConnectorRuntimeApplication.class, args);
}

@Bean
@Primary
public DocumentFactory documentFactory() {
return new DocumentFactoryImpl(InMemoryDocumentStore.INSTANCE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ public void uriAuthenticationReceiveMessageTest() throws Exception {

private void assertIntermediateCatchEventUsingModel(BpmnModelInstance model) throws Exception {
Object expectedJsonResponse =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readValue(
"{\"message\":{\"consumerTag\":\"myConsumerTag\",\"body\":{\"foo\": {\"bar\": \"barValue\"}},\"properties\":{}}}",
Object.class);
ConnectorsObjectMapperSupplier.getCopy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reuse it between the tests?

.readValue(
"{\"message\":{\"consumerTag\":\"myConsumerTag\",\"body\":{\"foo\": {\"bar\": \"barValue\"}},\"properties\":{}}}",
Object.class);

processStateStore.update(mockProcessDefinition(model));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ void execute_shouldSetConnectTime(final String input) throws Exception {
objectMapper.readValue(input, ObjectNode.class).get("graphql").toString(),
GraphQLRequest.GraphQL.class)
.connectionTimeoutInSeconds();
var graphQLRequestMapper =
new GraphQLRequestMapper(ConnectorsObjectMapperSupplier.DEFAULT_MAPPER);
var graphQLRequestMapper = new GraphQLRequestMapper(ConnectorsObjectMapperSupplier.getCopy());

// when
var request =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ private HttpCommonResult getResultForCloudFunction(
if (HttpStatusHelper.isError(code)) {
// unwrap as ErrorResponse
var errorResponse =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readValue(content, ErrorResponse.class);
ConnectorsObjectMapperSupplier.getCopy().readValue(content, ErrorResponse.class);
return new HttpCommonResult(code, headers, errorResponse, reason);
}
// Unwrap the response as a HttpCommonResult directly
return ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readValue(content, HttpCommonResult.class);
return ConnectorsObjectMapperSupplier.getCopy().readValue(content, HttpCommonResult.class);
}

/**
Expand All @@ -97,7 +97,7 @@ private Object extractBody(InputStream content) throws IOException {

if (StringUtils.isNotBlank(bodyString)) {
return isJsonStringValid(bodyString)
? ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readValue(bodyString, Object.class)
? ConnectorsObjectMapperSupplier.getCopy().readValue(bodyString, Object.class)
: bodyString;
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private HttpEntity createStringEntity(HttpCommonRequest request) {
? new StringEntity(
s, contentType.orElse(ContentType.TEXT_PLAIN.withCharset(StandardCharsets.UTF_8)))
: new StringEntity(
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(body),
ConnectorsObjectMapperSupplier.getCopy().writeValueAsString(body),
contentType.orElse(ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8)));
} catch (JsonProcessingException e) {
throw new ConnectorException("Failed to serialize request body:" + body, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ public HttpCommonRequest toCloudFunctionRequest(final HttpCommonRequest request)
try {
// Using the JsonHttpContent cannot work with an element on the root content,
// hence write it ourselves:
String contentAsJson =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(request);
String contentAsJson = ConnectorsObjectMapperSupplier.getCopy().writeValueAsString(request);
String token = credentials.getOAuthToken(getProxyFunctionUrl());
return createCloudFunctionRequest(contentAsJson, token);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

public class JsonHelper {

private static final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private static final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy();

public static JsonNode getAsJsonElement(Object body) {
if (body instanceof String stringBody) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public class HttpServiceTest {
private final HttpService httpService = new HttpService(cloudFunctionService);
private final HttpService httpServiceWithoutCloudFunction =
new HttpService(disabledCloudFunctionService);
private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy();

@BeforeAll
public static void setUp() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeNotProvided() throw
assertThat(httpRequest.getEntity().getContentType())
.isEqualTo(ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString());
var jsonNode =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readTree(
httpRequest.getEntity().getContent());
ConnectorsObjectMapperSupplier.getCopy().readTree(httpRequest.getEntity().getContent());
assertThat(jsonNode.get("key").asText()).isEqualTo("value");
}

Expand All @@ -374,8 +373,7 @@ public void shouldNotSetJsonBody_whenBodySupportedAndContentTypeProvided() throw
assertThat(httpRequest.getEntity().getContentType())
.isEqualTo(ContentType.TEXT_PLAIN.withCharset(StandardCharsets.UTF_8).toString());
var jsonNode =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readTree(
httpRequest.getEntity().getContent());
ConnectorsObjectMapperSupplier.getCopy().readTree(httpRequest.getEntity().getContent());
assertThat(jsonNode.get("key").asText()).isEqualTo("value");
}

Expand All @@ -400,8 +398,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeProvided() throws E
assertThat(httpRequest.getEntity().getContentType())
.isEqualTo(ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString());
var jsonNode =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readTree(
httpRequest.getEntity().getContent());
ConnectorsObjectMapperSupplier.getCopy().readTree(httpRequest.getEntity().getContent());
assertThat(jsonNode.get("key").asText()).isEqualTo("value");
}

Expand All @@ -427,8 +424,7 @@ public void shouldSetJsonBody_whenBodySupportedAndContentTypeProvidedAndBodyIsMa
assertThat(httpRequest.getEntity().getContentType())
.isEqualTo(ContentType.APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString());
var jsonNode =
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.readTree(
httpRequest.getEntity().getContent());
ConnectorsObjectMapperSupplier.getCopy().readTree(httpRequest.getEntity().getContent());
assertThat(jsonNode.get("key").asText()).isEqualTo("value");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
public class CustomApacheHttpClientTest {

private final CustomApacheHttpClient customApacheHttpClient = CustomApacheHttpClient.getDefault();
private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.DEFAULT_MAPPER;
private final ObjectMapper objectMapper = ConnectorsObjectMapperSupplier.getCopy();
private final CamundaDocumentStore store = InMemoryDocumentStore.INSTANCE;

@Nested
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void shouldHandleJsonResponse_whenCloudFunctionEnabled() throws Exception
new HttpCommonResult(200, Map.of("X-Header", "value"), Map.of("key", "value"));
response.setEntity(
new StringEntity(
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(cloudFunctionResult)));
ConnectorsObjectMapperSupplier.getCopy().writeValueAsString(cloudFunctionResult)));

// when
HttpCommonResult result = handler.handleResponse(response);
Expand All @@ -107,8 +107,8 @@ public void shouldHandleError_whenCloudFunctionEnabled() throws Exception {
response.setHeaders(headers);
response.setEntity(
new StringEntity(
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(
new ErrorResponse("500", "Custom message", null))));
ConnectorsObjectMapperSupplier.getCopy()
.writeValueAsString(new ErrorResponse("500", "Custom message", null))));

// when
HttpCommonResult result = handler.handleResponse(response);
Expand All @@ -134,7 +134,7 @@ public void shouldHandleJsonAsTextResponse_whenCloudFunctionEnabled() throws Exc
new HttpCommonResult(200, Map.of("X-Header", "value"), "{\"key\":\"value\"}");
response.setEntity(
new StringEntity(
ConnectorsObjectMapperSupplier.DEFAULT_MAPPER.writeValueAsString(cloudFunctionResult)));
ConnectorsObjectMapperSupplier.getCopy().writeValueAsString(cloudFunctionResult)));

// when
HttpCommonResult result = handler.handleResponse(response);
Expand Down
Loading
Loading