[WIP] Get computation statuses for branch#986
Open
sBouzols wants to merge 4 commits into
Open
Conversation
Signed-off-by: sBouzols <sylvain.bouzols@gmail.com>
📝 WalkthroughWalkthroughThis PR refactors computation services to use batch status retrieval APIs. Services now support fetching status for multiple result UUIDs in single calls, replacing per-UUID GET requests with bulk POST operations. Status return types change from ChangesBatch Status Retrieval Pattern Refactoring
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.2)src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javaTip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
SlimaneAmar
reviewed
May 5, 2026
| } | ||
| } | ||
|
|
||
| public void assertLoadFlowsNotRunning(List<UUID> resultUuids) { |
Contributor
There was a problem hiding this comment.
Remove assertLoadFlowNotRunning
| } | ||
| } | ||
|
|
||
| public void assertLoadFlowsNotRunning(List<UUID> resultUuids) { |
Contributor
There was a problem hiding this comment.
Rename by assertNoLoadFlowRunning ?
Comment on lines
+222
to
+224
| if (loadFlowStatuses != null && loadFlowStatuses.containsValue(LoadFlowStatus.RUNNING)) { | ||
| throw new StudyException(COMPUTATION_RUNNING); | ||
| } |
Contributor
There was a problem hiding this comment.
Set<LoadFlowStatus> values = new HashSet<>(loadFlowStatuses.values()); if (loadFlowStatuses != null && values.contains(LoadFlowStatus.RUNNING)) { throw new StudyException(COMPUTATION_RUNNING); }
…computation Co-authored-by: Copilot <copilot@github.com> Signed-off-by: sBouzols <sylvain.bouzols@gmail.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: 18
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/study/server/service/LoadFlowService.java (1)
155-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden batch status response against null body
Line 155 returns
getBody()directly; if the load-flow service responds with an empty body, downstream calls (e.g.,assertNoLoadFlowRunning) can throw NPE. Return an empty map fallback here to keep callers safe.Proposed fix
- return restTemplate.exchange(loadFlowServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, LoadFlowStatus>>() { - }).getBody(); + Map<UUID, LoadFlowStatus> statuses = restTemplate.exchange( + loadFlowServerBaseUri + path, + HttpMethod.POST, + httpEntity, + new ParameterizedTypeReference<Map<UUID, LoadFlowStatus>>() { + } + ).getBody(); + return statuses != null ? statuses : Map.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/study/server/service/LoadFlowService.java` around lines 155 - 156, The call in LoadFlowService that returns restTemplate.exchange(...).getBody() can return null; update the code in the method that calls restTemplate.exchange (the POST to loadFlowServerBaseUri + path in LoadFlowService) to guard against a null body by returning an empty Map<UUID, LoadFlowStatus> when getBody() is null (e.g., assign the result to a local variable and return result != null ? result : Collections.emptyMap()) so downstream callers like assertNoLoadFlowRunning never see a null.
🧹 Nitpick comments (5)
src/main/java/org/gridsuite/study/server/service/dynamicsecurityanalysis/DynamicSecurityAnalysisService.java (1)
137-143: 💤 Low valueConsider avoiding unnecessary HashSet allocation.
Creating a
HashSetfrom the map values is unnecessary. You can check for the presence ofRUNNINGstatus directly on theCollection<DynamicSecurityAnalysisStatus>returned by.values().♻️ Proposed refactor
public void assertNoDynamicSecurityAnalysisRunning(List<UUID> resultUuids) { Map<UUID, DynamicSecurityAnalysisStatus> dynamicSecurityAnalysisStatuses = getDynamicSecurityAnalysisStatuses(resultUuids); - Set<DynamicSecurityAnalysisStatus> values = new HashSet<>(dynamicSecurityAnalysisStatuses.values()); - if (values.contains(DynamicSecurityAnalysisStatus.RUNNING)) { + if (dynamicSecurityAnalysisStatuses.values().contains(DynamicSecurityAnalysisStatus.RUNNING)) { throw new StudyException(COMPUTATION_RUNNING); } }🤖 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/study/server/service/dynamicsecurityanalysis/DynamicSecurityAnalysisService.java` around lines 137 - 143, The method assertNoDynamicSecurityAnalysisRunning creates an unnecessary HashSet from dynamicSecurityAnalysisStatuses.values(); instead check for RUNNING directly on the Collection returned by getDynamicSecurityAnalysisStatuses(resultUuids) to avoid allocation—e.g. replace the new HashSet<>(dynamicSecurityAnalysisStatuses.values()) usage with a direct contains/stream anyMatch on dynamicSecurityAnalysisStatuses.values() and keep throwing StudyException(COMPUTATION_RUNNING) when DynamicSecurityAnalysisStatus.RUNNING is found.src/main/java/org/gridsuite/study/server/service/shortcircuit/ShortCircuitService.java (1)
202-215: 💤 Low valueInconsistent null-safety in empty list check.
Line 203 uses
resultUuids.isEmpty()which will throw NPE ifresultUuidsis null, while other similar services (PccMinService, StateEstimationService, VoltageInitService, SensitivityAnalysisService) useCollectionUtils.isEmpty(resultUuids)for null-safe checking. Consider usingCollectionUtils.isEmpty()for consistency and defensive coding.🤖 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/study/server/service/shortcircuit/ShortCircuitService.java` around lines 202 - 215, In ShortCircuitService.getShortCircuitAnalysisStatuses, replace the null-unsafe resultUuids.isEmpty() check with a null-safe CollectionUtils.isEmpty(resultUuids) to match other services (PccMinService, StateEstimationService, VoltageInitService, SensitivityAnalysisService); ensure you import org.springframework.util.CollectionUtils if not already present and keep the early return Map.of() behavior when the collection is empty or null.src/main/java/org/gridsuite/study/server/service/VoltageInitService.java (1)
13-13: 💤 Low valueInconsistent CollectionUtils import.
This file imports
org.apache.commons.collections4.CollectionUtilswhile other services in this PR (PccMinService, StateEstimationService, SensitivityAnalysisService) useorg.springframework.util.CollectionUtils. Consider using Spring's CollectionUtils for consistency across the codebase.🤖 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/study/server/service/VoltageInitService.java` at line 13, VoltageInitService imports org.apache.commons.collections4.CollectionUtils which is inconsistent with other services; change the import to org.springframework.util.CollectionUtils and update any calls if needed to match Spring's API (e.g., CollectionUtils.isEmpty/hasLength semantics) so usages in the VoltageInitService class (look for references inside the VoltageInitService methods/constructor) compile and follow the same convention as PccMinService, StateEstimationService, and SensitivityAnalysisService.src/main/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java (1)
208-208: ⚡ Quick winUse
.build()instead of.buildAndExpand().Since no path variables are being expanded,
.build()is more appropriate than.buildAndExpand().♻️ Suggested simplification
- var uriComponents = UriComponentsBuilder.fromUriString(endPointUrl + "/statuses").buildAndExpand(); + var uriComponents = UriComponentsBuilder.fromUriString(endPointUrl + "/statuses").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/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java` at line 208, Replace the unnecessary call to buildAndExpand() with build() when constructing the URI for statuses: in DynamicMarginCalculationClient where uriComponents is assigned (var uriComponents = UriComponentsBuilder.fromUriString(endPointUrl + "/statuses")...), use .build() instead of .buildAndExpand() because there are no path variables to expand.src/main/java/org/gridsuite/study/server/service/dynamicmargincalculation/DynamicMarginCalculationService.java (1)
135-135: ⚡ Quick winUnnecessary
HashSetcreation.Creating a
HashSetfromvalues()adds overhead. Thecontainscheck can be performed directly onCollection<DynamicMarginCalculationStatus>returned by.values().♻️ Simplified implementation
public void assertNoDynamicMarginCalculationRunning(List<UUID> resultUuids) { Map<UUID, DynamicMarginCalculationStatus> dynamicMarginCalculationStatuses = getDynamicMarginCalculationStatuses(resultUuids); - Set<DynamicMarginCalculationStatus> values = new HashSet<>(dynamicMarginCalculationStatuses.values()); - if (values.contains(DynamicMarginCalculationStatus.RUNNING)) { + if (dynamicMarginCalculationStatuses.values().contains(DynamicMarginCalculationStatus.RUNNING)) { throw new StudyException(COMPUTATION_RUNNING); } }🤖 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/study/server/service/dynamicmargincalculation/DynamicMarginCalculationService.java` at line 135, Replace the unnecessary HashSet allocation in DynamicMarginCalculationService: instead of creating Set<DynamicMarginCalculationStatus> values = new HashSet<>(dynamicMarginCalculationStatuses.values()), use the collection returned by dynamicMarginCalculationStatuses.values() directly (e.g. assign to Collection<DynamicMarginCalculationStatus> or call .values().contains(...) inline) to perform the contains check without the extra copy/overhead.
🤖 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/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java`:
- Around line 197-199: getStatus currently may return null when
getStatuses(List.of(resultUuid)) doesn't contain resultUuid; change getStatus to
return Optional<DynamicMarginCalculationStatus> and wrap the map lookup with
Optional.ofNullable(getStatuses(List.of(resultUuid)).get(resultUuid)) so callers
must handle missing status explicitly; update any callers of
DynamicMarginCalculationClient.getStatus to handle Optional (or alternatively
throw a clear checked/unchecked exception with resultUuid in the message if you
prefer fail-fast semantics).
- Around line 215-216: The REST call currently returns
getRestTemplate().exchange(...).getBody() which can be null and cause NPEs; in
DynamicMarginCalculationClient replace the direct getBody() usage with a
defensive check: capture the response body into a local Map (from the exchange
call using the same ParameterizedTypeReference), then if the body is null return
Collections.emptyMap() (or throw a clear IllegalStateException if you prefer
failing fast) so callers always receive a non-null Map; ensure to import or
reference java.util.Collections for the empty map fallback.
- Around line 201-217: The remote /statuses POST may return a null body or omit
entries, so update DynamicMarginCalculationClient.getStatuses(List<UUID>) to
defensively handle null responses (e.g., if exchange().getBody() is null return
an empty Map or a map with requested UUIDs mapped to a default/UNKNOWN
DynamicMarginCalculationStatus) and ensure callers (notably getStatus(UUID))
check the returned map and handle missing keys instead of assuming non-null
values; adjust getStatus(UUID) to null-check the map and return an explicit
Optional/throw a clear exception or a default status when the requested UUID is
not present.
In
`@src/main/java/org/gridsuite/study/server/service/client/dynamicsecurityanalysis/DynamicSecurityAnalysisClient.java`:
- Around line 184-186: The getStatus method delegates to
getStatuses(List.of(resultUuid)).get(resultUuid) which can return null; update
DynamicSecurityAnalysisClient.getStatus to handle a missing entry by checking
the map result from getStatuses(List.of(resultUuid)) and if the value for
resultUuid is null, throw a clear runtime exception (e.g., IllegalStateException
or NoSuchElementException) with a message containing the resultUuid, otherwise
return the non-null DynamicSecurityAnalysisStatus; reference the getStatus(...)
method and the getStatuses(...) call when making this change.
- Around line 188-205: Annotate the getStatuses(List<UUID> resultUuids)
parameter with the same `@NonNull` used elsewhere in
DynamicSecurityAnalysisClient, and add a null-safe handling of the REST
response: capture the ResponseEntity<Map<UUID, DynamicSecurityAnalysisStatus>>
from getRestTemplate().exchange(...) into a variable, check its getBody() for
null and return Map.of() (or Objects.requireNonNullElse(body, Map.of())) when
null instead of returning getBody() directly; keep existing empty-input check
using CollectionUtils.isEmpty and reuse getRestTemplate() and the existing
ParameterizedTypeReference to locate the exchange call.
In
`@src/main/java/org/gridsuite/study/server/service/client/dynamicsimulation/impl/DynamicSimulationClientImpl.java`:
- Around line 208-210: The getStatus method delegates to
getStatuses(List.of(resultUuid)).get(resultUuid) which can NPE if getStatuses
returns null or the map lacks the key; update getStatus(UUID resultUuid) to call
getStatuses(List.of(resultUuid)) into a local Map variable, check for null and
for containsKey(resultUuid) and handle both cases safely (e.g., return null or
Optional<DynamicSimulationStatus> per project convention) instead of calling
.get() directly; reference methods getStatus, getStatuses and the type
DynamicSimulationStatus/resultUuid when making the checks and returning the safe
value.
- Around line 213-229: The getStatuses method in DynamicSimulationClientImpl may
return null because RestTemplate.exchange(...).getBody() can be null; update
getStatuses to defensively handle a null response by capturing the result of
getRestTemplate().exchange(...).getBody(), and if it is null return an empty map
(e.g. Map.of() or Collections.emptyMap()) so callers (like getStatus() which
calls .get() on the map) never get a null; apply the same null-safe pattern to
the corresponding methods in DynamicMarginCalculationClient and
DynamicSecurityAnalysisClient.
In
`@src/main/java/org/gridsuite/study/server/service/dynamicmargincalculation/DynamicMarginCalculationService.java`:
- Around line 88-90: getStatus(UUID resultUuid) calls
getDynamicMarginCalculationStatuses(List.of(resultUuid)).get(resultUuid) which
can return null if the map lacks the UUID; add a defensive null check in
getStatus to avoid returning null (e.g., check the map result for resultUuid and
either throw a clear exception such as
NotFoundException/IllegalArgumentException or change the method to return
Optional<DynamicMarginCalculationStatus>) so callers never receive a raw null;
update getStatus to locate the map entry from
getDynamicMarginCalculationStatuses and handle the null case accordingly.
- Around line 133-139: Add defensive checks in
assertNoDynamicMarginCalculationRunning: validate the input resultUuids (throw
IllegalArgumentException if null or empty) and null-safe the call to
getDynamicMarginCalculationStatuses (treat a null map as empty). When building
values from dynamicMarginCalculationStatuses.values(), guard against a null
collection to avoid NPEs; only check for DynamicMarginCalculationStatus.RUNNING
if the map/values are non-null. Use the existing
StudyException(COMPUTATION_RUNNING) only for the running-state detection and use
a clear IllegalArgumentException for bad input.
In
`@src/main/java/org/gridsuite/study/server/service/dynamicsecurityanalysis/DynamicSecurityAnalysisService.java`:
- Around line 89-91: The call in getStatus delegates to
getDynamicSecurityAnalysisStatuses(List.of(resultUuid)) and then directly
.get(resultUuid), which can return null and cause NPEs; update getStatus to
first capture the Map returned by getDynamicSecurityAnalysisStatuses, check for
the presence of resultUuid (e.g. containsKey or get != null) and return a safe
value (null or an Optional) or throw a clear exception instead of dereferencing
a potentially missing entry; refer to the methods getStatus and
getDynamicSecurityAnalysisStatuses and the type DynamicSecurityAnalysisStatus to
locate where to add the null-safety check.
In
`@src/main/java/org/gridsuite/study/server/service/dynamicsimulation/impl/DynamicSimulationServiceImpl.java`:
- Around line 245-251: The method assertNoDynamicSimulationRunning currently
assumes getDynamicSimulationStatuses(...) returns a non-null Map and will NPE
when it's null; update assertNoDynamicSimulationRunning to defensively handle a
null (or empty) result from getDynamicSimulationStatuses by treating a null map
as empty before creating the HashSet or checking values, then proceed to check
for DynamicSimulationStatus.RUNNING and throw
StudyException(COMPUTATION_RUNNING) if found; reference the
assertNoDynamicSimulationRunning method, getDynamicSimulationStatuses, and
DynamicSimulationStatus.RUNNING when locating and applying the fix.
- Around line 206-208: The getStatus method delegates to
getDynamicSimulationStatuses but can NPE if that method returns null or the map
lacks the key; update DynamicSimulationServiceImpl.getStatus to first call
Map<UUID, DynamicSimulationStatus> statuses =
getDynamicSimulationStatuses(List.of(resultUuid)), check if statuses is null and
return null (or an Optional if your API prefers), then return
statuses.get(resultUuid) only after confirming the key exists (or use
statuses.getOrDefault(resultUuid, null)) to avoid a NullPointerException when
the map is missing the entry.
- Line 246: The local variable name loadFlowStatuses in
DynamicSimulationServiceImpl is misleading; rename it to
dynamicSimulationStatuses (or simulationStatuses) where it's assigned from
getDynamicSimulationStatuses(computationResultUuids) and update all subsequent
references in the method to use the new name so the variable accurately reflects
it holds DynamicSimulationStatus values.
In `@src/main/java/org/gridsuite/study/server/service/PccMinService.java`:
- Around line 134-144: The RestTemplate call is using a relative path only, so
prepend the configured base URI (pccMinServerBaseUri) to the built path before
calling restTemplate.exchange; update the code around where PCC_MIN_URI +
DELIMITER + "results/statuses" is composed (in PccMinService) to build a full
URL (e.g., concatenate pccMinServerBaseUri and the path) and then call
restTemplate.exchange(fullUrl, HttpMethod.POST, httpEntity, new
ParameterizedTypeReference<Map<UUID, PccMinStatus>>() { }). Ensure you keep the
existing headers and HttpEntity<List<UUID>> httpEntity and return the response
body as before.
In
`@src/main/java/org/gridsuite/study/server/service/SecurityAnalysisService.java`:
- Around line 189-191: getSecurityAnalysisStatus currently calls
getSecurityAnalysisStatuses(List.of(resultUuid)) which will throw NPE when
resultUuid is null; add a null guard in getSecurityAnalysisStatus (check
resultUuid == null) and handle that case explicitly (e.g., return null or an
appropriate default SecurityAnalysisStatus) instead of calling List.of, so
callers like RootNetworkNodeInfoService which use getComputationResultUuid()
that may be null won't break the call chain; reference the methods
getSecurityAnalysisStatus and getSecurityAnalysisStatuses and the caller
RootNetworkNodeInfoService#getComputationResultUuid when making the change.
In
`@src/main/java/org/gridsuite/study/server/service/SensitivityAnalysisService.java`:
- Around line 179-193: The call in getSensitivityAnalysisStatuses builds a
relative path but calls restTemplate.exchange(path, ...) without prepending
sensitivityAnalysisServerBaseUri, causing failed requests; fix by resolving the
full URL before the exchange (e.g., prefixing the path with
sensitivityAnalysisServerBaseUri or using
UriComponentsBuilder.fromHttpUrl(sensitivityAnalysisServerBaseUri).path(path) to
build the absoluteUri) and then call restTemplate.exchange(absoluteUri,
HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID,
SensitivityAnalysisStatus>>() { }).getBody() so the restTemplate uses the full
base URI.
In
`@src/main/java/org/gridsuite/study/server/service/StateEstimationService.java`:
- Around line 134-147: getStateEstimationStatuses builds a relative path and
calls restTemplate.exchange(path, ...) without prepending the configured base
URI (stateEstimationServerServerBaseUri), causing requests to fail; fix by
concatenating or resolving stateEstimationServerServerBaseUri with the local
path before calling restTemplate.exchange (use the existing path variable and
the stateEstimationServerServerBaseUri field), then call
restTemplate.exchange(fullUri, HttpMethod.POST, httpEntity, new
ParameterizedTypeReference<Map<UUID, StateEstimationStatus>>() { }).getBody() so
the request uses the full service URI.
In `@src/main/java/org/gridsuite/study/server/service/VoltageInitService.java`:
- Around line 134-146: getVoltageInitStatuses builds only a relative path and
calls restTemplate.exchange(path, ...) so requests fail; update the method to
prepend the configured base URI (voltageInitServerBaseUri) when building the
request URI (e.g. combine voltageInitServerBaseUri with the current path or use
UriComponentsBuilder starting from voltageInitServerBaseUri) and then call
restTemplate.exchange with that full URI while keeping the existing HttpEntity,
headers, and ParameterizedTypeReference to return the Map<UUID,
VoltageInitStatus>.
---
Duplicate comments:
In `@src/main/java/org/gridsuite/study/server/service/LoadFlowService.java`:
- Around line 155-156: The call in LoadFlowService that returns
restTemplate.exchange(...).getBody() can return null; update the code in the
method that calls restTemplate.exchange (the POST to loadFlowServerBaseUri +
path in LoadFlowService) to guard against a null body by returning an empty
Map<UUID, LoadFlowStatus> when getBody() is null (e.g., assign the result to a
local variable and return result != null ? result : Collections.emptyMap()) so
downstream callers like assertNoLoadFlowRunning never see a null.
---
Nitpick comments:
In
`@src/main/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java`:
- Line 208: Replace the unnecessary call to buildAndExpand() with build() when
constructing the URI for statuses: in DynamicMarginCalculationClient where
uriComponents is assigned (var uriComponents =
UriComponentsBuilder.fromUriString(endPointUrl + "/statuses")...), use .build()
instead of .buildAndExpand() because there are no path variables to expand.
In
`@src/main/java/org/gridsuite/study/server/service/dynamicmargincalculation/DynamicMarginCalculationService.java`:
- Line 135: Replace the unnecessary HashSet allocation in
DynamicMarginCalculationService: instead of creating
Set<DynamicMarginCalculationStatus> values = new
HashSet<>(dynamicMarginCalculationStatuses.values()), use the collection
returned by dynamicMarginCalculationStatuses.values() directly (e.g. assign to
Collection<DynamicMarginCalculationStatus> or call .values().contains(...)
inline) to perform the contains check without the extra copy/overhead.
In
`@src/main/java/org/gridsuite/study/server/service/dynamicsecurityanalysis/DynamicSecurityAnalysisService.java`:
- Around line 137-143: The method assertNoDynamicSecurityAnalysisRunning creates
an unnecessary HashSet from dynamicSecurityAnalysisStatuses.values(); instead
check for RUNNING directly on the Collection returned by
getDynamicSecurityAnalysisStatuses(resultUuids) to avoid allocation—e.g. replace
the new HashSet<>(dynamicSecurityAnalysisStatuses.values()) usage with a direct
contains/stream anyMatch on dynamicSecurityAnalysisStatuses.values() and keep
throwing StudyException(COMPUTATION_RUNNING) when
DynamicSecurityAnalysisStatus.RUNNING is found.
In
`@src/main/java/org/gridsuite/study/server/service/shortcircuit/ShortCircuitService.java`:
- Around line 202-215: In ShortCircuitService.getShortCircuitAnalysisStatuses,
replace the null-unsafe resultUuids.isEmpty() check with a null-safe
CollectionUtils.isEmpty(resultUuids) to match other services (PccMinService,
StateEstimationService, VoltageInitService, SensitivityAnalysisService); ensure
you import org.springframework.util.CollectionUtils if not already present and
keep the early return Map.of() behavior when the collection is empty or null.
In `@src/main/java/org/gridsuite/study/server/service/VoltageInitService.java`:
- Line 13: VoltageInitService imports
org.apache.commons.collections4.CollectionUtils which is inconsistent with other
services; change the import to org.springframework.util.CollectionUtils and
update any calls if needed to match Spring's API (e.g.,
CollectionUtils.isEmpty/hasLength semantics) so usages in the VoltageInitService
class (look for references inside the VoltageInitService methods/constructor)
compile and follow the same convention as PccMinService, StateEstimationService,
and SensitivityAnalysisService.
🪄 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: 2918cf6f-5027-4cfd-a02f-8d2fd133efae
📒 Files selected for processing (22)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/ShortCircuitAnalysisStatus.javasrc/main/java/org/gridsuite/study/server/repository/rootnetwork/RootNetworkNodeInfoRepository.javasrc/main/java/org/gridsuite/study/server/service/LoadFlowService.javasrc/main/java/org/gridsuite/study/server/service/PccMinService.javasrc/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javasrc/main/java/org/gridsuite/study/server/service/SecurityAnalysisService.javasrc/main/java/org/gridsuite/study/server/service/SensitivityAnalysisService.javasrc/main/java/org/gridsuite/study/server/service/StateEstimationService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/java/org/gridsuite/study/server/service/VoltageInitService.javasrc/main/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.javasrc/main/java/org/gridsuite/study/server/service/client/dynamicsecurityanalysis/DynamicSecurityAnalysisClient.javasrc/main/java/org/gridsuite/study/server/service/client/dynamicsimulation/DynamicSimulationClient.javasrc/main/java/org/gridsuite/study/server/service/client/dynamicsimulation/impl/DynamicSimulationClientImpl.javasrc/main/java/org/gridsuite/study/server/service/dynamicmargincalculation/DynamicMarginCalculationService.javasrc/main/java/org/gridsuite/study/server/service/dynamicsecurityanalysis/DynamicSecurityAnalysisService.javasrc/main/java/org/gridsuite/study/server/service/dynamicsimulation/DynamicSimulationService.javasrc/main/java/org/gridsuite/study/server/service/dynamicsimulation/impl/DynamicSimulationServiceImpl.javasrc/main/java/org/gridsuite/study/server/service/shortcircuit/ShortCircuitService.javasrc/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.java
Comment on lines
197
to
+199
| public DynamicMarginCalculationStatus getStatus(@NonNull UUID resultUuid) { | ||
| Objects.requireNonNull(resultUuid); | ||
| return getStatuses(List.of(resultUuid)).get(resultUuid); | ||
| } |
There was a problem hiding this comment.
Potential null return when status not found in map.
The .get(resultUuid) call will return null if the map from getStatuses() doesn't contain the requested UUID. This can happen if the remote service returns an incomplete response. Callers expecting a non-null DynamicMarginCalculationStatus may encounter NPEs.
🛡️ Suggested fix to handle missing status
public DynamicMarginCalculationStatus getStatus(`@NonNull` UUID resultUuid) {
- return getStatuses(List.of(resultUuid)).get(resultUuid);
+ Map<UUID, DynamicMarginCalculationStatus> statuses = getStatuses(List.of(resultUuid));
+ DynamicMarginCalculationStatus status = statuses.get(resultUuid);
+ if (status == null) {
+ throw new IllegalStateException("No status returned for result UUID: " + resultUuid);
+ }
+ return status;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public DynamicMarginCalculationStatus getStatus(@NonNull UUID resultUuid) { | |
| Objects.requireNonNull(resultUuid); | |
| return getStatuses(List.of(resultUuid)).get(resultUuid); | |
| } | |
| public DynamicMarginCalculationStatus getStatus(`@NonNull` UUID resultUuid) { | |
| Map<UUID, DynamicMarginCalculationStatus> statuses = getStatuses(List.of(resultUuid)); | |
| DynamicMarginCalculationStatus status = statuses.get(resultUuid); | |
| if (status == null) { | |
| throw new IllegalStateException("No status returned for result UUID: " + resultUuid); | |
| } | |
| return status; | |
| } |
🤖 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/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java`
around lines 197 - 199, getStatus currently may return null when
getStatuses(List.of(resultUuid)) doesn't contain resultUuid; change getStatus to
return Optional<DynamicMarginCalculationStatus> and wrap the map lookup with
Optional.ofNullable(getStatuses(List.of(resultUuid)).get(resultUuid)) so callers
must handle missing status explicitly; update any callers of
DynamicMarginCalculationClient.getStatus to handle Optional (or alternatively
throw a clear checked/unchecked exception with resultUuid in the message if you
prefer fail-fast semantics).
Comment on lines
+201
to
217
| public Map<UUID, DynamicMarginCalculationStatus> getStatuses(List<UUID> resultUuids) { | ||
| if (CollectionUtils.isEmpty(resultUuids)) { | ||
| return Map.of(); | ||
| } | ||
|
|
||
| String url = UriComponentsBuilder | ||
| .fromUriString(resultBaseUrl + "/{resultUuid}/status") | ||
| .buildAndExpand(resultUuid) | ||
| .toUriString(); | ||
| String endPointUrl = buildEndPointUrl(getBaseUri(), DYNAMIC_MARGIN_CALCULATION_API_VERSION, DYNAMIC_MARGIN_CALCULATION_END_POINT_RESULT); | ||
|
|
||
| // call dynamic-margin-calculation REST API | ||
| return getRestTemplate().getForObject(url, DynamicMarginCalculationStatus.class); | ||
| var uriComponents = UriComponentsBuilder.fromUriString(endPointUrl + "/statuses").buildAndExpand(); | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
|
|
||
| return getRestTemplate().exchange(uriComponents.toUriString(), HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicMarginCalculationStatus>>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for API documentation or tests that describe the /statuses endpoint behavior
rg -nP -C5 '(statuses|getStatuses).*endpoint|POST.*statuses' --type=java --type=mdRepository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Find test files for DynamicMarginCalculationClient
fd -type f -name '*DynamicMarginCalculation*Test*.java' -o -name '*Test*DynamicMarginCalculation*.java'Repository: gridsuite/study-server
Length of output: 237
🏁 Script executed:
# Search for test references to getStatuses method
rg -l 'getStatuses' --type=javaRepository: gridsuite/study-server
Length of output: 839
🏁 Script executed:
# Look for OpenAPI/Swagger specs or contract files
fd -type f \( -name '*.yaml' -o -name '*.yml' -o -name '*.json' -o -name '*contract*' -o -name '*openapi*' -o -name '*swagger*' \)Repository: gridsuite/study-server
Length of output: 237
🏁 Script executed:
# Check how getStatuses is used in the codebase
rg 'getStatuses\(' --type=java -B 2 -A 5Repository: gridsuite/study-server
Length of output: 10162
🏁 Script executed:
# Find test files for the client
fd 'DynamicMarginCalculation.*Test' -type fRepository: gridsuite/study-server
Length of output: 237
🏁 Script executed:
# Examine the getStatus method in DynamicMarginCalculationClient to see if it handles null
rg -A 3 'public DynamicMarginCalculationStatus getStatus' src/main/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.javaRepository: gridsuite/study-server
Length of output: 218
🏁 Script executed:
# Look for how the client methods are tested
fd 'Test' -type f | xargs grep -l 'getStatuses\|DynamicMarginCalculation' | head -5Repository: gridsuite/study-server
Length of output: 237
🏁 Script executed:
# Check the complete DynamicMarginCalculationClient implementation
cat src/main/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.javaRepository: gridsuite/study-server
Length of output: 11300
🏁 Script executed:
# Search for test files that test getStatuses or DynamicMarginCalculation
find . -name '*Test*.java' | xargs grep -l 'DynamicMarginCalculation\|getStatuses' 2>/dev/nullRepository: gridsuite/study-server
Length of output: 860
🏁 Script executed:
# Look for any documentation or comments about the statuses endpoint
rg -i 'statuses.*endpoint|endpoint.*statuses|contract|guarantee' --type=java -B 2 -A 2Repository: gridsuite/study-server
Length of output: 2825
🏁 Script executed:
# Check similar client implementations to see if they handle partial results
rg -A 10 'public Map.*getStatuses' src/main/java/org/gridsuite/study/server/service/client/Repository: gridsuite/study-server
Length of output: 5484
🏁 Script executed:
# Examine the DynamicMarginCalculationClientTest to see what it tests
cat src/test/java/org/gridsuite/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClientTest.javaRepository: gridsuite/study-server
Length of output: 19472
🏁 Script executed:
# Also check the service test to see how getStatuses is called and handled
cat src/test/java/org/gridsuite/study/server/service/dynamicmargincalculation/DynamicMarginCalculationServiceTest.javaRepository: gridsuite/study-server
Length of output: 8535
Verify remote API contract and handle potential null results.
The getStatus() method calls getStatuses() and immediately invokes .get(resultUuid) without null checking. If the remote /statuses endpoint returns:
- A null body,
getStatus()will throw NullPointerException - Partial results (missing entries for requested UUIDs), callers will receive null instead of a status
Either ensure the API contract guarantees complete coverage for all requested UUIDs, or add defensive null handling in getStatus() and callers that depend on map entries existing.
🤖 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/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java`
around lines 201 - 217, The remote /statuses POST may return a null body or omit
entries, so update DynamicMarginCalculationClient.getStatuses(List<UUID>) to
defensively handle null responses (e.g., if exchange().getBody() is null return
an empty Map or a map with requested UUIDs mapped to a default/UNKNOWN
DynamicMarginCalculationStatus) and ensure callers (notably getStatus(UUID))
check the returned map and handle missing keys instead of assuming non-null
values; adjust getStatus(UUID) to null-check the map and return an explicit
Optional/throw a clear exception or a default status when the requested UUID is
not present.
Comment on lines
+215
to
+216
| return getRestTemplate().exchange(uriComponents.toUriString(), HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicMarginCalculationStatus>>() { | ||
| }).getBody(); |
There was a problem hiding this comment.
Potential null return from REST call.
The getBody() method can return null in edge cases (e.g., certain HTTP responses). This would cause NPEs in calling code that expects a non-null map. Consider adding a null check and returning an empty map or throwing an exception.
🛡️ Suggested defensive null check
- return getRestTemplate().exchange(uriComponents.toUriString(), HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicMarginCalculationStatus>>() {
- }).getBody();
+ Map<UUID, DynamicMarginCalculationStatus> body = getRestTemplate().exchange(
+ uriComponents.toUriString(),
+ HttpMethod.POST,
+ httpEntity,
+ new ParameterizedTypeReference<Map<UUID, DynamicMarginCalculationStatus>>() {}
+ ).getBody();
+ return body != null ? body : Map.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/study/server/service/client/dynamicmargincalculation/DynamicMarginCalculationClient.java`
around lines 215 - 216, The REST call currently returns
getRestTemplate().exchange(...).getBody() which can be null and cause NPEs; in
DynamicMarginCalculationClient replace the direct getBody() usage with a
defensive check: capture the response body into a local Map (from the exchange
call using the same ParameterizedTypeReference), then if the body is null return
Collections.emptyMap() (or throw a clear IllegalStateException if you prefer
failing fast) so callers always receive a non-null Map; ensure to import or
reference java.util.Collections for the empty map fallback.
Comment on lines
184
to
+186
| public DynamicSecurityAnalysisStatus getStatus(@NonNull UUID resultUuid) { | ||
| Objects.requireNonNull(resultUuid); | ||
| return getStatuses(List.of(resultUuid)).get(resultUuid); | ||
| } |
There was a problem hiding this comment.
Add null safety check for the delegated batch call.
The delegation to getStatuses(List.of(resultUuid)).get(resultUuid) can return null if the server's response map does not contain the requested UUID. This would break the expected contract and cause NPEs in calling code that expects a non-null DynamicSecurityAnalysisStatus.
🛡️ Proposed fix to add null safety
public DynamicSecurityAnalysisStatus getStatus(`@NonNull` UUID resultUuid) {
- return getStatuses(List.of(resultUuid)).get(resultUuid);
+ DynamicSecurityAnalysisStatus status = getStatuses(List.of(resultUuid)).get(resultUuid);
+ if (status == null) {
+ throw new IllegalStateException("Status not found for result UUID: " + resultUuid);
+ }
+ return status;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public DynamicSecurityAnalysisStatus getStatus(@NonNull UUID resultUuid) { | |
| Objects.requireNonNull(resultUuid); | |
| return getStatuses(List.of(resultUuid)).get(resultUuid); | |
| } | |
| public DynamicSecurityAnalysisStatus getStatus(`@NonNull` UUID resultUuid) { | |
| DynamicSecurityAnalysisStatus status = getStatuses(List.of(resultUuid)).get(resultUuid); | |
| if (status == null) { | |
| throw new IllegalStateException("Status not found for result UUID: " + resultUuid); | |
| } | |
| return status; | |
| } |
🤖 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/study/server/service/client/dynamicsecurityanalysis/DynamicSecurityAnalysisClient.java`
around lines 184 - 186, The getStatus method delegates to
getStatuses(List.of(resultUuid)).get(resultUuid) which can return null; update
DynamicSecurityAnalysisClient.getStatus to handle a missing entry by checking
the map result from getStatuses(List.of(resultUuid)) and if the value for
resultUuid is null, throw a clear runtime exception (e.g., IllegalStateException
or NoSuchElementException) with a message containing the resultUuid, otherwise
return the non-null DynamicSecurityAnalysisStatus; reference the getStatus(...)
method and the getStatuses(...) call when making this change.
Comment on lines
+188
to
205
| public Map<UUID, DynamicSecurityAnalysisStatus> getStatuses(List<UUID> resultUuids) { | ||
| if (CollectionUtils.isEmpty(resultUuids)) { | ||
| return Map.of(); | ||
| } | ||
|
|
||
| String resultBaseUrl = buildEndPointUrl(getBaseUri(), DYNAMIC_SECURITY_ANALYSIS_API_VERSION, DYNAMIC_SECURITY_ANALYSIS_END_POINT_RESULT); | ||
|
|
||
| String url = UriComponentsBuilder.fromUriString(resultBaseUrl + "/{resultUuid}/status") | ||
| .buildAndExpand(resultUuid) | ||
| String url = UriComponentsBuilder.fromUriString(resultBaseUrl + "/statuses") | ||
| .toUriString(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
|
|
||
| // call dynamic-security-analysis REST API | ||
| return getRestTemplate().getForObject(url, DynamicSecurityAnalysisStatus.class); | ||
| return getRestTemplate().exchange(url, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicSecurityAnalysisStatus >>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
Add parameter validation and null check on REST response body.
Two issues:
- The
resultUuidsparameter lacks@NonNullannotation, inconsistent with other methods in this class. RestTemplate.exchange(...).getBody()can returnnullin certain failure scenarios, which would cause an NPE when the caller expects a non-nullMap.
🛡️ Proposed fix to add validation and null safety
-public Map<UUID, DynamicSecurityAnalysisStatus> getStatuses(List<UUID> resultUuids) {
+public Map<UUID, DynamicSecurityAnalysisStatus> getStatuses(`@NonNull` List<UUID> resultUuids) {
if (CollectionUtils.isEmpty(resultUuids)) {
return Map.of();
}
String resultBaseUrl = buildEndPointUrl(getBaseUri(), DYNAMIC_SECURITY_ANALYSIS_API_VERSION, DYNAMIC_SECURITY_ANALYSIS_END_POINT_RESULT);
String url = UriComponentsBuilder.fromUriString(resultBaseUrl + "/statuses")
.toUriString();
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers);
// call dynamic-security-analysis REST API
- return getRestTemplate().exchange(url, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicSecurityAnalysisStatus >>() {
- }).getBody();
+ Map<UUID, DynamicSecurityAnalysisStatus> body = getRestTemplate().exchange(url, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, DynamicSecurityAnalysisStatus >>() {
+ }).getBody();
+ if (body == null) {
+ throw new IllegalStateException("Received null response body from statuses endpoint");
+ }
+ return body;
}🤖 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/study/server/service/client/dynamicsecurityanalysis/DynamicSecurityAnalysisClient.java`
around lines 188 - 205, Annotate the getStatuses(List<UUID> resultUuids)
parameter with the same `@NonNull` used elsewhere in
DynamicSecurityAnalysisClient, and add a null-safe handling of the REST
response: capture the ResponseEntity<Map<UUID, DynamicSecurityAnalysisStatus>>
from getRestTemplate().exchange(...) into a variable, check its getBody() for
null and return Map.of() (or Objects.requireNonNullElse(body, Map.of())) when
null instead of returning getBody() directly; keep existing empty-input check
using CollectionUtils.isEmpty and reuse getRestTemplate() and the existing
ParameterizedTypeReference to locate the exchange call.
Comment on lines
134
to
144
| String path = UriComponentsBuilder | ||
| .fromPath(PCC_MIN_URI + DELIMITER + "results/{resultUuid}/status") | ||
| .buildAndExpand(resultUuid).toUriString(); | ||
| return restTemplate.getForObject(pccMinServerBaseUri + path, String.class); | ||
| .fromPath(PCC_MIN_URI + DELIMITER + "results/statuses") | ||
| .toUriString(); | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, PccMinStatus>>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
Missing base URI in RestTemplate call causes request to fail.
Line 142 invokes restTemplate.exchange(path, ...) with only the relative path, but the pccMinServerBaseUri is not prepended. This will cause the HTTP request to fail because RestTemplate doesn't know the target server.
🐛 Proposed fix
- return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, PccMinStatus>>() {
+ return restTemplate.exchange(pccMinServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, PccMinStatus>>() {
}).getBody();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String path = UriComponentsBuilder | |
| .fromPath(PCC_MIN_URI + DELIMITER + "results/{resultUuid}/status") | |
| .buildAndExpand(resultUuid).toUriString(); | |
| return restTemplate.getForObject(pccMinServerBaseUri + path, String.class); | |
| .fromPath(PCC_MIN_URI + DELIMITER + "results/statuses") | |
| .toUriString(); | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | |
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, PccMinStatus>>() { | |
| }).getBody(); | |
| } | |
| String path = UriComponentsBuilder | |
| .fromPath(PCC_MIN_URI + DELIMITER + "results/statuses") | |
| .toUriString(); | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | |
| return restTemplate.exchange(pccMinServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, PccMinStatus>>() { | |
| }).getBody(); | |
| } |
🤖 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/study/server/service/PccMinService.java` around
lines 134 - 144, The RestTemplate call is using a relative path only, so prepend
the configured base URI (pccMinServerBaseUri) to the built path before calling
restTemplate.exchange; update the code around where PCC_MIN_URI + DELIMITER +
"results/statuses" is composed (in PccMinService) to build a full URL (e.g.,
concatenate pccMinServerBaseUri and the path) and then call
restTemplate.exchange(fullUrl, HttpMethod.POST, httpEntity, new
ParameterizedTypeReference<Map<UUID, PccMinStatus>>() { }). Ensure you keep the
existing headers and HttpEntity<List<UUID>> httpEntity and return the response
body as before.
Comment on lines
189
to
+191
| public SecurityAnalysisStatus getSecurityAnalysisStatus(UUID resultUuid) { | ||
| return getSecurityAnalysisStatuses(List.of(resultUuid)).get(resultUuid); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all callers of getSecurityAnalysisStatus to verify null handling expectations.
# Search for method calls with context
rg -nP --type=java -C5 '\.getSecurityAnalysisStatus\s*\(' \
-g '!SecurityAnalysisService.java'Repository: gridsuite/study-server
Length of output: 3460
🏁 Script executed:
#!/bin/bash
# Find the getComputationResultUuid method implementation
rg -nP --type=java -A10 'private.*UUID\s+getComputationResultUuid' src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javaRepository: gridsuite/study-server
Length of output: 1200
🏁 Script executed:
#!/bin/bash
# Also check the getSecurityAnalysisStatuses implementation to understand null handling
rg -nP --type=java -A15 'public.*getSecurityAnalysisStatuses' src/main/java/org/gridsuite/study/server/service/SecurityAnalysisService.javaRepository: gridsuite/study-server
Length of output: 925
Add null guard for resultUuid parameter.
List.of(resultUuid) will throw NullPointerException if resultUuid is null. The caller in RootNetworkNodeInfoService (line 854) obtains resultUuid from getComputationResultUuid(), which can return null when the entity's result UUID field is uninitialized. Either add a null check before calling List.of() or handle the null case explicitly to prevent breaking the call chain.
🤖 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/study/server/service/SecurityAnalysisService.java`
around lines 189 - 191, getSecurityAnalysisStatus currently calls
getSecurityAnalysisStatuses(List.of(resultUuid)) which will throw NPE when
resultUuid is null; add a null guard in getSecurityAnalysisStatus (check
resultUuid == null) and handle that case explicitly (e.g., return null or an
appropriate default SecurityAnalysisStatus) instead of calling List.of, so
callers like RootNetworkNodeInfoService which use getComputationResultUuid()
that may be null won't break the call chain; reference the methods
getSecurityAnalysisStatus and getSecurityAnalysisStatuses and the caller
RootNetworkNodeInfoService#getComputationResultUuid when making the change.
Comment on lines
+179
to
193
| public Map<UUID, SensitivityAnalysisStatus> getSensitivityAnalysisStatuses(List<UUID> resultUuids) { | ||
| if (CollectionUtils.isEmpty(resultUuids)) { | ||
| return Map.of(); | ||
| } | ||
|
|
||
| String path = UriComponentsBuilder.fromPath(DELIMITER + SENSITIVITY_ANALYSIS_API_VERSION + "/results/{resultUuid}/status") | ||
| .buildAndExpand(resultUuid).toUriString(); | ||
| String path = UriComponentsBuilder.fromPath(DELIMITER + SENSITIVITY_ANALYSIS_API_VERSION + "/results/statuses") | ||
| .toUriString(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
|
|
||
| return restTemplate.getForObject(sensitivityAnalysisServerBaseUri + path, String.class); | ||
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, SensitivityAnalysisStatus>>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
Missing base URI in RestTemplate call causes request to fail.
Line 191 invokes restTemplate.exchange(path, ...) with only the relative path, but the sensitivityAnalysisServerBaseUri is not prepended.
🐛 Proposed fix
- return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, SensitivityAnalysisStatus>>() {
+ return restTemplate.exchange(sensitivityAnalysisServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, SensitivityAnalysisStatus>>() {
}).getBody();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Map<UUID, SensitivityAnalysisStatus> getSensitivityAnalysisStatuses(List<UUID> resultUuids) { | |
| if (CollectionUtils.isEmpty(resultUuids)) { | |
| return Map.of(); | |
| } | |
| String path = UriComponentsBuilder.fromPath(DELIMITER + SENSITIVITY_ANALYSIS_API_VERSION + "/results/{resultUuid}/status") | |
| .buildAndExpand(resultUuid).toUriString(); | |
| String path = UriComponentsBuilder.fromPath(DELIMITER + SENSITIVITY_ANALYSIS_API_VERSION + "/results/statuses") | |
| .toUriString(); | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | |
| return restTemplate.getForObject(sensitivityAnalysisServerBaseUri + path, String.class); | |
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, SensitivityAnalysisStatus>>() { | |
| }).getBody(); | |
| } | |
| public Map<UUID, SensitivityAnalysisStatus> getSensitivityAnalysisStatuses(List<UUID> resultUuids) { | |
| if (CollectionUtils.isEmpty(resultUuids)) { | |
| return Map.of(); | |
| } | |
| String path = UriComponentsBuilder.fromPath(DELIMITER + SENSITIVITY_ANALYSIS_API_VERSION + "/results/statuses") | |
| .toUriString(); | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | |
| return restTemplate.exchange(sensitivityAnalysisServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, SensitivityAnalysisStatus>>() { | |
| }).getBody(); | |
| } |
🤖 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/study/server/service/SensitivityAnalysisService.java`
around lines 179 - 193, The call in getSensitivityAnalysisStatuses builds a
relative path but calls restTemplate.exchange(path, ...) without prepending
sensitivityAnalysisServerBaseUri, causing failed requests; fix by resolving the
full URL before the exchange (e.g., prefixing the path with
sensitivityAnalysisServerBaseUri or using
UriComponentsBuilder.fromHttpUrl(sensitivityAnalysisServerBaseUri).path(path) to
build the absoluteUri) and then call restTemplate.exchange(absoluteUri,
HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID,
SensitivityAnalysisStatus>>() { }).getBody() so the restTemplate uses the full
base URI.
Comment on lines
+134
to
147
| public Map<UUID, StateEstimationStatus> getStateEstimationStatuses(List<UUID> resultUuids) { | ||
| if (CollectionUtils.isEmpty(resultUuids)) { | ||
| return Map.of(); | ||
| } | ||
| String path = UriComponentsBuilder | ||
| .fromPath(DELIMITER + STATE_ESTIMATION_API_VERSION + "/results/{resultUuid}/status") | ||
| .buildAndExpand(resultUuid).toUriString(); | ||
| return restTemplate.getForObject(stateEstimationServerServerBaseUri + path, String.class); | ||
| .fromPath(DELIMITER + STATE_ESTIMATION_API_VERSION + "/results/statuses") | ||
| .toUriString(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, StateEstimationStatus>>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
Missing base URI in RestTemplate call causes request to fail.
Line 145 invokes restTemplate.exchange(path, ...) with only the relative path, but the stateEstimationServerServerBaseUri is not prepended.
🐛 Proposed fix
- return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, StateEstimationStatus>>() {
+ return restTemplate.exchange(stateEstimationServerServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, StateEstimationStatus>>() {
}).getBody();🤖 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/study/server/service/StateEstimationService.java`
around lines 134 - 147, getStateEstimationStatuses builds a relative path and
calls restTemplate.exchange(path, ...) without prepending the configured base
URI (stateEstimationServerServerBaseUri), causing requests to fail; fix by
concatenating or resolving stateEstimationServerServerBaseUri with the local
path before calling restTemplate.exchange (use the existing path variable and
the stateEstimationServerServerBaseUri field), then call
restTemplate.exchange(fullUri, HttpMethod.POST, httpEntity, new
ParameterizedTypeReference<Map<UUID, StateEstimationStatus>>() { }).getBody() so
the request uses the full service URI.
Comment on lines
+134
to
146
| public Map<UUID, VoltageInitStatus> getVoltageInitStatuses(List<UUID> resultUuids) { | ||
| if (CollectionUtils.isEmpty(resultUuids)) { | ||
| return Map.of(); | ||
| } | ||
| String path = UriComponentsBuilder.fromPath(DELIMITER + VOLTAGE_INIT_API_VERSION + "/results/statuses").toUriString(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<List<UUID>> httpEntity = new HttpEntity<>(resultUuids, headers); | ||
|
|
||
| return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, VoltageInitStatus>>() { | ||
| }).getBody(); | ||
| } |
There was a problem hiding this comment.
Missing base URI in RestTemplate call causes request to fail.
Line 144 invokes restTemplate.exchange(path, ...) with only the relative path, but the voltageInitServerBaseUri is not prepended.
🐛 Proposed fix
- return restTemplate.exchange(path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, VoltageInitStatus>>() {
+ return restTemplate.exchange(voltageInitServerBaseUri + path, HttpMethod.POST, httpEntity, new ParameterizedTypeReference<Map<UUID, VoltageInitStatus>>() {
}).getBody();🤖 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/study/server/service/VoltageInitService.java`
around lines 134 - 146, getVoltageInitStatuses builds only a relative path and
calls restTemplate.exchange(path, ...) so requests fail; update the method to
prepend the configured base URI (voltageInitServerBaseUri) when building the
request URI (e.g. combine voltageInitServerBaseUri with the current path or use
UriComponentsBuilder starting from voltageInitServerBaseUri) and then call
restTemplate.exchange with that full URI while keeping the existing HttpEntity,
headers, and ParameterizedTypeReference to return the Map<UUID,
VoltageInitStatus>.
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
2 participants
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 Summary