From a94b34eb22580e41105d5b1962ca254e5afbc0db Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 26 Jan 2024 11:39:55 +0100 Subject: [PATCH] fix: Log trip and pattern id when negative trip-times occurs in routing. --- .../framework/lang/ObjectUtils.java | 13 ++++++++++++ .../framework/tostring/ToStringBuilder.java | 14 +++++++++++-- .../request/TripScheduleWithOffset.java | 4 +++- .../model/network/RoutingTripPattern.java | 2 +- .../framework/lang/ObjectUtilsTest.java | 13 ++++++++++++ .../tostring/ToStringBuilderTest.java | 20 +++++++++++++++++++ 6 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/framework/lang/ObjectUtils.java b/src/main/java/org/opentripplanner/framework/lang/ObjectUtils.java index 1e67b8f253e..a3f18748987 100644 --- a/src/main/java/org/opentripplanner/framework/lang/ObjectUtils.java +++ b/src/main/java/org/opentripplanner/framework/lang/ObjectUtils.java @@ -1,6 +1,7 @@ package org.opentripplanner.framework.lang; import java.util.function.Function; +import java.util.function.Supplier; import javax.annotation.Nullable; /** @@ -33,6 +34,18 @@ public static T ifNotNull( return ifNotNull(getter.apply(entity), defaultValue); } + /** + * Get the value or {@code null}, ignore any exceptions. This is useful if you must traverse + * a long call-chain like {@code a.b().c().d()...} when e.g. logging. + */ + public static T safeGetOrNull(Supplier body) { + try { + return body.get(); + } catch (Exception ignore) { + return null; + } + } + public static T requireNotInitialized(T oldValue, T newValue) { return requireNotInitialized(null, oldValue, newValue); } diff --git a/src/main/java/org/opentripplanner/framework/tostring/ToStringBuilder.java b/src/main/java/org/opentripplanner/framework/tostring/ToStringBuilder.java index cb5d26294db..6480b6f1187 100644 --- a/src/main/java/org/opentripplanner/framework/tostring/ToStringBuilder.java +++ b/src/main/java/org/opentripplanner/framework/tostring/ToStringBuilder.java @@ -13,9 +13,11 @@ import java.util.Collection; import java.util.Objects; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.opentripplanner.framework.lang.ObjectUtils; import org.opentripplanner.framework.lang.OtpNumberFormat; import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.framework.time.TimeUtils; @@ -134,12 +136,20 @@ public ToStringBuilder addObj(String name, Object value, @Nullable Object ignore return addIfNotIgnored(name, value, ignoreValue, Object::toString); } + /** + * Add the result of the given supplier. If the supplier return {@code null} or an exceptions + * is thrown, then nothing is added - the result is ignored. + */ + public ToStringBuilder addObjOpSafe(String name, Supplier body) { + return addObj(name, ObjectUtils.safeGetOrNull(body)); + } + /** * Use this if you would like a custom toString function to convert the value. If the given value * is null, then the value is not printed. *

* Implementation note! The "Op" (Operation) suffix is necessary to separate this from - * {@link #addObj(String, Object, Object)}, when the last argument is null. + * {@link #addObj(String, Object, Object)}, when the last argument is null. */ public ToStringBuilder addObjOp( String name, @@ -176,7 +186,7 @@ public ToStringBuilder addDoubles(String name, double[] value, double ignoreValu return addIt(name, Arrays.toString(value)); } - /** Add collection if not null or not empty, all elements are added */ + /** Add the collection if not null or not empty, all elements are added */ public ToStringBuilder addCol(String name, Collection c) { return addIfNotNull(name, c == null || c.isEmpty() ? null : c); } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TripScheduleWithOffset.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TripScheduleWithOffset.java index 3eb26150d97..7c5db27292e 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TripScheduleWithOffset.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/TripScheduleWithOffset.java @@ -107,7 +107,9 @@ public int getSecondsOffset() { public String toString() { return ToStringBuilder .of(TripScheduleWithOffset.class) - .addObj("trip", pattern.debugInfo()) + .addObj("info", pattern.debugInfo()) + .addObjOpSafe("id", () -> tripTimes.getTrip().getId().toString()) + .addObjOpSafe("pattern.id", () -> pattern.getTripPattern().getPattern().getId().toString()) .addServiceTime("depart", secondsOffset + getOriginalTripTimes().getDepartureTime(0)) .toString(); } diff --git a/src/main/java/org/opentripplanner/transit/model/network/RoutingTripPattern.java b/src/main/java/org/opentripplanner/transit/model/network/RoutingTripPattern.java index 00033b5f798..98d30432b32 100644 --- a/src/main/java/org/opentripplanner/transit/model/network/RoutingTripPattern.java +++ b/src/main/java/org/opentripplanner/transit/model/network/RoutingTripPattern.java @@ -117,7 +117,7 @@ public Route route() { @Override public String debugInfo() { - return pattern.logName() + " @" + index; + return pattern.logName() + " #" + index; } @Override diff --git a/src/test/java/org/opentripplanner/framework/lang/ObjectUtilsTest.java b/src/test/java/org/opentripplanner/framework/lang/ObjectUtilsTest.java index 9237b221fe5..274fb3e8700 100644 --- a/src/test/java/org/opentripplanner/framework/lang/ObjectUtilsTest.java +++ b/src/test/java/org/opentripplanner/framework/lang/ObjectUtilsTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.time.Duration; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; @@ -48,6 +49,18 @@ void requireNotInitialized() { assertEquals("Field is already set! Old value: old, new value: new.", ex.getMessage()); } + @Test + void safeGetOrNull() { + assertEquals("test", ObjectUtils.safeGetOrNull(() -> "test")); + assertEquals(3000, ObjectUtils.safeGetOrNull(() -> Duration.ofSeconds(3).toMillis())); + assertNull(ObjectUtils.safeGetOrNull(() -> null)); + assertNull( + ObjectUtils.safeGetOrNull(() -> { + throw new NullPointerException("Something went wrong - ignore"); + }) + ); + } + @Test void toStringTest() { assertEquals("1", ObjectUtils.toString(1)); diff --git a/src/test/java/org/opentripplanner/framework/tostring/ToStringBuilderTest.java b/src/test/java/org/opentripplanner/framework/tostring/ToStringBuilderTest.java index 3e87006e953..90254c321a1 100644 --- a/src/test/java/org/opentripplanner/framework/tostring/ToStringBuilderTest.java +++ b/src/test/java/org/opentripplanner/framework/tostring/ToStringBuilderTest.java @@ -111,6 +111,26 @@ public void addObj() { ); } + @Test + public void addObjOpSafe() { + assertEquals( + "ToStringBuilderTest{obj: Foo{a: 5, b: 'X'}}", + subject().addObjOpSafe("obj", () -> new Foo(5, "X")).toString() + ); + assertEquals("ToStringBuilderTest{}", subject().addObjOpSafe("obj", () -> null).toString()); + assertEquals( + "ToStringBuilderTest{}", + subject() + .addObjOpSafe( + "obj", + () -> { + throw new IllegalStateException("Ignore"); + } + ) + .toString() + ); + } + @Test public void addObjOp() { var duration = Duration.ofMinutes(1);