fix: Use inserted range when retracting range in toConnectedRanges (#…

…1056)

The `ConnectedRangesCalculator` incorrectly assumes the range for each
value is constant. The fix is to save the range on insert, and then pass
the saved range on retract. The other calculators do not have this
problem, because either:

- Their input should be immutable (ex: number)
- They used a map in their internal data structure that saved the
original value

Fixes #953

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/ConnectedRangesCalculator.java

@@ -5,9 +5,11 @@
 
 import ai.timefold.solver.core.api.score.stream.common.ConnectedRangeChain;
 import ai.timefold.solver.core.impl.score.stream.collector.connected_ranges.ConnectedRangeTracker;
+import ai.timefold.solver.core.impl.score.stream.collector.connected_ranges.Range;
 
 public final class ConnectedRangesCalculator<Interval_, Point_ extends Comparable<Point_>, Difference_ extends Comparable<Difference_>>
-        implements ObjectCalculator<Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>> {
+        implements
+        ObjectCalculator<Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>, Range<Interval_, Point_>> {
 
     private final ConnectedRangeTracker<Interval_, Point_, Difference_> context;
 
@@ -21,13 +23,15 @@ public ConnectedRangesCalculator(Function<? super Interval_, ? extends Point_> s
     }
 
     @Override
-    public void insert(Interval_ result) {
-        context.add(context.getRange(result));
+    public Range<Interval_, Point_> insert(Interval_ result) {
+        final var saved = context.getRange(result);
+        context.add(saved);
+        return saved;
     }
 
     @Override
-    public void retract(Interval_ result) {
-        context.remove(context.getRange(result));
+    public void retract(Range<Interval_, Point_> range) {
+        context.remove(range);
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/IntDistinctCountCalculator.java

@@ -5,18 +5,19 @@
 
 import ai.timefold.solver.core.impl.util.MutableInt;
 
-public final class IntDistinctCountCalculator<Input_> implements ObjectCalculator<Input_, Integer> {
+public final class IntDistinctCountCalculator<Input_> implements ObjectCalculator<Input_, Integer, Input_> {
     private final Map<Input_, MutableInt> countMap = new HashMap<>();
 
     @Override
-    public void insert(Input_ input) {
+    public Input_ insert(Input_ input) {
         countMap.computeIfAbsent(input, ignored -> new MutableInt()).increment();
+        return input;
     }
 
     @Override
-    public void retract(Input_ input) {
-        if (countMap.get(input).decrement() == 0) {
-            countMap.remove(input);
+    public void retract(Input_ mapped) {
+        if (countMap.get(mapped).decrement() == 0) {
+            countMap.remove(mapped);
         }
     }
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/LongDistinctCountCalculator.java

@@ -5,18 +5,19 @@
 
 import ai.timefold.solver.core.impl.util.MutableInt;
 
-public final class LongDistinctCountCalculator<Input_> implements ObjectCalculator<Input_, Long> {
+public final class LongDistinctCountCalculator<Input_> implements ObjectCalculator<Input_, Long, Input_> {
     private final Map<Input_, MutableInt> countMap = new HashMap<>();
 
     @Override
-    public void insert(Input_ input) {
+    public Input_ insert(Input_ input) {
         countMap.computeIfAbsent(input, ignored -> new MutableInt()).increment();
+        return input;
     }
 
     @Override
-    public void retract(Input_ input) {
-        if (countMap.get(input).decrement() == 0) {
-            countMap.remove(input);
+    public void retract(Input_ mapped) {
+        if (countMap.get(mapped).decrement() == 0) {
+            countMap.remove(mapped);
         }
     }
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/ObjectCalculator.java

@@ -1,11 +1,11 @@
 package ai.timefold.solver.core.impl.score.stream.collector;
 
-public sealed interface ObjectCalculator<Input_, Output_>
+public sealed interface ObjectCalculator<Input_, Output_, Mapped_>
         permits ConnectedRangesCalculator, IntDistinctCountCalculator, LongDistinctCountCalculator, ReferenceAverageCalculator,
         ReferenceSumCalculator, SequenceCalculator {
-    void insert(Input_ input);
+    Mapped_ insert(Input_ input);
 
-    void retract(Input_ input);
+    void retract(Mapped_ mapped);
 
     Output_ result();
 }

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/ReferenceAverageCalculator.java

@@ -8,7 +8,7 @@
 import java.util.function.BinaryOperator;
 import java.util.function.Supplier;
 
-public final class ReferenceAverageCalculator<Input_, Output_> implements ObjectCalculator<Input_, Output_> {
+public final class ReferenceAverageCalculator<Input_, Output_> implements ObjectCalculator<Input_, Output_, Input_> {
     int count = 0;
     Input_ sum;
     final BinaryOperator<Input_> adder;
@@ -51,15 +51,16 @@ public static Supplier<ReferenceAverageCalculator<Duration, Duration>> duration(
     }
 
     @Override
-    public void insert(Input_ input) {
+    public Input_ insert(Input_ input) {
         count++;
         sum = adder.apply(sum, input);
+        return input;
     }
 
     @Override
-    public void retract(Input_ input) {
+    public void retract(Input_ mapped) {
         count--;
-        sum = subtractor.apply(sum, input);
+        sum = subtractor.apply(sum, mapped);
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/ReferenceSumCalculator.java

@@ -2,7 +2,7 @@
 
 import java.util.function.BinaryOperator;
 
-public final class ReferenceSumCalculator<Result_> implements ObjectCalculator<Result_, Result_> {
+public final class ReferenceSumCalculator<Result_> implements ObjectCalculator<Result_, Result_, Result_> {
     private Result_ current;
     private final BinaryOperator<Result_> adder;
     private final BinaryOperator<Result_> subtractor;
@@ -14,13 +14,14 @@ public ReferenceSumCalculator(Result_ current, BinaryOperator<Result_> adder, Bi
     }
 
     @Override
-    public void insert(Result_ input) {
+    public Result_ insert(Result_ input) {
         current = adder.apply(current, input);
+        return input;
     }
 
     @Override
-    public void retract(Result_ input) {
-        current = subtractor.apply(current, input);
+    public void retract(Result_ mapped) {
+        current = subtractor.apply(current, mapped);
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/SequenceCalculator.java

@@ -7,7 +7,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.consecutive.ConsecutiveSetTree;
 
 public final class SequenceCalculator<Result_>
-        implements ObjectCalculator<Result_, SequenceChain<Result_, Integer>> {
+        implements ObjectCalculator<Result_, SequenceChain<Result_, Integer>, Result_> {
 
     private final ConsecutiveSetTree<Result_, Integer, Integer> context = new ConsecutiveSetTree<>(
             (Integer a, Integer b) -> b - a,
@@ -20,9 +20,10 @@ public SequenceCalculator(ToIntFunction<Result_> indexMap) {
     }
 
     @Override
-    public void insert(Result_ result) {
+    public Result_ insert(Result_ result) {
         var value = indexMap.applyAsInt(result);
         context.add(result, value);
+        return result;
     }
 
     @Override

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/AverageReferenceBiCollector.java

@@ -7,7 +7,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.ReferenceAverageCalculator;
 
 final class AverageReferenceBiCollector<A, B, Mapped_, Average_>
-        extends ObjectCalculatorBiCollector<A, B, Mapped_, Average_, ReferenceAverageCalculator<Mapped_, Average_>> {
+        extends ObjectCalculatorBiCollector<A, B, Mapped_, Average_, Mapped_, ReferenceAverageCalculator<Mapped_, Average_>> {
     private final Supplier<ReferenceAverageCalculator<Mapped_, Average_>> calculatorSupplier;
 
     AverageReferenceBiCollector(BiFunction<? super A, ? super B, ? extends Mapped_> mapper,

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/ConnectedRangesBiConstraintCollector.java

@@ -7,10 +7,11 @@
 
 import ai.timefold.solver.core.api.score.stream.common.ConnectedRangeChain;
 import ai.timefold.solver.core.impl.score.stream.collector.ConnectedRangesCalculator;
+import ai.timefold.solver.core.impl.score.stream.collector.connected_ranges.Range;
 
 final class ConnectedRangesBiConstraintCollector<A, B, Interval_, Point_ extends Comparable<Point_>, Difference_ extends Comparable<Difference_>>
         extends
-        ObjectCalculatorBiCollector<A, B, Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>, ConnectedRangesCalculator<Interval_, Point_, Difference_>> {
+        ObjectCalculatorBiCollector<A, B, Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>, Range<Interval_, Point_>, ConnectedRangesCalculator<Interval_, Point_, Difference_>> {
 
     private final Function<? super Interval_, ? extends Point_> startMap;
     private final Function<? super Interval_, ? extends Point_> endMap;

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/ConsecutiveSequencesBiConstraintCollector.java

@@ -10,7 +10,7 @@
 
 final class ConsecutiveSequencesBiConstraintCollector<A, B, Result_>
         extends
-        ObjectCalculatorBiCollector<A, B, Result_, SequenceChain<Result_, Integer>, SequenceCalculator<Result_>> {
+        ObjectCalculatorBiCollector<A, B, Result_, SequenceChain<Result_, Integer>, Result_, SequenceCalculator<Result_>> {
 
     private final ToIntFunction<Result_> indexMap;
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/CountDistinctIntBiCollector.java

@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.IntDistinctCountCalculator;
 
 final class CountDistinctIntBiCollector<A, B, Mapped_>
-        extends ObjectCalculatorBiCollector<A, B, Mapped_, Integer, IntDistinctCountCalculator<Mapped_>> {
+        extends ObjectCalculatorBiCollector<A, B, Mapped_, Integer, Mapped_, IntDistinctCountCalculator<Mapped_>> {
     CountDistinctIntBiCollector(BiFunction<? super A, ? super B, ? extends Mapped_> mapper) {
         super(mapper);
     }

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/CountDistinctLongBiCollector.java

@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.LongDistinctCountCalculator;
 
 final class CountDistinctLongBiCollector<A, B, Mapped_>
-        extends ObjectCalculatorBiCollector<A, B, Mapped_, Long, LongDistinctCountCalculator<Mapped_>> {
+        extends ObjectCalculatorBiCollector<A, B, Mapped_, Long, Mapped_, LongDistinctCountCalculator<Mapped_>> {
     CountDistinctLongBiCollector(BiFunction<? super A, ? super B, ? extends Mapped_> mapper) {
         super(mapper);
     }

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/ObjectCalculatorBiCollector.java

@@ -8,7 +8,7 @@
 import ai.timefold.solver.core.api.score.stream.bi.BiConstraintCollector;
 import ai.timefold.solver.core.impl.score.stream.collector.ObjectCalculator;
 
-abstract sealed class ObjectCalculatorBiCollector<A, B, Input_, Output_, Calculator_ extends ObjectCalculator<Input_, Output_>>
+abstract sealed class ObjectCalculatorBiCollector<A, B, Input_, Output_, Mapped_, Calculator_ extends ObjectCalculator<Input_, Output_, Mapped_>>
         implements BiConstraintCollector<A, B, Calculator_, Output_>
         permits AverageReferenceBiCollector, ConnectedRangesBiConstraintCollector, ConsecutiveSequencesBiConstraintCollector,
         CountDistinctIntBiCollector, CountDistinctLongBiCollector, SumReferenceBiCollector {
@@ -21,9 +21,9 @@ public ObjectCalculatorBiCollector(BiFunction<? super A, ? super B, ? extends In
     @Override
     public TriFunction<Calculator_, A, B, Runnable> accumulator() {
         return (calculator, a, b) -> {
-            final Input_ mapped = mapper.apply(a, b);
-            calculator.insert(mapped);
-            return () -> calculator.retract(mapped);
+            final var mapped = mapper.apply(a, b);
+            final var saved = calculator.insert(mapped);
+            return () -> calculator.retract(saved);
         };
     }
 
@@ -38,7 +38,7 @@ public boolean equals(Object object) {
             return true;
         if (object == null || getClass() != object.getClass())
             return false;
-        var that = (ObjectCalculatorBiCollector<?, ?, ?, ?, ?>) object;
+        var that = (ObjectCalculatorBiCollector<?, ?, ?, ?, ?, ?>) object;
         return Objects.equals(mapper, that.mapper);
     }
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/bi/SumReferenceBiCollector.java

@@ -8,7 +8,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.ReferenceSumCalculator;
 
 final class SumReferenceBiCollector<A, B, Result_>
-        extends ObjectCalculatorBiCollector<A, B, Result_, Result_, ReferenceSumCalculator<Result_>> {
+        extends ObjectCalculatorBiCollector<A, B, Result_, Result_, Result_, ReferenceSumCalculator<Result_>> {
     private final Result_ zero;
     private final BinaryOperator<Result_> adder;
     private final BinaryOperator<Result_> subtractor;

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/connected_ranges/RangeGapImpl.java

@@ -1,5 +1,7 @@
 package ai.timefold.solver.core.impl.score.stream.collector.connected_ranges;
 
+import java.util.Objects;
+
 import ai.timefold.solver.core.api.score.stream.common.ConnectedRange;
 import ai.timefold.solver.core.api.score.stream.common.RangeGap;
 
@@ -51,6 +53,21 @@ void setLength(Difference_ length) {
         this.length = length;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o)
+            return true;
+        if (!(o instanceof RangeGapImpl<?, ?, ?> rangeGap))
+            return false;
+        return Objects.equals(getPreviousRangeEnd(), rangeGap.getPreviousRangeEnd()) &&
+                Objects.equals(getNextRangeStart(), rangeGap.getNextRangeStart());
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(getPreviousRangeEnd(), getNextRangeStart());
+    }
+
     @Override
     public String toString() {
         return "RangeGap{" +

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/AverageReferenceQuadCollector.java

@@ -7,7 +7,8 @@
 import ai.timefold.solver.core.impl.score.stream.collector.ReferenceAverageCalculator;
 
 final class AverageReferenceQuadCollector<A, B, C, D, Mapped_, Average_>
-        extends ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Average_, ReferenceAverageCalculator<Mapped_, Average_>> {
+        extends
+        ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Average_, Mapped_, ReferenceAverageCalculator<Mapped_, Average_>> {
     private final Supplier<ReferenceAverageCalculator<Mapped_, Average_>> calculatorSupplier;
 
     AverageReferenceQuadCollector(QuadFunction<? super A, ? super B, ? super C, ? super D, ? extends Mapped_> mapper,

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/ConnectedRangesQuadConstraintCollector.java

@@ -8,10 +8,11 @@
 import ai.timefold.solver.core.api.function.QuadFunction;
 import ai.timefold.solver.core.api.score.stream.common.ConnectedRangeChain;
 import ai.timefold.solver.core.impl.score.stream.collector.ConnectedRangesCalculator;
+import ai.timefold.solver.core.impl.score.stream.collector.connected_ranges.Range;
 
 final class ConnectedRangesQuadConstraintCollector<A, B, C, D, Interval_, Point_ extends Comparable<Point_>, Difference_ extends Comparable<Difference_>>
         extends
-        ObjectCalculatorQuadCollector<A, B, C, D, Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>, ConnectedRangesCalculator<Interval_, Point_, Difference_>> {
+        ObjectCalculatorQuadCollector<A, B, C, D, Interval_, ConnectedRangeChain<Interval_, Point_, Difference_>, Range<Interval_, Point_>, ConnectedRangesCalculator<Interval_, Point_, Difference_>> {
 
     private final Function<? super Interval_, ? extends Point_> startMap;
     private final Function<? super Interval_, ? extends Point_> endMap;

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/ConsecutiveSequencesQuadConstraintCollector.java

@@ -10,7 +10,7 @@
 
 final class ConsecutiveSequencesQuadConstraintCollector<A, B, C, D, Result_>
         extends
-        ObjectCalculatorQuadCollector<A, B, C, D, Result_, SequenceChain<Result_, Integer>, SequenceCalculator<Result_>> {
+        ObjectCalculatorQuadCollector<A, B, C, D, Result_, SequenceChain<Result_, Integer>, Result_, SequenceCalculator<Result_>> {
 
     private final ToIntFunction<Result_> indexMap;
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/CountDistinctIntQuadCollector.java

@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.IntDistinctCountCalculator;
 
 final class CountDistinctIntQuadCollector<A, B, C, D, Mapped_>
-        extends ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Integer, IntDistinctCountCalculator<Mapped_>> {
+        extends ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Integer, Mapped_, IntDistinctCountCalculator<Mapped_>> {
     CountDistinctIntQuadCollector(QuadFunction<? super A, ? super B, ? super C, ? super D, ? extends Mapped_> mapper) {
         super(mapper);
     }

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/CountDistinctLongQuadCollector.java

@@ -6,7 +6,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.LongDistinctCountCalculator;
 
 final class CountDistinctLongQuadCollector<A, B, C, D, Mapped_>
-        extends ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Long, LongDistinctCountCalculator<Mapped_>> {
+        extends ObjectCalculatorQuadCollector<A, B, C, D, Mapped_, Long, Mapped_, LongDistinctCountCalculator<Mapped_>> {
     CountDistinctLongQuadCollector(QuadFunction<? super A, ? super B, ? super C, ? super D, ? extends Mapped_> mapper) {
         super(mapper);
     }

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/ObjectCalculatorQuadCollector.java

@@ -8,7 +8,7 @@
 import ai.timefold.solver.core.api.score.stream.quad.QuadConstraintCollector;
 import ai.timefold.solver.core.impl.score.stream.collector.ObjectCalculator;
 
-abstract sealed class ObjectCalculatorQuadCollector<A, B, C, D, Input_, Output_, Calculator_ extends ObjectCalculator<Input_, Output_>>
+abstract sealed class ObjectCalculatorQuadCollector<A, B, C, D, Input_, Output_, Mapped_, Calculator_ extends ObjectCalculator<Input_, Output_, Mapped_>>
         implements QuadConstraintCollector<A, B, C, D, Calculator_, Output_>
         permits AverageReferenceQuadCollector, ConnectedRangesQuadConstraintCollector,
         ConsecutiveSequencesQuadConstraintCollector, CountDistinctIntQuadCollector, CountDistinctLongQuadCollector,
@@ -23,9 +23,9 @@ public ObjectCalculatorQuadCollector(QuadFunction<? super A, ? super B, ? super
     @Override
     public PentaFunction<Calculator_, A, B, C, D, Runnable> accumulator() {
         return (calculator, a, b, c, d) -> {
-            final Input_ mapped = mapper.apply(a, b, c, d);
-            calculator.insert(mapped);
-            return () -> calculator.retract(mapped);
+            final var mapped = mapper.apply(a, b, c, d);
+            final var saved = calculator.insert(mapped);
+            return () -> calculator.retract(saved);
         };
     }
 
@@ -40,7 +40,7 @@ public boolean equals(Object object) {
             return true;
         if (object == null || getClass() != object.getClass())
             return false;
-        var that = (ObjectCalculatorQuadCollector<?, ?, ?, ?, ?, ?, ?>) object;
+        var that = (ObjectCalculatorQuadCollector<?, ?, ?, ?, ?, ?, ?, ?>) object;
         return Objects.equals(mapper, that.mapper);
     }
 

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/quad/SumReferenceQuadCollector.java

@@ -8,7 +8,7 @@
 import ai.timefold.solver.core.impl.score.stream.collector.ReferenceSumCalculator;
 
 final class SumReferenceQuadCollector<A, B, C, D, Result_>
-        extends ObjectCalculatorQuadCollector<A, B, C, D, Result_, Result_, ReferenceSumCalculator<Result_>> {
+        extends ObjectCalculatorQuadCollector<A, B, C, D, Result_, Result_, Result_, ReferenceSumCalculator<Result_>> {
     private final Result_ zero;
     private final BinaryOperator<Result_> adder;
     private final BinaryOperator<Result_> subtractor;

core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/tri/AverageReferenceTriCollector.java

@@ -7,7 +7,8 @@
 import ai.timefold.solver.core.impl.score.stream.collector.ReferenceAverageCalculator;
 
 final class AverageReferenceTriCollector<A, B, C, Mapped_, Average_>
-        extends ObjectCalculatorTriCollector<A, B, C, Mapped_, Average_, ReferenceAverageCalculator<Mapped_, Average_>> {
+        extends
+        ObjectCalculatorTriCollector<A, B, C, Mapped_, Average_, Mapped_, ReferenceAverageCalculator<Mapped_, Average_>> {
     private final Supplier<ReferenceAverageCalculator<Mapped_, Average_>> calculatorSupplier;
 
     AverageReferenceTriCollector(TriFunction<? super A, ? super B, ? super C, ? extends Mapped_> mapper,