From 801a07f5e733fda2e33bc08b7bf09186bdf9815b Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Fri, 22 May 2026 18:39:00 +0200 Subject: [PATCH 1/5] fix: keep temporary limits in REPLACE mode when nothing is applied In REPLACE mode the existing temporary limits were dropped unconditionally as soon as any limit was left untouched, even when every provided input was rejected by validation (missing field, duplicate name, no match). The limit set was emptied although no modification had actually been performed. The existing temporary limits are now kept untouched unless at least one input actually created, modified or deleted a temporary limit. The "no match" check (a MODIFY input targeting a non-existing temporary limit) is also moved before the duplicate check, so an ignored input has no side effect on the limit set. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../OperationalLimitsGroupModification.java | 60 +++++++++++++------ .../gridsuite/modification/reports.properties | 2 +- .../LimitSetModificationsTest.java | 10 ++-- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java index 97710139..dfc439f5 100644 --- a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java +++ b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java @@ -415,9 +415,13 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi } } + // tracks whether at least one input actually applied a change (created / modified / + // deleted a temporary limit); limits ignored by validation (missing fields, duplicates, + // no match...) do not count + boolean atLeastOneLimitApplied = false; if (currentLimitsInfos != null && currentLimitsInfos.getTemporaryLimits() != null) { for (CurrentTemporaryLimitModificationInfos limit : currentLimitsInfos.getTemporaryLimits()) { - applyTemporaryLimitModification( + atLeastOneLimitApplied |= applyTemporaryLimitModification( limitsAdder, currentLimits, limit, @@ -429,7 +433,7 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi } if (!unmodifiedTemporaryLimits.isEmpty()) { - if (areLimitsReplaced) { + if (areLimitsReplaced && atLeastOneLimitApplied) { // this needs to be logged only if there are unmodifiedTemporaryLimits left. // which means that they are going to be removed by the REPLACE mode addToLogsOnSide(ReportNode.newRootReportNode() @@ -492,6 +496,28 @@ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limi return true; } + /** + * A MODIFY row can only update an existing temporary limit, it cannot create one. Reject the row + * when no existing temporary limit matches its acceptable duration. + * @return true if the row must be skipped because it targets no existing temporary limit; false otherwise. + */ + private boolean isModifyWithoutMatch( + CurrentTemporaryLimitModificationInfos limit, + LoadingLimits.TemporaryLimit limitToModify, + OperationalLimitsGroupInfos.Applicability applicability) { + if (limit.getModificationType() != TemporaryLimitModificationType.MODIFY || limitToModify != null) { + return false; + } + addToLogsOnSide(ReportNode.newRootReportNode() + .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) + .withMessageTemplate("network.modification.temporaryLimitsNoMatch") + .withUntypedValue(LIMIT_ACCEPTABLE_DURATION, limit.getAcceptableDuration().getValue()) + .withSeverity(TypedValue.WARN_SEVERITY) + .build(), + applicability); + return true; + } + /** * For ADD / MODIFY / MODIFY_OR_ADD, refuse to introduce a duplicate name or duration in the limit set. * Conflicting limits being deleted in the same batch are not considered duplicates. @@ -556,7 +582,7 @@ private boolean wouldCreateDuplicate( * @param limit modification to be applied to the limit * @param unmodifiedTemporaryLimits list of all the unmodified limits that will be added at the end of the network modification */ - private void applyTemporaryLimitModification( + private boolean applyTemporaryLimitModification( CurrentLimitsAdder limitsAdder, CurrentLimits networkCurrentLimits, CurrentTemporaryLimitModificationInfos limit, @@ -565,24 +591,29 @@ private void applyTemporaryLimitModification( OperationalLimitsGroupInfos.Applicability applicability) { CurrentLimitsModificationInfos currentLimitsInfos = olgModifInfos.getCurrentLimits(); if (!preModificationCheck(limit, applicability)) { - return; + return false; + } + LoadingLimits.TemporaryLimit limitToModify = networkCurrentLimits != null + ? getTemporaryLimitToModify(networkCurrentLimits, limit, currentLimitsInfos) + : null; + if (isModifyWithoutMatch(limit, limitToModify, applicability)) { + return false; } if (wouldCreateDuplicate(workingTemporaryLimits, limit, applicability)) { - return; + return false; } int limitAcceptableDuration = limit.getAcceptableDuration().getValue(); double limitValue = hasValue(limit.getValue()) ? limit.getValue().getValue() : Double.MAX_VALUE; String limitDurationToReport = String.valueOf(limitAcceptableDuration); String limitValueToReport = limitValue == Double.MAX_VALUE ? NO_VALUE : String.valueOf(limitValue); - LoadingLimits.TemporaryLimit limitToModify = null; if (networkCurrentLimits != null) { - limitToModify = getTemporaryLimitToModify(networkCurrentLimits, limit, currentLimitsInfos); // this limit is modified by the network modification so we remove it from the list of unmodified temporary limits unmodifiedTemporaryLimits.removeIf(temporaryLimit -> temporaryLimit.getAcceptableDuration() == limitAcceptableDuration); } if (limitToModify == null && mayCreateLimit(limit.getModificationType())) { createTemporaryLimit(limitsAdder, limit, limitDurationToReport, limitValueToReport, limitValue, limitAcceptableDuration, applicability); workingTemporaryLimits.put(limitAcceptableDuration, limit.getName().getValue()); + return true; } else if (limitToModify != null) { // the limit already exists if (limit.getModificationType() == TemporaryLimitModificationType.DELETE) { @@ -600,16 +631,9 @@ private void applyTemporaryLimitModification( modifyTemporaryLimit(limitsAdder, limit, limitToModify, limitValue, limitDurationToReport, limitAcceptableDuration, applicability); workingTemporaryLimits.put(limitAcceptableDuration, limit.getName().getValue()); } - } else if (limit.getModificationType() == TemporaryLimitModificationType.MODIFY) { - // invalid modification - addToLogsOnSide(ReportNode.newRootReportNode() - .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) - .withMessageTemplate("network.modification.temporaryLimitsNoMatch") - .withUntypedValue(LIMIT_ACCEPTABLE_DURATION, limitAcceptableDuration) - .withSeverity(TypedValue.WARN_SEVERITY) - .build(), - applicability); + return true; } + return false; } /** @@ -668,10 +692,10 @@ public void modifyTemporaryLimit( // Check if there are any actual changes boolean nameChanged = !limitToModify.getName().equals(finalName); boolean valueChanged = Double.compare(limitToModify.getValue(), finalValue) != 0; + String finalValueToReport = finalValue == Double.MAX_VALUE ? NO_VALUE : String.valueOf(finalValue); if (valueChanged && !nameChanged) { // only the value changes - String finalValueToReport = finalValue == Double.MAX_VALUE ? NO_VALUE : String.valueOf(finalValue); addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) .withMessageTemplate("network.modification.temporaryLimitValueModified.name") @@ -690,7 +714,7 @@ public void modifyTemporaryLimit( .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) .withMessageTemplate("network.modification.temporaryLimitModified.name") .withUntypedValue(NAME, finalName) - .withUntypedValue(VALUE, finalValue) + .withUntypedValue(VALUE, finalValueToReport) .withUntypedValue(DURATION, limitAcceptableDuration) .withSeverity(TypedValue.DETAIL_SEVERITY) .build(), diff --git a/src/main/resources/org/gridsuite/modification/reports.properties b/src/main/resources/org/gridsuite/modification/reports.properties index b88f4e29..56a5993a 100644 --- a/src/main/resources/org/gridsuite/modification/reports.properties +++ b/src/main/resources/org/gridsuite/modification/reports.properties @@ -311,7 +311,7 @@ network.modification.temporaryLimitsReplaced = Previous temporary limits were re network.modification.temporaryLimitsMissingInfo = Missing info (name or duration) to find temporary limit to ${modificationType}: ignored network.modification.temporaryLimitsMissingModificationType = Missing modification type for temporary limit: ignored network.modification.temporaryLimitsDuplicate = Duplicate name or duration for temporary limit ${name} (duration: ${duration}) to ${modificationType}: ignored -network.modification.temporaryLimitsNoMatch = No existing temporary limit found with acceptableDuration = ${limitAcceptableDuration} matching is based on acceptableDuration if that helps +network.modification.temporaryLimitsNoMatch = No existing temporary limit found with acceptableDuration=${limitAcceptableDuration}: ignored network.modification.temporaryLimitsWrongModification = Temporary limit modification type can only be ADD when limit group modification type is not MODIFY: switched from ${modificationType} to ADD network.modification.terminal1Disconnected = Equipment with id=${id} disconnected on side 1 network.modification.terminal2Disconnected = Equipment with id=${id} disconnected on side 2 diff --git a/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java b/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java index 6b1905c1..ae2d64f6 100644 --- a/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java +++ b/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java @@ -437,8 +437,7 @@ private enum MissingField { NAME, DURATION } /** * Verify that a temporary limit modification with either {@code name} or {@code acceptableDuration} * missing is skipped and emits a {@code temporaryLimitsMissingInfo} log, for every supported - * {@link TemporaryLimitModificationType}. When the temporary-limit-level type is REPLACE, pre-existing - * limits are dropped by REPLACE semantics; otherwise they remain untouched. + * {@link TemporaryLimitModificationType}. */ @ParameterizedTest(name = "[{index}] {0} with missing {1} should log temporaryLimitsMissingInfo and skip the change") @MethodSource("missingTemporaryLimitFieldCases") @@ -454,8 +453,9 @@ void testTemporaryLimitMissingFieldIsSkipped(TemporaryLimitModificationType op, .getOperationalLimitsGroup1("DEFAULT").orElse(null)) .getCurrentLimits().orElse(null); assertNotNull(limits); - int expectedSize = (op == TemporaryLimitModificationType.REPLACE) ? 0 : 2; - assertEquals(expectedSize, limits.getTemporaryLimits().size()); + // the only input is invalid, so nothing is applied and the existing temporary limits + // are kept, including in REPLACE mode + assertEquals(2, limits.getTemporaryLimits().size()); assertLogMessageWithoutRank( "Missing info (name or duration) to find temporary limit to " + op + ": ignored", "network.modification.temporaryLimitsMissingInfo", reportNode); @@ -1105,7 +1105,7 @@ private void assertAfterNetworkModificationApplication(ReportNode reportNode) { assertLogMessageWithoutRank("Limit set DEFAULT has been modified on side 1", "network.modification.operationalLimitsGroupModified", reportNode); assertLogMessageWithoutRank("Previous temporary limits were removed", "network.modification.temporaryLimitsReplaced", reportNode); assertLogMessageWithoutRank("Cannot add DEFAULT operational limit group, one with the given name already exists", "network.modification.tabular.modification.exception", reportNode); - assertLogMessageWithoutRank("No existing temporary limit found with acceptableDuration = 3 matching is based on acceptableDuration if that helps", "network.modification.temporaryLimitsNoMatch", reportNode); + assertLogMessageWithoutRank("No existing temporary limit found with acceptableDuration=3: ignored", "network.modification.temporaryLimitsNoMatch", reportNode); assertLogMessageWithoutRank("limit set selected on side 2 : group0", "network.modification.limitSetSelectedOnSide2", reportNode); assertLogMessageWithoutRank("Limit set group0 has replaced the existing limit sets on side 2", "network.modification.operationalLimitsGroupReplaced", reportNode); assertLogMessageWithoutRank("Limit set DEFAULT added on side 1", "network.modification.operationalLimitsGroupAdded", reportNode); From 2e3d4ce32ff45da8adfd598e08068bd2c32b721f Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Mon, 25 May 2026 11:45:25 +0200 Subject: [PATCH 2/5] refactor: split temporary-limit validation logs per failed field Missing-info and duplicate-conflict checks each emitted a single log covering both fields. Split into per-field templates (temporaryLimitsMissingName / temporaryLimitsMissingDuration, temporaryLimitsDuplicateName / temporaryLimitsDuplicateDuration) and aggregate the validity result so every problem on a row is reported. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../OperationalLimitsGroupModification.java | 39 ++++++++++++---- .../gridsuite/modification/reports.properties | 6 ++- .../LimitSetModificationsTest.java | 44 ++++++++++--------- 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java index dfc439f5..b93bdb0b 100644 --- a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java +++ b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java @@ -456,6 +456,7 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi * @return true if the modification can proceed, false if it must be skipped. */ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limit, OperationalLimitsGroupInfos.Applicability applicability) { + boolean valid = true; if (olgModifInfos.getModificationType() == OperationalLimitsGroupModificationType.ADD || olgModifInfos.getModificationType() == OperationalLimitsGroupModificationType.REPLACE) { // If we aren't modifying or deleting an existing limit set, temporary limit modification is necessarily of ADD type @@ -479,21 +480,31 @@ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limi .withSeverity(TypedValue.WARN_SEVERITY) .build(), applicability); - return false; + valid = false; } } // Ensure that name and duration are present (duration is the identifier) - if (!hasValue(limit.getName()) || !hasValue(limit.getAcceptableDuration())) { + if (!hasValue(limit.getName())) { addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) - .withMessageTemplate("network.modification.temporaryLimitsMissingInfo") + .withMessageTemplate("network.modification.temporaryLimitsMissingName") .withUntypedValue("modificationType", limit.getModificationType() != null ? limit.getModificationType().name() : "null") .withSeverity(TypedValue.WARN_SEVERITY) .build(), applicability); - return false; + valid = false; } - return true; + if (!hasValue(limit.getAcceptableDuration())) { + addToLogsOnSide(ReportNode.newRootReportNode() + .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) + .withMessageTemplate("network.modification.temporaryLimitsMissingDuration") + .withUntypedValue("modificationType", limit.getModificationType() != null ? limit.getModificationType().name() : "null") + .withSeverity(TypedValue.WARN_SEVERITY) + .build(), + applicability); + valid = false; + } + return valid; } /** @@ -560,19 +571,29 @@ private boolean wouldCreateDuplicate( && !isThisLimitDeleted(batch, existingByName.get().getKey()); } - if (durationConflict || nameConflict) { + if (durationConflict) { addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) - .withMessageTemplate("network.modification.temporaryLimitsDuplicate") + .withMessageTemplate("network.modification.temporaryLimitsDuplicateDuration") .withUntypedValue("modificationType", type.name()) .withUntypedValue(NAME, name) .withUntypedValue(DURATION, String.valueOf(duration)) .withSeverity(TypedValue.WARN_SEVERITY) .build(), applicability); - return true; } - return false; + if (nameConflict) { + addToLogsOnSide(ReportNode.newRootReportNode() + .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) + .withMessageTemplate("network.modification.temporaryLimitsDuplicateName") + .withUntypedValue("modificationType", type.name()) + .withUntypedValue(NAME, name) + .withUntypedValue(DURATION, String.valueOf(duration)) + .withSeverity(TypedValue.WARN_SEVERITY) + .build(), + applicability); + } + return durationConflict || nameConflict; } /** diff --git a/src/main/resources/org/gridsuite/modification/reports.properties b/src/main/resources/org/gridsuite/modification/reports.properties index 56a5993a..8e300c6d 100644 --- a/src/main/resources/org/gridsuite/modification/reports.properties +++ b/src/main/resources/org/gridsuite/modification/reports.properties @@ -308,9 +308,11 @@ network.modification.temporaryLimitDeleted.name = ${name} (${duration}) deleted network.modification.temporaryLimitValueModified.name = Temporary limit ${name} (duration: ${duration}) : ${oldValue} -> ${value} network.modification.temporaryLimitModified.name = Temporary limit ${name}; value: ${value}; acceptable duration: ${duration} network.modification.temporaryLimitsReplaced = Previous temporary limits were removed -network.modification.temporaryLimitsMissingInfo = Missing info (name or duration) to find temporary limit to ${modificationType}: ignored +network.modification.temporaryLimitsMissingName = Missing name to find temporary limit to ${modificationType}: ignored +network.modification.temporaryLimitsMissingDuration = Missing acceptable duration to find temporary limit to ${modificationType}: ignored network.modification.temporaryLimitsMissingModificationType = Missing modification type for temporary limit: ignored -network.modification.temporaryLimitsDuplicate = Duplicate name or duration for temporary limit ${name} (duration: ${duration}) to ${modificationType}: ignored +network.modification.temporaryLimitsDuplicateName = Duplicate name for temporary limit ${name} (duration: ${duration}) to ${modificationType}: ignored +network.modification.temporaryLimitsDuplicateDuration = Duplicate duration for temporary limit ${name} (duration: ${duration}) to ${modificationType}: ignored network.modification.temporaryLimitsNoMatch = No existing temporary limit found with acceptableDuration=${limitAcceptableDuration}: ignored network.modification.temporaryLimitsWrongModification = Temporary limit modification type can only be ADD when limit group modification type is not MODIFY: switched from ${modificationType} to ADD network.modification.terminal1Disconnected = Equipment with id=${id} disconnected on side 1 diff --git a/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java b/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java index ae2d64f6..2108809f 100644 --- a/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java +++ b/src/test/java/org/gridsuite/modification/modifications/tabularmodifications/LimitSetModificationsTest.java @@ -436,12 +436,12 @@ private enum MissingField { NAME, DURATION } /** * Verify that a temporary limit modification with either {@code name} or {@code acceptableDuration} - * missing is skipped and emits a {@code temporaryLimitsMissingInfo} log, for every supported + * missing is skipped and emits the matching missing-field log, for every supported * {@link TemporaryLimitModificationType}. */ - @ParameterizedTest(name = "[{index}] {0} with missing {1} should log temporaryLimitsMissingInfo and skip the change") + @ParameterizedTest(name = "[{index}] {0} with missing {1} should log {2} and skip the change") @MethodSource("missingTemporaryLimitFieldCases") - void testTemporaryLimitMissingFieldIsSkipped(TemporaryLimitModificationType op, MissingField missing) { + void testTemporaryLimitMissingFieldIsSkipped(TemporaryLimitModificationType op, MissingField missing, String expectedKey, String expectedMessage) { ModificationInfos modificationInfos = buildMissingFieldModification(op, missing); ReportNode reportNode = modificationInfos.createSubReportNode(ReportNode.newRootReportNode() @@ -456,9 +456,7 @@ void testTemporaryLimitMissingFieldIsSkipped(TemporaryLimitModificationType op, // the only input is invalid, so nothing is applied and the existing temporary limits // are kept, including in REPLACE mode assertEquals(2, limits.getTemporaryLimits().size()); - assertLogMessageWithoutRank( - "Missing info (name or duration) to find temporary limit to " + op + ": ignored", - "network.modification.temporaryLimitsMissingInfo", reportNode); + assertLogMessageWithoutRank(expectedMessage, expectedKey, reportNode); } static Stream missingTemporaryLimitFieldCases() { @@ -469,8 +467,12 @@ static Stream missingTemporaryLimitFieldCases() { TemporaryLimitModificationType.DELETE, TemporaryLimitModificationType.REPLACE ).flatMap(op -> Stream.of( - Arguments.of(op, MissingField.NAME), - Arguments.of(op, MissingField.DURATION))); + Arguments.of(op, MissingField.NAME, + "network.modification.temporaryLimitsMissingName", + "Missing name to find temporary limit to " + op + ": ignored"), + Arguments.of(op, MissingField.DURATION, + "network.modification.temporaryLimitsMissingDuration", + "Missing acceptable duration to find temporary limit to " + op + ": ignored"))); } private static ModificationInfos buildMissingFieldModification(TemporaryLimitModificationType op, MissingField missing) { @@ -693,8 +695,8 @@ void testAddTemporaryLimitWithSameDurationIsSkipped() { .withMessageTemplate("test").build()); modificationInfos.toModification().apply(getNetwork(), reportNode); - assertLogMessageWithoutRank("Duplicate name or duration for temporary limit different_name (duration: 32) to ADD: ignored", - "network.modification.temporaryLimitsDuplicate", reportNode); + assertLogMessageWithoutRank("Duplicate duration for temporary limit different_name (duration: 32) to ADD: ignored", + "network.modification.temporaryLimitsDuplicateDuration", reportNode); } /** @@ -732,8 +734,8 @@ void testAddTemporaryLimitWithSameNameIsSkipped() { .withMessageTemplate("test").build()); modificationInfos.toModification().apply(getNetwork(), reportNode); - assertLogMessageWithoutRank("Duplicate name or duration for temporary limit name32 (duration: 99) to ADD: ignored", - "network.modification.temporaryLimitsDuplicate", reportNode); + assertLogMessageWithoutRank("Duplicate name for temporary limit name32 (duration: 99) to ADD: ignored", + "network.modification.temporaryLimitsDuplicateName", reportNode); } /** @@ -790,7 +792,7 @@ void testAddTemporaryLimitConflictingWithDeletedInSameBatchSucceeds() { /** * Test MODIFY where the new name collides with another existing limit's name on the same side: - * must be skipped with a temporaryLimitsDuplicate WARN. + * must be skipped with a temporaryLimitsDuplicateName WARN. */ @Test void testModifyTemporaryLimitWithDuplicateName() { @@ -831,13 +833,13 @@ void testModifyTemporaryLimitWithDuplicateName() { // limit at duration 32 unchanged assertEquals("name32", limits.getTemporaryLimit(32).getName()); assertEquals(15.0, limits.getTemporaryLimit(32).getValue(), 0.01); - assertLogMessageWithoutRank("Duplicate name or duration for temporary limit name33 (duration: 32) to MODIFY: ignored", - "network.modification.temporaryLimitsDuplicate", reportNode); + assertLogMessageWithoutRank("Duplicate name for temporary limit name33 (duration: 32) to MODIFY: ignored", + "network.modification.temporaryLimitsDuplicateName", reportNode); } /** * Test MODIFY_OR_ADD where the provided name collides with another existing limit's name on - * a different duration: must be skipped with a temporaryLimitsDuplicate WARN. + * a different duration: must be skipped with a temporaryLimitsDuplicateName WARN. */ @Test void testModifyOrAddTemporaryLimitWithDuplicateName() { @@ -879,8 +881,8 @@ void testModifyOrAddTemporaryLimitWithDuplicateName() { assertNull(limits.getTemporaryLimit(50)); // existing name33 unchanged assertEquals(15.0, limits.getTemporaryLimit(33).getValue(), 0.01); - assertLogMessageWithoutRank("Duplicate name or duration for temporary limit name33 (duration: 50) to MODIFY_OR_ADD: ignored", - "network.modification.temporaryLimitsDuplicate", reportNode); + assertLogMessageWithoutRank("Duplicate name for temporary limit name33 (duration: 50) to MODIFY_OR_ADD: ignored", + "network.modification.temporaryLimitsDuplicateName", reportNode); } /** @@ -1017,7 +1019,7 @@ void testReplaceTemporaryLimitWithEmptyValueDoesNotPreservePrevious() { /** * Test that the duplicate check sees in-batch additions: when two ADD rows in the same batch * target the same fresh duration (not present in the network), the second one must be rejected - * with a temporaryLimitsDuplicate WARN, because the working set already contains the first. + * with a temporaryLimitsDuplicateDuration WARN, because the working set already contains the first. */ @Test void testAddTemporaryLimitDuplicateWithEarlierInBatchAddIsSkipped() { @@ -1067,8 +1069,8 @@ void testAddTemporaryLimitDuplicateWithEarlierInBatchAddIsSkipped() { // only the first ADD survived at duration 50 assertEquals("first_new", limits.getTemporaryLimit(50).getName()); assertEquals(11., limits.getTemporaryLimit(50).getValue(), 0.01); - assertLogMessageWithoutRank("Duplicate name or duration for temporary limit second_new (duration: 50) to ADD: ignored", - "network.modification.temporaryLimitsDuplicate", reportNode); + assertLogMessageWithoutRank("Duplicate duration for temporary limit second_new (duration: 50) to ADD: ignored", + "network.modification.temporaryLimitsDuplicateDuration", reportNode); } @Override From cd5d0f6fa9606a66926f96a9eb890133c83ce60a Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Mon, 25 May 2026 12:09:47 +0200 Subject: [PATCH 3/5] fix: log only when name changes in operational limits modification Remove unnecessary value change check to ensure logs are emitted only when the name is modified, avoiding redundant log entries. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../modifications/olg/OperationalLimitsGroupModification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java index b93bdb0b..3a49b25a 100644 --- a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java +++ b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java @@ -729,7 +729,7 @@ public void modifyTemporaryLimit( .withSeverity(TypedValue.DETAIL_SEVERITY) .build(), applicability); - } else if (nameChanged || valueChanged) { + } else if (nameChanged) { // log only if there is at least one actual modification addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) From 1d8d4c631351540b93836af0d8f87c4f22c93c12 Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Mon, 25 May 2026 12:17:00 +0200 Subject: [PATCH 4/5] fix: avoid unnecessary value change check in name change logging Simplified condition to ensure logging occurs only when the name is modified, improving clarity and avoiding redundant code. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../modifications/olg/OperationalLimitsGroupModification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java index 3a49b25a..d568594c 100644 --- a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java +++ b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java @@ -729,7 +729,7 @@ public void modifyTemporaryLimit( .withSeverity(TypedValue.DETAIL_SEVERITY) .build(), applicability); - } else if (nameChanged) { + } else if (nameChanged) { // || valueChanged is not necessary, because we would enter above // log only if there is at least one actual modification addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) From 74cd5b067e4b9d005a5f2f33abf20f5b2a2b518d Mon Sep 17 00:00:00 2001 From: Florent MILLOT <75525996+flomillot@users.noreply.github.com> Date: Mon, 25 May 2026 12:59:28 +0200 Subject: [PATCH 5/5] refactor: extract handleUnmodifiedTemporaryLimits and split preModificationCheck - `handleUnmodifiedTemporaryLimits` factored out of `modifyTemporaryLimits`. - `preModificationCheck` split into `checkTemporaryLimitModificationType` (per-limit modificationType consistency) and `checkTemporaryLimitMandatoryFields` (presence of name and duration); both are always evaluated so every problem on a limit is reported. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com> --- .../OperationalLimitsGroupModification.java | 80 +++++++++++++------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java index d568594c..58a6e23a 100644 --- a/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java +++ b/src/main/java/org/gridsuite/modification/modifications/olg/OperationalLimitsGroupModification.java @@ -405,9 +405,9 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi unmodifiedTemporaryLimits.addAll(currentLimits.getTemporaryLimits()); } - // Working set tracking the projected limits after the rows accepted so far (duration -> name). - // Seeded from the network snapshot so that subsequent rows in the same batch see the effects - // of earlier accepted rows (additions, renames, deletions). + // Working set tracking the projected limits after the inputs accepted so far (duration -> name). + // Seeded from the network snapshot so that subsequent inputs in the same batch see the effects + // of earlier accepted inputs (additions, renames, deletions). Map workingTemporaryLimits = new HashMap<>(); if (currentLimits != null) { for (LoadingLimits.TemporaryLimit tl : currentLimits.getTemporaryLimits()) { @@ -419,6 +419,8 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi // deleted a temporary limit); limits ignored by validation (missing fields, duplicates, // no match...) do not count boolean atLeastOneLimitApplied = false; + + // APPLY MODIFICATIONS if (currentLimitsInfos != null && currentLimitsInfos.getTemporaryLimits() != null) { for (CurrentTemporaryLimitModificationInfos limit : currentLimitsInfos.getTemporaryLimits()) { atLeastOneLimitApplied |= applyTemporaryLimitModification( @@ -432,21 +434,33 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi } } - if (!unmodifiedTemporaryLimits.isEmpty()) { - if (areLimitsReplaced && atLeastOneLimitApplied) { - // this needs to be logged only if there are unmodifiedTemporaryLimits left. - // which means that they are going to be removed by the REPLACE mode - addToLogsOnSide(ReportNode.newRootReportNode() - .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) - .withMessageTemplate("network.modification.temporaryLimitsReplaced") - .withSeverity(TypedValue.DETAIL_SEVERITY) - .build(), - applicability); - } else { - // we add (back) the temporary limits that have not been modified - for (LoadingLimits.TemporaryLimit limit : unmodifiedTemporaryLimits) { - addTemporaryLimit(limitsAdder, limit.getName(), limit.getValue(), limit.getAcceptableDuration()); - } + // ADD BACK LIMITS (if necessary) + handleUnmodifiedTemporaryLimits(limitsAdder, unmodifiedTemporaryLimits, areLimitsReplaced, atLeastOneLimitApplied, applicability); + } + + /** + * In REPLACE mode with at least one applied input, the untouched limits are intentionally dropped and a log is emitted. + * Otherwise the untouched limits are re-added so they survive the modification. + */ + private void handleUnmodifiedTemporaryLimits( + CurrentLimitsAdder limitsAdder, + List unmodifiedTemporaryLimits, + boolean areLimitsReplaced, + boolean atLeastOneLimitApplied, + OperationalLimitsGroupInfos.Applicability applicability) { + if (unmodifiedTemporaryLimits.isEmpty()) { + return; + } + if (areLimitsReplaced && atLeastOneLimitApplied) { + addToLogsOnSide(ReportNode.newRootReportNode() + .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) + .withMessageTemplate("network.modification.temporaryLimitsReplaced") + .withSeverity(TypedValue.DETAIL_SEVERITY) + .build(), + applicability); + } else { + for (LoadingLimits.TemporaryLimit limit : unmodifiedTemporaryLimits) { + addTemporaryLimit(limitsAdder, limit.getName(), limit.getValue(), limit.getAcceptableDuration()); } } } @@ -456,7 +470,18 @@ protected void modifyTemporaryLimits(CurrentLimitsAdder limitsAdder, CurrentLimi * @return true if the modification can proceed, false if it must be skipped. */ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limit, OperationalLimitsGroupInfos.Applicability applicability) { - boolean valid = true; + boolean validModificationType = checkTemporaryLimitModificationType(limit, applicability); + boolean validMandatoryFields = checkTemporaryLimitMandatoryFields(limit, applicability); + return validModificationType && validMandatoryFields; + } + + /** + * Ensure that the per-limit {@code modificationType} is consistent with the enclosing limit group modification type. + * When the limit group is ADD or REPLACE, a non-ADD limit is auto-fixed to ADD (a detail log is emitted). + * When the limit group is MODIFY or MODIFY_OR_ADD, a missing per-limit {@code modificationType} rejects the limit. + * @return true if the limit may proceed, false if it must be skipped. + */ + private boolean checkTemporaryLimitModificationType(CurrentTemporaryLimitModificationInfos limit, OperationalLimitsGroupInfos.Applicability applicability) { if (olgModifInfos.getModificationType() == OperationalLimitsGroupModificationType.ADD || olgModifInfos.getModificationType() == OperationalLimitsGroupModificationType.REPLACE) { // If we aren't modifying or deleting an existing limit set, temporary limit modification is necessarily of ADD type @@ -480,10 +505,19 @@ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limi .withSeverity(TypedValue.WARN_SEVERITY) .build(), applicability); - valid = false; + return false; } } - // Ensure that name and duration are present (duration is the identifier) + return true; + } + + /** + * Ensure that name and duration are present on the limit (duration is the identifier). + * Each missing field emits its own log so a limit missing both is reported twice. + * @return true if both fields are present, false otherwise. + */ + private boolean checkTemporaryLimitMandatoryFields(CurrentTemporaryLimitModificationInfos limit, OperationalLimitsGroupInfos.Applicability applicability) { + boolean valid = true; if (!hasValue(limit.getName())) { addToLogsOnSide(ReportNode.newRootReportNode() .withResourceBundles(NetworkModificationReportResourceBundle.BASE_NAME) @@ -508,9 +542,9 @@ private boolean preModificationCheck(CurrentTemporaryLimitModificationInfos limi } /** - * A MODIFY row can only update an existing temporary limit, it cannot create one. Reject the row + * A MODIFY input can only update an existing temporary limit, it cannot create one. Reject the input * when no existing temporary limit matches its acceptable duration. - * @return true if the row must be skipped because it targets no existing temporary limit; false otherwise. + * @return true if the input must be skipped because it targets no existing temporary limit; false otherwise. */ private boolean isModifyWithoutMatch( CurrentTemporaryLimitModificationInfos limit,