feat(core): fix duplicate issue with auditing#95
Open
RVANDO12 wants to merge 5 commits into
Open
Conversation
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall coverage in the branch is 91%. The coverage in the branch is 90%. Show a code coverage summary of the most impacted files.
Updated |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| EntityJpaEntity jpa = mapper.toJpa(domain); | ||
|
|
||
| assertEquals(2, jpa.getProperties().size()); | ||
| assertTrue(jpa.getProperties() instanceof Set); |
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to address duplicate-key failures in Envers auditing during batch updates of entity relations by reducing unnecessary relation updates during merge operations, and by extending/strengthening test coverage around entity update payloads and persistence mapping.
Changes:
- Added relation change detection in
EntityPersistenceMapper.mergeRelations(...)to avoid updating unchanged relations (reducing unnecessary Envers audit writes). - Refactored
EntityControllerTestrequest payloads to use JSON files undersrc/test/resources/...and added new update-with-relations scenarios. - Added a comprehensive
EntityPersistenceMapperTestsuite covering domain↔JPA mapping and merge behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/EntityPersistenceMapper.java | Adds relation change detection during merge to avoid unnecessary updates/audit writes. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityControllerTest.java | Switches multiple payloads to JSON fixtures; adds PUT relation-merge test scenarios. |
| src/test/java/com/decathlon/idp_core/infrastructure/adapters/persistence/mapper/EntityPersistenceMapperTest.java | Adds extensive unit tests for persistence mapping and merge logic (including relation change detection). |
| src/test/resources/integration_test/json/entity/v1/putEntity_400_required_properties_missing.json | New JSON fixture for PUT validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/putEntity_400_property_type_mismatch.json | New JSON fixture for PUT validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/putEntity_400_property_rules_not_respected.json | New JSON fixture for PUT validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/putEntity_400_property_not_defined_and_relation_target_missing.json | New JSON fixture for PUT combined validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/putEntity_200_valid_update.json | New JSON fixture for a valid PUT update. |
| src/test/resources/integration_test/json/entity/v1/putEntity_200_update_with_add_relation.json | New JSON fixture for PUT update adding a relation. |
| src/test/resources/integration_test/json/entity/v1/putEntity_200_update_remove_all_relations.json | New JSON fixture for PUT update removing all relations. |
| src/test/resources/integration_test/json/entity/v1/postEntity_400_required_properties_missing.json | New JSON fixture for POST validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/postEntity_400_relation_target_entity_does_not_exist.json | New JSON fixture for POST validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/postEntity_400_property_type_mismatch.json | New JSON fixture for POST validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/postEntity_400_property_rules_not_respected.json | New JSON fixture for POST validation error scenario. |
| src/test/resources/integration_test/json/entity/v1/postEntity_400_property_not_defined_in_template.json | New JSON fixture for POST validation error scenario. |
Comment on lines
+173
to
+177
| if (existingRel.getTargetEntities().size() != newTargetEntities.size()) { | ||
| return true; | ||
| } | ||
|
|
||
| return !existingRel.getTargetEntities().equals(newTargetEntities); |
Comment on lines
+148
to
+152
| if (hasRelationChanged(existingRel, domainRel)) { | ||
| existingRel.setTargetTemplateIdentifier(domainRel.targetTemplateIdentifier()); | ||
| existingRel.setTargetEntities(domainRel.targetEntityIdentifiers() != null | ||
| ? resolveBatchTargetEntities(domainRel.targetTemplateIdentifier(), | ||
| domainRel.targetEntityIdentifiers()) |
Comment on lines
+137
to
+139
| /// **Optimization:** Change detection prevents false positives where Hibernate | ||
| /// marks collections as modified even though their values haven't changed, | ||
| /// which would cause Envers to create duplicate audit table entries. |
Comment on lines
+593
to
+596
| EntityJpaEntity existing = EntityJpaEntity.builder().id(existingId) | ||
| .templateIdentifier("template").name("name").id(existingId).templateIdentifier("template") | ||
| .name("name").identifier("id").properties(new HashSet<>()) | ||
| .relations(new HashSet<>(Set.of(rel1))).build(); |
Comment on lines
+707
to
+708
| @DisplayName("Should preserve relation IDs during merge when relation unchanged") | ||
| void putEntity_200_preserves_relation_ids_on_merge() throws Exception { |
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
Issue when batch update of relation between entity.
" Caused by: java.sql.BatchUpdateException: Batch entry 1 insert into idp_core.relation_target_entities_aud "
" ERROR: duplicate key value violates unique constraint "relation_target_entities_aud_pkey"
Fixes
Database: Expand the relation_target_entities_aud_pkey to include target_entity_uuid.
Code: Strip targetEntityUuid out of the equals() and hashCode() methods in RelationTargetJpaEntity.java so that the Java Set correctly prevents duplicates before they ever reach Hibernate Envers.
Review
The reviewer must double-check these points:
!after the type/scope to identify the breakingchange in the release note and ensure we will release a major version.
How to test
Update a huge information about entity to cause the issue and check is not appears