feat(api)!: update entity output for having relations grouped#90
Open
brandPittCode wants to merge 37 commits into
Open
feat(api)!: update entity output for having relations grouped#90brandPittCode wants to merge 37 commits into
brandPittCode wants to merge 37 commits into
Conversation
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall coverage in commit 492a130 in the Show a code coverage summary of the most impacted files.
Updated |
brandPittCode
changed the title
brandPittCode
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Entities API response contract to provide a unified relations structure that combines outbound and inbound relations, and enriches related entity summaries with template_identifier so clients can identify related entity types without additional lookups.
Changes:
- Reworked entity output DTOs (
EntityDtoOut,EntitySummaryDto) to support unified relations and includetemplate_identifierin relation summaries. - Updated relation-summary persistence query and mapping to provide inbound relation source template identifiers.
- Updated OpenAPI (
swagger.yaml) and integration-test JSON fixtures to reflect the new response schema (including removal ofrelations_as_targetfrom responses).
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutMapper.java | Builds unified relations map and injects template identifiers into relation summaries |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/out/entity/EntityDtoOut.java | Refactors entity response DTO to a unified relations-only shape |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/dto/out/entity/EntitySummaryDto.java | Adds templateIdentifier to related entity summaries and converts to record |
| src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/repository/JpaRelationRepository.java | Extends native query projection to include source template identifier for inbound relation summaries |
| src/main/java/com/decathlon/idp_core/domain/model/entity/RelationAsTargetSummary.java | Extends domain summary model to carry sourceTemplateIdentifier |
| docs/src/static/swagger.yaml | Updates OpenAPI schemas to remove relations_as_target from responses and add template_identifier to entity summaries |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_startsWith.json | Updates expected entity search response fixture for unified relations + template identifiers |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_orTemplates.json | Updates expected entity search response fixture for unified relations + template identifiers |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_neq.json | Updates expected entity search response fixture for unified relations |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_byTemplateAndProperty.json | Updates expected entity search response fixture for unified relations + template identifiers |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_byRelationsAsTargetPresence.json | Updates expected entity search response fixture reflecting inbound relations now in relations |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_byRelationsAsTarget.json | Updates expected entity search response fixture reflecting inbound relations now in relations |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_byRelationNameEq.json | Updates expected entity search response fixture for unified relations + template identifiers |
| src/test/resources/integration_test/json/entity/v1/searchEntities_200_byRelationNameContains.json | Updates expected entity search response fixture for unified relations + template identifiers |
| src/test/resources/integration_test/json/entity/v1/getEntities_200_relationsAsTargetIdentifier.json | Updates expected get-entities response fixture reflecting inbound relations now in relations |
| src/test/resources/integration_test/json/entity/v1/getEntities_200_identifierEquals.json | Updates expected get-entities response fixture for unified relations + template identifiers |
evebrnd
requested changes
Jul 8, 2026
evebrnd
left a comment
evebrnd
left a comment
Collaborator
There was a problem hiding this comment.
There are many mentions of relations_as_target in tests and the documentation that should be cleaned / updated.
PR title is missing a '!' to declare the breaking change
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
brandPittCode
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 28 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
docs/src/concepts/entities.md:247
- The response example under "Relations in Responses" is now missing
template_identifierin relation summaries, which the PR introduces as part of the unified relations API. This makes the docs inconsistent with the updated contract.
In API responses, relations are grouped by name and include summary information about each target entity:
```json
{
"relations": {
Comment on lines
+166
to
+172
| // Load entity graphs in batch | ||
| Map<UUID, Entity> entityGraphs = entityGraphRepositoryPort.findEntityGraph(entityUuids, 1, true, | ||
| EntityGraphTraversalMode.DIRECT_LINEAGE); | ||
|
|
||
| private static record GraphTraversalContext(Map<UUID, Entity> entityMap, | ||
| Map<EntityCompositeKey, UUID> textToUuidLookup, | ||
| Map<String, Map<String, List<UUID>>> inboundIndex, boolean includeProperties, | ||
| Set<String> propertyFilter, Set<String> relationFilter, Set<String> activeStack, | ||
| Map<String, EntityGraphNode> memoCache, // High-speed in-memory reuse cache | ||
| EntityGraphTraversalMode mode) { | ||
| } | ||
| // Call the helper to build graph nodes | ||
| Map<UUID, EntityGraphNode> graphsByUuid = entityGraphHelper.buildGraphNodesForEntityIds( | ||
| entityGraphs, true, Set.of(), Set.of(), EntityGraphTraversalMode.DIRECT_LINEAGE, 1); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrés Brand <andresbrand@gmail.com>
Unify graph retrieving methods
Unify graph retrieving methods
Unify graph retrieving methods
Unify graph retrieving methods
Unify graph retrieving methods Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrés Brand <andresbrand@gmail.com>
Unify graph retrieving methods Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrés Brand <andresbrand@gmail.com>
Unify graph retrieving methods
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 34 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphService.java:15
- Unused import
EntityTemplateNotFoundExceptionwill fail compilation (Javadoc references don’t count as usage). Remove it, or reference it in code if you intend to throw it explicitly from this class.
import com.decathlon.idp_core.domain.exception.entity_template.EntityTemplateNotFoundException;
Comment on lines
+9
to
+10
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; |
Collaborator
Author
There was a problem hiding this comment.
We will migrate this for all service that use this Spring Data (Page/Pageable)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 34 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:60
buildGraphNodesForEntityIdscomputes a reachable footprint + localized indices for every entry inentityGraphs(which includes neighbors, not just requested roots). For the paginated list path, this can multiply work significantly (O(N) BFS/DFS builds) even though only the page roots are actually returned.
IndexBundle globalIndices = buildIndices(entityGraphs, mode);
Map<UUID, EntityGraphNode> result = new HashMap<>();
for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) {
Entity entity = entry.getValue();
if (entity != null) {
Set<UUID> reachableFootprint = computeReachableSubGraph(entry.getKey(), entityGraphs,
globalIndices, depth, mode);
src/main/resources/application-local.yml:39
- This PR comments out
app.security.mock-enabledin the local profile, which changes local runtime behavior (likely re-enabling real OAuth2). This operational/config change isn’t described in the PR description and may break existing local workflows.
app:
# security:
# mock-enabled: true
full-refresh-at-startup: true
docs/src/concepts/entities.md:57
- The JSON example is malformed (missing closing braces/brackets for the relation item and array), which will confuse readers and breaks copy/paste.
"relations": {
"depends-on": [
{
"identifier": "web-api-1",
"name": "Web API 1",
"template_identifier": "web-service"
]
}
Comment on lines
+264
to
+269
| private Map<String, EntitySummaryDto> buildEntitiesSummariesMap(List<String> targetIdentifiers) { | ||
| return targetIdentifiers.isEmpty() | ||
| ? Collections.emptyMap() | ||
| : entityService.getEntitiesSummariesByIdentifiers(targetIdentifiers).stream() | ||
| .collect(Collectors.toMap(EntitySummary::identifier, | ||
| es -> new EntitySummaryDto(es.identifier(), es.name()))); | ||
| es -> new EntitySummaryDto(es.identifier(), es.name(), es.templateIdentifier()))); |
Comment on lines
+82
to
+85
|
|
||
| var state = new TraversalState(root.identifier(), new HashMap<>(), new HashMap<>(), | ||
| new HashMap<>(), new HashSet<>()); | ||
|
|
Comment on lines
+167
to
175
| // Load entity graphs in batch (includes roots + neighbors for relation | ||
| // resolution) | ||
| Map<UUID, Entity> entityGraphs = entityGraphRepositoryPort.findEntityGraph(entityUuids, depth, | ||
| true, EntityGraphTraversalMode.DIRECT_LINEAGE); | ||
|
|
||
| private static record GraphTraversalContext(Map<UUID, Entity> entityMap, | ||
| Map<EntityCompositeKey, UUID> textToUuidLookup, | ||
| Map<String, Map<String, List<UUID>>> inboundIndex, boolean includeProperties, | ||
| Set<String> propertyFilter, Set<String> relationFilter, Set<String> activeStack, | ||
| Map<String, EntityGraphNode> memoCache, // High-speed in-memory reuse cache | ||
| EntityGraphTraversalMode mode) { | ||
| } | ||
| // Call the helper to build graph nodes | ||
| Map<UUID, EntityGraphNode> graphsByUuid = entityGraphHelper.buildGraphNodesForEntityIds( | ||
| entityGraphs, true, Set.of(), Set.of(), EntityGraphTraversalMode.DIRECT_LINEAGE, depth); | ||
|
|
RVANDO12
reviewed
Jul 23, 2026
| /// callers that already own a transaction can reuse the logic directly without | ||
| /// triggering a new transaction via the Spring proxy. | ||
| /// | ||
| /// @param entityIds UUIDs of the root entities to build graphs for |
Collaborator
There was a problem hiding this comment.
/// @param entityGraphs map of entity UUIDs to their corresponding Entity objects
… of https://github.com/Decathlon/internal-developer-platform into feat/update-entity-output-for-having-relations-grouped
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 34 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:58
buildGraphNodesForEntityIds(...)iterates over every entry inentityGraphsand builds a full graph for each UUID (computeReachableSubGraph(...)+buildGraphNode(...)). ButentityGraphscomes fromfindEntityGraph(...)and includes roots and all discovered neighbors, so for typical calls you only need graphs for the root UUID(s), not for every discovered node. This can turn a single graph build into O(N) graph builds and become very expensive at higher depths / larger graphs. Consider changing the API to accept an explicitrootIdscollection and only build graphs for those roots (neighbors remain available in the indexed map).
for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) {
Entity entity = entry.getValue();
if (entity != null) {
Set<UUID> reachableFootprint = computeReachableSubGraph(entry.getKey(), entityGraphs,
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphService.java:12
- The Domain layer now depends on Spring Data types (
org.springframework.data.domain.Page/Pageable). Per the project’s Domain-layer constraints, Domain code should not depend on Spring Data (keep pagination/transport concerns in Infrastructure/Application). Consider movinggetEntityGraphPageByTemplate(...)out of the Domain service (or exposing a port that uses domain pagination models likePaginatedResult/PaginationCriteria) so the Domain remains framework-agnostic.
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
src/main/resources/application-local.yml:38
application-local.ymlno longer enablesapp.security.mock-enabled: true. BecauseSecurityConfigurationusesmatchIfMissing=truefor the real JWT resource server, commenting this out makes the local profile require a real JWKS/JWT setup, which is a significant local-dev regression unless intentionally changed. If the goal is to keep local development working out-of-the-box, restore the mock security flag (or document the new local requirements).
app:
# security:
# mock-enabled: true
Comment on lines
+326
to
+330
| schema: | ||
| type: array | ||
| example: | ||
| - depends-on | ||
| - relates-to |
RVANDO12
reviewed
Jul 23, 2026
RVANDO12
reviewed
Jul 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 40 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (6)
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:85
TraversalStatestoresrootIdentifieras the root’s identifier only. Since identifiers are not globally unique across templates (this PR introduces composite keys for that), the traversal can incorrectly treat a different-template node with the same identifier as the root and drop it from the output.
var state = new TraversalState(root.identifier(), new HashMap<>(), new HashMap<>(),
new HashMap<>(), new HashSet<>());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:162
- Root-exclusion logic compares only
identifieragainst the stored root value. With composite IDs, this should compare the fulltemplateIdentifier:identifierto avoid excluding unrelated entities that share an identifier across templates.
String targetId = nodeId(target.templateIdentifier(), target.identifier());
String signature = nodeId + "|" + targetId + "|" + relation.name();
boolean isNotRoot = !target.identifier().equalsIgnoreCase(state.rootIdentifier());
if (shouldCollectRelations && isNotRoot && state.emittedEdgeSignatures().add(signature)) {
state.relationsMap().computeIfAbsent(relation.name(), k -> new LinkedHashSet<>())
.add(new EntitySummaryDto(target.identifier(), target.name(),
target.templateIdentifier()));
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:206
- Inbound root-exclusion logic should also compare composite IDs (template + identifier). Otherwise a source entity from another template that happens to share the root identifier will be dropped incorrectly.
for (EntityGraphRelation relation : node.relationsAsTarget()) {
for (EntityGraphNode source : relation.targets()) {
String sourceId = nodeId(source.templateIdentifier(), source.identifier());
String signature = sourceId + "|" + nodeId + "|" + relation.name();
boolean isNotRoot = !source.identifier().equalsIgnoreCase(state.rootIdentifier());
if (shouldCollectRelations && isNotRoot && state.emittedEdgeSignatures().add(signature)) {
state.relationsMap().computeIfAbsent(relation.name(), k -> new LinkedHashSet<>())
.add(new EntitySummaryDto(source.identifier(), source.name(),
source.templateIdentifier()));
}
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:77
buildGraphNodesForEntityIdscurrently iterates over every entry inentityGraphsand builds a full reachable footprint + localized indices for each one. SinceentityGraphscontains the root(s) and all discovered neighbor nodes, this can turn a single-root graph build into O(N²) work and make paginated list endpoints unexpectedly expensive.
IndexBundle globalIndices = buildIndices(entityGraphs, mode);
Map<UUID, EntityGraphNode> result = new HashMap<>();
for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) {
Entity entity = entry.getValue();
if (entity != null) {
Set<UUID> reachableFootprint = computeReachableSubGraph(entry.getKey(), entityGraphs,
globalIndices, depth, mode);
Map<UUID, Entity> localizedEntityMap = entityGraphs.entrySet().stream()
.filter(e -> reachableFootprint.contains(e.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
IndexBundle localizedIndices = buildIndices(localizedEntityMap, mode);
Set<UUID> isolatedStack = new HashSet<>();
Map<UUID, EntityGraphNode> isolatedCache = new HashMap<>();
GraphTraversalContext localizedCtx = new GraphTraversalContext(localizedEntityMap,
localizedIndices.textToUuidLookup(), localizedIndices.inboundIndex(), includeProperties,
propertyFilter, relationFilter, isolatedStack, isolatedCache, mode);
EntityGraphNode node = buildGraphNode(entry.getKey(), localizedCtx);
result.put(entry.getKey(), node);
}
src/main/resources/application-local.yml:38
- Removing
app.security.mock-enabled: truechanges the local profile’s security behavior: becauseSecurityConfigurationusesmatchIfMissing = true, local will now require real JWT authentication unless the property is explicitly set elsewhere. If this is intentional, consider settingapp.security.mock-enabled: falseexplicitly (or documenting the new local setup) to avoid surprises for developers.
app:
full-refresh-at-startup: true
idp-core-prefix-url: http://localhost:8084
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/repository/JpaRelationRepository.java:47
- Inbound relation summaries are fetched by filtering only on
rte.target_entity_identifier. Since entity identity is (template_identifier, identifier) and this PR explicitly adds composite-key handling elsewhere, this query can conflate inbound relations for two different entities that share the same identifier across templates.
@Query(value = """
SELECT
rte.target_entity_identifier AS targetEntityIdentifier,
r.name AS relationName,
e.identifier AS sourceEntityIdentifier,
e.name AS sourceEntityName,
e.template_identifier AS sourceTemplateIdentifier
FROM idp_core.entity e
JOIN idp_core.entity_relations er ON er.entity_id = e.id
JOIN idp_core.relation r ON r.id = er.relation_id
JOIN idp_core.relation_target_entities rte ON rte.relation_id = r.id
WHERE rte.target_entity_identifier IN :targetEntityIdentifiers
""", nativeQuery = true)
List<RelationAsTargetSummary> findRelationsSummariesByTargetEntityIdentifiers(
@Param("targetEntityIdentifiers") List<String> targetEntityIdentifiers);
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 41 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
src/main/resources/application-local.yml:38
app.security.mock-enabledis no longer set for the local profile. BecauseSecurityConfigurationis enabled whenapp.security.mock-enabled=falseor missing (matchIfMissing=true), this changes local runs to require real JWT auth instead of usingMockSecurityConfiguration(which is explicitly documented as the local dev setup). If the intent is to keep mock auth for local dev, re-enable it inapplication-local.yml(or update local run docs accordingly).
app:
full-refresh-at-startup: true
idp-core-prefix-url: http://localhost:8084
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:84
TraversalStatestoresrootIdentifieras only the entity identifier. If different templates can share the same identifier, nodes from other templates with the same identifier will be treated as the root and wrongly excluded from collected relations.
var state = new TraversalState(root.identifier(), new HashMap<>(), new HashMap<>(),
new HashMap<>(), new HashSet<>());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:157
- Root-node exclusion compares only
identifier(target.identifier()vsstate.rootIdentifier()). This can incorrectly exclude non-root nodes that share the same identifier across templates, and it will also stop working correctly ifrootIdentifieris made composite. Compare using the already-built compositetargetIdinstead.
boolean isNotRoot = !target.identifier().equalsIgnoreCase(state.rootIdentifier());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:200
- Root-node exclusion compares only
identifier(source.identifier()vsstate.rootIdentifier()). In multi-template scenarios this can exclude the wrong node, and it will also stop working correctly ifrootIdentifieris made composite. Compare using the already-built compositesourceIdinstead.
boolean isNotRoot = !source.identifier().equalsIgnoreCase(state.rootIdentifier());
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:59
buildGraphNodesForEntityIds(...)iterates over every entry inentityGraphsand recomputes a reachable footprint + localized indices for each one. In current call sites,entityGraphsis the entire discovered subgraph (roots + neighbors), so this makes graph building O(N²) and does much more work than needed (especially for single-root calls). Consider changing the API to accept explicit root IDs and only build graphs for those roots, reusing the global indices where possible.
for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) {
Entity entity = entry.getValue();
if (entity != null) {
Set<UUID> reachableFootprint = computeReachableSubGraph(entry.getKey(), entityGraphs,
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:303
- Inbound lookup is keyed only by
entity.identifier(), which is ambiguous if identifiers are only unique per template. This can attach inbound relations from an entity in a different template that happens to share the same identifier. Use a composite key (template + identifier) consistently for inbound indexing and lookup.
// Process inbound relationships
List<EntityGraphRelation> inboundRelations = buildRelationsAsTargetFromIndex(
entity.identifier(), ctx);
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:407
inboundIndexis built using onlytargetIdas the key. If identifiers are not globally unique, this will mix inbound edges for different templates. Userelation.targetTemplateIdentifier() + ":" + targetId(normalized) as the inbound index key so inbound lookups can be template-aware.
if (targetId == null)
continue;
String normalizedTargetId = targetId.trim().toLowerCase();
inboundIndex.computeIfAbsent(normalizedTargetId, k -> new HashMap<>())
.computeIfAbsent(relation.name(), k -> new ArrayList<>()).add(sourceUuid);
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphService.java:10
- This introduces Spring Data types (
Page/Pageable) into the Domain layer. Per the project’s Domain layer constraints, Domain code should not depend on Spring Data; pagination concerns belong in Infrastructure/Application and the Domain API should use domain-friendly abstractions (e.g., criteria + List/Slice-like domain model) instead.
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
Comment on lines
+229
to
+233
| String normalizedId = entity.identifier() == null | ||
| ? "" | ||
| : entity.identifier().trim().toLowerCase(); | ||
| Map<String, List<UUID>> sourcesByRelation = globalIndices.inboundIndex() | ||
| .getOrDefault(normalizedId, Map.of()); |
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 41 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/EntityDtoOutFromEntityNodeMapper.java:84
TraversalStatetracks the root using onlyroot.identifier(). If identifiers are only unique within a template (as implied by the new composite-key work), this can wrongly treat a different-template entity with the same identifier as the root during traversal/dedup, leading to missing relations in the flattened map.
var state = new TraversalState(root.identifier(), new HashMap<>(), new HashMap<>(),
new HashMap<>(), new HashSet<>());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:158
- Root-exclusion logic compares only
target.identifier()tostate.rootIdentifier(). OncerootIdentifieris made template-aware (needed to avoid cross-template collisions), this check should compare the same composite id (templateIdentifier:identifier) to reliably exclude only the true root node.
String targetId = nodeId(target.templateIdentifier(), target.identifier());
String signature = nodeId + "|" + targetId + "|" + relation.name();
boolean isNotRoot = !target.identifier().equalsIgnoreCase(state.rootIdentifier());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutFromEntityNodeMapper.java:201
- Inbound root-exclusion uses
source.identifier()only. For consistency with template-aware IDs (and to avoid excluding a different-template entity that happens to share the root identifier), compare the compositesourceId(templateIdentifier:identifier) against the stored root id.
String sourceId = nodeId(source.templateIdentifier(), source.identifier());
String signature = sourceId + "|" + nodeId + "|" + relation.name();
boolean isNotRoot = !source.identifier().equalsIgnoreCase(state.rootIdentifier());
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/mapper/entity/EntityDtoOutMapper.java:176
- Inbound relations are looked up/grouped by
entity.identifier()only (relationTargetOwnershipsMap.get(entity.identifier())). If identifiers are not globally unique, this can merge inbound relations across templates and expose unrelated entities in responses. This should be keyed by an unambiguous identifier (preferablytarget_entity_uuid) rather than the bare identifier string.
// Add inbound relations (entity is target) — merge if key exists
List<RelationAsTargetSummary> inboundRelations = relationTargetOwnershipsMap
.get(entity.identifier());
if (inboundRelations != null) {
src/main/java/com/decathlon/idp_core/domain/service/entity_graph/EntityGraphHelper.java:56
buildGraphNodesForEntityIds(...)iterates over every entry inentityGraphs. Callers pass the full discovered graph footprint (roots + neighbors), so this builds a full rooted graph for each discovered node even though only the requested root(s) are needed. This can turn a single graph request into O(N) graph-builds and will not scale for larger graphs/pages.
IndexBundle globalIndices = buildIndices(entityGraphs, mode);
Map<UUID, EntityGraphNode> result = new HashMap<>();
for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) {
Entity entity = entry.getValue();
Comment on lines
34
to
+38
| SELECT | ||
| rte.target_entity_identifier AS targetEntityIdentifier, | ||
| r.name AS relationName, | ||
| e.identifier AS sourceEntityIdentifier, | ||
| e.name AS sourceEntityName | ||
| e.name AS sourceEntityName, |
Comment on lines
+721
to
+725
| "Optional filter query using a simple expression language. See | ||
| more details in the API documentation. Example: `name:idp` for entities | ||
| with names containing 'idp'. | ||
|
|
||
| " |
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
What this PR Provides
This Pull request add the capacity for retrieving the relations in a given depth for the
entities/template_identifier/identifierendpoint.It also update the EntityDtout contract to have all the relations grouped in the relations object and dropping the relations_as_taget section.
This pull request introduces several improvements and refactorings to the entity relationship graph functionality, enhances API documentation, and clarifies the structure of relationship data in the documentation. The main focus is on supporting batch graph traversal, improving inbound relation summaries, and simplifying the graph-building logic by delegating it to a new helper class. There are also updates to the documentation to clarify the use and structure of relations in API requests and responses.
Backend Logic Refactoring:
EntityDtoOutMapper) is refactored to build a unified relations map, merging outbound and inbound relations under the same key and including template identifiers in related entity summaries. Obsolete mapping methods are removedEntity Graph Traversal and Service Refactoring:
EntityGraphServicenow supports batch graph traversal for multiple root entities, delegating recursive graph construction to a newEntityGraphHelperclass, which improves modularity and performance. The service exposes a new paginated API to fetch entity graphs with both inbound and outbound relations resolved in a single transaction.findEntityGraphis changed to accept a collection of root entity UUIDs instead of a single UUID, enabling bulk graph resolution.FlowDirectionto represent graph traversal directions, supporting more flexible and explicit traversal logic.Documentation and API Description Improvements:
Documentation for entity relations is reorganized and clarified: the structure of relations in API requests and responses is more clearly explained, and the meaning of reverse relation filters is made more precise.
The OpenAPI/Swagger description for the "get entity by identifier" endpoint is updated to explain support for configurable relationship graph traversal, including inbound and outbound relations and direct lineage mode.
Inbound Relation Summaries:
RelationAsTargetSummaryrecord now includes thesourceTemplateIdentifierfield, making inbound relation summaries template-aware for more informative and unified API responses.Other Technical Changes:
These changes collectively make the entity relationship graph APIs more powerful, consistent, and easier to use, while improving documentation for both developers and API consumers.
Test and Documentation Updates:
These changes provide a more consistent and informative API for consumers, simplify relation handling, and enhance frontend capabilities.
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
Please refer (copy/paste) the test section from the User Story. This should include
Breaking changes (if any)
API JSON schema modification (existing resource / behavior)
N/A
Context of the Breaking Change
The EntityDtoOut contract has change.
Result of the Breaking Change
The relation_as_target fiel have been removed.
The relations (inbound and outbound) are now grouped in the relations section.
The relation summary have now the template identifer information