Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a MonitorController REST API and MonitorService client for process-configs, wires MonitorService into ExploreService and DirectoryService, updates local config (monitor-server), and adds tests exercising create, update, and duplicate flows; controller endpoints return 200 OK with empty bodies. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant MonitorController
participant ExploreService
participant DirectoryService
participant MonitorService
participant RemoteMonitor as "Remote Monitor Server"
Client->>MonitorController: POST /v1/explore/monitor/process-configs\n(name, description, parentDirectoryUuid, userId, body)
MonitorController->>ExploreService: createProcessConfig(name, body, description, userId, parentDirectoryUuid)
ExploreService->>MonitorService: createProcessConfig(processConfig)
MonitorService->>RemoteMonitor: POST /v1/process-configs (JSON)
RemoteMonitor-->>MonitorService: UUID
MonitorService-->>ExploreService: UUID
ExploreService->>DirectoryService: createElementWithNewName(name, description, parentDirectoryUuid, PROCESS_CONFIG, UUID)
DirectoryService-->>ExploreService: Element created
ExploreService-->>MonitorController: (void)
MonitorController-->>Client: HTTP 200 (empty)
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/gridsuite/explore/server/MonitorController.java`:
- Line 46: The `@PreAuthorize` SpEL is passing a single UUID where
AuthorizationService.isAuthorized expects List<UUID>; in MonitorController
update the expressions to wrap single UUIDs as lists (use
T(java.util.List).of(...)) when calling AuthorizationService.isAuthorized so
replace uses of `#parentDirectoryId` and `#id` with list-wrapped forms (e.g.,
T(java.util.List).of(`#parentDirectoryId`) and T(java.util.List).of(`#id`)) to match
the method signature and avoid runtime type mismatches.
In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java`:
- Line 58: The method duplicateProcessConfig has a typo in its parameter name
(sourcePrcoessConfigUuid); rename the parameter to sourceProcessConfigUuid
everywhere in the method signature and all usages within the class (and related
callers if any) so the symbol duplicateProcessConfig(UUID
sourceProcessConfigUuid) compiles and reads correctly; update method
implementations, internal references, and any tests or callers that pass the
incorrectly spelled variable name to use the corrected identifier.
- Around line 68-75: The delete method in MonitorService currently ignores the
userId parameter when building the HTTP request; modify the delete(UUID id,
String userId) implementation to add the userId to the HttpHeaders (e.g.,
headers.set("userId", userId) or equivalent) before creating the HttpEntity and
calling restTemplate.exchange, so the outgoing DELETE request includes the same
userId header behavior as NetworkModificationService.delete and any other
callers that require authentication/authorization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31b535e8-d082-490f-a5ac-2297c91efa1a
📒 Files selected for processing (6)
src/main/java/org/gridsuite/explore/server/MonitorController.javasrc/main/java/org/gridsuite/explore/server/services/DirectoryService.javasrc/main/java/org/gridsuite/explore/server/services/ExploreService.javasrc/main/java/org/gridsuite/explore/server/services/MonitorService.javasrc/main/resources/application-local.ymlsrc/test/java/org/gridsuite/explore/server/ExploreTest.java
| @PostMapping(value = "/process-config", consumes = MediaType.APPLICATION_JSON_VALUE) | ||
| @Operation(summary = "Create a process config") | ||
| @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Process config has been successfully created")}) | ||
| @PreAuthorize("@authorizationService.isAuthorized(#userId, #parentDirectoryId, null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for `@PreAuthorize` patterns using isAuthorized in the codebase
rg -n "@PreAuthorize.*isAuthorized" --type=java -A1 -B1Repository: gridsuite/explore-server
Length of output: 23069
🏁 Script executed:
find . -type f -name "AuthorizationService.java" | head -5Repository: gridsuite/explore-server
Length of output: 147
🏁 Script executed:
rg -n "isAuthorized.*\(.*UUID.*\)" --type=java -B2 -A5 | head -50Repository: gridsuite/explore-server
Length of output: 2040
🏁 Script executed:
rg -n "public.*isAuthorized" src/main/java/org/gridsuite/explore/server/services/AuthorizationService.java -A2Repository: gridsuite/explore-server
Length of output: 699
Type mismatch in @PreAuthorize annotations will cause runtime errors.
AuthorizationService.isAuthorized expects List<UUID> as the second parameter (see its signature at line 28), but the SpEL expressions pass a single UUID (#parentDirectoryId and #id). This will cause a runtime type mismatch error. Wrap single UUIDs in a list using T(java.util.List).of().
🐛 Proposed fix
`@PostMapping`(value = "/process-config", consumes = MediaType.APPLICATION_JSON_VALUE)
`@Operation`(summary = "Create a process config")
`@ApiResponses`(value = {`@ApiResponse`(responseCode = "200", description = "Process config has been successfully created")})
-@PreAuthorize("@authorizationService.isAuthorized(`#userId`, `#parentDirectoryId`, null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")
+@PreAuthorize("@authorizationService.isAuthorized(`#userId`, T(java.util.List).of(`#parentDirectoryId`), null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")
public ResponseEntity<Void> createProcessConfig(`@RequestParam`(QUERY_PARAM_NAME) String name, `@PutMapping`(value = "/process-config/{id}", consumes = MediaType.APPLICATION_JSON_VALUE)
`@Operation`(summary = "Modify a process config")
`@ApiResponses`(value = {`@ApiResponse`(responseCode = "200", description = "Process config has been successfully modified")})
-@PreAuthorize("@authorizationService.isAuthorized(`#userId`, `#id`, null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")
+@PreAuthorize("@authorizationService.isAuthorized(`#userId`, T(java.util.List).of(`#id`), null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")
public ResponseEntity<Void> updateProcessConfig(`@PathVariable` UUID id,Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/gridsuite/explore/server/MonitorController.java` at line
46, The `@PreAuthorize` SpEL is passing a single UUID where
AuthorizationService.isAuthorized expects List<UUID>; in MonitorController
update the expressions to wrap single UUIDs as lists (use
T(java.util.List).of(...)) when calling AuthorizationService.isAuthorized so
replace uses of `#parentDirectoryId` and `#id` with list-wrapped forms (e.g.,
T(java.util.List).of(`#parentDirectoryId`) and T(java.util.List).of(`#id`)) to match
the method signature and avoid runtime type mismatches.
src/main/java/org/gridsuite/explore/server/services/MonitorService.java
Outdated
Show resolved
Hide resolved
Comment on lines
+68
to
+75
| @Override | ||
| public void delete(UUID id, String userId) { | ||
| String path = UriComponentsBuilder.fromPath(DELIMITER + MONITOR_API_VERSION + DELIMITER + PROCESS_CONFIG_PATH + DELIMITER + "{id}") | ||
| .buildAndExpand(id) | ||
| .toUriString(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| restTemplate.exchange(monitorServerBaseUri + path, HttpMethod.DELETE, new HttpEntity<>(headers), Void.class); | ||
| } |
There was a problem hiding this comment.
Missing userId header in delete request.
The delete method receives userId as a parameter but doesn't include it in the request headers. Other implementations like NetworkModificationService.delete (see context snippet 2) add the userId header. If the monitor-server requires authentication/authorization, this omission will cause issues.
🛡️ Proposed fix to add userId header
`@Override`
public void delete(UUID id, String userId) {
String path = UriComponentsBuilder.fromPath(DELIMITER + MONITOR_API_VERSION + DELIMITER + PROCESS_CONFIG_PATH + DELIMITER + "{id}")
.buildAndExpand(id)
.toUriString();
HttpHeaders headers = new HttpHeaders();
+ headers.add(HEADER_USER_ID, userId);
restTemplate.exchange(monitorServerBaseUri + path, HttpMethod.DELETE, new HttpEntity<>(headers), Void.class);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java`
around lines 68 - 75, The delete method in MonitorService currently ignores the
userId parameter when building the HTTP request; modify the delete(UUID id,
String userId) implementation to add the userId to the HttpHeaders (e.g.,
headers.set("userId", userId) or equivalent) before creating the HttpEntity and
calling restTemplate.exchange, so the outgoing DELETE request includes the same
userId header behavior as NetworkModificationService.delete and any other
callers that require authentication/authorization.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main/java/org/gridsuite/explore/server/services/MonitorService.java (1)
68-75:⚠️ Potential issue | 🟠 MajorMissing
userIdheader in delete request breaks contract with sibling services.The
deletemethod receivesuserIdas a parameter but doesn't include it in the request headers. All other implementations ofIDirectoryElementsService(e.g.,ContingencyListService,StudyService) addheaders.add(HEADER_USER_ID, userId). This omission will cause authorization issues if the monitor-server requires it.,
🛡️ Proposed fix to add userId header
`@Override` public void delete(UUID id, String userId) { String path = UriComponentsBuilder.fromPath(DELIMITER + MONITOR_API_VERSION + DELIMITER + PROCESS_CONFIGS_PATH + DELIMITER + "{id}") .buildAndExpand(id) .toUriString(); HttpHeaders headers = new HttpHeaders(); + headers.add(HEADER_USER_ID, userId); restTemplate.exchange(monitorServerBaseUri + path, HttpMethod.DELETE, new HttpEntity<>(headers), Void.class); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java` around lines 68 - 75, The delete method in MonitorService is missing the userId header, breaking the IDirectoryElementsService contract; update MonitorService.delete(UUID id, String userId) to create HttpHeaders, add headers.add(HEADER_USER_ID, userId) (same header constant used by ContingencyListService/StudyService), include those headers in the HttpEntity passed to restTemplate.exchange, and ensure the request still uses HttpMethod.DELETE and Void.class for the response.src/main/java/org/gridsuite/explore/server/MonitorController.java (2)
59-59:⚠️ Potential issue | 🔴 CriticalType mismatch in
@PreAuthorizewill cause runtime errors.Same issue as above:
#idis a singleUUIDbutisAuthorizedexpectsList<UUID>.,
🐛 Proposed fix
- `@PreAuthorize`("@authorizationService.isAuthorized(`#userId`, `#id`, null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)") + `@PreAuthorize`("@authorizationService.isAuthorized(`#userId`, T(java.util.List).of(`#id`), null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/gridsuite/explore/server/MonitorController.java` at line 59, The PreAuthorize SpEL passes a UUID (`#id`) to authorizationService.isAuthorized which expects a List<UUID>; update the annotation on MonitorController so it wraps the single id into a List (for example use T(java.util.Collections).singletonList(`#id`) or another Collections helper) when calling isAuthorized, leaving the other arguments and PermissionType unchanged.
46-46:⚠️ Potential issue | 🔴 CriticalType mismatch in
@PreAuthorizewill cause runtime errors.
AuthorizationService.isAuthorizedexpectsList<UUID>as the second parameter, but#parentDirectoryIdis a singleUUID. Wrap it in a list.,
🐛 Proposed fix
- `@PreAuthorize`("@authorizationService.isAuthorized(`#userId`, `#parentDirectoryId`, null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)") + `@PreAuthorize`("@authorizationService.isAuthorized(`#userId`, T(java.util.List).of(`#parentDirectoryId`), null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/gridsuite/explore/server/MonitorController.java` at line 46, The `@PreAuthorize` call in MonitorController uses AuthorizationService.isAuthorized with a second argument `#parentDirectoryId` which is a single UUID but the method expects List<UUID>; update the SpEL expression to wrap the UUID in a List, e.g. replace "#parentDirectoryId" with "T(java.util.Arrays).asList(`#parentDirectoryId`)" so the call becomes `@PreAuthorize`("@authorizationService.isAuthorized(`#userId`, T(java.util.Arrays).asList(`#parentDirectoryId`), null, T(org.gridsuite.explore.server.dto.PermissionType).WRITE)"); this keeps the PermissionType.WRITE usage and targets the AuthorizationService.isAuthorized invocation.
🧹 Nitpick comments (2)
src/test/java/org/gridsuite/explore/server/MonitorTest.java (1)
96-117: Consider verifying the request body content.The test uses
verifyPostRequest(stubId, URL_PROCESS_CONFIGS, Map.of(), false)withfalsefor body verification, which means it doesn't validate thatPROCESS_CONFIGwas sent in the request body. Consider adding body verification to ensure the process config content is correctly forwarded to the monitor-server.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/gridsuite/explore/server/MonitorTest.java` around lines 96 - 117, In the createProcessConfig test, the verifyPostRequest call is currently skipping body verification; update the test to assert that PROCESS_CONFIG was sent by either changing verifyPostRequest(stubId, URL_PROCESS_CONFIGS, Map.of(), false) to include the expected body (e.g., the serialized PROCESS_CONFIG) and set the boolean to true, or call the overload that verifies request body content; ensure you reference the same stubId and URL_PROCESS_CONFIGS used to create the WireMock stub and keep other assertions (elementAttributesCaptor, directoryService checks) intact so the test verifies the forwarded request body matches PROCESS_CONFIG.src/main/java/org/gridsuite/explore/server/services/MonitorService.java (1)
40-66: Consider addinguserIdheader to create/update/duplicate methods for consistency.The
createProcessConfig,updateProcessConfig, andduplicateProcessConfigmethods don't include auserIdheader. If the monitor-server performs authorization checks, you may need to add this header to these methods as well, similar to how other services handle authenticated requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java` around lines 40 - 66, createProcessConfig, updateProcessConfig, and duplicateProcessConfig are missing the userId request header used elsewhere for authenticated calls; add the same userId header to each method by populating the HttpHeaders (headers.set("userId", <current user id>)) before constructing the HttpEntity and sending the request (use the same helper or SecurityContext call your project uses to obtain the current user id so the header value matches other service calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/gridsuite/explore/server/MonitorController.java`:
- Line 43: The `@PostMapping` annotation in MonitorController is mis-indented at
column 0; adjust the indentation to match the surrounding class members (use 4
spaces) so the annotation and the following method declaration align with the
class's member indentation style (locate the `@PostMapping`(value =
"/process-configs", consumes = MediaType.APPLICATION_JSON_VALUE) above the
corresponding controller method in class MonitorController and indent it by 4
spaces).
In `@src/test/java/org/gridsuite/explore/server/MonitorTest.java`:
- Around line 88-94: Add an `@AfterEach` tearDown method to stop the
WireMockServer to prevent resource leaks: implement a method named tearDown()
that calls wireMockServer.stop() and annotate it with `@AfterEach` (matching the
existing setUp() that starts WireMockServer), and add the import for
org.junit.jupiter.api.AfterEach; ensure it references the same wireMockServer
field used in setUp() so the server is always stopped after each test.
---
Duplicate comments:
In `@src/main/java/org/gridsuite/explore/server/MonitorController.java`:
- Line 59: The PreAuthorize SpEL passes a UUID (`#id`) to
authorizationService.isAuthorized which expects a List<UUID>; update the
annotation on MonitorController so it wraps the single id into a List (for
example use T(java.util.Collections).singletonList(`#id`) or another Collections
helper) when calling isAuthorized, leaving the other arguments and
PermissionType unchanged.
- Line 46: The `@PreAuthorize` call in MonitorController uses
AuthorizationService.isAuthorized with a second argument `#parentDirectoryId`
which is a single UUID but the method expects List<UUID>; update the SpEL
expression to wrap the UUID in a List, e.g. replace "#parentDirectoryId" with
"T(java.util.Arrays).asList(`#parentDirectoryId`)" so the call becomes
`@PreAuthorize`("@authorizationService.isAuthorized(`#userId`,
T(java.util.Arrays).asList(`#parentDirectoryId`), null,
T(org.gridsuite.explore.server.dto.PermissionType).WRITE)"); this keeps the
PermissionType.WRITE usage and targets the AuthorizationService.isAuthorized
invocation.
In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java`:
- Around line 68-75: The delete method in MonitorService is missing the userId
header, breaking the IDirectoryElementsService contract; update
MonitorService.delete(UUID id, String userId) to create HttpHeaders, add
headers.add(HEADER_USER_ID, userId) (same header constant used by
ContingencyListService/StudyService), include those headers in the HttpEntity
passed to restTemplate.exchange, and ensure the request still uses
HttpMethod.DELETE and Void.class for the response.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java`:
- Around line 40-66: createProcessConfig, updateProcessConfig, and
duplicateProcessConfig are missing the userId request header used elsewhere for
authenticated calls; add the same userId header to each method by populating the
HttpHeaders (headers.set("userId", <current user id>)) before constructing the
HttpEntity and sending the request (use the same helper or SecurityContext call
your project uses to obtain the current user id so the header value matches
other service calls).
In `@src/test/java/org/gridsuite/explore/server/MonitorTest.java`:
- Around line 96-117: In the createProcessConfig test, the verifyPostRequest
call is currently skipping body verification; update the test to assert that
PROCESS_CONFIG was sent by either changing verifyPostRequest(stubId,
URL_PROCESS_CONFIGS, Map.of(), false) to include the expected body (e.g., the
serialized PROCESS_CONFIG) and set the boolean to true, or call the overload
that verifies request body content; ensure you reference the same stubId and
URL_PROCESS_CONFIGS used to create the WireMock stub and keep other assertions
(elementAttributesCaptor, directoryService checks) intact so the test verifies
the forwarded request body matches PROCESS_CONFIG.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 631352ef-8928-45de-88c4-d001eb94d7f4
📒 Files selected for processing (3)
src/main/java/org/gridsuite/explore/server/MonitorController.javasrc/main/java/org/gridsuite/explore/server/services/MonitorService.javasrc/test/java/org/gridsuite/explore/server/MonitorTest.java
src/main/java/org/gridsuite/explore/server/MonitorController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/explore/server/MonitorTest.java (1)
102-164: Add a direct test for the monitor delete endpoint.This class covers create/update/duplicate, but not delete. Since this PR introduces process-config management endpoints, adding an explicit delete test here would close the contract gap and prevent regressions on
/v1/explore/monitor/process-configs/{id}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/gridsuite/explore/server/MonitorTest.java` around lines 102 - 164, Add a new test method in MonitorTest: stub a WireMock DELETE for URL_PROCESS_CONFIGS + "/" + ID returning ok and capture its UUID; perform mockMvc.delete(URL_EXPLORE_MONITOR_PROCESS_CONFIGS + "/" + ID).header(QUERY_PARAM_USER_ID, USER_ID).andExpect(status().isOk()); then verify directoryService.checkPermission(List.of(ID), null, USER_ID, PermissionType.WRITE) and verify(directoryService, times(1)).deleteElement(eq(ID), eq(USER_ID)); finally call wireMockUtils.verifyDeleteRequest(stubId, URL_PROCESS_CONFIGS + "/" + ID, Map.of(), false) to assert the downstream delete call. Ensure you use the existing ID, USER_ID, URL_PROCESS_CONFIGS and URL_EXPLORE_MONITOR_PROCESS_CONFIGS constants and the same mocking style as other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/org/gridsuite/explore/server/MonitorTest.java`:
- Around line 102-164: Add a new test method in MonitorTest: stub a WireMock
DELETE for URL_PROCESS_CONFIGS + "/" + ID returning ok and capture its UUID;
perform mockMvc.delete(URL_EXPLORE_MONITOR_PROCESS_CONFIGS + "/" +
ID).header(QUERY_PARAM_USER_ID, USER_ID).andExpect(status().isOk()); then verify
directoryService.checkPermission(List.of(ID), null, USER_ID,
PermissionType.WRITE) and verify(directoryService,
times(1)).deleteElement(eq(ID), eq(USER_ID)); finally call
wireMockUtils.verifyDeleteRequest(stubId, URL_PROCESS_CONFIGS + "/" + ID,
Map.of(), false) to assert the downstream delete call. Ensure you use the
existing ID, USER_ID, URL_PROCESS_CONFIGS and
URL_EXPLORE_MONITOR_PROCESS_CONFIGS constants and the same mocking style as
other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0be83ec-c3ac-439f-90af-af5d8d220c9c
📒 Files selected for processing (3)
src/test/java/org/gridsuite/explore/server/ExploreTest.javasrc/test/java/org/gridsuite/explore/server/MonitorTest.javasrc/test/java/org/gridsuite/explore/server/SingleLineDiagramTest.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/explore/server/services/MonitorService.java (1)
71-78:⚠️ Potential issue | 🟠 MajorPropagate
userIdheader in delete request.
Line 72receivesuserId, butLine 76–Line 77sends empty headers. This can break auth/audit behavior expected by downstream services.🔧 Proposed fix
`@Override` public void delete(UUID id, String userId) { String path = UriComponentsBuilder.fromPath(PROCESS_CONFIGS_PATH + DELIMITER + "{id}") .buildAndExpand(id) .toUriString(); HttpHeaders headers = new HttpHeaders(); + headers.add("userId", userId); restTemplate.exchange(monitorServerBaseUri + path, HttpMethod.DELETE, new HttpEntity<>(headers), Void.class); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java` around lines 71 - 78, The delete(UUID id, String userId) method builds the DELETE request but sends empty headers; populate the HttpHeaders with the userId header before calling restTemplate.exchange so downstream auth/audit receives it. Locate the delete method and the HttpHeaders instance, add headers.set("userId", userId) (or use the existing header constant if one exists), then pass the HttpEntity<>(headers) as currently done to restTemplate.exchange(PROCESS_CONFIGS_PATH + DELIMITER...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/gridsuite/explore/server/services/MonitorService.java`:
- Around line 71-78: The delete(UUID id, String userId) method builds the DELETE
request but sends empty headers; populate the HttpHeaders with the userId header
before calling restTemplate.exchange so downstream auth/audit receives it.
Locate the delete method and the HttpHeaders instance, add headers.set("userId",
userId) (or use the existing header constant if one exists), then pass the
HttpEntity<>(headers) as currently done to
restTemplate.exchange(PROCESS_CONFIGS_PATH + DELIMITER...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4185ead4-c258-4c44-861b-31662e30fde6
📒 Files selected for processing (2)
src/main/java/org/gridsuite/explore/server/services/MonitorService.javasrc/test/java/org/gridsuite/explore/server/ExploreTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/org/gridsuite/explore/server/ExploreTest.java
khouadrired
reviewed
Mar 25, 2026
| } | ||
|
|
||
| public void createProcessConfig(String name, String processConfig, String description, String userId, UUID parentDirectoryUuid) { | ||
| UUID processConfigUuid = monitorService.createProcessConfig(processConfig); |
Contributor
There was a problem hiding this comment.
handle the use case where the call to monitor fails
There was a problem hiding this comment.
We use a custom exception handling mechanism based on a ControllerAdvice, implemented in the following dependency:
https://github.com/powsybl/powsybl-ws-commons/blob/main/src/main/java/com/powsybl/ws/commons/error/BaseExceptionHandler.java
The powsybl-ws-commons repository primarily provides shared Spring configurations across various Powsybl and GridSuite backend services.
For more details, you can refer to the documentation available on GoPro:
https://gopro-collaboratif.rte-france.com/spaces/GRD/pages/509641848/Guidelines+gestion+des+erreurs
In summary, HTTP exceptions are handled automatically by this advisor.
There was a problem hiding this comment.
@carojeandat We could also start implementing business error management (i.e, with translation support) in the monitor servers. This is not a heavy change, so it can be included in the same PR for this exception:
https://github.com/gridsuite/monitor-core/pull/74/changes#diff-1f2fe7a6ce6fd53a18803e2090bd2c303f8f7010f21bc5bda4e9511935c16b7dR48
See package error in others servers as the references
khouadrired
reviewed
Mar 25, 2026
khouadrired
reviewed
Mar 25, 2026
khouadrired
reviewed
Mar 25, 2026
src/main/java/org/gridsuite/explore/server/services/MonitorService.java
Outdated
Show resolved
Hide resolved
khouadrired
reviewed
Mar 25, 2026
src/main/java/org/gridsuite/explore/server/MonitorController.java
Outdated
Show resolved
Hide resolved
khouadrired
reviewed
Mar 25, 2026
khouadrired
requested changes
Mar 25, 2026
khouadrired
reviewed
Mar 26, 2026
src/main/java/org/gridsuite/explore/server/MonitorController.java
Outdated
Show resolved
Hide resolved
|
khouadrired
approved these changes
Mar 27, 2026
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
3 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
New endpoints to create, modify, duplicate and delete a Process Config