Fix preference modification

opentripplanner · Dec 11, 2023 · 3d98683 · 3d98683
1 parent 2d540f8
commit 3d98683
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 23 deletions.
diff --git a/src/main/java/org/opentripplanner/routing/api/request/preference/BikePreferences.java b/src/main/java/org/opentripplanner/routing/api/request/preference/BikePreferences.java
@@ -1,6 +1,7 @@
 package org.opentripplanner.routing.api.request.preference;
 
 import static org.opentripplanner.framework.lang.DoubleUtils.doubleEquals;
+import static org.opentripplanner.framework.lang.ObjectUtils.ifNotNull;
 
 import java.io.Serializable;
 import java.util.Objects;
@@ -294,9 +295,7 @@ public Builder withSwitchCost(int switchCost) {
     }
 
     public Builder withParking(Consumer<VehicleParkingPreferences.Builder> body) {
-      var builder = VehicleParkingPreferences.of();
-      body.accept(builder);
-      this.parking = builder.build();
+      this.parking = ifNotNull(this.parking, original.parking).copyOf().apply(body).build();
       return this;
     }
 

diff --git a/src/main/java/org/opentripplanner/routing/api/request/preference/CarPreferences.java b/src/main/java/org/opentripplanner/routing/api/request/preference/CarPreferences.java
@@ -1,5 +1,7 @@
 package org.opentripplanner.routing.api.request.preference;
 
+import static org.opentripplanner.framework.lang.ObjectUtils.ifNotNull;
+
 import java.io.Serializable;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -197,9 +199,7 @@ public Builder withReluctance(double reluctance) {
     }
 
     public Builder withParking(Consumer<VehicleParkingPreferences.Builder> body) {
-      var builder = VehicleParkingPreferences.of();
-      body.accept(builder);
-      this.parking = builder.build();
+      this.parking = ifNotNull(this.parking, original.parking).copyOf().apply(body).build();
       return this;
     }
 

diff --git a/...in/java/org/opentripplanner/routing/api/request/preference/VehicleParkingPreferences.java b/...in/java/org/opentripplanner/routing/api/request/preference/VehicleParkingPreferences.java
@@ -5,6 +5,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Consumer;
 import org.opentripplanner.framework.model.Cost;
 import org.opentripplanner.framework.tostring.ToStringBuilder;
 import org.opentripplanner.routing.api.request.preference.filter.VehicleParkingFilter;
@@ -54,6 +55,10 @@ public static VehicleParkingPreferences.Builder of() {
     return new Builder(DEFAULT);
   }
 
+  public VehicleParkingPreferences.Builder copyOf() {
+    return new Builder(this);
+  }
+
   /**
    * What cost is applied to using parking that is not preferred.
    */
@@ -190,6 +195,11 @@ public Builder withParkTime(Duration duration) {
       return this;
     }
 
+    public Builder apply(Consumer<Builder> body) {
+      body.accept(this);
+      return this;
+    }
+
     public VehicleParkingPreferences build() {
       var newObj = new VehicleParkingPreferences(this);
       return original.equals(newObj) ? original : newObj;

diff --git a/...ava/org/opentripplanner/routing/api/request/preference/VehicleParkingPreferencesTest.java b/...ava/org/opentripplanner/routing/api/request/preference/VehicleParkingPreferencesTest.java
@@ -1,7 +1,6 @@
 package org.opentripplanner.routing.api.request.preference;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.opentripplanner.routing.api.request.preference.ImmutablePreferencesAsserts.assertEqualsAndHashCode;
 
 import java.time.Duration;
@@ -49,22 +48,10 @@ void parkTime() {
   }
 
   @Test
-  void testEqualsAndHashCode() {
-    // Return same object if no value is set
-    assertSame(VehicleParkingPreferences.DEFAULT, VehicleParkingPreferences.of().build());
-
-    // Create other with different values and same with the same values as the subject
-    var other = VehicleParkingPreferences
-      .of()
-      .withPreferredVehicleParkingTags(Set.of())
-      .withNotPreferredVehicleParkingTags(Set.of())
-      .withUnpreferredVehicleParkingTagCost(0)
-      .withRequiredVehicleParkingTags(Set.of())
-      .withBannedVehicleParkingTags(Set.of())
-      .withParkCost(0)
-      .withParkTime(0)
-      .build();
-    var same = createPreferences();
+  void testCopyOfEqualsAndHashCode() {
+    // Create a copy, make a change and set it back again to force creating a new object
+    var other = subject.copyOf().withParkCost(10).build();
+    var same = other.copyOf().withParkCost(PARKING_COST.toSeconds()).build();
     assertEqualsAndHashCode(subject, other, same);
   }