feat(api)!: complete entity dynamic mapping dry-run workflow - #99
Open
foukou19 wants to merge 9 commits into
Open
feat(api)!: complete entity dynamic mapping dry-run workflow#99foukou19 wants to merge 9 commits into
foukou19 wants to merge 9 commits into
Conversation
…object and array payloads Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
foukou19
marked this pull request as draft
July 21, 2026 11:26
foukou19
changed the title
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall coverage in commit e73413c in the Show a code coverage summary of the most impacted files.
Updated |
foukou19
changed the title
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a complete entity dynamic mapping dry-run flow to the API and refactors dynamic-mapping relations from a JSON object/map to an ordered list, enabling richer relation extraction (scalar/array/object payload shapes) and better validation/error reporting.
Changes:
- Introduce
POST /api/v1/entity-dynamic-mappings/dry-runwith request/response DTOs and mappers, returning per-mapping dry-run results. - Refactor
EntityDynamicMapping.relationsfromMap<String,String>toList<RelationMapping>across domain/API/persistence, including Flyway migration and deserialization support for both legacy and new API formats. - Add a JSLT-based
MappingEnginePortimplementation with expression evaluation fallback and improved validation/exception mapping (incl. 422 responses).
Reviewed changes
Copilot reviewed 47 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/db/test/R__4_insert_webhook_test_data.sql | Adjust test seed data relations JSON shape to array ([]). |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/EntityDynamicMappingAdaptorTest.java | Update persistence adapter tests for relations list. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltMappingEngineAdapterTest.java | New unit tests for relation extraction across payload shapes. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltEntityMappingValidatorTest.java | Update validator tests for new relations model + new exception type. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/webhook/InboundWebhookMapperTest.java | Update mapper test to use RelationMapping. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandlerTest.java | Update expected HTTP status codes (400 → 422) for mapping validation errors. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/EntityMappingDtoInDeserializationTest.java | New tests ensuring relations can deserialize from object-map or array formats. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/InboundWebhookManagementControllerTest.java | Update controller tests payload relations JSON to array. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingControllerTest.java | Add dry-run endpoint tests + update relations JSON + 422 expectations. |
| src/test/java/com/decathlon/idp_core/domain/service/webhook/WebhookConnectorValidationServiceTest.java | Update domain tests for relations list. |
| src/test/java/com/decathlon/idp_core/domain/service/webhook/WebhookConnectorServiceTest.java | Update domain tests for relations list. |
| src/test/java/com/decathlon/idp_core/domain/service/webhook/EntityDynamicMappingValidationServiceTest.java | Update mapping validation tests + helper conversion to RelationMapping. |
| src/test/java/com/decathlon/idp_core/domain/service/webhook/DynamicMappingServiceTest.java | Update mapping merge tests for relations list. |
| src/test/java/com/decathlon/idp_core/domain/service/relation/RelationValidationServiceTest.java | Add dry-run relation validation behavior test (skip DB existence checks). |
| src/test/java/com/decathlon/idp_core/domain/service/entity_dynamic_mapping/EntityDynamicMappingDryRunServiceTest.java | New unit tests for dry-run service behavior and error paths. |
| src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql | New Flyway migration to convert relations JSON from object to array. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/EntityDynamicMappingPersistenceMapper.java | Switch relations mapping to list (new MapStruct qualifiers). |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java | Add JSONB (de)serialization helpers for relations list. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltMappingEngineAdapter.java | New mapping engine adapter extracting entity + relations from payload via JSLT. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltExpressionEvaluator.java | New evaluator with current-node/root fallback and dedicated failure exception. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltEntityMappingValidator.java | Refactor validator to use ExpressionEngine abstraction + relation list validation. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltEngine.java | New JSLT engine implementing expression validation/evaluation abstraction. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java | New Spring configuration for JSLT mapping components. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/engine/ExpressionEngine.java | New infra abstraction for expression engines (JSLT/JQ/etc.). |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingMapper.java | Map inbound/outbound relations via RelationMapping list DTOs. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoOutMapper.java | New mapper from domain dry-run result to API response DTO. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java | New mapper normalizing payload object/string to raw JSON string. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java | Map additional mapping/evaluation exceptions to HTTP 422. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/out/entity_dynamic_mapping/RelationMappingDtoOut.java | New outbound DTO for relation mapping definition. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/out/entity_dynamic_mapping/EntityDynamicMappingDtoOut.java | Update outbound DTO to expose relations as ordered list. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/out/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoOut.java | New dry-run response DTO (snake_case). |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/RelationMappingsDeserializer.java | New deserializer accepting relations as legacy map or new array format. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/RelationMappingDtoIn.java | New inbound DTO for relation mappings. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/EntityMappingDtoIn.java | Update inbound entity mapping DTO to relations list with custom deserializer. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/EntityDynamicMappingDryRunDtoIn.java | New dry-run request DTO allowing payload as string or object. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingController.java | Add /dry-run endpoint wiring DTO mappers + domain service. |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerDescription.java | Add Swagger strings for new dry-run endpoint and payload description. |
| src/main/java/com/decathlon/idp_core/domain/service/relation/RelationValidationService.java | Add dry-run relation validation method skipping DB existence checks. |
| src/main/java/com/decathlon/idp_core/domain/service/entity/EntityValidationService.java | Add dry-run entity validation method (skip uniqueness + target existence). |
| src/main/java/com/decathlon/idp_core/domain/service/entity_dynamic_mapping/EntityDynamicMappingValidationService.java | Update template relation-name validation for ordered relation list. |
| src/main/java/com/decathlon/idp_core/domain/service/entity_dynamic_mapping/EntityDynamicMappingService.java | Rename injected validation service field and use it consistently. |
| src/main/java/com/decathlon/idp_core/domain/service/entity_dynamic_mapping/EntityDynamicMappingDryRunService.java | New domain service executing mapping dry-run and validating result. |
| src/main/java/com/decathlon/idp_core/domain/port/MappingEnginePort.java | New domain port for mapping engine operations. |
| src/main/java/com/decathlon/idp_core/domain/model/enums/ErrorType.java | New enum classifying dry-run errors (skipped/jslt/validation). |
| src/main/java/com/decathlon/idp_core/domain/model/entity_mapping/RelationMapping.java | New domain record for ordered relation mappings with multiple expressions. |
| src/main/java/com/decathlon/idp_core/domain/model/entity_mapping/EntityDynamicMapping.java | Change relations type to List<RelationMapping> and normalize nulls. |
| src/main/java/com/decathlon/idp_core/domain/model/entity_mapping/DryRunResult.java | New domain model representing dry-run outcomes. |
| src/main/java/com/decathlon/idp_core/domain/exception/entity_dynamic_mapping/ExpressionEvaluationFailedException.java | New exception for expression evaluation failure on both current/root nodes. |
| src/main/java/com/decathlon/idp_core/domain/exception/entity_dynamic_mapping/EntityDynamicMappingJsltErrorException.java | New exception category for 422 mapping/JSLT errors. |
| src/main/java/com/decathlon/idp_core/domain/constant/ValidationMessages.java | Add dry-run validation/result messages. |
Comments suppressed due to low confidence (1)
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:26
- The column comment documents the migrated JSON format using an 'expression' field, but the code serializes/deserializes RelationMapping with an 'expressions' array. Keeping the comment consistent helps avoid inserting the wrong JSON shape and ending up with null expressions.
Comment on lines
+12
to
+16
| SELECT COALESCE( | ||
| jsonb_agg(jsonb_build_object('name', key, 'expression', value) ORDER BY key), | ||
| '[]'::jsonb | ||
| ) | ||
| FROM jsonb_each_text(relations) |
Comment on lines
+57
to
+61
| /// Converts JSONB array string to `List<RelationMapping>`. | ||
| /// | ||
| /// Expected format: `[{"name": "owner", "expression": ".sender.login"}]` | ||
| /// | ||
| /// Used when loading relations from database. |
Comment on lines
+1
to
+19
| package com.decathlon.idp_core.infrastructure.adapters.entity_mapping.jslt; | ||
|
|
||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
|
|
||
| /// Spring configuration for JSLT mapping adapter infrastructure. | ||
| /// Provides shared JSON serialization for payload parsing and expression evaluation. | ||
| @Configuration | ||
| public class JsltConfiguration { | ||
|
|
||
| /// Provides a shared ObjectMapper bean for mapping adapters. | ||
| /// Bean scope is singleton and safe for concurrent reuse. | ||
| @Bean | ||
| public ObjectMapper objectMapper() { | ||
| return new ObjectMapper(); | ||
| } | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Already fixed with @ConditionalOnMissingBean(ObjectMapper.class)
Comment on lines
+16
to
+21
| @Component | ||
| @RequiredArgsConstructor | ||
| public class EntityDynamicMappingDryRunDtoInMapper { | ||
|
|
||
| private final ObjectMapper objectMapper = new ObjectMapper(); | ||
|
|
Comment on lines
+3
to
+6
| import java.util.Optional; | ||
| import java.util.stream.Stream; | ||
| import java.util.stream.StreamSupport; | ||
|
|
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 24, 2026 09:55
b2e78a6 to
3c808f8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 50 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:15
- The migration converts legacy relation objects into array items with an
expressionstring field, but the Java model/persistence mapper expectsRelationMappingwith anexpressionslist. As-is, existing rows will deserialize incorrectly (or fail) after the migration.
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:6 - The migration header comment documents the new relations format using
{name, expression}but the codebase now models relations as{name, expressions:[...]}. Keeping this comment aligned will avoid confusion for future migrations/operations.
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:26 - The column comment still describes the old
{name, expression}format. After the migration (and withRelationMapping.expressions), the persisted JSON should documentexpressionsas an array.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:21 - The class-level docs say relations are stored as
{name, expression}but persistence now serializes/deserializesRelationMapping, whose JSON shape will be{name, expressions:[...]}(record component name). The docs should match the actual persisted format to prevent future mismigrations.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltExpressionEvaluator.java:5 java.util.stream.Streamis imported but never used; unused imports are compilation errors in Java.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:21- This Spring component creates its own
ObjectMapperinstance, which bypasses the application’s Jackson configuration (modules, features, naming, etc.) and makes behavior diverge from the rest of the API. Prefer constructor injection of the sharedObjectMapperbean.
@Component
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
- Defining a bare
@Bean ObjectMapperhere will disable Spring Boot’s Jackson auto-configuration and can subtly break JSON (de)serialization across the whole API (missing modules like JavaTimeModule, custom naming/features, etc.). Consider removing this bean and injecting the Boot-managedObjectMapper, or naming/qualifying a dedicated mapper for JSLT only so it doesn’t replace the global one.
@Configuration
public class JsltConfiguration {
/// Provides a shared ObjectMapper bean for mapping adapters.
/// Bean scope is singleton and safe for concurrent reuse.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
Comment on lines
41
to
48
| if (mapping.relations() != null && !mapping.relations().isEmpty()) { | ||
| mapping.relations().forEach((key, expr) -> checkExpression(errors, "relations." + key, expr)); | ||
| mapping.relations().forEach(relation -> { | ||
| if (relation.expressions() != null && !relation.expressions().isEmpty()) { | ||
| relation.expressions().forEach( | ||
| expr -> checkExpression(errors, "relations." + relation.name() + "[]", expr)); | ||
| } | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 50 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:13
- The migration converts the old map format to objects with an
expressionfield (string), but the application now persists/deserializes relations asRelationMappingrecords with anexpressionsfield (list). This mismatch will leaveexpressionsnull after migration and can break mapping validation/runtime.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:21 - This Spring component instantiates its own ObjectMapper, bypassing the application-configured Jackson ObjectMapper (modules, naming strategies, etc.) and defeating constructor injection despite @requiredargsconstructor. This can cause inconsistent serialization of the dry-run payload compared to the rest of the API.
@Component
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
Comment on lines
+174
to
+181
| /// Returns node text value and fails when node is null or missing. | ||
| private String requireStringValue(JsonNode node, String fieldName) { | ||
| if (node == null || node.isNull() || node.isMissingNode()) { | ||
| throw new EntityDynamicMappingConfigurationException( | ||
| "Expression for '" + fieldName + "' returned null"); | ||
| } | ||
| return node.asText(); | ||
| } |
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 24, 2026 10:46
3c808f8 to
f741d07
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 51 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:20
- This mapper always creates its own
ObjectMapper, which ignores the application’s configured Jackson settings/modules and prevents Spring from injecting the shared mapper (despite@RequiredArgsConstructor). This can lead to inconsistent serialization (dates, naming, modules) between the API layer and dry-run payload normalization.
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
- Registering a bare
new ObjectMapper()as a Spring bean overrides Spring Boot’s auto-configuredObjectMapper(and any custom Jackson modules/settings). That can unintentionally change JSON (de)serialization across the whole application.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltMappingEngineAdapter.java:179
requireStringValuereturnsnode.asText()for any non-null node. For non-scalar results (objects/arrays) Jackson returns an empty string, which can silently produce blank identifiers/names instead of failing fast. This should reject non-scalar and blank results for required fields like entity identifier/name.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:61toRelationListdeserializes directly intoRelationMapping, which only supports anexpressionsfield. However, the Javadoc here indicates a legacy/singularexpressionfield, and the accompanying test claims legacy support. As-is, legacy rows/doc examples usingexpressionwould fail to deserialize.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltExpressionEvaluator.java:5java.util.stream.Streamis imported but never used, which may fail strict style/lint checks.
Comment on lines
+40
to
+42
| if (node == null || node.isNull()) { | ||
| return List.of(); | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 52 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:63
toRelationListcurrently relies on Jackson binding toRelationMapping, which expects anexpressionsarray. However this file’s Javadoc (and the corresponding unit test name) refer to a legacyexpressionfield. If any persisted JSON uses{ "expression": "..." }, deserialization will fail or produce invalidRelationMappings. Consider supporting both keys (expressionsand legacyexpression) during read, and align the documented expected format with the actual schema/migration (expressions).
src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelperTest.java:23- This test claims to cover legacy V6.4
expressionfield deserialization, but the JSON uses the currentexpressionsarray. If legacy compatibility is intended, the fixture should useexpressionso the test actually exercises the fallback behavior.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:20 EntityDynamicMappingDryRunDtoInMappercreates its ownObjectMapperinstance. This can diverge from the application’s configured Jackson modules/features (and makes@RequiredArgsConstructormisleading). Prefer constructor-injecting the Spring-managedObjectMapperto keep serialization consistent across the API.
@Component
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
- Defining a new
ObjectMapperbean viareturn new ObjectMapper()can override Spring Boot’s auto-configured mapper and drop important customizations/modules (Java time, Kotlin, naming, etc.). Prefer building the mapper via Spring’sJackson2ObjectMapperBuilder(or remove this bean entirely and inject the default mapper) to preserve the application-wide Jackson configuration.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
Comment on lines
+56
to
+64
| } catch (EntityDynamicMappingJsltErrorException | ||
| | EntityDynamicMappingConfigurationException e) { | ||
| throw e; | ||
| } catch (ExpressionEvaluationFailedException e) { | ||
| throw new EntityDynamicMappingJsltErrorException(e.getMessage()); | ||
| } catch (Exception e) { | ||
| return List.of(DryRunEntityResult.failure(templateIdentifier, ErrorType.JSLT_ERROR, | ||
| "Unexpected transformation error: " + e.getMessage())); | ||
| } |
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 24, 2026 15:29
004bda7 to
b3c6756
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 52 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:21
- The relation JSON format in this helper’s JavaDoc uses a singular
expressionfield, but the persisted format (migration + RelationMapping record) usesexpressions(array). Keeping the doc wrong will mislead future migrations/debugging and makes it look like legacy support exists when it doesn’t.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:21 - This mapper creates its own
ObjectMapperinstance, which bypasses the application’s Jackson configuration (modules, naming strategy, etc.) and violates the project’s constructor-injection pattern. It also makes@RequiredArgsConstructormisleading since there are no injected deps.
@Component
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
JsltConfigurationdeclares a new globalObjectMapperbean. In Spring Boot, that will replace the auto-configured (customized) mapper and can break JSON handling elsewhere (missing modules, custom settings). If the goal is only to share a mapper, prefer injecting the existing Boot-providedObjectMapperand remove this bean definition.
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import com.fasterxml.jackson.databind.ObjectMapper;
/// Spring configuration for JSLT mapping adapter infrastructure.
/// Provides shared JSON serialization for payload parsing and expression evaluation.
@Configuration
public class JsltConfiguration {
/// Provides a shared ObjectMapper bean for mapping adapters.
/// Bean scope is singleton and safe for concurrent reuse.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelperTest.java:27
- This test claims to cover legacy single-field
expressioncompatibility, but the JSON sample still uses the currentexpressionsarray. As written, it won’t detect regressions (or absence) of legacy support.
Comment on lines
+62
to
+73
| @Named("jsonStringToRelationList") | ||
| public List<RelationMapping> toRelationList(String json) { | ||
| if (json == null || json.trim().isEmpty()) { | ||
| return List.of(); | ||
| } | ||
| try { | ||
| return OBJECT_MAPPER.readValue(json, new TypeReference<List<RelationMapping>>() { | ||
| }); | ||
| } catch (JsonProcessingException e) { | ||
| throw new IllegalArgumentException("Invalid JSON relation list configuration", e); | ||
| } | ||
| } |
Comment on lines
+23
to
+26
| -- Normalize NULL relations to empty array (defensive guard for any unexpected NULLs) | ||
| UPDATE entity_dynamic_mapping | ||
| SET relations = '[]'::jsonb | ||
| WHERE relations IS NULL; |
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 27, 2026 09:36
b3c6756 to
e766205
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 54 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/main/resources/db/migration/V6_4__migrate_entity_dynamic_mapping_relations_to_array.sql:26
- The migration only normalizes SQL NULL ("relations IS NULL"), but Postgres JSONB can also contain a JSON null literal (relations = 'null'::jsonb). Those rows will not be converted to an empty array and will remain inconsistent with the new array contract.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18 - Defining an
ObjectMapperbean here will replace Spring Boot’s auto-configuredObjectMapperfor the entire application (Boot backs off when anObjectMapperbean exists). That can inadvertently drop important Jackson modules/configuration used by the REST API (e.g., Java Time handling, naming strategies, security-related settings). The mapping adapter can just inject the existing application-wideObjectMapper.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltExpressionEvaluator.java:6
java.util.stream.Streamis imported but not used. If the build runs with unused-import checks (Spotless/Checkstyle), this will fail CI.
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 55 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (5)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:20
- This Spring
@Componentcreates its own ObjectMapper instance, which bypasses the application’s Jackson configuration (modules, naming strategies, etc.) and makes testing harder (final field). Prefer constructor injection of the Spring-managed ObjectMapper bean.
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:22
- The class-level documentation describes relations JSON using a singular 'expression' field, but the actual persisted format and domain model use an 'expressions' array (see RelationMapping.expressions and migration V6_4). This mismatch can mislead future maintenance and debugging.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:60 - The method documentation for relation JSONB deserialization still references the old singular 'expression' field, but the expected format is now 'expressions' (array). Keeping this accurate is important because this helper is the canonical reference for the persistence JSON contract.
src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelperTest.java:20 - This test’s display name says it validates legacy singular 'expression' field support, but the JSON fixture uses the current 'expressions' array field. As written, it doesn’t cover any legacy compatibility branch and is redundant with the next test.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:79 - The relation JSONB serialization JavaDoc still documents an output with a singular 'expression' field, but persistence now writes RelationMapping (name + expressions list) and the migration/comment describe an 'expressions' array. The doc should reflect the actual persisted JSON shape.
Comment on lines
+34
to
+37
| @SuppressWarnings("unused") | ||
| @AssertTrue(message = PAYLOAD_DRY_RUN_MANDATORY) | ||
| public boolean isPayloadNotBlankWhenString() { | ||
| return !(payload instanceof String payloadString) || !payloadString.isBlank(); |
Comment on lines
+122
to
+126
| @ApiResponse(responseCode = BAD_REQUEST_CODE, description = RESPONSE_ENTITY_DYNAMIC_MAPPING_DATA, content = { | ||
| @Content(schema = @Schema(implementation = ApiExceptionHandler.ErrorResponse.class))}) | ||
| @ApiResponse(responseCode = NOT_FOUND_CODE, description = RESPONSE_ENTITY_DYNAMIC_MAPPING_NOT_FOUND_IDENTIFIER, content = { | ||
| @Content(schema = @Schema(implementation = ApiExceptionHandler.ErrorResponse.class))}) | ||
| @PostMapping("/dry-run") |
Collaborator
Author
There was a problem hiding this comment.
Already fixed
422 OK
404 = "Entity template not found"
Comment on lines
+602
to
+606
| "400": | ||
| description: Invalid entity dynamic mapping data provided | ||
| content: | ||
| "*/*": | ||
| schema: |
Collaborator
Author
There was a problem hiding this comment.
Already fixed
422 OK
404 = "Entity template not found"
Comment on lines
+1510
to
+1514
| payload: | ||
| description: Sample JSON payload to test against the webhook mapping configuration | ||
| payload_not_blank_when_string: | ||
| type: boolean | ||
| required: |
foukou19
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 55 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/EntityDynamicMappingDryRunDtoIn.java:37
@AssertTruemethod is being surfaced as a model property in the generated OpenAPI (payload_not_blank_when_stringshows up in swagger.yaml). This leaks an internal validation guard into the public contract and can confuse clients.
@SuppressWarnings("unused")
@AssertTrue(message = PAYLOAD_DRY_RUN_MANDATORY)
public boolean isPayloadNotBlankWhenString() {
return !(payload instanceof String payloadString) || !payloadString.isBlank();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:22
EntityDynamicMappingDryRunDtoInMapperinstantiates its ownObjectMapper(new ObjectMapper()), which bypasses the application-wide Jackson configuration/modules and also defeats constructor injection (Lombok won’t include initialized finals). This can lead to inconsistent serialization between dry-run and the rest of the API.
@Component
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
public String toRawPayload(Object payload) {
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
- Defining a custom
ObjectMapperbean vianew ObjectMapper()will override Spring Boot’s auto-configured mapper (modules, naming strategies, date/time handling, etc.). Prefer building from the BootJackson2ObjectMapperBuilderso the mapper used by the JSLT adapter stays consistent with the rest of the application.
/// Provides a shared ObjectMapper bean for mapping adapters.
/// Bean scope is singleton and safe for concurrent reuse.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
src/main/java/com/decathlon/idp_core/domain/service/entity_dynamic_mapping/EntityDynamicMappingDryRunService.java:64
ExpressionEvaluationFailedExceptionis being wrapped intoEntityDynamicMappingJsltErrorException, which prevents the dedicated@ExceptionHandler(ExpressionEvaluationFailedException)from ever running for the dry-run endpoint and drops the structured context (expression,reason) that the handler logs. Let it propagate so the API returns a consistent 422 with the correct handler/logging.
try {
return mapAndValidateEntity(mapping, rawPayload, templateIdentifier);
} catch (EntityDynamicMappingJsltErrorException
| EntityDynamicMappingConfigurationException e) {
throw e;
} catch (ExpressionEvaluationFailedException e) {
throw new EntityDynamicMappingJsltErrorException(e.getMessage());
} catch (Exception e) {
return List.of(DryRunEntityResult.failure(templateIdentifier, ErrorType.JSLT_ERROR,
"Unexpected transformation error: " + e.getMessage()));
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:21
- The class-level documentation says relations are stored with an
expressionfield, but the actual persisted format isexpressions(array) (seeRelationMapping#expressionsand the Flyway V6_4 migration). Keeping this doc accurate is important for future maintenance/debugging of JSONB contents.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:60 - The relation JSON format examples still reference
expression(singular), but the code serializes/deserializesRelationMappingwhich usesexpressions(list). These examples should match the real JSONB payload to avoid misleading future migrations/debugging.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:78 - The output JSON format example says
expression(singular), butRelationMappingis serialized with theexpressionsarray field. Align the doc with the actual JSON produced byObjectMapper.writeValueAsString(relations).
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 27, 2026 13:41
1570b41 to
8f55645
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 55 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltMappingEngineAdapter.java:181
requireStringValueonly checks for null/missing nodes, but allows blank values (e.g., when the expression evaluates to an object,asText()can become empty). This can produce entities with empty identifier/name and bypass dry-run validation sincevalidateForDryRundoesn’t checkEntity’s@NotBlankconstraints. Treat blank results as a configuration error.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:24EntityDynamicMappingDryRunDtoInMapperalways constructs its ownObjectMapper, ignoring the application’s configured JacksonObjectMapper(modules, naming, etc.) and forcing tests to use reflection to replace afinalfield. Prefer constructor injection of the Spring-managedObjectMapper.
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
public String toRawPayload(Object payload) {
if (payload instanceof String payloadString) {
return payloadString;
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
JsltConfigurationdefines a brand newObjectMapperbean (new ObjectMapper()), which will override Spring Boot’s auto-configured mapper and may drop globally registered modules (e.g., JavaTimeModule) used by the rest of the API. The mapping adapters should instead reuse the existing Boot-managedObjectMapper.
/// Spring configuration for JSLT mapping adapter infrastructure.
/// Provides shared JSON serialization for payload parsing and expression evaluation.
@Configuration
public class JsltConfiguration {
/// Provides a shared ObjectMapper bean for mapping adapters.
/// Bean scope is singleton and safe for concurrent reuse.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 55 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/entity_mapping/jslt/JsltConfiguration.java:18
- Defining a custom
@Bean ObjectMapperhere will replace Spring Boot’s auto-configuredObjectMapper(modules, naming, serialization settings), which can cause broad JSON (de)serialization regressions across the API. Prefer relying on the Boot-providedObjectMappervia constructor injection (or, if you need isolation, inject it and useobjectMapper.copy()locally) and remove this configuration class.
/// Provides a shared ObjectMapper bean for mapping adapters.
/// Bean scope is singleton and safe for concurrent reuse.
@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity_dynamic_mapping/EntityDynamicMappingDryRunDtoInMapper.java:20
EntityDynamicMappingDryRunDtoInMapperinstantiates its ownObjectMapperand the field initializer prevents Spring from injecting the shared mapper. This also makes the unit tests rely on reflection to swap afinalfield. Prefer constructor injection ofObjectMapper(Spring Boot already provides one).
@RequiredArgsConstructor
public class EntityDynamicMappingDryRunDtoInMapper {
private final ObjectMapper objectMapper = new ObjectMapper();
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/in/EntityDynamicMappingDryRunDtoIn.java:36
- The
@AssertTruemethod introduces a syntheticpayload_not_blank_when_stringfield in the generated OpenAPI schema (and the committedswagger.yaml). This is misleading for clients because it looks like a required request property. Consider moving this to a class-level custom constraint, or at least hide the method from OpenAPI (@Schema(hidden = true)) and Jackson (@JsonIgnore), then regenerate/updatedocs/src/static/swagger.yaml.
/// Keeps syntactic validation equivalent to previous String-based contract.
///
/// Null is handled by `@NotNull`; this guard prevents blank String payloads.
@SuppressWarnings("unused")
@AssertTrue(message = PAYLOAD_DRY_RUN_MANDATORY)
public boolean isPayloadNotBlankWhenString() {
return !(payload instanceof String payloadString) || !payloadString.isBlank();
}
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:22
- The Javadoc examples for relations still use a singular
expressionfield, but the new persisted shape isexpressions: [ ... ](see migrationV6_4__...andRelationMapping.expressions). This makes the helper documentation misleading.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:60 - The documented expected relations JSON format uses
expression, butRelationMappingactually expectsexpressions(list). Update this comment to match the persisted schema.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelper.java:77 - The documented output relations JSON format uses
expression, but serialization producesexpressions(list) viaRelationMapping. Update the comment so it matches the actual JSONB written to Postgres.
src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/common/EntityDynamicMappingJsonbHelperTest.java:28 - This test’s display name claims it verifies legacy
expressionfield support, but the JSON usesexpressionsand therefore duplicates the next test. Either rename the test to match what it actually asserts, or add a real legacy fixture and implement compatibility handling.
| @DisplayName("EntityDynamicMappingDryRunDtoInMapper Unit Tests") | ||
| class EntityDynamicMappingDryRunDtoInMapperTest { | ||
|
|
||
| private final EntityDynamicMappingDryRunDtoInMapper mapper = new EntityDynamicMappingDryRunDtoInMapper(); |
foukou19
marked this pull request as ready for review
July 27, 2026 14:17
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 27, 2026 14:20
8f55645 to
46e3e4a
Compare
Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
foukou19
force-pushed
the
feat/idp-core-entity-dynamic-mapping-dry-run
branch
from
July 27, 2026 14:40
46e3e4a to
7eea5b7
Compare
…orkflow Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
…workflow Signed-off-by: foukou19 <ferial.oukoukas@decathlon.com>
|
This file contains hidden or 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
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.
PR Description
feat(entity-dynamic-mapping): implement dry-run endpoint and add relations support to entity dynamic mappings
What this PR Provides
New Feature: Dry-Run Endpoint
/api/v1/entity_dynamic_mappings/dry-runPOST endpoint: Allows users to validate JSLT mapping definitions against sample payloads without persisting dataEntityDynamicMappingDryRunDtoIn- Request input with mapping definition and sample payloadEntityDynamicMappingDryRunDtoOut- Response withDryRunEntityDtocontaining properties AND relationsDryRunRelationDto- Serialization format for extracted relationsRelations Support for Entity Dynamic Mappings
EntityMappingDtoInnow includesrelations: List<RelationMappingDtoIn>(array format)RelationMappingDtoIncontainsnameandtarget_entity_identifiers: List<String>RelationMappingstores name and list of JSLT expressionsexpressionsarray format (not singularexpression)Enhanced Test Coverage
RelationMappingDtoOutserializationapi-linkrelation to microservice templateReview
The reviewer must double-check these points:
{"name": "expr"}) and array ([{"name":"...","target_entity_identifiers":[...]}]) formatsHow to test
Initial State
1.Microservice and APIM API templates exist with relation definitions
What and How to Test
Test 1: Simple Dry-Run (Properties Only)
/api/v1/entity_dynamic_mappings/dry-run:{ "mapping": { "identifier": "github-dry-run", "entity_template_identifier": "microservice", "filter": ".action == \"pushed\"", "name": "GitHub mapping", "entity": { "identifier": ".repository.full_name", "name": ".repository.name", "properties": { "applicationName": ".repository.name", "ownerEmail": ".sender.email", "environment": "\"DEV\"", "version": ".ref", "port": "8080", "programmingLanguage": ".repository.language" }, "relations": [] } }, "payload": { "action": "pushed", "repository": { "full_name": "org/repo", "name": "repo", "language": "Java" }, "ref": "1.0.0", "sender": {"email": "user@example.com"} } }Test 2: Dry-Run with Relations (APIM API)
/api/v1/entity_dynamic_mappings/dry-runwith APIM API mapping containing relations:{ "mapping": { "entity_template_identifier": "apim-api", "entity": { "relations": [ { "name": "apim-api-consumed_by-component", "target_entity_identifiers": [".relations.\"apim-api-consumed_by-component\""] } ] } }, "payload": { "relations": { "apim-api-consumed_by-component": [ {"identifier": "e5d9813e-b16f-4f24-bdf9-376f18f3a9ac", "name": "sportyswap-be"}, {"identifier": "f2e2ab44-5d19-44de-a77a-42ef6aa51676", "name": "user-profile-sync"} ] } } }entity.relations[0].name: "apim-api-consumed_by-component"entity.relations[0].target_entity_identifiers: array with 2 component UUIDsTest 3: Dry-Run Error - Missing Required Properties
Test 4: Create Mapping with Relations
/api/v1/entity_dynamic_mappingswith mapping containing relationsentity.relationsin returned objectTest 5: Get Mapping - Relations Persistence
/api/v1/entity_dynamic_mappings/microservice-mappingentity.relations[]with correct formatTest 6: Relations Format Compatibility
Expected Results
target_entity_identifiersas arrayBreaking Changes
N/A
No breaking changes introduced:
relationscolumn already exists; format now consistent across all operationsContext & Rationale
Why Dry-Run?
Previously, users had to save a mapping and manually verify it worked. The dry-run endpoint provides: