transmit CompositesToBeInserted to network modifications server

[Mathieu-Deharbe]

PR Summary

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (2)

src/main/java/org/gridsuite/study/server/service/DirectoryService.java (1)

110-110: 💤 Low value

Fix parameter naming convention.

The parameter name elementsUuids has an inconsistent pluralization pattern. Standard Java convention would use elementUuids (plural noun before "Uuids"). This also applies to the javadoc on line 106.

♻️ Naming fix

 /**
  *
- * `@param` elementsUuids all the element uuids of the shared composites that need to be referenced in directory server
+ * `@param` elementUuids all the element uuids of the shared composites that need to be referenced in directory server
  * `@param` userId id of the user who started the insertion
  * `@param` targetNodeUuid where the new references will point
  */
-public void addReferencesToSharedComposites(List<UUID> elementsUuids, String userId, UUID targetNodeUuid) {
-    elementsUuids.forEach(elementUuid -> {
+public void addReferencesToSharedComposites(List<UUID> elementUuids, String userId, UUID targetNodeUuid) {
+    elementUuids.forEach(elementUuid -> {

🤖 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/DirectoryService.java` at
line 110, The parameter name `elementsUuids` in the method
addReferencesToSharedComposites uses inconsistent pluralization naming. Rename
the parameter from `elementsUuids` to `elementUuids` to follow standard Java
naming conventions where the plural noun precedes the type suffix. Update all
references to this parameter throughout the method body and also update the
corresponding javadoc comment that documents this parameter.

src/main/java/org/gridsuite/study/server/dto/DeleteStudyInfos.java (1)

26-26: 💤 Low value

Consider a more concise field name.

The field name modificationGroupUuidsNodeUuids is descriptive but slightly verbose. Since this is already a breaking change (type changed from List<UUID> to List<Pair<UUID, UUID>>), consider renaming to something like groupNodePairs or modificationGroupNodePairs for better readability.

💡 Alternative naming

-    private List<Pair<UUID, UUID>> modificationGroupUuidsNodeUuids;
+    private List<Pair<UUID, UUID>> modificationGroupNodePairs;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/org/gridsuite/study/server/dto/DeleteStudyInfos.java` at line
26, The field `modificationGroupUuidsNodeUuids` in the DeleteStudyInfos class is
overly verbose and repeats "Uuids" twice, reducing readability. Rename this
field to a more concise name such as `modificationGroupNodePairs` or
`groupNodePairs` to better reflect that it represents pairs of UUIDs. Remember
to update the field declaration and any corresponding getter/setter methods or
constructors that reference this field name.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/org/gridsuite/study/server/controller/StudyController.java`:
- Around line 709-711: Update the javadoc comment for the compositesToBeInserted
parameter in the StudyController to reflect the current implementation. Replace
the outdated statement that claims there is no need to create a specific DTO
with an accurate description explaining that study-server now uses the specific
CompositesToBeInserted DTO and consumes the isShared field to manage shared
composite references before forwarding the data to network-modification-server.

In `@src/main/java/org/gridsuite/study/server/service/DirectoryService.java`:
- Around line 128-140: The removeReference method is missing javadoc
documentation and parameter validation. Add comprehensive javadoc to the method
that explains its purpose, documents all three parameters (referenceUuid,
userId, sharedElementUuid) and any exceptions it throws. Additionally, add null
safety checks at the beginning of the removeReference method to validate that
referenceUuid, userId, and sharedElementUuid are not null, throwing
IllegalArgumentException with descriptive messages if any are null, similar to
the pattern used in the addReferencesToSharedComposites method.
- Around line 110-126: The addReferencesToSharedComposites method does not
validate the elementsUuids parameter before using it in the forEach operation,
which will cause a NullPointerException if a null list is passed. Add a null
safety check at the beginning of the method using either a guard clause that
returns early if elementsUuids is null, or use Objects.requireNonNull to fail
fast with a descriptive error message. This validation should occur before the
forEach call to prevent null pointer exceptions.

In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 600-602: The deleteModificationsFromGroup method is called without
passing the actual caller's userId, causing downstream calls to
directoryService.removeReference to use a literal "userId" string instead of the
real user's identity, which breaks authorization and auditing. Modify the
deleteModificationsFromGroup method signature to accept userId as a parameter,
update the forEach lambda to pass the actual userId from the calling context,
and use this passed userId in all directoryService.removeReference calls instead
of the placeholder literal "userId" string.
- Around line 2403-2414: The addReferencesToSharedComposites call is sending all
composite IDs instead of only the shared ones, and it executes before the actual
insertion happens in duplicateModificationsOrInsertComposites, which can leave
dangling directory references if insertion fails. Filter the compositesInfos
stream to extract only shared composites (those where isShared() returns true)
when passing IDs to addReferencesToSharedComposites, and move this call to
execute after duplicateModificationsOrInsertComposites completes successfully to
ensure references are only created for composites that were actually inserted.

In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java`:
- Around line 2101-2102: The testInsertComposite test method only creates
CompositesToBeInserted instances with isShared set to false, which means the new
shared-reference synchronization behavior is not being tested. Add at least one
additional test case where a CompositesToBeInserted instance is created with
isShared=true, and include an assertion to verify that the expected
directory-reference side effect call is properly invoked for the shared
composite scenario. This will ensure the new shared-composite path introduced in
this PR is properly exercised and validated.

---

Nitpick comments:
In `@src/main/java/org/gridsuite/study/server/dto/DeleteStudyInfos.java`:
- Line 26: The field `modificationGroupUuidsNodeUuids` in the DeleteStudyInfos
class is overly verbose and repeats "Uuids" twice, reducing readability. Rename
this field to a more concise name such as `modificationGroupNodePairs` or
`groupNodePairs` to better reflect that it represents pairs of UUIDs. Remember
to update the field declaration and any corresponding getter/setter methods or
constructors that reference this field name.

In `@src/main/java/org/gridsuite/study/server/service/DirectoryService.java`:
- Line 110: The parameter name `elementsUuids` in the method
addReferencesToSharedComposites uses inconsistent pluralization naming. Rename
the parameter from `elementsUuids` to `elementUuids` to follow standard Java
naming conventions where the plural noun precedes the type suffix. Update all
references to this parameter throughout the method body and also update the
corresponding javadoc comment that documents this parameter.

🪄 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: a596461c-24af-4028-afbe-5dc8d30b6c8d

📥 Commits

Reviewing files that changed from the base of the PR and between b04a6a2 and e434747.

📒 Files selected for processing (8)

• src/main/java/org/gridsuite/study/server/controller/StudyController.java
• src/main/java/org/gridsuite/study/server/dto/DeleteStudyInfos.java
• src/main/java/org/gridsuite/study/server/dto/ReferenceAttributes.java
• src/main/java/org/gridsuite/study/server/dto/modification/CompositesToBeInserted.java
• src/main/java/org/gridsuite/study/server/service/DirectoryService.java
• src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java
• src/main/java/org/gridsuite/study/server/service/StudyService.java
• src/test/java/org/gridsuite/study/server/NetworkModificationTest.java

✅ Files skipped from review due to trivial changes (1)

• src/main/java/org/gridsuite/study/server/dto/modification/CompositesToBeInserted.java

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
85.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud