Skip to content

Replicate composite submodifications root network applicability after copy #984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Meklo wants to merge 8 commits into
base: main
Choose a base branch
from
+143 −26
Open

Replicate composite submodifications root network applicability after copy #984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c94c353
Replicate composite submodifications applicability after copy
Apr 29, 2026
0c45d13
Adapt tests
Apr 29, 2026
56f18f5
Clean up indents
May 4, 2026
587fc70
Merge remote-tracking branch 'origin/main' into marcellinh/fix-compos…
May 4, 2026
e4f4de7
Add test
May 4, 2026
c65c53a
Tweak
May 4, 2026
9c5bd70
Fix
May 4, 2026
7e205b7
Stub tightening
May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@

public void verifyModifications(UUID groupUuid, Set<UUID> modificationUuids) {
var path = UriComponentsBuilder.fromPath(GROUP_PATH + DELIMITER + NETWORK_MODIFICATIONS_PATH + DELIMITER + "verify")
.queryParam("uuids", modificationUuids)

Check failure on line 375 in src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "uuids" 3 times. 


See more on https://sonarcloud.io/project/issues?id=org.gridsuite%3Astudy-server&issues=AZ3ZQ04qxqhRDr6QvsPN&open=AZ3ZQ04qxqhRDr6QvsPN&pullRequest=984
.buildAndExpand(groupUuid)
.toUriString();

Expand Down Expand Up @@ -450,4 +450,20 @@
new ParameterizedTypeReference<Set<UUID>>() { }
).getBody();
}

public List<UUID> findAllChildrenUuids(List<UUID> compositeUuids) {
if (compositeUuids.isEmpty()) {
return List.of();
}
var path = UriComponentsBuilder.fromPath(COMPOSITE_PATH + "children-uuids")
.queryParam("uuids", compositeUuids)
.toUriString();

return restTemplate.exchange(
getNetworkModificationServerURI(false) + path,
HttpMethod.GET,
null,
new ParameterizedTypeReference<List<UUID>>() { }
).getBody();
}
}
11 changes: 10 additions & 1 deletion src/main/java/org/gridsuite/study/server/service/StudyService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@
@Transactional(readOnly = true)
public void assertNoBlockedNodeInStudy(@NonNull UUID studyUuid, @NonNull UUID nodeUuid) {
List<UUID> nodesUuids = networkModificationTreeService.getNodeTreeUuids(nodeUuid);
getStudyRootNetworks(studyUuid).stream().forEach(rootNetwork ->

Check warning on line 1173 in src/main/java/org/gridsuite/study/server/service/StudyService.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Simplify the code by replacing .stream().forEach() with .forEach(). 


See more on https://sonarcloud.io/project/issues?id=org.gridsuite%3Astudy-server&issues=AZ3yMVR-icYIPaGiaMvD&open=AZ3yMVR-icYIPaGiaMvD&pullRequest=984
rootNetworkNodeInfoService.assertNoBlockedNode(rootNetwork.getId(), nodesUuids)
);
}
Expand Down Expand Up @@ -2494,8 +2494,17 @@
List<UUID> modificationsUuids,
NetworkModificationsResult networkModificationResults) {
Map<UUID, UUID> mappingModificationsUuids = new HashMap<>();
List<UUID> copyUuids = networkModificationResults.modificationUuids();

// Map root-level modifications
for (int i = 0; i < modificationsUuids.size(); i++) {
mappingModificationsUuids.put(modificationsUuids.get(i), networkModificationResults.modificationUuids().get(i));
mappingModificationsUuids.put(modificationsUuids.get(i), copyUuids.get(i));
}

List<UUID> originalChildren = networkModificationService.findAllChildrenUuids(modificationsUuids);
List<UUID> copyChildren = networkModificationService.findAllChildrenUuids(copyUuids);
for (int i = 0; i < originalChildren.size(); i++) {
mappingModificationsUuids.put(originalChildren.get(i), copyChildren.get(i));
}

rootNetworkNodeInfoService.copyModificationsToExcludeFromTags(originNodeUuid, targetNodeUuid, mappingModificationsUuids);
Expand Down
142 changes: 117 additions & 25 deletions src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import com.github.tomakehurst.wiremock.stubbing.Scenario;
import com.google.common.collect.ImmutableSet;
import com.powsybl.commons.datasource.ReadOnlyDataSource;
import com.powsybl.commons.datasource.ResourceDataSource;
Expand Down Expand Up @@ -87,11 +88,16 @@
import static org.gridsuite.study.server.utils.TestUtils.checkUpdateStatusMessagesReceived;
import static org.gridsuite.study.server.utils.TestUtils.synchronizeStudyServerExecutionService;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
import static org.springframework.http.MediaType.APPLICATION_JSON;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

/**
Expand Down Expand Up @@ -2122,13 +2128,21 @@ void testDuplicateModification() throws Exception {
UUID nodeUuid1 = node1.getId();
UUID modification1 = UUID.randomUUID();
UUID modification2 = UUID.randomUUID();
String modificationUuidListBody = mapper.writeValueAsString(Arrays.asList(modification1, modification2));
List<UUID> modificationUuids = List.of(modification1, modification2);
String modificationUuidListBody = mapper.writeValueAsString(modificationUuids);

UUID groupStubId = wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
List<UUID> copyUuids = List.of(UUID.randomUUID(), UUID.randomUUID());
wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
.withQueryParam("action", WireMock.equalTo("COPY"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(List.of(UUID.randomUUID(), UUID.randomUUID()), List.of(Optional.empty()))))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))).getId();
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(copyUuids, List.of())))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

// uuids is sent as repeated params (?uuids=x&uuids=y) — match on path only, both calls return empty list
wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(List.of()))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

// duplicate 2 modifications in node1
mockMvc.perform(put("/v1/studies/{studyUuid}/nodes/{nodeUuid}?originStudyUuid={originStudyUuid}&originNodeUuid={originNodeUuid}&action=COPY",
Expand All @@ -2142,12 +2156,13 @@ void testDuplicateModification() throws Exception {
checkEquipmentUpdatingFinishedMessagesReceived(studyUuid, nodeUuid1);
checkElementUpdatedMessageSent(studyUuid, userId);

Pair<List<UUID>, List<ModificationApplicationContext>> modificationBody = Pair.of(List.of(modification1, modification2), List.of(rootNetworkNodeInfoService.getNetworkModificationApplicationContext(firstRootNetworkUuid, node1.getId(), NETWORK_UUID)));
Pair<List<UUID>, List<ModificationApplicationContext>> modificationBody = Pair.of(modificationUuids, List.of(rootNetworkNodeInfoService.getNetworkModificationApplicationContext(firstRootNetworkUuid, node1.getId(), NETWORK_UUID)));
String expectedBody = mapper.writeValueAsString(modificationBody);
String url = "/v1/groups/" + node1.getModificationGroupUuid();
WireMockUtils.verifyPutRequest(wireMockServer, groupStubId, url, true, Map.of(
"action", WireMock.equalTo("COPY")),
expectedBody);
WireMockUtilsCriteria.verifyPutRequest(wireMockServer, url, Map.of("action", WireMock.equalTo("COPY")), expectedBody);

// Verify both findAllChildrenUuids calls were made (originals + copies)
WireMockUtilsCriteria.verifyGetRequest(wireMockServer, "/v1/network-composite-modifications/children-uuids", Map.of("uuids", WireMock.matching(".*")), 2);

verify(rootNetworkNodeInfoService, times(1)).copyModificationsToExcludeFromTags(any(), any(), any());

Expand All @@ -2156,11 +2171,13 @@ void testDuplicateModification() throws Exception {
rootNetworkNodeInfo1Entity.setNodeBuildStatus(NodeBuildStatusEmbeddable.from(BuildStatus.BUILT));
rootNetworkNodeInfoRepository.save(rootNetworkNodeInfo1Entity);

groupStubId = wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
List<UUID> copyUuids2 = List.of(UUID.randomUUID(), UUID.randomUUID());

wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
.withQueryParam("action", WireMock.equalTo("COPY"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(List.of(UUID.randomUUID(), UUID.randomUUID()), List.of(Optional.of(NetworkModificationResult.builder().build())))))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))).getId();
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(copyUuids2, List.of(Optional.of(NetworkModificationResult.builder().build())))))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

mockMvc.perform(put("/v1/studies/{studyUuid}/nodes/{nodeUuid}?originStudyUuid={originStudyUuid}&originNodeUuid={originNodeUuid}&action=COPY",
studyUuid, nodeUuid1, studyUuid, nodeUuid1)
Expand All @@ -2176,10 +2193,76 @@ void testDuplicateModification() throws Exception {
checkEquipmentMessagesReceived(studyUuid, List.of(nodeUuid1), expectedPayload);
checkEquipmentUpdatingFinishedMessagesReceived(studyUuid, nodeUuid1);

url = "/v1/groups/" + node1.getModificationGroupUuid();
WireMockUtils.verifyPutRequest(wireMockServer, groupStubId, url, true, Map.of(
"action", WireMock.equalTo("COPY")),
expectedBody);
WireMockUtilsCriteria.verifyPutRequest(wireMockServer, url, Map.of("action", WireMock.equalTo("COPY")), expectedBody);
WireMockUtilsCriteria.verifyGetRequest(wireMockServer, "/v1/network-composite-modifications/children-uuids", Map.of("uuids", WireMock.matching(".*")), 2);
}

@Test
void testDuplicateModificationReplicateChildExcludedUuids() throws Exception {
// Verifies that when findAllChildrenUuids returns non-empty lists, the child UUIDs
// are included in the mapping passed to copyModificationsToExcludeFromTags.
String userId = "userId";
StudyEntity studyEntity = insertDummyStudy(UUID.fromString(NETWORK_UUID_STRING), CASE_UUID, "UCTE");
UUID studyUuid = studyEntity.getId();
UUID rootNodeUuid = getRootNode(studyUuid).getId();
NetworkModificationNode node1 = createNetworkModificationNode(studyUuid, rootNodeUuid,
UUID.randomUUID(), VARIANT_ID, "node for child mapping", userId);
UUID nodeUuid1 = node1.getId();

UUID modification1 = UUID.randomUUID();
List<UUID> modificationUuids = List.of(modification1);
String modificationUuidListBody = mapper.writeValueAsString(modificationUuids);

UUID copyUuid1 = UUID.randomUUID();
List<UUID> copyUuids = List.of(copyUuid1);
UUID originalChild = UUID.randomUUID();
UUID copyChild = UUID.randomUUID();

wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
.withQueryParam("action", WireMock.equalTo("COPY"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(copyUuids, List.of())))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

// First call (originals) returns originalChild; second call (copies) returns copyChild
wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
.inScenario("childMapping")
.whenScenarioStateIs(Scenario.STARTED)
.withQueryParam("uuids", WireMock.equalTo(modification1.toString()))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(List.of(originalChild)))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))
.willSetStateTo("secondCall"));
wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
.inScenario("childMapping")
.whenScenarioStateIs("secondCall")
.withQueryParam("uuids", WireMock.equalTo(copyUuid1.toString()))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(List.of(copyChild)))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

mockMvc.perform(put("/v1/studies/{studyUuid}/nodes/{nodeUuid}?originStudyUuid={originStudyUuid}&originNodeUuid={originNodeUuid}&action=COPY",
studyUuid, nodeUuid1, studyUuid, nodeUuid1)
.contentType(MediaType.APPLICATION_JSON)
.content(modificationUuidListBody)
.header(USER_ID_HEADER, userId))
.andExpect(status().isOk());

checkUpdateStatusMessagesReceived(studyUuid, nodeUuid1, output);
checkEquipmentUpdatingMessagesReceived(studyUuid, nodeUuid1);
checkEquipmentUpdatingFinishedMessagesReceived(studyUuid, nodeUuid1);
checkElementUpdatedMessageSent(studyUuid, userId);

Pair<List<UUID>, List<ModificationApplicationContext>> modificationBody = Pair.of(modificationUuids, List.of(rootNetworkNodeInfoService.getNetworkModificationApplicationContext(studyTestUtils.getOneRootNetworkUuid(studyUuid), node1.getId(), NETWORK_UUID)));
WireMockUtilsCriteria.verifyPutRequest(wireMockServer, "/v1/groups/" + node1.getModificationGroupUuid(), Map.of("action", WireMock.equalTo("COPY")), mapper.writeValueAsString(modificationBody));
WireMockUtilsCriteria.verifyGetRequest(wireMockServer, "/v1/network-composite-modifications/children-uuids", Map.of("uuids", WireMock.matching(".*")), 2);
Comment on lines +2227 to +2258
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Constrain the scenario stubs to the expected UUID lists.

Both children-uuids stubs match only on path and scenario state, so this test still passes if the implementation calls findAllChildrenUuids(...) twice with the original UUIDs, or in the wrong order. Since the production code in StudyService.copyModificationsToExclude(...) depends on original -> copy pairing, the stub should assert modificationUuids on the first call and copyUuids on the second.

Suggested tightening 
         wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
                 .inScenario("childMapping")
                 .whenScenarioStateIs(Scenario.STARTED)
+                .withQueryParam("uuids", WireMock.equalTo(modification1.toString()))
                 .willReturn(WireMock.ok()
                         .withBody(mapper.writeValueAsString(List.of(originalChild)))
                         .withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))
                 .willSetStateTo("secondCall"));
         wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
                 .inScenario("childMapping")
                 .whenScenarioStateIs("secondCall")
+                .withQueryParam("uuids", WireMock.equalTo(copyUuid1.toString()))
                 .willReturn(WireMock.ok()
                         .withBody(mapper.writeValueAsString(List.of(copyChild)))
                         .withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java` around
lines 2227 - 2256, The two WireMock stubs for
"/v1/network-composite-modifications/children-uuids" are only constrained by
path and scenario state, so tighten them to match the expected UUID lists:
update the first stub (returning originalChild) to require the query param
"uuids" equals the serialized modificationUuids list, and update the second stub
(returning copyChild) to require "uuids" equals the serialized copyUuids list;
locate these stubs in NetworkModificationTest and relate them to
StudyService.copyModificationsToExclude and the variables originalChild,
copyChild, modificationUuids and copyUuids so the test fails if the code calls
children-uuids with the wrong UUID set or order.

✅ Addressed in commit 7e205b7

Copy link
Copy Markdown
Contributor

@Mathieu-Deharbe Mathieu-Deharbe May 5, 2026

Choose a reason for hiding this comment

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

I kind of agreed with coderabbit but I tried to do it and it got surprisingly complicated. I don't think this is worth it.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

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

@Mathieu-Deharbe, understood — the scenario-based stub ordering combined with WireMock's query parameter matching can get tricky fast, especially when parameters are repeated (e.g. ?uuids=x&uuids=y). The existing coverage with the argThat matcher on copyModificationsToExcludeFromTags already provides meaningful protection for the core pairing logic. Happy to leave it as-is.


// The mapping must contain both the root-level pair AND the child pair
verify(rootNetworkNodeInfoService, times(1)).copyModificationsToExcludeFromTags(
any(), any(),
argThat((Map<UUID, UUID> mapping) ->
copyUuid1.equals(mapping.get(modification1))
&& copyChild.equals(mapping.get(originalChild))));
}

@Test
Expand All @@ -2194,15 +2277,23 @@ void testDuplicateModificationBetweenStudies() throws Exception {
StudyEntity studyEntity2 = insertDummyStudy(UUID.fromString(NETWORK_UUID_2_STRING), CASE_UUID, "UCTE");
NetworkModificationNode node2 = createNetworkModificationNode(studyEntity2.getId(), getRootNode(studyEntity2.getId()).getId(), UUID.randomUUID(), VARIANT_ID_2, "New node 2", "userId");

UUID groupStubId = wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
List<UUID> modifications = List.of(UUID.randomUUID());
List<UUID> copyUuids = List.of(UUID.randomUUID());
String modificationUuidListBody = mapper.writeValueAsString(modifications);

wireMockServer.stubFor(WireMock.any(WireMock.urlPathMatching("/v1/groups/.*"))
.withQueryParam("action", WireMock.equalTo("COPY"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(List.of(UUID.randomUUID(), UUID.randomUUID()), List.of(Optional.empty()))))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE))).getId();
.withBody(mapper.writeValueAsString(new NetworkModificationsResult(copyUuids, List.of())))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

// uuids is sent as repeated params — match on path only
wireMockServer.stubFor(WireMock.get(WireMock.urlPathMatching("/v1/network-composite-modifications/children-uuids"))
.willReturn(WireMock.ok()
.withBody(mapper.writeValueAsString(List.of()))
.withHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)));

// Duplicate modification from node2 (study2) to node1 (study1)
List<UUID> modifications = List.of(UUID.randomUUID());
String modificationUuidListBody = mapper.writeValueAsString(modifications);
mockMvc.perform(put("/v1/studies/{studyUuid}/nodes/{nodeUuid}?originStudyUuid={originStudyUuid}&originNodeUuid={originNodeUuid}&action=COPY",
studyEntity1.getId(), node1.getId(), studyEntity2.getId(), node2.getId())
.contentType(MediaType.APPLICATION_JSON)
Expand All @@ -2217,9 +2308,10 @@ void testDuplicateModificationBetweenStudies() throws Exception {
Pair<List<UUID>, List<ModificationApplicationContext>> modificationBody = Pair.of(modifications, List.of(rootNetworkNodeInfoService.getNetworkModificationApplicationContext(studyTestUtils.getOneRootNetworkUuid(studyEntity1.getId()), node1.getId(), NETWORK_UUID)));
String expectedBody = mapper.writeValueAsString(modificationBody);
String url = "/v1/groups/" + node1.getModificationGroupUuid();
WireMockUtils.verifyPutRequest(wireMockServer, groupStubId, url, true, Map.of(
"action", WireMock.equalTo("COPY")),
expectedBody);
WireMockUtilsCriteria.verifyPutRequest(wireMockServer, url, Map.of("action", WireMock.equalTo("COPY")), expectedBody);

// Verify both findAllChildrenUuids calls were made (originals + copies)
WireMockUtilsCriteria.verifyGetRequest(wireMockServer, "/v1/network-composite-modifications/children-uuids", Map.of("uuids", WireMock.matching(".*")), 2);

verify(rootNetworkNodeInfoService, times(1)).copyModificationsToExcludeFromTags(any(), any(), any());

Expand Down
Loading