Handle async network modification application results#994
Open
ayolab wants to merge 4 commits into
Open
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThis PR refactors network modification workflows from synchronous result handling to asynchronous message-driven completion. The changes introduce ChangesAsynchronous modification result messaging
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.43.0)src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/test/java/org/gridsuite/study/server/NetworkModificationTest.javaThanks 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: 6
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/study/server/NetworkModificationTest.java (1)
695-703: 🏗️ Heavy liftExtract the repeated async-modification assertion flow into a helper.
The same sequence is duplicated throughout this file (request → in-progress assertions →
sendApplicationResult(...)→ finished assertions). Centralizing it would reduce brittleness when message ordering/contracts evolve.♻️ Helper sketch
+private void assertAsyncModificationFlow( + UUID studyUuid, + UUID nodeUuid, + Runnable triggerRequest, + Runnable preResultAssertions, + Runnable postResultAssertions) { + triggerRequest.run(); + checkUpdateStatusMessagesReceived(studyUuid, nodeUuid, output); + checkEquipmentCreatingMessagesReceived(studyUuid, nodeUuid); + preResultAssertions.run(); + studyTestUtils.sendApplicationResult(studyUuid, nodeUuid); + checkEquipmentUpdatingFinishedMessagesReceived(studyUuid, nodeUuid); + postResultAssertions.run(); +}🤖 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/NetworkModificationTest.java` around lines 695 - 703, Extract the repeated async-modification flow into a single helper method (e.g., assertAsyncModificationFlow or performAsyncModificationAndAssert) that encapsulates the POST call and the four assertions: mockMvc.perform(post(URI_NETWORK_MODIF, studyNameUserIdUuid, modificationNode1Uuid).content(bodyJson).contentType(MediaType.APPLICATION_JSON).header(USER_ID_HEADER, userId)).andExpect(status().isOk()), checkUpdateStatusMessagesReceived(studyNameUserIdUuid, modificationNode1Uuid, output), checkEquipmentCreatingMessagesReceived(studyNameUserIdUuid, modificationNode1Uuid), studyTestUtils.sendApplicationResult(studyNameUserIdUuid, modificationNode1Uuid), and checkEquipmentUpdatingFinishedMessagesReceived(studyNameUserIdUuid, modificationNode1Uuid); make the helper accept the needed parameters (studyNameUserIdUuid, modificationNode1Uuid, bodyJson, userId, output) and replace all duplicated sequences in NetworkModificationTest with calls to this new helper.
🤖 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/ConsumerService.java`:
- Around line 223-229: The catch blocks in consumeApplicationResult and
consumeApplicationFailed currently swallow all exceptions by logging only
e.toString(), which prevents retries/DLQ; update those methods (and other
consumer handlers named consumeQueuedCommand, consumeSessionStart,
consumeSessionEnd, consumeApplicationResult, consumeApplicationFailed, etc.) to
rethrow the exception after logging (or wrap and throw a RuntimeException)
instead of returning normally—locate the try/catch around objectMapper.readValue
and studyService.handleApplicationResult/handleApplicationFailed and replace the
silent LOGGER.error(e.toString()) with LOGGER.error(..., e) followed by throw e;
or if checked, throw new RuntimeException("Error processing consumer message",
e) so the framework can trigger retries/DLQ.
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 2433-2441: The loop in handleApplicationResult assumes
modificationResults() and receiver.rootNetworkUuids() are the same size and may
skip emitApplicationEnd if an index error or emitNetworkModificationImpacts
throws; validate sizes first (compute minSize =
Math.min(modificationResults.size(), rootNetworkUuids.size())) and iterate only
to minSize, and wrap the per-item call to emitNetworkModificationImpacts in a
try/catch so one failure doesn’t abort the loop; move the
emitApplicationEnd(receiver) call into a finally block so it always runs
regardless of exceptions, referencing the existing methods modificationResults,
rootNetworkUuids, emitNetworkModificationImpacts, and emitApplicationEnd.
- Around line 3385-3387: The finally block currently calls
networkModificationTreeService.unblockNodeTree(rootNetworkUuid, nodeUuid)
immediately, but duplicateModificationsFromGroup(...) is now asynchronous so
unblocking there can race with the remote apply; remove the unblock from the
finally and instead invoke
networkModificationTreeService.unblockNodeTree(rootNetworkUuid, nodeUuid) inside
the async duplicateModificationsFromGroup(...) completion handlers (both success
and failure paths) so the tree is only unblocked after the async apply finishes;
locate the callsite invoking duplicateModificationsFromGroup(...) and move the
unblock logic into its async callback/then/exception handling blocks.
- Around line 2501-2515: In copyModificationsToExclude, before doing positional
mapping into mappingModificationsUuids, validate that savedUuids.size() ==
modificationsUuids.size() and copyChildren.size() == originalChildren.size(); if
either size differs, fail fast (throw an IllegalStateException or a
domain-specific exception) with a clear message identifying the mismatched
lists; then proceed to build the maps using the verified lists (references:
method copyModificationsToExclude, variables mappingModificationsUuids,
modificationsUuids, savedUuids, originalChildren, copyChildren, and calls to
networkModificationService.findAllChildrenUuids).
In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java`:
- Around line 2228-2229: The test sends two modification UUIDs via copyUuids2
but only supplies one result entry, so update the call to
studyTestUtils.sendApplicationResult to provide a result element for each UUID
(e.g., pass a List with two Optional<NetworkModificationResult> entries) so
NetworkModificationsResult aligns with copyUuids2; adjust the List.of(...)
argument currently wrapping
Optional.of(NetworkModificationResult.builder().build()) to include a second
Optional (or an explicit Optional.empty() if intended) so
checkEquipmentMessagesReceived receives payloads for both duplicated
modification UUIDs.
In `@src/test/java/org/gridsuite/study/server/utils/TestUtils.java`:
- Around line 114-117: The test helper currently injects InputDestination
optionally which leads to a late NPE in sendApplicationResult(...) when the test
binder is missing; make the dependency required so startup fails fast (change
the setter injection for InputDestination in setInput(InputDestination) to
`@Autowired` with required=true or drop required=false) or alternatively add a
`@PostConstruct` check that throws an IllegalStateException with a clear message
referencing sendApplicationResult when input is null; apply the same pattern to
the analogous setter/field referenced around lines 139-145 so both input/output
test-binder dependencies fail fast with a clear setup error.
---
Nitpick comments:
In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java`:
- Around line 695-703: Extract the repeated async-modification flow into a
single helper method (e.g., assertAsyncModificationFlow or
performAsyncModificationAndAssert) that encapsulates the POST call and the four
assertions: mockMvc.perform(post(URI_NETWORK_MODIF, studyNameUserIdUuid,
modificationNode1Uuid).content(bodyJson).contentType(MediaType.APPLICATION_JSON).header(USER_ID_HEADER,
userId)).andExpect(status().isOk()),
checkUpdateStatusMessagesReceived(studyNameUserIdUuid, modificationNode1Uuid,
output), checkEquipmentCreatingMessagesReceived(studyNameUserIdUuid,
modificationNode1Uuid),
studyTestUtils.sendApplicationResult(studyNameUserIdUuid,
modificationNode1Uuid), and
checkEquipmentUpdatingFinishedMessagesReceived(studyNameUserIdUuid,
modificationNode1Uuid); make the helper accept the needed parameters
(studyNameUserIdUuid, modificationNode1Uuid, bodyJson, userId, output) and
replace all duplicated sequences in NetworkModificationTest with calls to this
new helper.
🪄 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: 077a9acf-f37f-45ca-8939-fa8e20c0e760
📒 Files selected for processing (11)
src/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/resources/config/application.yamlsrc/test/java/org/gridsuite/study/server/NetworkModificationTest.javasrc/test/java/org/gridsuite/study/server/VoltageInitTest.javasrc/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/StudyTest.javasrc/test/java/org/gridsuite/study/server/utils/TestUtils.java
ayolab
force-pushed
the
ayolab/handle-async-midification-application-results
branch
from
5ddd50d to
2f2e3d8
Compare
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/RebuildNodeService.java (1)
30-32:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDefer rebuilds until async application completion.
These entry points still run through
handleRebuildNode(...), which rebuilds previously built root networks immediately afterstudyService.createNetworkModification(...)/studyService.moveNetworkModifications(...)returns. After this refactor those calls only submit async application work, so the rebuild can now run before the modification result callback has applied the change and unblocked the tree. Please trigger the rebuild from the async completion path instead of keeping these operations under the synchronoushandleRebuildNode(...)flow.Also applies to: 37-42, 84-86, 123-131
🤖 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/RebuildNodeService.java` around lines 30 - 32, The current createNetworkModification (and the other similar entry points that call handleRebuildNode) invokes handleRebuildNode synchronously around the call to handleCreateNetworkModification, which causes rebuilds to run before the async application of the modification completes; instead remove the synchronous handleRebuildNode wrapper and invoke handleRebuildNode (or the rebuild logic) from the async completion/callback/future returned by the studyService call (the path that actually applies the modification result), so change createNetworkModification, moveNetworkModifications and the other listed methods to only submit the async work (call handleCreateNetworkModification / handleMoveNetworkModifications) and attach a continuation that calls handleRebuildNode(studyUuid, nodeUuid, userId) after the studyService.createNetworkModification / studyService.moveNetworkModifications completion promise/callback indicates the change is applied.
🧹 Nitpick comments (1)
src/main/java/org/gridsuite/study/server/service/StudyService.java (1)
2445-2457: 💤 Low valuePast review concern partially addressed:
finallyblock now ensures cleanup.The
emitApplicationEnd(receiver)is now in afinallyblock, so the node will always be unblocked even ifemitNetworkModificationImpactsthrows. However, ifmodificationResults.size() > rootNetworkUuids.size(), anIndexOutOfBoundsExceptionwould occur at line 2452. While cleanup will still happen, adding a size check would provide clearer error handling.🛡️ Optional defensive size validation
public void handleApplicationResult(ModificationReceiver receiver, NetworkModificationsResult result) { try { List<Optional<NetworkModificationResult>> modificationResults = result.modificationResults(); List<UUID> rootNetworkUuids = receiver.rootNetworkUuids(); + if (modificationResults.size() != rootNetworkUuids.size()) { + LOGGER.warn("Mismatch between modification results ({}) and root networks ({})", + modificationResults.size(), rootNetworkUuids.size()); + } + int limit = Math.min(modificationResults.size(), rootNetworkUuids.size()); - for (int i = 0; i < modificationResults.size(); i++) { + for (int i = 0; i < limit; i++) { final int index = i;🤖 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 2445 - 2457, In handleApplicationResult, guard against a possible IndexOutOfBoundsException when modificationResults.size() > rootNetworkUuids.size() by validating sizes before accessing rootNetworkUuids: compute the smaller of result.modificationResults().size() and receiver.rootNetworkUuids().size() (or explicitly check and log/throw a descriptive IllegalStateException) and iterate up to that bound when calling emitNetworkModificationImpacts(receiver.studyUuid(), receiver.nodeUuid(), rootNetworkUuids.get(index), r); keep the existing finally block with emitApplicationEnd(receiver) unchanged so cleanup still runs.
🤖 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/RebuildNodeService.java`:
- Around line 30-32: The current createNetworkModification (and the other
similar entry points that call handleRebuildNode) invokes handleRebuildNode
synchronously around the call to handleCreateNetworkModification, which causes
rebuilds to run before the async application of the modification completes;
instead remove the synchronous handleRebuildNode wrapper and invoke
handleRebuildNode (or the rebuild logic) from the async
completion/callback/future returned by the studyService call (the path that
actually applies the modification result), so change createNetworkModification,
moveNetworkModifications and the other listed methods to only submit the async
work (call handleCreateNetworkModification / handleMoveNetworkModifications) and
attach a continuation that calls handleRebuildNode(studyUuid, nodeUuid, userId)
after the studyService.createNetworkModification /
studyService.moveNetworkModifications completion promise/callback indicates the
change is applied.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 2445-2457: In handleApplicationResult, guard against a possible
IndexOutOfBoundsException when modificationResults.size() >
rootNetworkUuids.size() by validating sizes before accessing rootNetworkUuids:
compute the smaller of result.modificationResults().size() and
receiver.rootNetworkUuids().size() (or explicitly check and log/throw a
descriptive IllegalStateException) and iterate up to that bound when calling
emitNetworkModificationImpacts(receiver.studyUuid(), receiver.nodeUuid(),
rootNetworkUuids.get(index), r); keep the existing finally block with
emitApplicationEnd(receiver) unchanged so cleanup still runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 405080c3-8cfe-4e3c-9a8b-e4a0403e7d98
📒 Files selected for processing (13)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationService.javasrc/main/java/org/gridsuite/study/server/service/RebuildNodeService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/resources/config/application.yamlsrc/test/java/org/gridsuite/study/server/NetworkModificationTest.javasrc/test/java/org/gridsuite/study/server/VoltageInitTest.javasrc/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/StudyTest.javasrc/test/java/org/gridsuite/study/server/utils/TestUtils.java
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.java
- src/main/resources/config/application.yaml
- src/test/java/org/gridsuite/study/server/utils/TestUtils.java
- src/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.java
- src/test/java/org/gridsuite/study/server/studycontroller/StudyTest.java
- src/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.java
- src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
- src/main/java/org/gridsuite/study/server/service/ConsumerService.java
- src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java
- src/test/java/org/gridsuite/study/server/VoltageInitTest.java
Signed-off-by: Ayoub LABIDI <ayoub.labidi@protonmail.com>
ayolab
force-pushed
the
ayolab/handle-async-midification-application-results
branch
from
2f2e3d8 to
61d2ea7
Compare
EtienneLt
reviewed
Jun 10, 2026
| } | ||
| } | ||
|
|
||
| public void handleApplicationResult(ModificationReceiver receiver, NetworkModificationsResult result) { |
Contributor
There was a problem hiding this comment.
Suggested change
| public void handleApplicationResult(ModificationReceiver receiver, NetworkModificationsResult result) { | |
| public void handleModificationApplicationResult(ModificationReceiver receiver, NetworkModificationsResult result) { |
what do you think ?
| } | ||
|
|
||
| @Bean | ||
| public Consumer<Message<NetworkModificationsResult>> consumeApplicationResult() { |
Contributor
There was a problem hiding this comment.
consumeModificationApplicationResult ?
| ModificationReceiver receiverObj = objectMapper.readValue( | ||
| URLDecoder.decode(receiver, StandardCharsets.UTF_8), ModificationReceiver.class); | ||
| studyService.handleApplicationResult(receiverObj, message.getPayload()); | ||
| } catch (Exception e) { |
Contributor
There was a problem hiding this comment.
exception is not thrown ?
Contributor
There was a problem hiding this comment.
because for notification we just logs error ?
| } | ||
|
|
||
| @Bean | ||
| public Consumer<Message<String>> consumeApplicationFailed() { |
Contributor
There was a problem hiding this comment.
consumeModificationApplicationFailed ?
|
|
||
| public NetworkModificationsResult createModification(UUID groupUuid, | ||
| Pair<String, List<ModificationApplicationContext>> modificationContextInfos) { | ||
| public List<UUID> createModification(UUID groupUuid, ModificationReceiver receiver, |
Contributor
There was a problem hiding this comment.
the return of the method is never used, should it return void ?
Signed-off-by: Ayoub LABIDI <ayoub.labidi@protonmail.com>
Signed-off-by: Ayoub LABIDI <ayoub.labidi@protonmail.com>
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java (2)
139-149:⚠️ Potential issue | 🟠 Major | ⚡ Quick winComplete cleanup on receiver-null bulk moves. When
isTargetInDifferentNodeTreeis false,StudyServicepassesreceiver = null, so no async callback will run. The success path still needs synchronous cleanup.
src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java#L139-L149: after a successful receiver-null move, unblockoriginNodeUuidbecause the callback will not do it.src/main/java/org/gridsuite/study/server/service/StudyService.java#L2266-L2269: inside thereceiver == nullbranch, also emit the end notification fororiginNodeUuidwhenisTargetDifferentNode.🤖 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/RebuildNodeService.java` around lines 139 - 149, The issue spans two locations. In RebuildNodeService.java within the handleMoveNetworkModifications method, after the successful moveNetworkModifications call completes (before the catch block), add synchronous cleanup to unblock the originNodeUuid since the async callback will not execute when receiver is null. In StudyService.java within the receiver == null branch (around lines 2266-2269), add logic to emit the end notification for originNodeUuid when isTargetDifferentNode is true, to ensure cleanup occurs synchronously when no callback will run.
181-192:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDelay rebuilds until async modification application completes.
action.get()can now return right after scheduling an async modification apply, but this code immediately rebuilds previously built nodes. For create/cross-tree move flows, that can rebuild descendants before the parent node has received its async application result.Move the rebuild trigger into the async success path, or carry enough rebuild context in the async receiver/callback to rebuild only after
handleModificationApplicationResult(...)has applied impacts and cleaned up the node state.🤖 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/RebuildNodeService.java` around lines 181 - 192, The rebuild loop triggered by the forEach on rootNetworkUuidsByNodeBuilt executes immediately after action.get() returns, but this is too early when async modification application is still pending. The rebuild calls to studyService.buildNode need to be deferred until after the async modification has completed. Either move the entire forEach rebuild block into the async success callback path where it will execute after handleModificationApplicationResult has applied impacts and cleaned up node state, or capture the rootNetworkUuidsByNodeBuilt map and pass it through the async receiver/callback mechanism so the rebuild can be triggered at the correct time once the async processing finishes.
🤖 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 3709-3711: The call to doUnbuildNodeTree with
skipDeleteVariants=true skips variant deletion for all root networks in the
study, but rootNetworkService.invalidateRootNetworkRemoteInfos only invalidates
the specific rootNetworkUuid. This leaves stale variants on other root networks.
Modify the logic to skip variant deletion only for the specific rootNetworkUuid
being invalidated; call doUnbuildNodeTree once with skipDeleteVariants=true
specifically for that root network, and then call it again for any other root
networks with skipDeleteVariants=false to ensure their variants are properly
cleaned up.
- Around line 2320-2326: The finally block in the StudyService code only
unblocks the receiver's root network tree nodes but fails to unblock the origin
tree for cross-tree async moves. When receiver.originNodeUuid() is not null
(indicating a cross-tree move), the origin tree remains blocked after the
callback. In the finally block, after unblocking receiver.rootNetworkUuids(),
add additional logic to unblock the origin tree by calling
networkModificationTreeService.unblockNodeTree() with receiver.originNodeUuid()
and nodeUuid, but only when receiver.originNodeUuid() is not null. This ensures
both the origin and receiver trees are properly unblocked on async move
completion.
- Around line 3188-3193: The code conditionally deactivates modifications on
other root networks only when savedUuids contains exactly one element, but the
resetModificationsGroupUuid() call after this block executes unconditionally. If
duplicateModificationsFromGroup() returns 0 or more than 1 UUID, the
deactivation is silently skipped while the group reset still proceeds, leaving
modifications active on unintended root networks. Add a fail-fast validation
check immediately before the resetModificationsGroupUuid() call to ensure
savedUuids.size() equals 1, throwing an exception if the invariant is violated
instead of silently proceeding.
In `@src/test/java/org/gridsuite/study/server/WorkflowTest.java`:
- Around line 124-126: The tests are sending raw JSON strings in the
HEADER_RECEIVER headers, but ConsumerService expects URL-encoded content before
calling URLDecoder.decode(). At both locations (line 125 where
objectMapper.writeValueAsString(receiver) is written to the header, and line 138
with the same pattern), you need to URL-encode the JSON string before putting it
into the headers map. Apply URLEncoder.encode() to the
objectMapper.writeValueAsString(receiver) result to match the actual async wire
format that ConsumerService expects, ensuring the tests properly validate the
end-to-end contract.
---
Outside diff comments:
In `@src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java`:
- Around line 139-149: The issue spans two locations. In RebuildNodeService.java
within the handleMoveNetworkModifications method, after the successful
moveNetworkModifications call completes (before the catch block), add
synchronous cleanup to unblock the originNodeUuid since the async callback will
not execute when receiver is null. In StudyService.java within the receiver ==
null branch (around lines 2266-2269), add logic to emit the end notification for
originNodeUuid when isTargetDifferentNode is true, to ensure cleanup occurs
synchronously when no callback will run.
- Around line 181-192: The rebuild loop triggered by the forEach on
rootNetworkUuidsByNodeBuilt executes immediately after action.get() returns, but
this is too early when async modification application is still pending. The
rebuild calls to studyService.buildNode need to be deferred until after the
async modification has completed. Either move the entire forEach rebuild block
into the async success callback path where it will execute after
handleModificationApplicationResult has applied impacts and cleaned up node
state, or capture the rootNetworkUuidsByNodeBuilt map and pass it through the
async receiver/callback mechanism so the rebuild can be triggered at the correct
time once the async processing finishes.
🪄 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: ccde3891-fd4f-46df-ae62-a30fde5ad8de
📒 Files selected for processing (14)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationService.javasrc/main/java/org/gridsuite/study/server/service/RebuildNodeService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/resources/config/application.yamlsrc/test/java/org/gridsuite/study/server/NetworkModificationTest.javasrc/test/java/org/gridsuite/study/server/VoltageInitTest.javasrc/test/java/org/gridsuite/study/server/WorkflowTest.javasrc/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/StudyTest.javasrc/test/java/org/gridsuite/study/server/utils/TestUtils.java
🚧 Files skipped from review as they are similar to previous changes (9)
- src/main/java/org/gridsuite/study/server/controller/StudyController.java
- src/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.java
- src/test/java/org/gridsuite/study/server/studycontroller/StudyTest.java
- src/main/java/org/gridsuite/study/server/service/ConsumerService.java
- src/test/java/org/gridsuite/study/server/utils/TestUtils.java
- src/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.java
- src/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.java
- src/test/java/org/gridsuite/study/server/VoltageInitTest.java
- src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java (2)
139-149:⚠️ Potential issue | 🟠 Major | ⚡ Quick winComplete cleanup on receiver-null bulk moves. When
isTargetInDifferentNodeTreeis false,StudyServicepassesreceiver = null, so no async callback will run. The success path still needs synchronous cleanup.
src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java#L139-L149: after a successful receiver-null move, unblockoriginNodeUuidbecause the callback will not do it.src/main/java/org/gridsuite/study/server/service/StudyService.java#L2266-L2269: inside thereceiver == nullbranch, also emit the end notification fororiginNodeUuidwhenisTargetDifferentNode.🤖 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/RebuildNodeService.java` around lines 139 - 149, The issue spans two locations. In RebuildNodeService.java within the handleMoveNetworkModifications method, after the successful moveNetworkModifications call completes (before the catch block), add synchronous cleanup to unblock the originNodeUuid since the async callback will not execute when receiver is null. In StudyService.java within the receiver == null branch (around lines 2266-2269), add logic to emit the end notification for originNodeUuid when isTargetDifferentNode is true, to ensure cleanup occurs synchronously when no callback will run.
181-192:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDelay rebuilds until async modification application completes.
action.get()can now return right after scheduling an async modification apply, but this code immediately rebuilds previously built nodes. For create/cross-tree move flows, that can rebuild descendants before the parent node has received its async application result.Move the rebuild trigger into the async success path, or carry enough rebuild context in the async receiver/callback to rebuild only after
handleModificationApplicationResult(...)has applied impacts and cleaned up the node state.🤖 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/RebuildNodeService.java` around lines 181 - 192, The rebuild loop triggered by the forEach on rootNetworkUuidsByNodeBuilt executes immediately after action.get() returns, but this is too early when async modification application is still pending. The rebuild calls to studyService.buildNode need to be deferred until after the async modification has completed. Either move the entire forEach rebuild block into the async success callback path where it will execute after handleModificationApplicationResult has applied impacts and cleaned up node state, or capture the rootNetworkUuidsByNodeBuilt map and pass it through the async receiver/callback mechanism so the rebuild can be triggered at the correct time once the async processing finishes.
🤖 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 3709-3711: The call to doUnbuildNodeTree with
skipDeleteVariants=true skips variant deletion for all root networks in the
study, but rootNetworkService.invalidateRootNetworkRemoteInfos only invalidates
the specific rootNetworkUuid. This leaves stale variants on other root networks.
Modify the logic to skip variant deletion only for the specific rootNetworkUuid
being invalidated; call doUnbuildNodeTree once with skipDeleteVariants=true
specifically for that root network, and then call it again for any other root
networks with skipDeleteVariants=false to ensure their variants are properly
cleaned up.
- Around line 2320-2326: The finally block in the StudyService code only
unblocks the receiver's root network tree nodes but fails to unblock the origin
tree for cross-tree async moves. When receiver.originNodeUuid() is not null
(indicating a cross-tree move), the origin tree remains blocked after the
callback. In the finally block, after unblocking receiver.rootNetworkUuids(),
add additional logic to unblock the origin tree by calling
networkModificationTreeService.unblockNodeTree() with receiver.originNodeUuid()
and nodeUuid, but only when receiver.originNodeUuid() is not null. This ensures
both the origin and receiver trees are properly unblocked on async move
completion.
- Around line 3188-3193: The code conditionally deactivates modifications on
other root networks only when savedUuids contains exactly one element, but the
resetModificationsGroupUuid() call after this block executes unconditionally. If
duplicateModificationsFromGroup() returns 0 or more than 1 UUID, the
deactivation is silently skipped while the group reset still proceeds, leaving
modifications active on unintended root networks. Add a fail-fast validation
check immediately before the resetModificationsGroupUuid() call to ensure
savedUuids.size() equals 1, throwing an exception if the invariant is violated
instead of silently proceeding.
In `@src/test/java/org/gridsuite/study/server/WorkflowTest.java`:
- Around line 124-126: The tests are sending raw JSON strings in the
HEADER_RECEIVER headers, but ConsumerService expects URL-encoded content before
calling URLDecoder.decode(). At both locations (line 125 where
objectMapper.writeValueAsString(receiver) is written to the header, and line 138
with the same pattern), you need to URL-encode the JSON string before putting it
into the headers map. Apply URLEncoder.encode() to the
objectMapper.writeValueAsString(receiver) result to match the actual async wire
format that ConsumerService expects, ensuring the tests properly validate the
end-to-end contract.
---
Outside diff comments:
In `@src/main/java/org/gridsuite/study/server/service/RebuildNodeService.java`:
- Around line 139-149: The issue spans two locations. In RebuildNodeService.java
within the handleMoveNetworkModifications method, after the successful
moveNetworkModifications call completes (before the catch block), add
synchronous cleanup to unblock the originNodeUuid since the async callback will
not execute when receiver is null. In StudyService.java within the receiver ==
null branch (around lines 2266-2269), add logic to emit the end notification for
originNodeUuid when isTargetDifferentNode is true, to ensure cleanup occurs
synchronously when no callback will run.
- Around line 181-192: The rebuild loop triggered by the forEach on
rootNetworkUuidsByNodeBuilt executes immediately after action.get() returns, but
this is too early when async modification application is still pending. The
rebuild calls to studyService.buildNode need to be deferred until after the
async modification has completed. Either move the entire forEach rebuild block
into the async success callback path where it will execute after
handleModificationApplicationResult has applied impacts and cleaned up node
state, or capture the rootNetworkUuidsByNodeBuilt map and pass it through the
async receiver/callback mechanism so the rebuild can be triggered at the correct
time once the async processing finishes.
🪄 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: ccde3891-fd4f-46df-ae62-a30fde5ad8de
📒 Files selected for processing (14)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationService.javasrc/main/java/org/gridsuite/study/server/service/RebuildNodeService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/resources/config/application.yamlsrc/test/java/org/gridsuite/study/server/NetworkModificationTest.javasrc/test/java/org/gridsuite/study/server/VoltageInitTest.javasrc/test/java/org/gridsuite/study/server/WorkflowTest.javasrc/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.javasrc/test/java/org/gridsuite/study/server/studycontroller/StudyTest.javasrc/test/java/org/gridsuite/study/server/utils/TestUtils.java
🚧 Files skipped from review as they are similar to previous changes (9)
- src/main/java/org/gridsuite/study/server/controller/StudyController.java
- src/main/java/org/gridsuite/study/server/dto/modification/ModificationReceiver.java
- src/test/java/org/gridsuite/study/server/studycontroller/StudyTest.java
- src/main/java/org/gridsuite/study/server/service/ConsumerService.java
- src/test/java/org/gridsuite/study/server/utils/TestUtils.java
- src/test/java/org/gridsuite/study/server/studycontroller/NodeControllerTest.java
- src/test/java/org/gridsuite/study/server/rootnetworks/ModificationToExcludeTest.java
- src/test/java/org/gridsuite/study/server/VoltageInitTest.java
- src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
🛑 Comments failed to post (4)
src/main/java/org/gridsuite/study/server/service/StudyService.java (3)
2320-2326:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnblock the origin tree on async move completion.
This ends origin-node notifications, but the
finallyonly unblocksreceiver.nodeUuid(). For cross-tree async moves, the origin tree was invalidated before the request andRebuildNodeServiceno longer unblocks it on success, so the origin can remain blocked after the callback.Suggested fix
} finally { - receiver.rootNetworkUuids().forEach(rn -> networkModificationTreeService.unblockNodeTree(rn, nodeUuid)); + receiver.rootNetworkUuids().forEach(rn -> { + networkModificationTreeService.unblockNodeTree(rn, nodeUuid); + if (receiver.originNodeUuid() != null) { + networkModificationTreeService.unblockNodeTree(rn, receiver.originNodeUuid()); + } + }); }🤖 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 2320 - 2326, The finally block in the StudyService code only unblocks the receiver's root network tree nodes but fails to unblock the origin tree for cross-tree async moves. When receiver.originNodeUuid() is not null (indicating a cross-tree move), the origin tree remains blocked after the callback. In the finally block, after unblocking receiver.rootNetworkUuids(), add additional logic to unblock the origin tree by calling networkModificationTreeService.unblockNodeTree() with receiver.originNodeUuid() and nodeUuid, but only when receiver.originNodeUuid() is not null. This ensures both the origin and receiver trees are properly unblocked on async move completion.
3188-3193:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect declared Java/toolchain targets before relying on List.getFirst(). set -euo pipefail fd -HI '^(pom.xml|build.gradle|build.gradle.kts|gradle.properties|\.java-version|toolchains.xml|Dockerfile.*|.*\.ya?ml)$' \ | xargs -r rg -n -C2 '(<maven\.compiler\.(source|target|release)>|<release>|sourceCompatibility|targetCompatibility|languageVersion|java-version|temurin|openjdk|eclipse-temurin|JAVA_VERSION)'Repository: gridsuite/study-server
Length of output: 50
🏁 Script executed:
# Find and inspect the StudyService.java file fd -H 'StudyService\.java' | head -5Repository: gridsuite/study-server
Length of output: 133
🏁 Script executed:
# Once we find the file, read the relevant lines with context sed -n '3170,3210p' "$(fd -H 'StudyService\.java' | head -1)"Repository: gridsuite/study-server
Length of output: 2719
🏁 Script executed:
# Search for Java version configuration in build files fd -H '(pom\.xml|build\.gradle|\.java-version)' | head -10Repository: gridsuite/study-server
Length of output: 74
🏁 Script executed:
# Read pom.xml to check Java version configuration head -100 "$(fd -H 'pom\.xml' | head -1)" | rg -A5 -B5 '(maven\.compiler\.|<java|<release>|sourceCompatibility|targetCompatibility)'Repository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Also check if there are other build files fd -H '(build\.gradle|gradle\.properties)'Repository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Read the full pom.xml to find Java version configuration cat "$(fd -H 'pom\.xml' | head -1)" | head -200Repository: gridsuite/study-server
Length of output: 7637
🏁 Script executed:
# Search for Java version markers anywhere in pom.xml rg '(java|release|compiler|version)' "$(fd -H 'pom\.xml' | head -1)" -i | head -20Repository: gridsuite/study-server
Length of output: 727
🏁 Script executed:
# Check for Java version configuration in parent or anywhere rg 'java.version|java.compiler|java.source|java.target|<java>|<release>' "$(fd -H 'pom\.xml' | head -1)" -A2 -B2Repository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Look for .java-version, .sdkmanrc, or similar files fd -H '\.(java-version|sdkmanrc|tool-versions|jdk)'Repository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Check if List.getFirst() is used elsewhere in the codebase rg 'getFirst\(\)' --type java | head -20Repository: gridsuite/study-server
Length of output: 5061
🏁 Script executed:
# Check the powsybl-parent-ws to understand Java version targeting # First, look for common Java version patterns more broadly rg 'source|target|release' "$(fd -H 'pom\.xml' | head -1)" --type xml | grep -i 'java\|release\|compiler\|source\|target' | head -20Repository: gridsuite/study-server
Length of output: 193
🏁 Script executed:
# Since the project has a parent pom, search for any Java toolchain configuration rg 'java\.version|maven\.compiler|maven-compiler-plugin' --type xmlRepository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Check what the actual Java version requirement is by looking at Maven compiler plugin or similar cat "$(fd -H 'pom\.xml' | head -1)" | tail -200Repository: gridsuite/study-server
Length of output: 7173
🏁 Script executed:
# Search for Java version documentation or requirements fd -H 'README|CHANGELOG|\.md' | xargs rg -l 'java|jdk|21|20|19' | head -5Repository: gridsuite/study-server
Length of output: 76
🏁 Script executed:
# Check if there's a .github/workflows that shows Java version fd -H '\.github/workflows' -type dRepository: gridsuite/study-server
Length of output: 237
🏁 Script executed:
# Look for GitHub Actions workflow files to see what Java version is used fd -H '\.yml$|\.yaml$' '.github/workflows' 2>/dev/null | head -5Repository: gridsuite/study-server
Length of output: 190
🏁 Script executed:
# Check the build workflow to see Java version used cat .github/workflows/build.yml | head -50Repository: gridsuite/study-server
Length of output: 564
🏁 Script executed:
# Also look for any maven or gradle configuration that specifies Java version rg 'java-version|setup-java|jdk' .github/workflows/ -A2 -B2Repository: gridsuite/study-server
Length of output: 48
🏁 Script executed:
# Check README for Java version requirements cat README.md | head -100Repository: gridsuite/study-server
Length of output: 1214
🏁 Script executed:
# Search for Java version in powsybl dependencies or documentation rg 'java|jdk|21|20' README.md -iRepository: gridsuite/study-server
Length of output: 281
🏁 Script executed:
# Look at the code context more carefully - what happens after the if block? sed -n '3185,3205p' src/main/java/org/gridsuite/study/server/service/StudyService.javaRepository: gridsuite/study-server
Length of output: 1259
🏁 Script executed:
# Check if there are other usages of this pattern (duplicateModificationsFromGroup) to understand expectations rg 'duplicateModificationsFromGroup' --type java -B3 -A10Repository: gridsuite/study-server
Length of output: 5826
🏁 Script executed:
# Understand the data flow: what does resetModificationsGroupUuid do? rg 'resetModificationsGroupUuid' --type java -B2 -A5Repository: gridsuite/study-server
Length of output: 1930
🏁 Script executed:
# Check what voltageInitService.resetModificationsGroupUuid does specifically rg 'class.*VoltageInitService' -A 100 --type java | head -150Repository: gridsuite/study-server
Length of output: 12392
Add validation to ensure voltage-init modification count is exactly one before deactivation.
The code's documentation indicates a single-copy invariant, but
resetModificationsGroupUuid()is called unconditionally after the deactivation block (lines 3194+). IfduplicateModificationsFromGroup()returns 0 or >1 UUIDs, the deactivation is silently skipped while the voltage-init group is still reset, leaving modifications active on unintended root networks. Add a fail-fast check before reset.Suggested fix
- // We expect a single voltageInit modification, deactivate it for all other root networks synchronously - if (savedUuids.size() == 1) { - for (UUID otherRootNetwork : rootNetworkToDeactivateUuids) { - rootNetworkNodeInfoService.updateModificationsToExclude(nodeUuid, otherRootNetwork, Set.of(savedUuids.getFirst()), false); - } + if (savedUuids.size() != 1) { + throw new IllegalStateException("Expected exactly one voltage-init modification copy, got " + savedUuids.size()); } + + // We expect a single voltageInit modification, deactivate it for all other root networks synchronously + UUID savedUuid = savedUuids.get(0); + for (UUID otherRootNetwork : rootNetworkToDeactivateUuids) { + rootNetworkNodeInfoService.updateModificationsToExclude(nodeUuid, otherRootNetwork, Set.of(savedUuid), false); + }📝 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.if (savedUuids.size() != 1) { throw new IllegalStateException("Expected exactly one voltage-init modification copy, got " + savedUuids.size()); } // We expect a single voltageInit modification, deactivate it for all other root networks synchronously UUID savedUuid = savedUuids.get(0); for (UUID otherRootNetwork : rootNetworkToDeactivateUuids) { rootNetworkNodeInfoService.updateModificationsToExclude(nodeUuid, otherRootNetwork, Set.of(savedUuid), false); }🤖 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 3188 - 3193, The code conditionally deactivates modifications on other root networks only when savedUuids contains exactly one element, but the resetModificationsGroupUuid() call after this block executes unconditionally. If duplicateModificationsFromGroup() returns 0 or more than 1 UUID, the deactivation is silently skipped while the group reset still proceeds, leaving modifications active on unintended root networks. Add a fail-fast validation check immediately before the resetModificationsGroupUuid() call to ensure savedUuids.size() equals 1, throwing an exception if the invariant is violated instead of silently proceeding.
3709-3711:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly skip variant deletion for the root network being invalidated.
doUnbuildNodeTree(..., skipDeleteVariants=true)applies to every root network in the study, but the subsequent remote invalidation only targetsrootNetworkUuid. That skips variant deletion for the other root networks while still invalidating their node tree state, leaving stale remote variants behind.Suggested direction
- doUnbuildNodeTree(studyUuid, rootNodeUuid, true, true, userId); + // Skip variant deletion only for rootNetworkUuid, whose remote data is erased below. + // Other root networks still need their invalidated variants deleted. + doUnbuildNodeTreeForRootNetworkInvalidation(studyUuid, rootNodeUuid, rootNetworkUuid, userId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/study/server/service/StudyService.java` around lines 3709 - 3711, The call to doUnbuildNodeTree with skipDeleteVariants=true skips variant deletion for all root networks in the study, but rootNetworkService.invalidateRootNetworkRemoteInfos only invalidates the specific rootNetworkUuid. This leaves stale variants on other root networks. Modify the logic to skip variant deletion only for the specific rootNetworkUuid being invalidated; call doUnbuildNodeTree once with skipDeleteVariants=true specifically for that root network, and then call it again for any other root networks with skipDeleteVariants=false to ensure their variants are properly cleaned up.src/test/java/org/gridsuite/study/server/WorkflowTest.java (1)
124-126:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEncode
HEADER_RECEIVERin tests to match the consumer contract.At Line 125 and Line 138, the tests send raw JSON while
ConsumerServiceexpects URL-encoded header content beforeURLDecoder.decode(...). Please encode these headers so the tests actually validate the async wire format.Proposed test fix
@@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; @@ - headers.put(HEADER_RECEIVER, objectMapper.writeValueAsString(receiver)); + headers.put(HEADER_RECEIVER, + URLEncoder.encode(objectMapper.writeValueAsString(receiver), StandardCharsets.UTF_8)); @@ - headers.put(HEADER_RECEIVER, objectMapper.writeValueAsString(receiver)); + headers.put(HEADER_RECEIVER, + URLEncoder.encode(objectMapper.writeValueAsString(receiver), StandardCharsets.UTF_8));Also applies to: 137-139
🤖 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/WorkflowTest.java` around lines 124 - 126, The tests are sending raw JSON strings in the HEADER_RECEIVER headers, but ConsumerService expects URL-encoded content before calling URLDecoder.decode(). At both locations (line 125 where objectMapper.writeValueAsString(receiver) is written to the header, and line 138 with the same pattern), you need to URL-encode the JSON string before putting it into the headers map. Apply URLEncoder.encode() to the objectMapper.writeValueAsString(receiver) result to match the actual async wire format that ConsumerService expects, ensuring the tests properly validate the end-to-end contract.
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.
No description provided.