feat(api)!: update entity output for having relations grouped#90
feat(api)!: update entity output for having relations grouped#90brandPittCode wants to merge 34 commits into
Conversation
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall coverage in commit f88fa09 in the Show a code coverage summary of the most impacted files.
Updated |
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
left a comment
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
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": {
| // 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
… of https://github.com/Decathlon/internal-developer-platform into feat/update-entity-output-for-having-relations-grouped
| IndexBundle globalIndices = buildIndices(entityGraphs, mode); | ||
| Map<UUID, EntityGraphNode> result = new HashMap<>(); | ||
|
|
||
| for (Map.Entry<UUID, Entity> entry : entityGraphs.entrySet()) { | ||
| Entity entity = entry.getValue(); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrés Brand <andresbrand@gmail.com>
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;
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; |
| app: | ||
| security: | ||
| mock-enabled: true | ||
| # security: | ||
| # mock-enabled: true | ||
| full-refresh-at-startup: true |
| "relations": { | ||
| "depends-on": [ | ||
| { | ||
| "identifier": "web-api-1", | ||
| "name": "Web API 1" | ||
| } | ||
| "name": "Web API 1", |
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"
]
}
| 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()))); |
|
|
||
| var state = new TraversalState(root.identifier(), new HashMap<>(), new HashMap<>(), | ||
| new HashMap<>(), new HashSet<>()); | ||
|
|
| // 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); | ||
|
|
| /// 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 |
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
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
|
|
||
| EntityGraphNode entityGraphNode = entityGraphService.getEntityGraph(templateIdentifier, | ||
| entityIdentifier, relationsDepth, true, | ||
| relationsToDisplay == null ? Set.of() : relationsToDisplay, Set.of(), | ||
| EntityGraphTraversalMode.DIRECT_LINEAGE); | ||
| return entityDtoOutFromEntityNodeMapper.toDto(entityGraphNode, templateIdentifier, | ||
| relationsDepth); |
| schema: | ||
| type: array | ||
| example: | ||
| - depends-on | ||
| - relates-to |
| ### Relations in Responses | ||
|
|
||
| When multiple related entities are allowed, list several identifiers: | ||
| In API responses, relations are grouped by name and include summary information about each target entity: |
| private Set<ReachableState> initializeAnchorStates(UUID rootId, EntityGraphTraversalMode mode, | ||
| Set<ReachableState> visited) { | ||
| Set<ReachableState> currentLevel = new HashSet<>(); | ||
|
|
||
| if (mode == EntityGraphTraversalMode.OUTBOUND_ONLY | ||
| || mode == EntityGraphTraversalMode.DIRECT_LINEAGE) { | ||
| ReachableState anchor = new ReachableState(rootId, FlowDirection.OUTBOUND); | ||
| visited.add(anchor); | ||
| currentLevel.add(anchor); | ||
| } | ||
| if (mode == EntityGraphTraversalMode.DIRECT_LINEAGE) { | ||
| ReachableState anchor = new ReachableState(rootId, FlowDirection.INBOUND); | ||
| if (visited.add(anchor)) { | ||
| currentLevel.add(anchor); | ||
| } | ||
| } | ||
| if (mode == EntityGraphTraversalMode.BIDIRECTIONAL) { | ||
| ReachableState anchor = new ReachableState(rootId, FlowDirection.ANY); | ||
| visited.add(anchor); | ||
| currentLevel.add(anchor); | ||
| } | ||
|
|
||
| return currentLevel; | ||
| } |
There was a problem hiding this comment.
`private Set initializeAnchorStates(UUID rootId, EntityGraphTraversalMode mode,
Set visited) {
Set currentLevel = new HashSet<>();
switch (mode) {
case OUTBOUND_ONLY -> addAnchorState(rootId, FlowDirection.OUTBOUND, visited, currentLevel);
case DIRECT_LINEAGE -> {
addAnchorState(rootId, FlowDirection.OUTBOUND, visited, currentLevel);
addAnchorState(rootId, FlowDirection.INBOUND, visited, currentLevel);
}
case BIDIRECTIONAL -> addAnchorState(rootId, FlowDirection.ANY, visited, currentLevel);
}
return currentLevel;
}
private void addAnchorState(UUID rootId, FlowDirection direction,
Set visited, Set currentLevel) {
ReachableState anchor = new ReachableState(rootId, direction);
if (visited.add(anchor)) {
currentLevel.add(anchor);
}
}`
| private Set<ReachableState> expandCurrentLevel(Set<ReachableState> currentLevel, | ||
| Map<UUID, Entity> entityMap, IndexBundle globalIndices, Set<ReachableState> visited) { | ||
| Set<ReachableState> nextLevel = new HashSet<>(); | ||
|
|
||
| for (ReachableState state : currentLevel) { | ||
| Entity entity = entityMap.get(state.id()); | ||
| if (entity == null) | ||
| continue; | ||
|
|
||
| propagateOutboundPaths(state, entity, entityMap, globalIndices, visited, nextLevel); | ||
| propagateInboundPaths(state, entity, entityMap, globalIndices, visited, nextLevel); | ||
| } | ||
|
|
||
| return nextLevel; | ||
| } |
There was a problem hiding this comment.
proposal :
currentLevel.forEach(state -> {
Entity entity = entityMap.get(state.id());
if (entity == null)
return;
propagateOutboundPaths(state, entity, entityMap, globalIndices, visited, nextLevel);
propagateInboundPaths(state, entity, entityMap, globalIndices, visited, nextLevel);
});
just for beautify ^^
|
|
||
| for (Relation rel : entity.relations()) { | ||
| for (String targetId : rel.targetEntityIdentifiers()) { | ||
| UUID targetUuid = globalIndices.textToUuidLookup() | ||
| .get(new EntityCompositeKey(rel.targetTemplateIdentifier(), targetId)); | ||
| if (targetUuid != null && entityMap.containsKey(targetUuid)) { | ||
| ReachableState nextState = new ReachableState(targetUuid, FlowDirection.OUTBOUND); | ||
| if (visited.add(nextState)) { | ||
| nextLevel.add(nextState); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
globalIndices.textToUuidLookup() was extracted outside the loops so getter calls aren't made repeatedly.
rel.targetTemplateIdentifier() was extracted outside the inner loop since it remains identical for every targetId in that relation.
if (state.flow() != FlowDirection.OUTBOUND && state.flow() != FlowDirection.ANY) {
return;
}
Map<EntityCompositeKey, UUID> lookupMap = globalIndices.textToUuidLookup();
for (Relation rel : entity.relations()) {
String templateId = rel.targetTemplateIdentifier();
for (String targetId : rel.targetEntityIdentifiers()) {
UUID targetUuid = lookupMap.get(new EntityCompositeKey(templateId, targetId));
if (targetUuid != null && entityMap.containsKey(targetUuid)) {
ReachableState nextState = new ReachableState(targetUuid, FlowDirection.OUTBOUND);
if (visited.add(nextState)) {
nextLevel.add(nextState);
}
}
}
}
}
should be better but need to validate loop
| for (List<UUID> sources : sourcesByRelation.values()) { | ||
| for (UUID sourceUuid : sources) { | ||
| if (entityMap.containsKey(sourceUuid)) { | ||
| ReachableState nextState = new ReachableState(sourceUuid, FlowDirection.INBOUND); | ||
| if (visited.add(nextState)) { | ||
| nextLevel.add(nextState); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
i love for loop, no ?
sourcesByRelation.values()
.stream()
.flatMap(Collection::stream)
.filter(entityMap::containsKey)
.map(sourceUuid -> new ReachableState(sourceUuid, FlowDirection.INBOUND))
.filter(visited::add)
.forEach(nextLevel::add);
| EntityGraphTraversalMode mode) { | ||
| } | ||
|
|
||
| private static record EntityCompositeKey(String templateIdentifier, String identifier) { |
There was a problem hiding this comment.
i make often the same little mistake in java 25 , static declaration is by default on inner record
- Java Language Specification Rules
According to the Java Language Specification (JLS §8.10):
Nested records are implicitly static.
The Java compiler automatically treats any nested record declaration as static, whether or not you explicitly write the static keyword.
- Records Cannot Hold an Enclosing Reference
Regular inner classes in Java retain an implicit reference to an instance of their outer class (EntityGraphHelper.this).
Records are intended purely as immutable data carriers. To prevent subtle memory leaks and enforce immutability, Java explicitly disallows records from holding an implicit reference to an enclosing class instance. As a result, nested records must be static, so Java makes them static automatically.
|



PR Description
What this PR Provides
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.
Entity 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.
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 removedTest 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