fix: RemoveDeletionFlag filter should work on local set of itineraries

src/main/java/org/opentripplanner/model/plan/Itinerary.java

@@ -6,6 +6,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
@@ -187,8 +188,8 @@ public void flagForDeletion(SystemNotice notice) {
   /**
    * Remove all deletion flags of this itinerary, in effect undeleting it from the result.
    */
-  public void removeDeletionFlags() {
-    systemNotices.clear();
+  public void removeDeletionFlags(Set<String> removeTags) {
+    systemNotices.removeIf(it -> removeTags.contains(it.tag()));
   }
 
   public boolean isFlaggedForDeletion() {

src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java

@@ -483,6 +483,8 @@ private List<ItineraryListFilter> buildGroupBySameRoutesAndStopsFilter() {
    * The filter name is dynamically created: similar-legs-filter-68p-1
    */
   private List<ItineraryListFilter> buildGroupByTripIdAndDistanceFilters() {
+    var sysTags = new ArrayList<String>();
+
     List<GroupBySimilarity> groupBy = groupBySimilarity
       .stream()
       .sorted(Comparator.comparingDouble(o -> o.groupByP))
@@ -491,16 +493,18 @@ private List<ItineraryListFilter> buildGroupByTripIdAndDistanceFilters() {
     List<ItineraryListFilter> groupByFilters = new ArrayList<>();
 
     for (GroupBySimilarity group : groupBy) {
-      String name =
+      String tag =
         "similar-legs-filter-%.0fp-%dx".formatted(
             100d * group.groupByP,
             group.maxNumOfItinerariesPerGroup
           );
+      sysTags.add(tag);
 
       List<ItineraryListFilter> nested = new ArrayList<>();
 
       if (group.nestedGroupingByAllSameStations) {
-        final String innerGroupName = name + "-group-by-all-same-stations";
+        final String innerGroupName = tag + "-group-by-all-same-stations";
+        sysTags.add(tag);
         nested.add(
           new GroupByFilter<>(
             GroupByAllSameStations::new,
@@ -513,19 +517,17 @@ private List<ItineraryListFilter> buildGroupByTripIdAndDistanceFilters() {
       }
 
       if (group.maxCostOtherLegsFactor > 1.0) {
-        nested.add(
-          new DeletionFlaggingFilter(
-            new OtherThanSameLegsMaxGeneralizedCostFilter(group.maxCostOtherLegsFactor)
-          )
-        );
+        var flagger = new OtherThanSameLegsMaxGeneralizedCostFilter(group.maxCostOtherLegsFactor);
+        sysTags.add(flagger.name());
+        nested.add(new DeletionFlaggingFilter(flagger));
       }
 
       nested.add(new SortingFilter(generalizedCostComparator()));
       nested.add(
-        new DeletionFlaggingFilter(new MaxLimitFilter(name, group.maxNumOfItinerariesPerGroup))
+        new DeletionFlaggingFilter(new MaxLimitFilter(tag, group.maxNumOfItinerariesPerGroup))
       );
 
-      nested.add(new RemoveDeletionFlagForLeastTransfersItinerary());
+      nested.add(new RemoveDeletionFlagForLeastTransfersItinerary(sysTags));
 
       groupByFilters.add(
         new GroupByFilter<>(it -> new GroupByDistance(it, group.groupByP), nested)

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filter/RemoveDeletionFlagForLeastTransfersItinerary.java

@@ -2,22 +2,36 @@
 
 import static org.opentripplanner.routing.algorithm.filterchain.comparator.SortOrderComparator.numberOfTransfersComparator;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
+import org.opentripplanner.model.SystemNotice;
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.routing.algorithm.filterchain.ItineraryListFilter;
 
 /**
  * This filter makes sure that the itinerary with the least amount of transfers is not marked for
- * deletion
+ * deletion. It iterates over the itineraries and remove the SystemNotice if it contains
+ * the provided set of {@code filterKeys}. The itinerary must match all {@code filterKeys}, and
+ * if so the given keys are removed. Other system-notices are ignored.
  */
 public class RemoveDeletionFlagForLeastTransfersItinerary implements ItineraryListFilter {
 
+  private final Set<String> filterKeys;
+
+  public RemoveDeletionFlagForLeastTransfersItinerary(List<String> filterKeys) {
+    this.filterKeys = new HashSet<>(filterKeys);
+  }
+
   @Override
   public List<Itinerary> filter(List<Itinerary> itineraries) {
     itineraries
       .stream()
       .min(numberOfTransfersComparator())
-      .ifPresent(Itinerary::removeDeletionFlags);
+      .filter(it ->
+        filterKeys.containsAll(it.getSystemNotices().stream().map(SystemNotice::tag).toList())
+      )
+      .ifPresent(it -> it.removeDeletionFlags(filterKeys));
 
     return itineraries;
   }

src/test/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainTest.java

@@ -18,6 +18,7 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
+import org.opentripplanner.model.SystemNotice;
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.model.plan.PlanTestConstants;
 import org.opentripplanner.model.plan.TestItineraryBuilder;
@@ -94,11 +95,12 @@ public void testDebugFilterChain() {
 
     // Walk first, then transit sorted on arrival-time
     assertEquals(toStr(List.of(i1, i2, i3)), toStr(chain.filter(List.of(i1, i2, i3))));
-    assertTrue(i1.getSystemNotices().isEmpty());
-    assertFalse(i2.getSystemNotices().isEmpty());
-    assertFalse(i3.getSystemNotices().isEmpty());
-    assertEquals("transit-vs-street-filter", i2.getSystemNotices().get(0).tag());
-    assertEquals("outside-search-window", i3.getSystemNotices().get(0).tag());
+    assertEquals("[]", toStringOfTags(i1.getSystemNotices()));
+    assertEquals(
+      "[transit-vs-street-filter, transit-vs-walk-filter]",
+      toStringOfTags(i2.getSystemNotices())
+    );
+    assertEquals("[outside-search-window]", toStringOfTags(i3.getSystemNotices()));
   }
 
   @Test
@@ -320,4 +322,10 @@ public void removeTransitWithHigherCostThanBestOnStreetOnlyEnabled() {
       assertEquals(toStr(List.of(walk)), toStr(chain.filter(List.of(walk, bus))));
     }
   }
+
+  private static String toStringOfTags(List<SystemNotice> systemNotices) {
+    return systemNotices == null
+      ? "[]"
+      : systemNotices.stream().map(SystemNotice::tag).toList().toString();
+  }
 }

src/test/java/org/opentripplanner/routing/algorithm/filterchain/filter/GroupByFilterTest.java

@@ -5,6 +5,7 @@
 import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary;
 
 import java.util.List;
+import java.util.Set;
 import org.junit.jupiter.api.Test;
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.model.plan.PlanTestConstants;
@@ -14,6 +15,8 @@
 
 public class GroupByFilterTest implements PlanTestConstants {
 
+  private static final String TEST_FILTER_TAG = "test";
+
   /**
    * This test group by exact trip ids and test that the reduce function works properly. It does not
    * merge any groups.
@@ -35,8 +38,11 @@ public void aSimpleTestGroupByMatchingTripIdsNoMerge() {
     assertFalse(i2a.isFlaggedForDeletion());
     assertTrue(i2b.isFlaggedForDeletion());
 
-    // Remove notice after asserting
-    i2b.removeDeletionFlags();
+    // Remove an none existing set of tags
+    i2b.removeDeletionFlags(Set.of("ANY_TAG"));
+    assertTrue(i2b.isFlaggedForDeletion());
+
+    i2b.removeDeletionFlags(Set.of(TEST_FILTER_TAG));
 
     // With min Limit = 2, we get two from each group
     createFilter(2).filter(all);
@@ -78,9 +84,9 @@ public void testMerging() {
 
       // Remove notices after asserting
       assertTrue(i11.isFlaggedForDeletion());
-      i11.removeDeletionFlags();
+      i11.removeDeletionFlags(Set.of());
       assertTrue(i12.isFlaggedForDeletion());
-      i12.removeDeletionFlags();
+      i12.removeDeletionFlags(Set.of());
     }
   }
 
@@ -92,7 +98,9 @@ private GroupByFilter<AGroupId> createFilter(int maxNumberOfItinerariesPrGroup)
       i -> new AGroupId(i.firstLeg().getTrip().getId().getId()),
       List.of(
         new SortingFilter(SortOrderComparator.defaultComparatorDepartAfter()),
-        new DeletionFlaggingFilter(new MaxLimitFilter("test", maxNumberOfItinerariesPrGroup))
+        new DeletionFlaggingFilter(
+          new MaxLimitFilter(TEST_FILTER_TAG, maxNumberOfItinerariesPrGroup)
+        )
       )
     );
   }