feat: add an endpoint returning the elements using a shared element#200
Open
flomillot wants to merge 17 commits into
Open
feat: add an endpoint returning the elements using a shared element#200flomillot wants to merge 17 commits into
flomillot wants to merge 17 commits into
Conversation
Aggregate the references of a shared element into displayable rows, one per reference: the referenced nodes come from study-server, their study metadata and path from directory-server, and user labels are resolved here so that the response holds no uuid. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a consumer-element information API that resolves shared-element references through study, directory, and user-identity services, returns metadata and labels, and omits unreadable studies. New DTOs, remote lookups, aggregation logic, and integration tests are included. ChangesShared element information flow
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExploreController
participant ExploreService
participant StudyService
participant DirectoryService
participant UserIdentityService
Client->>ExploreController: GET consumer-element-infos
ExploreController->>ExploreService: getConsumerElementInfos(elementUuid, userId)
ExploreService->>StudyService: getNodesInfos(nodeUuids)
ExploreService->>DirectoryService: getElementsInfos(studyUuids, userId, false)
ExploreService->>DirectoryService: getElementsPaths(studyUuids, userId)
ExploreService->>UserIdentityService: getUsersIdentitiesMap(subs)
ExploreService-->>ExploreController: List<ConsumerElementInfos>
ExploreController-->>Client: JSON response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Match the field order of the directory-server DTO. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/explore/server/services/ExploreService.java (1)
467-469: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winShort-circuit if there are no studies to fetch.
If the
study-serverreturns no nodes,studyUuidswill be empty. Checking for this and returning early avoids making useless downstream HTTP calls for element information and user identities.⚡ Proposed fix for early exit
Map<UUID, NodeInfos> nodeInfosByUuid = studyService.getNodesInfos(referencedNodeUuids.stream().distinct().toList()) .stream().collect(Collectors.toMap(NodeInfos::nodeUuid, Function.identity())); List<UUID> studyUuids = nodeInfosByUuid.values().stream().map(NodeInfos::studyUuid).distinct().toList(); + + if (studyUuids.isEmpty()) { + return List.of(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java` around lines 467 - 469, Update the flow after `studyUuids` is computed from `nodeInfosByUuid` to return immediately when no study UUIDs are present, before triggering downstream element-information or user-identity HTTP calls. Preserve the existing processing path when `studyUuids` is non-empty.src/main/java/org/gridsuite/explore/server/services/DirectoryService.java (1)
309-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShort-circuit on empty input and safely build query parameters.
Calling this method with an empty list of UUIDs results in an unnecessary network request (e.g.,
?ids=). Additionally, manual string concatenation for query parameters is fragile; usingUriComponentsBuilderto construct the entire URL is a more robust approach.♻️ Proposed refactor
private List<ElementAttributes> getElementsInfos(List<UUID> elementsUuids, List<String> elementTypes, String userId, boolean strictMode) { + if (elementsUuids.isEmpty()) { + return List.of(); + } var ids = elementsUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); - String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH).toUriString() + "?ids=" + ids; + UriComponentsBuilder builder = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH) + .queryParam("ids", ids); if (!strictMode) { - path += "&strictMode=false"; + builder.queryParam("strictMode", false); } if (!CollectionUtils.isEmpty(elementTypes)) { - path += "&elementTypes=" + elementTypes.stream().collect(Collectors.joining(",")); + builder.queryParam("elementTypes", String.join(",", elementTypes)); } - List<ElementAttributes> elementAttributesList; HttpHeaders headers = new HttpHeaders(); headers.add(HEADER_USER_ID, userId); - elementAttributesList = restTemplate.exchange(directoryServerBaseUri + path, HttpMethod.GET, new HttpEntity<>(headers), + List<ElementAttributes> elementAttributesList = restTemplate.exchange( + directoryServerBaseUri + builder.toUriString(), HttpMethod.GET, new HttpEntity<>(headers), new ParameterizedTypeReference<List<ElementAttributes>>() { }).getBody(); return Objects.requireNonNullElse(elementAttributesList, Collections.emptyList()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java` around lines 309 - 328, Update getElementsInfos to immediately return an empty list when elementsUuids is empty, avoiding the network request. Replace manual query-string concatenation with UriComponentsBuilder query-parameter methods for ids, strictMode, and elementTypes, then use the builder’s encoded URI when calling restTemplate.exchange.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/gridsuite/explore/server/dto/ElementAttributes.java`:
- Around line 50-56: Update the explicit ElementAttributes constructors to
preserve non-null collection defaults for specificMetadata and references
instead of passing null through the constructor chain. Ensure both fields remain
initialized as empty collections when callers omit them, while retaining
provided metadata in the overload that accepts specificMetadata.
In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java`:
- Around line 473-475: Update the parent-directory resolution around
getParentDirectoryNames to avoid sequential per-study HTTP calls by using
parallel stream processing as the temporary mitigation. Preserve the existing
studyUuid-to-directory-names mapping and userId propagation, while leaving
getIdentityBySub unchanged.
In `@src/main/java/org/gridsuite/explore/server/services/StudyService.java`:
- Around line 92-102: Update getNodesInfos to return an empty list immediately
when nodeUuids is empty, avoiding the remote call. For non-empty input, preserve
the existing exchange request but normalize a null response body to an empty
list before returning it, ensuring callers can safely stream the result.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java`:
- Around line 309-328: Update getElementsInfos to immediately return an empty
list when elementsUuids is empty, avoiding the network request. Replace manual
query-string concatenation with UriComponentsBuilder query-parameter methods for
ids, strictMode, and elementTypes, then use the builder’s encoded URI when
calling restTemplate.exchange.
In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java`:
- Around line 467-469: Update the flow after `studyUuids` is computed from
`nodeInfosByUuid` to return immediately when no study UUIDs are present, before
triggering downstream element-information or user-identity HTTP calls. Preserve
the existing processing path when `studyUuids` is non-empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 903ddad7-92e5-4540-851d-ae12b2f3f834
📒 Files selected for processing (11)
src/main/java/org/gridsuite/explore/server/ExploreController.javasrc/main/java/org/gridsuite/explore/server/dto/ElementAttributes.javasrc/main/java/org/gridsuite/explore/server/dto/NodeInfos.javasrc/main/java/org/gridsuite/explore/server/dto/ReferenceAttributes.javasrc/main/java/org/gridsuite/explore/server/dto/SharedElementInfos.javasrc/main/java/org/gridsuite/explore/server/dto/UsersIdentities.javasrc/main/java/org/gridsuite/explore/server/services/DirectoryService.javasrc/main/java/org/gridsuite/explore/server/services/ExploreService.javasrc/main/java/org/gridsuite/explore/server/services/StudyService.javasrc/main/java/org/gridsuite/explore/server/services/UserIdentityService.javasrc/test/java/org/gridsuite/explore/server/SharedElementInfosTest.java
Also drop the subtype: the elements using a shared element are studies, which have none, and it was never filled anyway. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Keep the naming rules next to the identities they read. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Use the new directory-server batch endpoint instead of querying the path of each study separately. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
It returns the infos about the usages of one shared element, matching the SharedElementInfos name. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/gridsuite/explore/server/services/UserIdentityService.java (1)
53-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not hide identity-service failures as an empty result.
Catching
RestClientExceptionand returningMap.of()makes an identity-server outage indistinguishable from everysubhaving no known identity.ExploreServicecan therefore return successful but incomplete entries without the required owner/last-modifier labels. Propagate the failure or return an explicit endpoint error, while reserving an empty map for a successful response with no matching identities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/UserIdentityService.java` around lines 53 - 65, The getUsersIdentitiesMap method must not convert RestClientException failures into an empty map. Remove the fallback return from the catch path and propagate the failure or expose it as an explicit endpoint error, while retaining Map.of() only for empty subscriptions or successful responses with no identity data.
🧹 Nitpick comments (1)
src/main/java/org/gridsuite/explore/server/services/DirectoryService.java (1)
156-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
UriComponentsBuilderfor query parameters instead of manual string concatenation.Both of these methods extract a URI string from
UriComponentsBuilderand then manually append query parameters using string concatenation (+ "?ids=" + ...). This bypasses the builder's built-in URL parameter management and encoding mechanisms.
src/main/java/org/gridsuite/explore/server/services/DirectoryService.java#L156-L158: Use.queryParam("ids", ids)directly on the builder before calling.toUriString().src/main/java/org/gridsuite/explore/server/services/DirectoryService.java#L308-L318: Chain.queryParam()calls forids,strictMode, andelementTypesinstead of conditionally concatenating them to the string.♻️ Proposed refactoring
For
getElementsPaths(around line 156):- var ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); - String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH + "/paths").toUriString() + "?ids=" + ids; + var ids = elementUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); + String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH + "/paths") + .queryParam("ids", ids) + .toUriString();For
getElementsInfos(around line 308):- var ids = elementsUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); - String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH).toUriString() + "?ids=" + ids; - - if (!strictMode) { - path += "&strictMode=false"; - } - - if (!CollectionUtils.isEmpty(elementTypes)) { - path += "&elementTypes=" + elementTypes.stream().collect(Collectors.joining(",")); - } + var ids = elementsUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); + UriComponentsBuilder builder = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH) + .queryParam("ids", ids); + + if (!strictMode) { + builder.queryParam("strictMode", false); + } + + if (!CollectionUtils.isEmpty(elementTypes)) { + builder.queryParam("elementTypes", String.join(",", elementTypes)); + } + String path = builder.toUriString();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java` around lines 156 - 158, Replace manual query-string concatenation in DirectoryService.getElementsPaths at src/main/java/org/gridsuite/explore/server/services/DirectoryService.java:156-158 with UriComponentsBuilder.queryParam("ids", ids) before toUriString(). In DirectoryService.getElementsInfos at src/main/java/org/gridsuite/explore/server/services/DirectoryService.java:308-318, build the URI by chaining queryParam calls for ids, strictMode, and elementTypes while preserving the existing conditional parameter behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/main/java/org/gridsuite/explore/server/services/UserIdentityService.java`:
- Around line 53-65: The getUsersIdentitiesMap method must not convert
RestClientException failures into an empty map. Remove the fallback return from
the catch path and propagate the failure or expose it as an explicit endpoint
error, while retaining Map.of() only for empty subscriptions or successful
responses with no identity data.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java`:
- Around line 156-158: Replace manual query-string concatenation in
DirectoryService.getElementsPaths at
src/main/java/org/gridsuite/explore/server/services/DirectoryService.java:156-158
with UriComponentsBuilder.queryParam("ids", ids) before toUriString(). In
DirectoryService.getElementsInfos at
src/main/java/org/gridsuite/explore/server/services/DirectoryService.java:308-318,
build the URI by chaining queryParam calls for ids, strictMode, and elementTypes
while preserving the existing conditional parameter behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2f30bb8-b883-4de2-8e90-56517352cdec
📒 Files selected for processing (9)
src/main/java/org/gridsuite/explore/server/ExploreController.javasrc/main/java/org/gridsuite/explore/server/dto/NodeInfos.javasrc/main/java/org/gridsuite/explore/server/dto/ReferenceAttributes.javasrc/main/java/org/gridsuite/explore/server/dto/SharedElementInfos.javasrc/main/java/org/gridsuite/explore/server/dto/UsersIdentities.javasrc/main/java/org/gridsuite/explore/server/services/DirectoryService.javasrc/main/java/org/gridsuite/explore/server/services/ExploreService.javasrc/main/java/org/gridsuite/explore/server/services/UserIdentityService.javasrc/test/java/org/gridsuite/explore/server/SharedElementInfosTest.java
💤 Files with no reviewable changes (2)
- src/main/java/org/gridsuite/explore/server/dto/NodeInfos.java
- src/main/java/org/gridsuite/explore/server/dto/ReferenceAttributes.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/org/gridsuite/explore/server/ExploreController.java
- src/main/java/org/gridsuite/explore/server/services/ExploreService.java
- src/test/java/org/gridsuite/explore/server/SharedElementInfosTest.java
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
The record describes an element consuming a shared element, not the shared element itself. Rename the endpoint accordingly. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Simplify and rephrase the class description to better explain its purpose. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/explore/server/services/ExploreService.java (2)
486-502: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSafeguard against null paths when the study path cannot be resolved.
If the directory server fails to resolve the path for a study (e.g., if it was partially deleted or corrupted),
parentDirectoryNamesByStudyUuid.get(...)will returnnull. UsinggetOrDefaultensures thepathlist is empty rather thannull, which provides a more consistent JSON response and avoids potential downstreamNullPointerExceptionsin the UI.🛡️ Proposed fix
private ConsumerElementInfos toConsumerElementInfos(NodeInfos nodeInfos, ElementAttributes study, Map<UUID, List<String>> parentDirectoryNamesByStudyUuid, Map<String, UsersIdentities.UserIdentity> identityBySub) { if (study == null) { // the user cannot read this study, or it no longer exists return null; } return ConsumerElementInfos.builder() .elementName(study.getElementName()) .type(study.getType()) - .path(parentDirectoryNamesByStudyUuid.get(study.getElementUuid())) + .path(parentDirectoryNamesByStudyUuid.getOrDefault(study.getElementUuid(), List.of())) .node(nodeInfos.nodeName()) .ownerLabel(UsersIdentities.toLabel(study.getOwner(), identityBySub)) .lastModificationDate(study.getLastModificationDate()) .lastModifiedByLabel(UsersIdentities.toLabel(study.getLastModifiedBy(), identityBySub)) .build(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java` around lines 486 - 502, Update toConsumerElementInfos so the path lookup for study.getElementUuid() uses parentDirectoryNamesByStudyUuid.getOrDefault with an empty list fallback, ensuring ConsumerElementInfos.path is never null when the directory path cannot be resolved.
470-471: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an early return if
studyUuidsis empty to avoid unnecessary downstream calls.If all referenced nodes have been deleted (resulting in an empty
nodeInfosByUuid),studyUuidswill be empty. Returning early here prevents three unnecessary downstream API calls (directoryService.getElementsInfosNotStrict,directoryService.getElementsPaths, anduserIdentityService.getUsersIdentitiesMap), which could also fail if the downstream servers do not gracefully handle empty lists.⚡ Proposed fix
List<UUID> studyUuids = nodeInfosByUuid.values().stream().map(NodeInfos::studyUuid).distinct().toList(); + if (studyUuids.isEmpty()) { + return List.of(); + } Map<UUID, ElementAttributes> studyByUuid = directoryService.getElementsInfosNotStrict(studyUuids, null, userId)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java` around lines 470 - 471, After constructing studyUuids in the surrounding service method, return immediately when it is empty, before invoking directoryService.getElementsInfosNotStrict, directoryService.getElementsPaths, or userIdentityService.getUsersIdentitiesMap. Preserve the existing processing path for non-empty study UUID lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/org/gridsuite/explore/server/services/ExploreService.java`:
- Around line 486-502: Update toConsumerElementInfos so the path lookup for
study.getElementUuid() uses parentDirectoryNamesByStudyUuid.getOrDefault with an
empty list fallback, ensuring ConsumerElementInfos.path is never null when the
directory path cannot be resolved.
- Around line 470-471: After constructing studyUuids in the surrounding service
method, return immediately when it is empty, before invoking
directoryService.getElementsInfosNotStrict, directoryService.getElementsPaths,
or userIdentityService.getUsersIdentitiesMap. Preserve the existing processing
path for non-empty study UUID lists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2389aad4-7fb0-4d54-8185-5f5d0f962b74
📒 Files selected for processing (5)
src/main/java/org/gridsuite/explore/server/ExploreController.javasrc/main/java/org/gridsuite/explore/server/dto/ConsumerElementInfos.javasrc/main/java/org/gridsuite/explore/server/services/DirectoryService.javasrc/main/java/org/gridsuite/explore/server/services/ExploreService.javasrc/test/java/org/gridsuite/explore/server/ConsumerElementInfosTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/gridsuite/explore/server/services/DirectoryService.java
…rgument Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/explore/server/services/DirectoryService.java (2)
156-171: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winShort-circuit if
elementUuidsis empty.If the provided list of
elementUuidsis empty, this method will still make an HTTP call to the directory server without any IDs, potentially resulting in an unnecessary network request or a bad request. Adding an early return prevents this.♻️ Proposed refactor
public Map<UUID, List<ElementAttributes>> getElementsPaths(List<UUID> elementUuids, String userId) { + if (CollectionUtils.isEmpty(elementUuids)) { + return Collections.emptyMap(); + } + String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH + "/paths") .queryParam(PARAM_IDS, elementUuids) .buildAndExpand() .toUriString();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java` around lines 156 - 171, Update getElementsPaths to return an empty map immediately when elementUuids is empty, before building the URI or invoking restTemplate.exchange. Preserve the existing HTTP request and response handling for non-empty lists.
306-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShort-circuit empty inputs and use
UriComponentsBuildersafely.
- Early return: If
elementsUuidsis empty, skip the HTTP call and return an empty list immediately to avoid unnecessary network requests.- URL construction: Manually concatenating query strings is fragile and bypasses automatic URL encoding. Use
UriComponentsBuilderto construct the URI safely. Spring handles collections directly inqueryParam(as already done elsewhere in this class).♻️ Proposed refactor
public List<ElementAttributes> getElementsInfos(List<UUID> elementsUuids, List<String> elementTypes, String userId, boolean strictMode) { - var ids = elementsUuids.stream().map(UUID::toString).collect(Collectors.joining(",")); - String path = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH).toUriString() + "?ids=" + ids; - - if (!strictMode) { - path += "&strictMode=false"; - } - - if (!CollectionUtils.isEmpty(elementTypes)) { - path += "&elementTypes=" + elementTypes.stream().collect(Collectors.joining(",")); - } + if (CollectionUtils.isEmpty(elementsUuids)) { + return Collections.emptyList(); + } + + UriComponentsBuilder builder = UriComponentsBuilder.fromPath(ELEMENTS_SERVER_ROOT_PATH) + .queryParam(PARAM_IDS, elementsUuids); + + if (!strictMode) { + builder.queryParam("strictMode", false); + } + + if (!CollectionUtils.isEmpty(elementTypes)) { + builder.queryParam(PARAM_ELEMENT_TYPES, elementTypes); + } + + String path = builder.toUriString(); List<ElementAttributes> elementAttributesList;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java` around lines 306 - 317, Update getElementsInfos to return an empty list immediately when elementsUuids is empty, before constructing or calling the endpoint. Replace manual query-string concatenation with UriComponentsBuilder queryParam calls for ids, strictMode when applicable, and elementTypes, then build the URI using Spring’s encoding support while preserving the existing request behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/org/gridsuite/explore/server/services/DirectoryService.java`:
- Around line 156-171: Update getElementsPaths to return an empty map
immediately when elementUuids is empty, before building the URI or invoking
restTemplate.exchange. Preserve the existing HTTP request and response handling
for non-empty lists.
- Around line 306-317: Update getElementsInfos to return an empty list
immediately when elementsUuids is empty, before constructing or calling the
endpoint. Replace manual query-string concatenation with UriComponentsBuilder
queryParam calls for ids, strictMode when applicable, and elementTypes, then
build the URI using Spring’s encoding support while preserving the existing
request behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccfab1ff-d655-42b0-9f35-ba03af11db8d
📒 Files selected for processing (3)
src/main/java/org/gridsuite/explore/server/services/DirectoryService.javasrc/main/java/org/gridsuite/explore/server/services/ExploreService.javasrc/test/java/org/gridsuite/explore/server/ConsumerElementInfosTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/org/gridsuite/explore/server/ConsumerElementInfosTest.java
- src/main/java/org/gridsuite/explore/server/services/ExploreService.java
Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
Exercise the directory, study and user-identity remote calls end-to-end with WireMock instead of mocking the services, and add focused tests for UsersIdentities.toLabel and UserIdentityService.getUsersIdentitiesMap. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.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
1 participant
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.
Add
GET /v1/explore/elements/{elementUuid}/consumer-element-infos, listing the elements using a shared element, to fill the "sharing links" dialog.There is one entry per reference of the shared element: an element referencing it from several nodes appears once per node, and two entries are identical when a same node references it twice. Each entry holds the name, type, path and node, plus the owner, last modification date and last modifier of the element.
The response holds no uuid, so user labels are resolved here rather than by the caller.
Each entry is aggregated from:
GET /v1/elements?ids=) and path (GET /v1/elements/paths?ids=) of the elements using it,POST /v1/nodes/infos),Each server is queried once, whatever the number of references.
Elements the user cannot read are omitted rather than failing the whole call.
Depends on gridsuite/study-server#1020 and gridsuite/directory-server#259.
Used by gridsuite/gridexplore-app#895.