Skip to content

specific composite controller#781

Merged
Mathieu-Deharbe merged 11 commits intomainfrom
specific-composite-controller
Mar 30, 2026
Merged

specific composite controller#781
Mathieu-Deharbe merged 11 commits intomainfrom
specific-composite-controller

Conversation

@Mathieu-Deharbe
Copy link
Copy Markdown
Contributor

  • Creates a specific controller for the composite modifications
    • new composite controler TU class
  • Split some endpoints, enum etc to separate both

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
@Mathieu-Deharbe Mathieu-Deharbe self-assigned this Mar 17, 2026
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
public CompletableFuture<ResponseEntity<NetworkModificationsResult>> insertCompositeModifications(
@Parameter(description = "updated group UUID, where modifications are inserted") @PathVariable("groupUuid") UUID targetGroupUuid,
@Parameter(description = "Insertion method", required = true) @RequestParam(value = "action") CompositeModificationAction action,
@RequestBody Pair<List<Pair<UUID, String>>, List<ModificationApplicationContext>> modificationContextInfos) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why the need of a Pair inbetween List<Pair<UUID, String>> and List I'm not sure I get the logic here, there can be multiple instances of List<Pair<UUID, String> as input ?

Copy link
Copy Markdown
Contributor Author

@Mathieu-Deharbe Mathieu-Deharbe Mar 20, 2026

Choose a reason for hiding this comment

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

I am not sure that I understand your question.
Pair<List<Pair<UUID, String>>, List<ModificationApplicationContext>>
mans :

  • one List<Pair<UUID, String> : yes there might be several composite modifications inserted simultaneously.
  • one List<ModificationApplicationContext> : I didn't look into it much but there are several ModificationApplicationContext, one for each root network in the targetted study.

But no there can't be multiple instances of List<Pair<UUID, String> as input.

I don't really know why this data is sent as a pair though. I just kept the previous system from handleNetworkModifications. It could be separated, but they are in the body so this is probably the reason.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah i just checked it follows what was done in handleNetworkModifications, that said it is getting really confusing the way those data structure get imbricated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Maybe in the end we will have to use a dto and copy it to study-server...

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

Adds a new REST controller CompositeController for network composite modifications, removes composite handling from NetworkModificationController, and updates service, repository, and tests to use Pair-based payloads (UUID+name pairs plus application contexts) for composite operations.

Changes

Cohort / File(s) Summary
Composite controller & tests
src/main/java/org/gridsuite/modification/server/CompositeController.java, src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java
New CompositeController exposing endpoints under /v{API_VERSION}/network-composite-modifications (actions: SPLIT, INSERT) plus create/list/duplicate/update endpoints; tests cover composite lifecycle (split, insert, duplicate, update).
Controller refactor
src/main/java/org/gridsuite/modification/server/NetworkModificationController.java
Removed composite-specific enum values and four composite endpoints; changed handleNetworkModifications request body from Pair<List<ModificationsToCopyInfos>, ...>Pair<List<UUID>, ...> and removed composite branching logic.
Service API changes
src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
Updated signatures: splitCompositeModifications(...) and insertCompositeModifications(...) now accept Pair<List<Pair<UUID,String>>, List<ModificationApplicationContext>>; implementations derive UUIDs from the Pair and forward contexts.
Repository changes
src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
Replaced insertCompositeModificationsIntoGroup signature to accept List<Pair<UUID,String>>; bulk-loads ModificationInfos, applies provided names to found CompositeModificationInfos, logs and skips missing UUIDs; removed cloneCompositeModification(...).
Test utilities & API helpers
src/test/java/org/gridsuite/modification/server/utils/TestUtils.java, src/test/java/org/gridsuite/modification/server/utils/ApiUtils.java
Renamed getJsonBodyModificationsToCopyInfosgetJsonBodyModificationCompositeInfos (accepts List<Pair<UUID,String>>); putGroupsWithCopy and other helpers switched to serializing List<UUID> for non-composite COPY requests.
Tests update / removals
src/test/java/org/gridsuite/modification/server/ModificationControllerTest.java, src/test/java/org/gridsuite/modification/server/modifications/AbstractNetworkModificationTest.java, src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java
Removed composite-related tests from ModificationControllerTest; updated various tests to use new payload shapes (List or Pair structures), adjusted helpers and minor parsing/stream cleanups.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CompositeController
    participant NetworkModificationService as Service
    participant NetworkModificationRepository as Repo
    participant NetworkStoreService as Store

    Client->>CompositeController: PUT /v{v}/network-composite-modifications/groups/{groupUuid}?action=INSERT (body: Pair<List<Pair<UUID,String>>, List<Context>>)
    CompositeController->>Service: insertCompositeModifications(groupUuid, modificationContextInfos)
    Service->>Repo: insertCompositeModifications(groupUuid, modificationContextInfos.getFirst())
    Repo->>Store: getModificationsInfosNonTransactional(listOfUUIDs)
    Store-->>Repo: List<ModificationInfos>
    Repo-->>Service: persisted ModificationInfos (names applied)
    Service-->>CompositeController: CompletableFuture<NetworkModificationsResult>
    CompositeController-->>Client: HTTP 200 + NetworkModificationsResult
Loading

Suggested reviewers

  • antoinebhs
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'specific composite controller' accurately describes the main change: creating a dedicated controller for composite modifications and separating composite-related endpoints from the general network modification controller.
Description check ✅ Passed The description is directly related to the changeset, explaining that it creates a specific controller for composite modifications, adds test classes, and splits endpoints/enums to separate composite functionality.

✏️ 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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java (2)

298-299: Prefer non-exact exception assertion for resilience.

Asserting the full message string is brittle. It’s safer to assert exception type and key fragment(s) (e.g., error code and UUID).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`
around lines 298 - 299, Update the test in CompositeControllerTest to avoid
asserting the full exception message; instead call
assertThrows(NetworkModificationException.class, () ->
modificationRepository.getModificationInfo(returnedNewId)) to capture the
exception, then assert that the exception message contains the expected
fragments (e.g., "MODIFICATION_NOT_FOUND" and the returnedNewId) using
contains/containsAll-style assertions; reference the existing
modificationRepository.getModificationInfo(returnedNewId), returnedNewId and
NetworkModificationException to locate and modify the assertion accordingly.

326-332: Strengthen update verification beyond list size.

Line 331 checks only count. The test can still pass if wrong modifications are returned with the same size. Consider asserting UUID equality with newModificationUuids.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`
around lines 326 - 332, The test currently only checks list size; strengthen it
by verifying returned modification UUIDs match expected newModificationUuids:
after deserializing into updatedCompositeContent (List<ModificationInfos>),
extract the UUIDs from each ModificationInfos (use the UUID/getId accessor on
ModificationInfos) and compare to newModificationUuids — ideally compare as
unordered sets to avoid order sensitivity; use the same request/response
variables (mvcResult, mapper, URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT,
compositeModificationUuid) and replace or augment the
assertEquals(newModificationsNumber, updatedCompositeContent.size()) with an
assertion that the sets of UUIDs are equal.
🤖 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/modification/server/NetworkModificationController.java`:
- Around line 97-105: The new handleNetworkModifications signature narrows
supported actions and changes the request body shape, breaking existing clients;
restore backward compatibility by accepting the previous payload shape and
legacy action values: update handleNetworkModifications to detect both the new
Pair<List<UUID>,List<ModificationApplicationContext>> body and the old payload
(e.g., a wrapper object or plain List<UUID>) and branch accordingly, and allow
legacy GroupModificationAction values (or map them to the new enum) when parsing
the action request param; mark the old-path handling as deprecated (log a
deprecation warning) so you can safely remove it in the next release.

In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`:
- Around line 790-804: The loop in the method that processes compositesUuidName
can throw ClassCastException and reuses the same DTO for duplicate UUIDs; fix by
first building a Map<UUID, Deque<CompositeModificationInfos>> from the
List<ModificationInfos> returned by getModificationsInfosNonTransactional(...)
by filtering only instances of CompositeModificationInfos and grouping them into
Deque queues per UUID, then iterate compositesUuidName and for each Pair poll()
one CompositeModificationInfos from the map entry for
compositeUuidName.getFirst(), setName(compositeUuidName.getSecond()) on that
polled instance and add it to newCompositeModifications; if the map has no entry
or the deque is empty, log via LOGGER.error that the composite UUID/name could
not be applied.

In
`@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`:
- Around line 66-67: Tests construct endpoints with duplicated slashes because
URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT ends with a '/' and callers concatenate
values that also start with '/', causing fragile URLs; fix by normalizing
constants and usages: remove the trailing slash from
URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT (and any other constants that end with
'/') or remove leading slashes from the concatenated segments so concatenation
like URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT + URI_NETWORK_MODIF_BASE produces a
single slash; update all affected constants/usages referenced
(URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT, URI_NETWORK_MODIF_BASE and the other
similar constants around the commented ranges) to consistently store paths
without duplicate slashes.

---

Nitpick comments:
In
`@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`:
- Around line 298-299: Update the test in CompositeControllerTest to avoid
asserting the full exception message; instead call
assertThrows(NetworkModificationException.class, () ->
modificationRepository.getModificationInfo(returnedNewId)) to capture the
exception, then assert that the exception message contains the expected
fragments (e.g., "MODIFICATION_NOT_FOUND" and the returnedNewId) using
contains/containsAll-style assertions; reference the existing
modificationRepository.getModificationInfo(returnedNewId), returnedNewId and
NetworkModificationException to locate and modify the assertion accordingly.
- Around line 326-332: The test currently only checks list size; strengthen it
by verifying returned modification UUIDs match expected newModificationUuids:
after deserializing into updatedCompositeContent (List<ModificationInfos>),
extract the UUIDs from each ModificationInfos (use the UUID/getId accessor on
ModificationInfos) and compare to newModificationUuids — ideally compare as
unordered sets to avoid order sensitivity; use the same request/response
variables (mvcResult, mapper, URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT,
compositeModificationUuid) and replace or augment the
assertEquals(newModificationsNumber, updatedCompositeContent.size()) with an
assertion that the sets of UUIDs are equal.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a05d48d2-8788-4b08-8db3-af9e3f7d32aa

📥 Commits

Reviewing files that changed from the base of the PR and between b48db88 and 395135a.

📒 Files selected for processing (10)
  • src/main/java/org/gridsuite/modification/server/CompositeController.java
  • src/main/java/org/gridsuite/modification/server/NetworkModificationController.java
  • src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
  • src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
  • src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java
  • src/test/java/org/gridsuite/modification/server/ModificationControllerTest.java
  • src/test/java/org/gridsuite/modification/server/modifications/AbstractNetworkModificationTest.java
  • src/test/java/org/gridsuite/modification/server/service/ModificationIndexationTest.java
  • src/test/java/org/gridsuite/modification/server/utils/ApiUtils.java
  • src/test/java/org/gridsuite/modification/server/utils/TestUtils.java

Comment on lines +97 to +105
@Operation(summary = "For a list of network modifications passed in body, Move them before another one or at the end of the list, or Duplicate them at the end of the list")
@ApiResponse(responseCode = "200", description = "The modification list of the group has been updated.")
public CompletableFuture<ResponseEntity<NetworkModificationsResult>> handleNetworkModifications(
@Parameter(description = "updated group UUID, where modifications are pasted") @PathVariable("groupUuid") UUID targetGroupUuid,
@Parameter(description = "kind of modification", required = true) @RequestParam(value = "action") GroupModificationAction action,
@Parameter(description = "the modification Uuid to move before (MOVE option, empty means moving at the end)") @RequestParam(value = "before", required = false) UUID beforeModificationUuid,
@Parameter(description = "origin group UUID, where modifications are copied or cut") @RequestParam(value = "originGroupUuid", required = false) UUID originGroupUuid,
@Parameter(description = "modifications can be applied (default is true)") @RequestParam(value = "build", required = false, defaultValue = "true") Boolean canApply,
@RequestBody Pair<List<ModificationsToCopyInfos>, List<ModificationApplicationContext>> modificationContextInfos) {
List<UUID> modificationsUuids = modificationContextInfos.getFirst().stream().map(ModificationsToCopyInfos::getUuid).toList();
@RequestBody Pair<List<UUID>, List<ModificationApplicationContext>> modificationContextInfos) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This narrows an existing v1 endpoint in a breaking way.

The /v1/groups/{groupUuid} contract now only binds COPY|MOVE, and the request body is now a bare UUID list instead of the previous object payload shape. Existing clients will start getting 400s until they are redeployed against the new composite route and payload. Please keep a deprecated compatibility layer for one release, or version this API change explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/gridsuite/modification/server/NetworkModificationController.java`
around lines 97 - 105, The new handleNetworkModifications signature narrows
supported actions and changes the request body shape, breaking existing clients;
restore backward compatibility by accepting the previous payload shape and
legacy action values: update handleNetworkModifications to detect both the new
Pair<List<UUID>,List<ModificationApplicationContext>> body and the old payload
(e.g., a wrapper object or plain List<UUID>) and branch accordingly, and allow
legacy GroupModificationAction values (or map them to the new enum) when parsing
the action request param; mark the old-path handling as deprecated (log a
deprecation warning) so you can safely remove it in the next release.

Comment on lines +790 to +804
@NonNull List<Pair<UUID, String>> compositesUuidName) {
List<UUID> compositeUuids = compositesUuidName.stream().map(Pair::getFirst).toList();
List<ModificationInfos> newCompositeModifications = new ArrayList<>();
for (ModificationsToCopyInfos compositeModification : compositeModifications) {
CompositeModificationInfos newCompositeModification = cloneCompositeModification(compositeModification);
newCompositeModifications.add(newCompositeModification);
List<ModificationInfos> modificationInfos = getModificationsInfosNonTransactional(compositeUuids);
// apply the new composite name to the corresponding composite modifications
for (Pair<UUID, String> compositeUuidName : compositesUuidName) {
CompositeModificationInfos newCompositeModification = (CompositeModificationInfos) modificationInfos.stream()
.filter(modif -> modif.getUuid().equals(compositeUuidName.getFirst()))
.findFirst().orElse(null);
if (newCompositeModification != null) {
newCompositeModification.setName(compositeUuidName.getSecond());
newCompositeModifications.add(newCompositeModification);
} else {
LOGGER.error("Could not find composite modification with uuid {} to apply its name {}", compositeUuidName.getFirst(), compositeUuidName.getSecond());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate the fetched DTO type and consume one match per requested insert.

Line 796 can throw a ClassCastException when a non-composite UUID slips into this endpoint, and the findFirst() lookup on Lines 796-798 reuses the same DTO when the same composite UUID is inserted twice. For a request like [(A, "name-1"), (A, "name-2")], both saved copies end up carrying the last assigned name. Build a validated Map<UUID, Deque<CompositeModificationInfos>> (or fetch composites only) and poll() one DTO per input pair.

💡 Sketch of a safe lookup
-        List<ModificationInfos> modificationInfos = getModificationsInfosNonTransactional(compositeUuids);
-        // apply the new composite name to the corresponding composite modifications
-        for (Pair<UUID, String> compositeUuidName : compositesUuidName) {
-            CompositeModificationInfos newCompositeModification = (CompositeModificationInfos) modificationInfos.stream()
-                    .filter(modif -> modif.getUuid().equals(compositeUuidName.getFirst()))
-                    .findFirst().orElse(null);
+        Map<UUID, Deque<CompositeModificationInfos>> compositesById = getModificationsInfosNonTransactional(compositeUuids).stream()
+                .map(modif -> {
+                    if (!(modif instanceof CompositeModificationInfos composite)) {
+                        throw new NetworkModificationException(MODIFICATION_ERROR,
+                                String.format("Modification (%s) is not a composite modification", modif.getUuid()));
+                    }
+                    return composite;
+                })
+                .collect(Collectors.groupingBy(
+                        CompositeModificationInfos::getUuid,
+                        LinkedHashMap::new,
+                        Collectors.toCollection(ArrayDeque::new)
+                ));
+        for (Pair<UUID, String> compositeUuidName : compositesUuidName) {
+            CompositeModificationInfos newCompositeModification = Optional.ofNullable(compositesById.get(compositeUuidName.getFirst()))
+                    .map(Deque::pollFirst)
+                    .orElse(null);
             if (newCompositeModification != null) {
                 newCompositeModification.setName(compositeUuidName.getSecond());
                 newCompositeModifications.add(newCompositeModification);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`
around lines 790 - 804, The loop in the method that processes compositesUuidName
can throw ClassCastException and reuses the same DTO for duplicate UUIDs; fix by
first building a Map<UUID, Deque<CompositeModificationInfos>> from the
List<ModificationInfos> returned by getModificationsInfosNonTransactional(...)
by filtering only instances of CompositeModificationInfos and grouping them into
Deque queues per UUID, then iterate compositesUuidName and for each Pair poll()
one CompositeModificationInfos from the map entry for
compositeUuidName.getFirst(), setName(compositeUuidName.getSecond()) on that
polled instance and add it to newCompositeModifications; if the map has no entry
or the deque is empty, log via LOGGER.error that the composite UUID/name could
not be applied.

Comment on lines +66 to +67
private static final String URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT = "/v1/network-composite-modifications/";
private static final String URI_NETWORK_MODIF_BASE = "/v1/network-modifications";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid double-slash endpoint construction in test URLs.

Line 66 already ends with /, and later concatenations also prepend /, producing URLs like ...//network-modifications. This is fragile across path matchers.

Proposed fix
-    private static final String URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT = "/v1/network-composite-modifications/";
+    private static final String URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT = "/v1/network-composite-modifications";

Also applies to: 125-127, 150-151, 156-157, 327-328, 362-363

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/modification/server/CompositeControllerTest.java`
around lines 66 - 67, Tests construct endpoints with duplicated slashes because
URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT ends with a '/' and callers concatenate
values that also start with '/', causing fragile URLs; fix by normalizing
constants and usages: remove the trailing slash from
URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT (and any other constants that end with
'/') or remove leading slashes from the concatenated segments so concatenation
like URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT + URI_NETWORK_MODIF_BASE produces a
single slash; update all affected constants/usages referenced
(URI_GET_COMPOSITE_NETWORK_MODIF_CONTENT, URI_NETWORK_MODIF_BASE and the other
similar constants around the commented ranges) to consistently store paths
without duplicate slashes.

@Mathieu-Deharbe Mathieu-Deharbe requested a review from Meklo March 20, 2026 12:41
networkModificationService.splitCompositeModifications(targetGroupUuid, modificationContextInfos)
.thenApply(ResponseEntity.ok()::body);
case INSERT ->
networkModificationService.insertCompositeModificationsIntoGroup(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename all insertCompositeModificationsIntoGroup by insertCompositeModifications

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :f1fa886

Signed-off-by: Mathieu DEHARBE <mathieu.deharbe@rte-france.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java (1)

793-804: ⚠️ Potential issue | 🟠 Major

Fail fast and dequeue composite DTOs per request item.

Line 796 still does a blind cast on the first UUID match. That leaves three bugs here: a non-composite UUID fails with ClassCastException, duplicate inputs like [(A, "name-1"), (A, "name-2")] reuse the same DTO instance so both saved copies end up with "name-2", and the null branch on Lines 802-803 turns bad input into a partial-success write with fewer inserted IDs than requested. Build a validated Map<UUID, Deque<CompositeModificationInfos>>, pollFirst() one DTO per pair, and throw when a UUID cannot be resolved.

💡 Sketch of a safe lookup
-        List<ModificationInfos> modificationInfos = getModificationsInfosNonTransactional(compositeUuids);
+        Map<UUID, Deque<CompositeModificationInfos>> compositesById = getModificationsInfosNonTransactional(compositeUuids).stream()
+                .map(modif -> {
+                    if (!(modif instanceof CompositeModificationInfos composite)) {
+                        throw new NetworkModificationException(
+                                MODIFICATION_ERROR,
+                                String.format("Modification (%s) is not a composite modification", modif.getUuid()));
+                    }
+                    return composite;
+                })
+                .collect(Collectors.groupingBy(
+                        CompositeModificationInfos::getUuid,
+                        LinkedHashMap::new,
+                        Collectors.toCollection(ArrayDeque::new)
+                ));
         // apply the new composite name to the corresponding composite modifications
         for (Pair<UUID, String> compositeUuidName : compositesUuidName) {
-            CompositeModificationInfos newCompositeModification = (CompositeModificationInfos) modificationInfos.stream()
-                    .filter(modif -> modif.getUuid().equals(compositeUuidName.getFirst()))
-                    .findFirst().orElse(null);
-            if (newCompositeModification != null) {
-                newCompositeModification.setName(compositeUuidName.getSecond());
-                newCompositeModifications.add(newCompositeModification);
-            } else {
-                LOGGER.error("Could not find composite modification with uuid {} to apply its name {}", compositeUuidName.getFirst(), compositeUuidName.getSecond());
-            }
+            CompositeModificationInfos newCompositeModification = Optional.ofNullable(compositesById.get(compositeUuidName.getFirst()))
+                    .map(Deque::pollFirst)
+                    .orElseThrow(() -> {
+                        LOGGER.error("Could not find composite modification with uuid {} to apply its name {}",
+                                compositeUuidName.getFirst(), compositeUuidName.getSecond());
+                        return new NetworkModificationException(
+                                MODIFICATION_NOT_FOUND,
+                                String.format(MODIFICATION_NOT_FOUND_MESSAGE, compositeUuidName.getFirst()));
+                    });
+            newCompositeModification.setName(compositeUuidName.getSecond());
+            newCompositeModifications.add(newCompositeModification);
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java`
around lines 793 - 804, The current loop vulnerably casts the first matching
ModificationInfos to CompositeModificationInfos and reuses the same DTO for
duplicate UUID inputs, and silently logs missing UUIDs causing partial writes;
instead, after calling getModificationsInfosNonTransactional(compositeUuids)
build a Map<UUID, Deque<CompositeModificationInfos>> by filtering
modificationInfos for instances of CompositeModificationInfos and grouping them
into deques keyed by getUuid(), then iterate compositesUuidName and for each
Pair pollFirst() one CompositeModificationInfos from the deque for that UUID,
setName(...) and add to newCompositeModifications; if the deque is empty or the
UUID key is missing throw an exception (do not just log) to fail fast; update
references to CompositeModificationInfos, modificationInfos, compositesUuidName,
newCompositeModifications and LOGGER accordingly.
🤖 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/modification/server/repositories/NetworkModificationRepository.java`:
- Around line 793-804: The current loop vulnerably casts the first matching
ModificationInfos to CompositeModificationInfos and reuses the same DTO for
duplicate UUID inputs, and silently logs missing UUIDs causing partial writes;
instead, after calling getModificationsInfosNonTransactional(compositeUuids)
build a Map<UUID, Deque<CompositeModificationInfos>> by filtering
modificationInfos for instances of CompositeModificationInfos and grouping them
into deques keyed by getUuid(), then iterate compositesUuidName and for each
Pair pollFirst() one CompositeModificationInfos from the deque for that UUID,
setName(...) and add to newCompositeModifications; if the deque is empty or the
UUID key is missing throw an exception (do not just log) to fail fast; update
references to CompositeModificationInfos, modificationInfos, compositesUuidName,
newCompositeModifications and LOGGER accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef47ada7-f769-451c-acc9-628e50d18d0a

📥 Commits

Reviewing files that changed from the base of the PR and between eaea202 and c32da69.

📒 Files selected for processing (3)
  • src/main/java/org/gridsuite/modification/server/CompositeController.java
  • src/main/java/org/gridsuite/modification/server/repositories/NetworkModificationRepository.java
  • src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/java/org/gridsuite/modification/server/service/NetworkModificationService.java
  • src/main/java/org/gridsuite/modification/server/CompositeController.java

@sonarqubecloud
Copy link
Copy Markdown

@Mathieu-Deharbe Mathieu-Deharbe merged commit 338c8d1 into main Mar 30, 2026
4 checks passed
@Mathieu-Deharbe Mathieu-Deharbe deleted the specific-composite-controller branch March 30, 2026 11:16
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.

3 participants