Skip to content

refactor: move modificationUuids from ProcessConfig to ModifyingProcessConfig interface #43

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

Merged
KoloMenek merged 1 commit into from
Feb 26, 2026
Merged

refactor: move modificationUuids from ProcessConfig to ModifyingProcessConfig interface #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions monitor-commons/src/main/java/org/gridsuite/monitor/commons/ModifyingProcessConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Copyright (c) 2026, RTE (http://www.rte-france.com)
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
package org.gridsuite.monitor.commons;
Copy link
Contributor

@antoinebhs antoinebhs Feb 24, 2026

Choose a reason for hiding this comment

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

Missing license

Copy link
Member Author

@KoloMenek KoloMenek Feb 25, 2026

Choose a reason for hiding this comment

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

Added


import java.util.List;
import java.util.UUID;

/**
* @author Kamil MARUT {@literal <kamil.marut at rte-france.com>}
*/
public interface ModifyingProcessConfig extends ProcessConfig {

List<UUID> modificationUuids();

}
4 changes: 0 additions & 4 deletions monitor-commons/src/main/java/org/gridsuite/monitor/commons/ProcessConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;

import java.util.List;
import java.util.UUID;

/**
* @author Antoine Bouhours <antoine.bouhours at rte-france.com>
*/
Expand All @@ -25,5 +22,4 @@
public interface ProcessConfig {
ProcessType processType();

List<UUID> modificationUuids();
}
3 changes: 2 additions & 1 deletion monitor-commons/src/main/java/org/gridsuite/monitor/commons/SecurityAnalysisConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ public record SecurityAnalysisConfig(
UUID parametersUuid,
List<String> contingencies,
List<UUID> modificationUuids
) implements ProcessConfig {
) implements ModifyingProcessConfig {

@Override
public ProcessType processType() {
return ProcessType.SECURITY_ANALYSIS;
Expand Down
4 changes: 2 additions & 2 deletions ...a/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import com.powsybl.iidm.network.Network;
import org.apache.commons.collections4.CollectionUtils;
import org.gridsuite.modification.dto.ModificationInfos;
import org.gridsuite.monitor.commons.ProcessConfig;
import org.gridsuite.monitor.commons.ModifyingProcessConfig;
import org.gridsuite.monitor.worker.server.core.AbstractProcessStep;
import org.gridsuite.monitor.worker.server.core.ProcessStepExecutionContext;
import org.gridsuite.monitor.worker.server.services.*;
Expand All @@ -29,7 +29,7 @@
* @author Antoine Bouhours <antoine.bouhours at rte-france.com>
*/
@Component
public class ApplyModificationsStep<C extends ProcessConfig> extends AbstractProcessStep<C> {
public class ApplyModificationsStep<C extends ModifyingProcessConfig> extends AbstractProcessStep<C> {

private final NetworkModificationService networkModificationService;
private final NetworkModificationRestService networkModificationRestService;
Expand Down
44 changes: 32 additions & 12 deletions ...g/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import org.gridsuite.modification.dto.LoadModificationInfos;
import org.gridsuite.modification.dto.ModificationInfos;
import org.gridsuite.modification.dto.OperationType;
import org.gridsuite.monitor.commons.ProcessConfig;
import org.gridsuite.monitor.commons.ModifyingProcessConfig;
import org.gridsuite.monitor.worker.server.core.ProcessStepExecutionContext;
import org.gridsuite.monitor.worker.server.dto.ReportInfos;
import org.gridsuite.monitor.worker.server.services.*;
Expand All @@ -27,6 +27,7 @@

import java.io.IOException;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -54,30 +55,27 @@ class ApplyModificationsStepTest {
private S3Service s3Service;

@Mock
private ProcessConfig config;
private ModifyingProcessConfig config;

private ApplyModificationsStep<ProcessConfig> applyModificationsStep;
private ApplyModificationsStep<ModifyingProcessConfig> applyModificationsStep;

@Mock
private ProcessStepExecutionContext<ProcessConfig> stepContext;
private ProcessStepExecutionContext<ModifyingProcessConfig> stepContext;

private static final UUID MODIFICATION_UUID = UUID.randomUUID();
private static final UUID REPORT_UUID = UUID.randomUUID();

@BeforeEach
void setUp() {
applyModificationsStep = new ApplyModificationsStep<>(networkModificationService, networkModificationRestService, s3Service, filterService);
when(config.modificationUuids()).thenReturn(List.of(MODIFICATION_UUID));
when(stepContext.getConfig()).thenReturn(config);
ReportInfos reportInfos = new ReportInfos(REPORT_UUID, ReportNode.newRootReportNode()
.withResourceBundles("i18n.reports")
.withMessageTemplate("test")
.build());
when(stepContext.getReportInfos()).thenReturn(reportInfos);

applyModificationsStep = new ApplyModificationsStep<>(networkModificationService, networkModificationRestService, s3Service, filterService);
}

@Test
void executeApplyModifications() {
void executeApplyModificationsWhenModificationUuidsNotEmpty() {
setUpReportInfos();
when(config.modificationUuids()).thenReturn(List.of(MODIFICATION_UUID));
String stepType = applyModificationsStep.getType().getName();
assertEquals("APPLY_MODIFICATIONS", stepType);

Expand All @@ -93,8 +91,22 @@ void executeApplyModifications() {
verify(networkModificationService).applyModifications(any(Network.class), any(List.class), any(ReportNode.class), any(FilterService.class));
}

@Test
void executeDoesNothingWhenModificationUuidsEmpty() {
when(config.modificationUuids()).thenReturn(Collections.emptyList());

applyModificationsStep.execute(stepContext);

verifyNoInteractions(networkModificationService);
verifyNoInteractions(networkModificationRestService);
verifyNoInteractions(filterService);
verifyNoInteractions(s3Service);
}
Comment on lines +100 to +104
Copy link

@coderabbitai coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

executeDoesNothingWhenModificationUuidsEmpty should also assert no S3 side effects.

The test says “does nothing”, but it currently does not guard s3Service. Add a no-interaction assertion to fully lock the contract.

Suggested patch 
         verifyNoInteractions(networkModificationService);
         verifyNoInteractions(networkModificationRestService);
         verifyNoInteractions(filterService);
+        verifyNoInteractions(s3Service);
📝 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.

Suggested change
verifyNoInteractions(networkModificationService);
verifyNoInteractions(networkModificationRestService);
verifyNoInteractions(filterService);
}
verifyNoInteractions(networkModificationService);
verifyNoInteractions(networkModificationRestService);
verifyNoInteractions(filterService);
verifyNoInteractions(s3Service);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java`
around lines 100 - 103, The test executeDoesNothingWhenModificationUuidsEmpty is
missing an assertion that S3 is untouched; add a no-interaction verification for
the s3Service mock (e.g., verifyNoInteractions(s3Service) or equivalent)
alongside the existing verifyNoInteractions(networkModificationService),
verifyNoInteractions(networkModificationRestService), and
verifyNoInteractions(filterService) to ensure the test fully asserts there are
no S3 side effects.


@Test
void executeApplyModificationsDebugOn() throws IOException {
setUpReportInfos();
when(config.modificationUuids()).thenReturn(List.of(MODIFICATION_UUID));
String stepType = applyModificationsStep.getType().getName();
assertEquals("APPLY_MODIFICATIONS", stepType);

Expand Down Expand Up @@ -135,4 +147,12 @@ void executeApplyModificationsDebugOn() throws IOException {

verify(network).write("XIIDM", null, mockedPath);
}

private void setUpReportInfos() {
ReportInfos reportInfos = new ReportInfos(REPORT_UUID, ReportNode.newRootReportNode()
.withResourceBundles("i18n.reports")
.withMessageTemplate("test")
.build());
when(stepContext.getReportInfos()).thenReturn(reportInfos);
}
}