add asymmetrical load computation#1015
Conversation
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
📝 WalkthroughWalkthroughAdds asymmetrical load computation support end-to-end: new persisted fields and DTOs, a remote client service, controller routes, study and supervision orchestration, database columns, notification wiring, and integration tests. ChangesAsymmetrical Load computation feature
Sequence Diagram(s)sequenceDiagram
participant Controller as StudyController
participant StudyService
participant AsymmetricalLoadService
participant PccMinServer
participant ConsumerService
Controller->>StudyService: runAsymmetricalLoad(studyUuid, nodeUuid, rootNetworkUuid, userId)
StudyService->>StudyService: handleAsymmetricalLoadRequest(...)
StudyService->>AsymmetricalLoadService: runAsymmetricalLoad(networkUuid, variantId, parametersInfos, reportInfos, receiver, userId)
AsymmetricalLoadService->>PccMinServer: POST run-and-save
PccMinServer-->>ConsumerService: result/stopped/failed message
ConsumerService->>StudyService: consumeCalculationResult/Stopped/Failed(ASYMMETRICAL_LOAD)
StudyService-->>Controller: notification emitted
sequenceDiagram
participant Controller as StudyController
participant NodeInfoService as RootNetworkNodeInfoService
participant AsymmetricalLoadService
participant PccMinServer
Controller->>NodeInfoService: getAsymmetricalLoadResult(nodeUuid, rootNetworkUuid, filters, globalFilters, pageable)
NodeInfoService->>AsymmetricalLoadService: getAsymmetricalLoadResults(resultParameters, filters, globalFilters, pageable)
AsymmetricalLoadService->>PccMinServer: GET results (paged)
PccMinServer-->>AsymmetricalLoadService: paged JSON result
AsymmetricalLoadService-->>NodeInfoService: result body
NodeInfoService-->>Controller: 200 with body / 204 if null
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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.44.1)src/main/java/org/gridsuite/study/server/service/StudyService.javaast-grep timed out on this file 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.java (1)
382-408: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftMissing reset of
asymmetricalLoadResultUuidon invalidation.
invalidateComputationResultsnulls every other computation result UUID (includingpccMinResultUuidright above), but never resetsasymmetricalLoadResultUuid. SincefillComputationResultUuids(lines 456-457) already queues this UUID for remote deletion on invalidation, the entity is left with a dangling reference to a deleted remote result after any node invalidation — leading to stale/broken status and result lookups.🐛 Proposed fix
rootNetworkNodeInfoEntity.setStateEstimationResultUuid(null); rootNetworkNodeInfoEntity.setPccMinResultUuid(null); + rootNetworkNodeInfoEntity.setAsymmetricalLoadResultUuid(null);🤖 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/RootNetworkNodeInfoService.java` around lines 382 - 408, `invalidateComputationResults` is missing cleanup for `asymmetricalLoadResultUuid`, leaving a stale reference after invalidation. Update this method in `RootNetworkNodeInfoService` to null out `asymmetricalLoadResultUuid` alongside the other computation result UUIDs when results are not preserved, matching the deletion behavior already handled by `fillComputationResultUuids`.src/main/java/org/gridsuite/study/server/service/StudyService.java (1)
1307-1325: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalidate asymmetrical-load status on short-circuit changes, and fix the helper wiring
RunAsymmetricalLoadParametersInfosdepends onstudyEntity.getShortCircuitParametersUuid(), so short-circuit parameter updates should clear asymmetrical-load status too.invalidateAsymmetricalLoadStatusOnAllNodesis currently wired toPCC_MINresult UUIDs, so it needs to targetASYMMETRICAL_LOADresults before being added here.🤖 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/StudyService.java` around lines 1307 - 1325, Short-circuit parameter updates in StudyService.setShortCircuitParameters should also invalidate asymmetrical-load status. Update the helper wiring so invalidateAsymmetricalLoadStatusOnAllNodes is associated with ASYMMETRICAL_LOAD result UUIDs instead of PCC_MIN, then add that invalidation helper to the setComputationParameters call alongside invalidateShortCircuitStatusOnAllNodes and invalidatePccMinStatusOnAllNodes.src/main/java/org/gridsuite/study/server/dto/RootNetworkNodeInfo.java (1)
63-83: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
toEntity()is missingasymmetricalLoadResultUuidmapping — silent data loss on persistence.The new
asymmetricalLoadResultUuidfield was added to the DTO (line 57) and is correctly mapped in the entity'stoDto(), buttoEntity()omits.asymmetricalLoadResultUuid(asymmetricalLoadResultUuid)from the builder chain. This means any DTO-to-entity conversion will silently null out the asymmetrical load result UUID, losing persisted results.🐛 Proposed fix: add missing builder call
.stateEstimationResultUuid(stateEstimationResultUuid) .pccMinResultUuid(pccMinResultUuid) + .asymmetricalLoadResultUuid(asymmetricalLoadResultUuid) .nodeBuildStatus(nodeBuildStatus.toEntity()) .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/dto/RootNetworkNodeInfo.java` around lines 63 - 83, RootNetworkNodeInfo.toEntity() is missing the new asymmetricalLoadResultUuid field, causing DTO-to-entity conversion to drop that value. Update the builder chain in RootNetworkNodeInfo.toEntity() to include the asymmetricalLoadResultUuid mapping alongside the other result UUID fields, keeping it consistent with RootNetworkNodeInfo.toDto() and the RootNetworkNodeInfoEntity builder.
🧹 Nitpick comments (2)
src/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java (1)
586-642: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew asymmetrical-load stubs duplicate prefix-aware logic already available in
ComputationServerStubs, inside a@Deprecatedclass.
stubAsymmetricalLoadRun/verifyAsymmetricalLoadRun/stubAsymmetricalLoadFailed/verifyAsymmetricalLoadFail/stubPagedAsymmetricalLoadResult/verifyAsymmetricalLoadPagedGet/verifyExportAsymmetricalLoadResultall hardcode the/v1/asymmetrical-load/...paths, duplicating whatComputationServerStubsalready supports generically via its newprefixparameter (stubComputationRun(..., prefix),stubGetResultStatus(..., prefix), etc.). SinceWireMockStubsis marked@Deprecated, growing it with new asymmetrical-load-specific methods works against the stated migration direction and adds more surface area to maintain/duplicate for future computation types.Consider using
computationServerStubs.stubComputationRun(networkUuid, variantId, resultUuid, ASYMMETRICAL_LOAD_PREFIX)etc. instead of adding these to the deprecated class.Also applies to: 685-697
🤖 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/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java` around lines 586 - 642, These asymmetrical-load helpers in the deprecated WireMockStubs class duplicate the prefix-aware behavior now available in ComputationServerStubs. Remove or avoid the new hardcoded /v1/asymmetrical-load/... stubs and route the call sites through ComputationServerStubs methods such as stubComputationRun, stubGetResultStatus, and the related verify helpers, passing the ASYMMETRICAL_LOAD_PREFIX instead. Keep WireMockStubs from gaining more asymmetrical-load-specific surface area and preserve the existing generic prefix-based implementation.src/test/java/org/gridsuite/study/server/utils/wiremock/ComputationServerStubs.java (1)
62-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
stubComputationStoplacks a prefix overload, unlike itsverifyComputationStopcounterpart.
verifyComputationStopgained aprefixparameter (Line 89-91), butstubComputationStop(Line 62-65) did not. This forces callers needing a prefixed stub path (e.g./v1/asymmetrical-load/results/.../stop) to bypass this helper and hand-rollwireMockServer.stubFor(...)calls directly, as seen inAsymmetricalLoadTest.testStop(). Consider adding astubComputationStop(String resultUuid, String prefix)overload for consistency with the other prefix-aware methods added in this diff.♻️ Proposed fix
- public void stubComputationStop(String resultUuid) { - wireMock.stubFor(WireMock.put(WireMock.urlPathEqualTo("/v1/results/" + resultUuid + "/stop")) - .willReturn(WireMock.ok())); - } + public void stubComputationStop(String resultUuid) { + stubComputationStop(resultUuid, ""); + } + + public void stubComputationStop(String resultUuid, String prefix) { + wireMock.stubFor(WireMock.put(WireMock.urlPathEqualTo("/v1/" + prefix + "results/" + resultUuid + "/stop")) + .willReturn(WireMock.ok())); + }Also applies to: 89-91
🤖 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/test/java/org/gridsuite/study/server/utils/wiremock/ComputationServerStubs.java` around lines 62 - 65, `stubComputationStop` is missing the prefix-aware overload that `verifyComputationStop` already has, so add a `stubComputationStop(String resultUuid, String prefix)` variant in `ComputationServerStubs` and route the existing single-argument method through it with the default prefix. Make the new overload build the stop URL from the supplied prefix plus the result UUID, matching the pattern used by the other prefix-aware stub helpers in this class, so callers like the asymmetrical-load tests can use the shared helper instead of hand-rolling `wireMock.stubFor(...)`.
🤖 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/StudyService.java`:
- Around line 1498-1501: `invalidateAsymmetricalLoadStatusOnAllNodes` is using
the wrong computation type when fetching result UUIDs, so update the lookup in
`StudyService` to use `ASYMMETRICAL_LOAD` instead of `PCC_MIN`. Make the change
in the call to `rootNetworkNodeInfoService.getComputationResultUuids(...)` so
`asymmetricalLoadService.invalidateAsymmetricalLoadStatus(...)` receives the
asymmetrical-load UUIDs that are actually updated by
`setAsymmetricalLoadParameters`.
---
Outside diff comments:
In `@src/main/java/org/gridsuite/study/server/dto/RootNetworkNodeInfo.java`:
- Around line 63-83: RootNetworkNodeInfo.toEntity() is missing the new
asymmetricalLoadResultUuid field, causing DTO-to-entity conversion to drop that
value. Update the builder chain in RootNetworkNodeInfo.toEntity() to include the
asymmetricalLoadResultUuid mapping alongside the other result UUID fields,
keeping it consistent with RootNetworkNodeInfo.toDto() and the
RootNetworkNodeInfoEntity builder.
In
`@src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.java`:
- Around line 382-408: `invalidateComputationResults` is missing cleanup for
`asymmetricalLoadResultUuid`, leaving a stale reference after invalidation.
Update this method in `RootNetworkNodeInfoService` to null out
`asymmetricalLoadResultUuid` alongside the other computation result UUIDs when
results are not preserved, matching the deletion behavior already handled by
`fillComputationResultUuids`.
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 1307-1325: Short-circuit parameter updates in
StudyService.setShortCircuitParameters should also invalidate asymmetrical-load
status. Update the helper wiring so invalidateAsymmetricalLoadStatusOnAllNodes
is associated with ASYMMETRICAL_LOAD result UUIDs instead of PCC_MIN, then add
that invalidation helper to the setComputationParameters call alongside
invalidateShortCircuitStatusOnAllNodes and invalidatePccMinStatusOnAllNodes.
---
Nitpick comments:
In
`@src/test/java/org/gridsuite/study/server/utils/wiremock/ComputationServerStubs.java`:
- Around line 62-65: `stubComputationStop` is missing the prefix-aware overload
that `verifyComputationStop` already has, so add a `stubComputationStop(String
resultUuid, String prefix)` variant in `ComputationServerStubs` and route the
existing single-argument method through it with the default prefix. Make the new
overload build the stop URL from the supplied prefix plus the result UUID,
matching the pattern used by the other prefix-aware stub helpers in this class,
so callers like the asymmetrical-load tests can use the shared helper instead of
hand-rolling `wireMock.stubFor(...)`.
In `@src/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java`:
- Around line 586-642: These asymmetrical-load helpers in the deprecated
WireMockStubs class duplicate the prefix-aware behavior now available in
ComputationServerStubs. Remove or avoid the new hardcoded
/v1/asymmetrical-load/... stubs and route the call sites through
ComputationServerStubs methods such as stubComputationRun, stubGetResultStatus,
and the related verify helpers, passing the ASYMMETRICAL_LOAD_PREFIX instead.
Keep WireMockStubs from gaining more asymmetrical-load-specific surface area and
preserve the existing generic prefix-based implementation.
🪄 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: 57eeaaff-5a89-4ba8-9ef4-bf0b95464bc7
📒 Files selected for processing (28)
src/main/java/org/gridsuite/study/server/StudyConstants.javasrc/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/ComputationType.javasrc/main/java/org/gridsuite/study/server/dto/InvalidateNodeInfos.javasrc/main/java/org/gridsuite/study/server/dto/RemoteDeletionInfos.javasrc/main/java/org/gridsuite/study/server/dto/RootNetworkNodeInfo.javasrc/main/java/org/gridsuite/study/server/dto/RunAsymmetricalLoadParametersInfos.javasrc/main/java/org/gridsuite/study/server/dto/UserProfileInfos.javasrc/main/java/org/gridsuite/study/server/dto/computation/ComputationParameterUUIDs.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/entities/RootNetworkNodeInfoEntity.javasrc/main/java/org/gridsuite/study/server/notification/NotificationService.javasrc/main/java/org/gridsuite/study/server/repository/StudyEntity.javasrc/main/java/org/gridsuite/study/server/repository/rootnetwork/RootNetworkNodeInfoRepository.javasrc/main/java/org/gridsuite/study/server/service/AsymmetricalLoadService.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/java/org/gridsuite/study/server/service/SupervisionService.javasrc/main/resources/db/changelog/changesets/changelog_20260703T090337Z.xmlsrc/main/resources/db/changelog/db.changelog-master.yamlsrc/test/java/org/gridsuite/study/server/AsymmetricalLoadTest.javasrc/test/java/org/gridsuite/study/server/PccMinTest.javasrc/test/java/org/gridsuite/study/server/SensitivityAnalysisTest.javasrc/test/java/org/gridsuite/study/server/ShortCircuitTest.javasrc/test/java/org/gridsuite/study/server/utils/TestUtils.javasrc/test/java/org/gridsuite/study/server/utils/wiremock/ComputationServerStubs.javasrc/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.javasrc/test/resources/asymmetricalload-result-paged.json
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/study/server/service/StudyService.java (1)
3345-3351: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalidate asymmetrical-load status on short-circuit parameter updates.
handleAsymmetricalLoadRequestusesstudyEntity.getShortCircuitParametersUuid(), sosetShortCircuitParametersshould also callinvalidateAsymmetricalLoadStatusOnAllNodesand emitUPDATE_TYPE_ASYMMETRICAL_LOAD_STATUS, not just short-circuit and PCC Min statuses.🤖 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/StudyService.java` around lines 3345 - 3351, The short-circuit parameter update path in StudyService is missing asymmetrical-load invalidation, so the UI/status can remain stale after changes. Update setShortCircuitParameters to also call invalidateAsymmetricalLoadStatusOnAllNodes and publish UPDATE_TYPE_ASYMMETRICAL_LOAD_STATUS, alongside the existing short-circuit and PCC Min status updates, so it stays consistent with handleAsymmetricalLoadRequest and the studyEntity.getShortCircuitParametersUuid() dependency.
🤖 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/study/server/service/StudyService.java`:
- Around line 3345-3351: The short-circuit parameter update path in StudyService
is missing asymmetrical-load invalidation, so the UI/status can remain stale
after changes. Update setShortCircuitParameters to also call
invalidateAsymmetricalLoadStatusOnAllNodes and publish
UPDATE_TYPE_ASYMMETRICAL_LOAD_STATUS, alongside the existing short-circuit and
PCC Min status updates, so it stays consistent with
handleAsymmetricalLoadRequest and the
studyEntity.getShortCircuitParametersUuid() dependency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c23dc08-8923-4283-9c17-54bf2be61b40
📒 Files selected for processing (2)
src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.java
|
1 participant
PR Summary