Skip to content

Create dedicated controllers for each model to lighten studyController file#1019

Open
basseche wants to merge 8 commits into
mainfrom
refactor_study_controller
Open

Create dedicated controllers for each model to lighten studyController file#1019
basseche wants to merge 8 commits into
mainfrom
refactor_study_controller

Conversation

@basseche

Copy link
Copy Markdown
Contributor

PR Summary

For each model :

  • we create a separate controller.

@basseche basseche self-assigned this Jul 15, 2026
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Load-flow endpoints move from StudyController into dedicated controllers. A local LoadFlowService is introduced alongside the renamed REST client, dependent services are rewired, and tests update their service mocks and controller targets.

Changes

Load-flow controller extraction

Layer / File(s) Summary
Dedicated load-flow controllers
src/main/java/.../controller/loadflow/*, src/main/java/.../controller/StudyController.java
Adds dedicated load-flow execution, result, stop, parameter, and provider endpoints while removing the corresponding endpoints from StudyController.
Load-flow service split
src/main/java/.../service/loadflow/*, src/main/java/.../service/StudyService.java, src/main/java/.../service/{ConsumerService,RootNetworkNodeInfoService,SupervisionService}.java, src/main/java/.../service/common/ComputationParametersService.java
Renames the REST client, adds study-backed parameter reads, and redirects load-flow operations to the appropriate service.
Test wiring migration
src/test/java/...
Updates load-flow mocks, spies, URI setup, controller targets, and verifications for the split services and dedicated controller.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant LoadFlowController
  participant StudyService
  participant LoadFlowServiceRest
  Client->>LoadFlowController: Start or rerun load-flow
  LoadFlowController->>StudyService: Validate and orchestrate execution
  StudyService->>LoadFlowServiceRest: Create status and run load-flow
  LoadFlowServiceRest-->>StudyService: Return computation response
  StudyService-->>LoadFlowController: Complete request
  LoadFlowController-->>Client: Return HTTP response
Loading

Suggested reviewers: slimaneamar

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: splitting StudyController into dedicated controllers per model.
Description check ✅ Passed The description is brief but clearly aligned with the change to create separate controllers for each model.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java (1)

95-95: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix parameter order for runLoadFlow.

The nodeUuid and rootNetworkUuid parameters are swapped in this method call. LoadFlowController.runLoadFlow expects the order to be studyUuid, rootNetworkUuid, nodeUuid (as correctly done on line 111).

While this test currently passes because the assertions use any() matchers, the incorrect order should be fixed to prevent confusion and test failures if stricter matchers are introduced in the future.

💡 Proposed fix
-        controller.runLoadFlow(studyUuid, nodeUuid, rootNetworkUuid, false, userId);
+        controller.runLoadFlow(studyUuid, rootNetworkUuid, nodeUuid, false, 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/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java` at
line 95, Update the runLoadFlow call in LoadFLowUnitTest so its arguments follow
LoadFlowController.runLoadFlow’s expected order: studyUuid, rootNetworkUuid,
then nodeUuid, matching the correct invocation elsewhere in the test.
🧹 Nitpick comments (3)
src/main/java/org/gridsuite/study/server/controller/DynamicSimulationController.java (2)

155-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove contentType from the bodyless response.

Setting the Content-Type header (to application/json) on a response that has no body (build()) is semantically incorrect in HTTP.

♻️ Proposed fix
-        return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).build();
+        return ResponseEntity.ok().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/controller/DynamicSimulationController.java`
at line 155, Update the bodyless response in DynamicSimulationController to
remove the contentType(MediaType.APPLICATION_JSON) call, returning the
ResponseEntity from ok().build() while preserving the existing status and
no-body behavior.

91-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document the 204 No Content response.

The implementation correctly returns a 204 No Content response when the event is not found, but this is not documented in the @ApiResponses. Please add the 204 response to keep the OpenAPI specification accurate.

📝 Proposed fix
     `@ApiResponses`(value = {
         `@ApiResponse`(responseCode = "200", description = "The dynamic simulation event was returned"),
+        `@ApiResponse`(responseCode = "204", description = "The dynamic simulation event was not found"),
         `@ApiResponse`(responseCode = "404", description = "The study/node is not found")})
     public ResponseEntity<EventInfos> getDynamicSimulationEvent(`@Parameter`(description = "Study UUID") `@PathVariable`("studyUuid") UUID studyUuid,
🤖 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/controller/DynamicSimulationController.java`
around lines 91 - 99, Add an `@ApiResponse` for HTTP 204 No Content to
getDynamicSimulationEvent, documenting the no-body response returned when
studyService.getDynamicSimulationEvent yields no event, while preserving the
existing 200 and 404 responses.
src/main/java/org/gridsuite/study/server/controller/LoadFlowController.java (1)

79-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Prevent the manual rollback from masking the original exception.

If studyService.deleteLoadflowResult(...) fails and throws an exception inside this catch block, it will swallow the original exception e. Consider wrapping the rollback in a nested try-catch and attaching its exception as suppressed, ensuring the root cause of the failure is not lost.

🛡️ Proposed fix
         } catch (Exception e) {
             if (loadflowResultUuid != null) {
-                studyService.deleteLoadflowResult(studyUuid, nodeUuid, rootNetworkUuid, loadflowResultUuid);
+                try {
+                    studyService.deleteLoadflowResult(studyUuid, nodeUuid, rootNetworkUuid, loadflowResultUuid);
+                } catch (Exception deletionException) {
+                    e.addSuppressed(deletionException);
+                }
             }
             throw e;
         }
🤖 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/controller/LoadFlowController.java`
around lines 79 - 84, Update the exception handler in LoadFlowController so the
cleanup call to studyService.deleteLoadflowResult is wrapped in a nested
try-catch; if rollback fails, attach that failure as suppressed to the original
exception e, then always rethrow e so the root cause is preserved.
🤖 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/controller/SecurityAnalysisController.java`:
- Around line 76-90: Update SecurityAnalysisController.getSecurityAnalysisResult
to return ResponseEntity<byte[]>, return noContent when the service result is
null, and set Content-Type and Content-Disposition headers for successful CSV
downloads. In SensitivityAnalysisController’s CSV export endpoint, add the same
null-result check and return ResponseEntity.noContent().build(); apply changes
at
src/main/java/org/gridsuite/study/server/controller/SecurityAnalysisController.java
lines 76-90 and
src/main/java/org/gridsuite/study/server/controller/SensitivityAnalysisController.java
lines 85-102.

---

Outside diff comments:
In `@src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java`:
- Line 95: Update the runLoadFlow call in LoadFLowUnitTest so its arguments
follow LoadFlowController.runLoadFlow’s expected order: studyUuid,
rootNetworkUuid, then nodeUuid, matching the correct invocation elsewhere in the
test.

---

Nitpick comments:
In
`@src/main/java/org/gridsuite/study/server/controller/DynamicSimulationController.java`:
- Line 155: Update the bodyless response in DynamicSimulationController to
remove the contentType(MediaType.APPLICATION_JSON) call, returning the
ResponseEntity from ok().build() while preserving the existing status and
no-body behavior.
- Around line 91-99: Add an `@ApiResponse` for HTTP 204 No Content to
getDynamicSimulationEvent, documenting the no-body response returned when
studyService.getDynamicSimulationEvent yields no event, while preserving the
existing 200 and 404 responses.

In `@src/main/java/org/gridsuite/study/server/controller/LoadFlowController.java`:
- Around line 79-84: Update the exception handler in LoadFlowController so the
cleanup call to studyService.deleteLoadflowResult is wrapped in a nested
try-catch; if rollback fails, attach that failure as suppressed to the original
exception e, then always rethrow e so the root cause is preserved.
🪄 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: 7c548ae7-b9d5-4249-991b-a75b2ef975de

📥 Commits

Reviewing files that changed from the base of the PR and between 8767c81 and 4fb8da6.

📒 Files selected for processing (10)
  • src/main/java/org/gridsuite/study/server/controller/DynamicSimulationController.java
  • src/main/java/org/gridsuite/study/server/controller/LoadFlowController.java
  • src/main/java/org/gridsuite/study/server/controller/PccMinController.java
  • src/main/java/org/gridsuite/study/server/controller/SecurityAnalysisController.java
  • src/main/java/org/gridsuite/study/server/controller/SensitivityAnalysisController.java
  • src/main/java/org/gridsuite/study/server/controller/ShortCircuitController.java
  • src/main/java/org/gridsuite/study/server/controller/StateEstimationController.java
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
  • src/main/java/org/gridsuite/study/server/controller/VoltageInitController.java
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java

Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
@basseche
basseche force-pushed the refactor_study_controller branch from 4fb8da6 to 8c6088c Compare July 16, 2026 11:38
@basseche basseche changed the title Create new Controllers for each Model instead of put it in studyController Create dedicated controllers for each model to lighten studycontroller file Jul 16, 2026
@basseche basseche changed the title Create dedicated controllers for each model to lighten studycontroller file Create dedicated controllers for each model to lighten studyController file Jul 16, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowController.java (2)

121-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update API response descriptions to match the endpoint.

The descriptions for the responses mention "computation infos" instead of "modifications", which appears to be a copy-paste oversight from the /computation-infos endpoint.

📝 Proposed fix
-    `@ApiResponses`(value = {`@ApiResponse`(responseCode = "200", description = "The loadflow computation infos"),
-        `@ApiResponse`(responseCode = "404", description = "The loadflow computation has not been found")})
+    `@ApiResponses`(value = {`@ApiResponse`(responseCode = "200", description = "The loadflow modifications"),
+        `@ApiResponse`(responseCode = "404", description = "The loadflow modifications have not been found")})
🤖 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/controller/loadflow/LoadFlowController.java`
around lines 121 - 122, Update the `@ApiResponses` annotation on the relevant
LoadFlowController endpoint so the 200 response description refers to loadflow
modifications rather than computation infos, while leaving the 404 description
unchanged.

74-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Prevent original exception masking during rollback.

If studyService.deleteLoadflowResult throws an exception, it will mask the original exception e that caused the failure. Wrapping the rollback in a try-catch and using addSuppressed ensures the original failure cause is preserved for debugging.

♻️ Proposed fix
         } catch (Exception e) {
             if (loadflowResultUuid != null) {
-                studyService.deleteLoadflowResult(studyUuid, nodeUuid, rootNetworkUuid, loadflowResultUuid);
+                try {
+                    studyService.deleteLoadflowResult(studyUuid, nodeUuid, rootNetworkUuid, loadflowResultUuid);
+                } catch (Exception ex) {
+                    e.addSuppressed(ex);
+                }
             }
             throw e;
         }
🤖 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/controller/loadflow/LoadFlowController.java`
around lines 74 - 79, Update the catch block in the LoadFlowController flow to
wrap studyService.deleteLoadflowResult in its own try-catch when
loadflowResultUuid is non-null; if rollback fails, attach that rollback
exception to the original exception e using addSuppressed, then rethrow e
unchanged.
🤖 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/study/server/controller/loadflow/LoadFlowController.java`:
- Around line 121-122: Update the `@ApiResponses` annotation on the relevant
LoadFlowController endpoint so the 200 response description refers to loadflow
modifications rather than computation infos, while leaving the 404 description
unchanged.
- Around line 74-79: Update the catch block in the LoadFlowController flow to
wrap studyService.deleteLoadflowResult in its own try-catch when
loadflowResultUuid is non-null; if rollback fails, attach that rollback
exception to the original exception e using addSuppressed, then rethrow e
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8ea4ec95-dc04-4430-b7bd-d94739e4d4ce

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb8da6 and 8c6088c.

📒 Files selected for processing (4)
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
  • src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowController.java
  • src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowControllerParameters.java
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java
💤 Files with no reviewable changes (1)
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java

basseche added 3 commits July 16, 2026 14:06
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
…Rest

Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java (1)

99-99: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the argument order for controller.runLoadFlow.

The arguments for rootNetworkUuid and nodeUuid are swapped. Based on the signature of runLoadFlow in the newly extracted LoadFlowController, rootNetworkUuid should precede nodeUuid. Although the test passes because of the any() matchers used in stubbing, this call is functionally incorrect.

🐛 Proposed fix
-        controller.runLoadFlow(studyUuid, nodeUuid, rootNetworkUuid, false, userId);
+        controller.runLoadFlow(studyUuid, rootNetworkUuid, nodeUuid, false, 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/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java` at
line 99, Correct the invocation of LoadFlowController.runLoadFlow in the test by
passing rootNetworkUuid before nodeUuid, matching the extracted controller
method signature. Keep the remaining arguments and test setup unchanged.
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java (1)

134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Include loadFlowService in the reset.

Since loadFlowService was added as a @MockitoSpyBean, it's a good practice to include it in the reset call along with loadFlowServiceRest to avoid state leaking between test executions.

♻️ Proposed refactor
-        reset(studyService, networkModificationTreeService, networkModificationService, notificationService, loadFlowServiceRest, studyRepository);
+        reset(studyService, networkModificationTreeService, networkModificationService, notificationService, loadFlowServiceRest, loadFlowService, studyRepository);
🤖 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/loadflow/LoadFLowUnitTest.java` at
line 134, Update the test setup reset call in LoadFLowUnitTest to include the
loadFlowService MockitoSpyBean alongside loadFlowServiceRest and the other
services, ensuring it is reset between test executions.
🤖 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/loadflow/LoadFlowService.java`:
- Around line 39-48: Update getLoadFlowParametersId to use a writable
transaction so mutations made by getLoadFlowParametersOrDefaultsUuid are
persisted through JPA dirty checking. Annotate getLoadFlowProvider with
`@Transactional` and obtain the parameters UUID via
getLoadFlowParametersOrDefaultsUuid(studyEntity) before calling
loadflowServiceRest.getLoadFlowProvider, preserving initialization for
uninitialized studies.

In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Line 821: The load-flow parameter fetch mutates detached or read-only
StudyEntity instances without persisting initialized UUIDs. Add a transactional
LoadFlowService method accepting the study UUID that initializes and saves
parameters in a short-lived transaction, then replace the studyEntity-based
calls at StudyService.java lines 821, 841, 925, and 3296 with this method,
passing each study UUID.

In `@src/test/java/org/gridsuite/study/server/WorkflowTest.java`:
- Around line 59-60: Change the LoadFlowService field injection in WorkflowTest
from a standard Spring `@Autowired` bean to a Mockito-injected mock, while
preserving Spring test injection so the existing verify() calls operate on that
mock.

---

Outside diff comments:
In `@src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java`:
- Line 99: Correct the invocation of LoadFlowController.runLoadFlow in the test
by passing rootNetworkUuid before nodeUuid, matching the extracted controller
method signature. Keep the remaining arguments and test setup unchanged.

---

Nitpick comments:
In `@src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java`:
- Line 134: Update the test setup reset call in LoadFLowUnitTest to include the
loadFlowService MockitoSpyBean alongside loadFlowServiceRest and the other
services, ensuring it is reset between test executions.
🪄 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: 83161c93-7c17-48b0-a266-ca93006acc42

📥 Commits

Reviewing files that changed from the base of the PR and between 8c6088c and e242963.

📒 Files selected for processing (30)
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
  • src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowController.java
  • src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowControllerParameters.java
  • src/main/java/org/gridsuite/study/server/service/ConsumerService.java
  • src/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.java
  • src/main/java/org/gridsuite/study/server/service/StudyService.java
  • src/main/java/org/gridsuite/study/server/service/SupervisionService.java
  • src/main/java/org/gridsuite/study/server/service/common/ComputationParametersService.java
  • src/main/java/org/gridsuite/study/server/service/loadflow/LoadFlowService.java
  • src/main/java/org/gridsuite/study/server/service/loadflow/LoadFlowServiceRest.java
  • src/test/java/org/gridsuite/study/server/NetworkAreaDiagramTest.java
  • src/test/java/org/gridsuite/study/server/NetworkMapTest.java
  • src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
  • src/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.java
  • src/test/java/org/gridsuite/study/server/SensitivityAnalysisTest.java
  • src/test/java/org/gridsuite/study/server/SingleLineDiagramTest.java
  • src/test/java/org/gridsuite/study/server/SingleLineDiagramWiremockTest.java
  • src/test/java/org/gridsuite/study/server/StudyControllerDynamicMarginCalculationTest.java
  • src/test/java/org/gridsuite/study/server/StudyControllerDynamicSecurityAnalysisTest.java
  • src/test/java/org/gridsuite/study/server/StudyControllerDynamicSimulationTest.java
  • src/test/java/org/gridsuite/study/server/VoltageInitTest.java
  • src/test/java/org/gridsuite/study/server/WorkflowTest.java
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFLowIntegrationTest.java
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFLowUnitTest.java
  • src/test/java/org/gridsuite/study/server/loadflow/LoadFlowTest.java
  • src/test/java/org/gridsuite/study/server/rootnetworks/RootNetworkControllerTest.java
  • src/test/java/org/gridsuite/study/server/rootnetworks/RootNetworkTest.java
  • src/test/java/org/gridsuite/study/server/rootnetworks/SecurityAnalysisTest.java
  • src/test/java/org/gridsuite/study/server/studycontroller/StudyControllerCreationTest.java
  • src/test/java/org/gridsuite/study/server/studycontroller/StudyTestBase.java
💤 Files with no reviewable changes (1)
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/org/gridsuite/study/server/controller/loadflow/LoadFlowController.java

Comment thread src/main/java/org/gridsuite/study/server/service/loadflow/LoadFlowService.java Outdated
UUID nodeUuidToSearchIn = getNodeUuidToSearchIn(nodeUuid, rootNetworkUuid, inUpstreamBuiltParentNode);
StudyEntity studyEntity = getStudy(studyUuid);
LoadFlowParameters loadFlowParameters = getLoadFlowParameters(studyEntity);
LoadFlowParameters loadFlowParameters = loadFlowService.getLoadFlowParameters(studyEntity);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

Lost update hazard: mutated detached entity is not saved.

All these sites pass a detached or read-only studyEntity to loadFlowService.getLoadFlowParameters(studyEntity). The shared root cause is that if the parameters UUID is uninitialized, the underlying REST client creates new default parameters remotely and mutates the studyEntity. Because these map-data operations lack a read-write transaction (and holding a DB transaction open over remote map calls is an anti-pattern), the mutated UUID is never saved to the database. This leads to orphaned remote parameters created on every request.

Consider exposing a @Transactional getLoadFlowParameters(UUID studyUuid) in LoadFlowService that safely initializes and saves the parameters in a short-lived transaction, and calling that here instead.

  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L821-L821: Delegate to a transaction-safe parameters fetch instead of passing the detached entity.
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L841-L841: Delegate to a transaction-safe parameters fetch instead of passing the detached entity.
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L925-L925: Delegate to a transaction-safe parameters fetch instead of passing the detached entity.
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L3296-L3296: Delegate to a transaction-safe parameters fetch instead of passing the read-only entity.
📍 Affects 1 file
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L821-L821 (this comment)
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L841-L841
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L925-L925
  • src/main/java/org/gridsuite/study/server/service/StudyService.java#L3296-L3296
🤖 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` at line
821, The load-flow parameter fetch mutates detached or read-only StudyEntity
instances without persisting initialized UUIDs. Add a transactional
LoadFlowService method accepting the study UUID that initializes and saves
parameters in a short-lived transaction, then replace the studyEntity-based
calls at StudyService.java lines 821, 841, 925, and 3296 with this method,
passing each study UUID.

Comment thread src/test/java/org/gridsuite/study/server/WorkflowTest.java Outdated
basseche added 4 commits July 20, 2026 11:32
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant