From 629ec1b0129bdfbd78f7467869a28ef13806250c Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 2 Jan 2024 17:56:47 +0100 Subject: [PATCH 01/17] feature: Filter away none optimal access/egress paths for multi-criteria Raptor --- .../transit/AccessEgressFunctions.java | 94 +++++++++++++++---- .../rangeraptor/transit/AccessPaths.java | 5 +- .../rangeraptor/transit/EgressPaths.java | 5 +- .../transit/AccessEgressFunctionsTest.java | 91 ++++++++++++++++-- .../rangeraptor/transit/EgressPathsTest.java | 11 +-- 5 files changed, 171 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java index 5059be84312..275a0f85cf3 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java @@ -5,12 +5,15 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.function.Predicate; import java.util.function.ToIntFunction; import java.util.stream.Collectors; import org.opentripplanner.raptor.api.model.RaptorAccessEgress; import org.opentripplanner.raptor.util.paretoset.ParetoComparator; import org.opentripplanner.raptor.util.paretoset.ParetoSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class contains functions used by the {@link AccessPaths} and {@link EgressPaths} classes. @@ -23,6 +26,8 @@ */ public final class AccessEgressFunctions { + private static final Logger LOG = LoggerFactory.getLogger(AccessEgressFunctions.class); + /** * Filter standard(not multi-criteria) Raptor access and egress paths. A path is pareto optimal * for a given stop if @@ -47,32 +52,64 @@ public final class AccessEgressFunctions { * */ private static final ParetoComparator STANDARD_COMPARATOR = (l, r) -> - (l.stopReachedOnBoard() && !r.stopReachedOnBoard()) || - r.hasOpeningHours() || - (l.numberOfRides() < r.numberOfRides()) || - (l.durationInSeconds() < r.durationInSeconds()); + ( + l.stopReachedOnBoard() && + !r.stopReachedOnBoard() || + r.hasOpeningHours() || + l.numberOfRides() < r.numberOfRides() || + l.durationInSeconds() < r.durationInSeconds() + ); + + /** + * Filter Multi-criteria Raptor access and egress paths. This can be used to wash + * access/egress paths - paths that are not optimal using this should not be passed into + * Raptor - it is a bug. + */ + private static final ParetoComparator MC_COMPARATOR = (l, r) -> + ( + (l.stopReachedOnBoard() && !r.stopReachedOnBoard()) || + r.hasOpeningHours() || + l.numberOfRides() < r.numberOfRides() || + l.durationInSeconds() < r.durationInSeconds() || + l.c1() < r.c1() + ); /** private constructor to prevent instantiation of utils class. */ private AccessEgressFunctions() {} + /** + * Filter non-optimal paths away for the standard search. This method does not + * look at the c1 value. + */ static Collection removeNoneOptimalPathsForStandardRaptor( Collection paths ) { - // To avoid too many items in the pareto set we first group the paths by stop, - // for each stop we filter it down to the optimal pareto set. We could do this - // for multi-criteria as well, but it is likely not so important. The focus for - // the mc-set should be that the list of access/egress created in OTP should not - // contain to many non-optimal paths. - var mapByStop = groupByStop(paths); - var set = new ParetoSet<>(STANDARD_COMPARATOR); - Collection result = new ArrayList<>(); + return removeNoneOptimalPaths(paths, STANDARD_COMPARATOR); + } - mapByStop.forEachValue(list -> { - set.clear(); - set.addAll(list); - result.addAll(set); - return true; - }); + /** + * Filter non-optimal paths away for the multi-criteria search. This method should in theory + * not remove any paths since the caller should not pass in duplicates, but it turns out that + * this happens, so we do it. + */ + static Collection removeNoneOptimalPathsForMcRaptor( + Collection paths + ) { + var result = removeNoneOptimalPaths(paths, MC_COMPARATOR); + if (LOG.isDebugEnabled() && result.size() < paths.size()) { + var duplicates = new ArrayList<>(paths); + duplicates.removeAll(result); + // Note! This does not provide enough information to solve/debug this problem, but this is + // not a problem in Raptor, so we do not want to add more specific logging here - this does + // however document that the problem exist. Turn on debug logging and move the start/end + // coordinate around until you see this message. + // + // See https://github.com/opentripplanner/OpenTripPlanner/issues/5601 + LOG.warn( + "Duplicate access/egress paths passed into raptor:\n\t" + + duplicates.stream().map(Objects::toString).collect(Collectors.joining("\n\t")) + ); + } return result; } @@ -96,6 +133,27 @@ static TIntObjectMap> groupByStop(Collection removeNoneOptimalPaths( + Collection paths, + ParetoComparator comparator + ) { + var mapByStop = groupByStop(paths); + var set = new ParetoSet<>(comparator); + var result = new ArrayList(); + + for (int stop : mapByStop.keys()) { + var list = mapByStop.get(stop); + set.clear(); + set.addAll(list); + result.addAll(set); + } + return result; + } + private static List getOrCreate( int key, TIntObjectMap> map diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java index 940937f82db..dd757ac0415 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java @@ -1,6 +1,7 @@ package org.opentripplanner.raptor.rangeraptor.transit; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByRound; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; import gnu.trove.map.TIntObjectMap; @@ -57,7 +58,9 @@ public int calculateMaxNumberOfRides() { * This method is static and package local to enable unit-testing. */ public static AccessPaths create(Collection paths, RaptorProfile profile) { - if (!profile.is(RaptorProfile.MULTI_CRITERIA)) { + if (profile.is(RaptorProfile.MULTI_CRITERIA)) { + paths = removeNoneOptimalPathsForMcRaptor(paths); + } else { paths = removeNoneOptimalPathsForStandardRaptor(paths); } return new AccessPaths( diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java index 2038ab543df..a1688ff8f5b 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java @@ -2,6 +2,7 @@ import static org.opentripplanner.raptor.api.request.RaptorProfile.MULTI_CRITERIA; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByStop; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; import gnu.trove.map.TIntObjectMap; @@ -31,7 +32,9 @@ private EgressPaths(TIntObjectMap> pathsByStop) { * This method is static and package local to enable unit-testing. */ public static EgressPaths create(Collection paths, RaptorProfile profile) { - if (!MULTI_CRITERIA.is(profile)) { + if (MULTI_CRITERIA.is(profile)) { + paths = removeNoneOptimalPathsForMcRaptor(paths); + } else { paths = removeNoneOptimalPathsForStandardRaptor(paths); } return new EgressPaths(groupByStop(paths)); diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java index eab9ee418cf..041d74786d7 100644 --- a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java +++ b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java @@ -6,6 +6,7 @@ import static org.opentripplanner.raptor._data.transit.TestAccessEgress.flexAndWalk; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByRound; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByStop; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; import java.util.Arrays; @@ -24,19 +25,26 @@ class AccessEgressFunctionsTest implements RaptorTestConstants { public static final int TRANSFER_SLACK = D1m; private static final int STOP = 8; - - private static final RaptorAccessEgress WALK_10m = TestAccessEgress.walk(STOP, D10m); - private static final RaptorAccessEgress WALK_8m = TestAccessEgress.walk(STOP, D8m); - private static final RaptorAccessEgress FLEX_1x_10m = flex(STOP, D10m, 1); - private static final RaptorAccessEgress FLEX_1x_8m = flex(STOP, D8m, 1); - private static final RaptorAccessEgress FLEX_2x_8m = flex(STOP, D8m, 2); - private static final RaptorAccessEgress FLEX_AND_WALK_1x_8m = flexAndWalk(STOP, D8m, 1); + private static final int C1 = 1000; + private static final int C1_LOW = 999; + + private static final RaptorAccessEgress WALK_10m = TestAccessEgress.walk(STOP, D10m, C1); + private static final RaptorAccessEgress WALK_10m_C1_LOW = TestAccessEgress.walk( + STOP, + D10m, + C1_LOW + ); + private static final RaptorAccessEgress WALK_8m = TestAccessEgress.walk(STOP, D8m, C1); + private static final RaptorAccessEgress FLEX_1x_10m = flex(STOP, D10m, 1, C1); + private static final RaptorAccessEgress FLEX_1x_8m = flex(STOP, D8m, 1, C1); + private static final RaptorAccessEgress FLEX_2x_8m = flex(STOP, D8m, 2, C1); + private static final RaptorAccessEgress FLEX_AND_WALK_1x_8m = flexAndWalk(STOP, D8m, 1, C1); private static final RaptorAccessEgress WALK_W_OPENING_HOURS_8m = TestAccessEgress - .walk(STOP, D8m) + .walk(STOP, D8m, C1) .openingHours(T00_00, T01_00); private static final RaptorAccessEgress WALK_W_OPENING_HOURS_8m_OTHER = TestAccessEgress - .walk(STOP, D8m) + .walk(STOP, D8m, C1) .openingHours(T00_10, T01_00); @Test @@ -101,6 +109,71 @@ void removeNoneOptimalPathsForStandardRaptorTest() { ); } + @Test + void removeNoneOptimalPathsForMcRaptorTest() { + // Empty set + assertElements(List.of(), removeNoneOptimalPathsForMcRaptor(List.of())); + + // One element + assertElements(List.of(WALK_8m), removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m))); + + // Lowest cost + assertElements( + List.of(WALK_10m_C1_LOW), + removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_10m_C1_LOW)) + ); + + // Shortest duration + assertElements(List.of(WALK_8m), removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_10m))); + + // Fewest rides + assertElements( + List.of(FLEX_1x_8m), + removeNoneOptimalPathsForMcRaptor(List.of(FLEX_1x_8m, FLEX_2x_8m)) + ); + + // Arriving at the stop on-board, and by-foot. + // OnBoard is better because we can do a transfer walk to nearby stops. + assertElements( + List.of(FLEX_1x_8m), + removeNoneOptimalPathsForMcRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_8m)) + ); + + // Flex+walk is faster, flex arrive on-board, both is optimal + assertElements( + List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m), + removeNoneOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m)) + ); + + // Walk has few rides, and Flex is faster - both is optimal + assertElements( + List.of(WALK_10m, FLEX_1x_8m), + removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, FLEX_1x_8m)) + ); + + // Walk without opening hours is better than with, because it can be time-shifted without + // any constraints + assertElements( + List.of(WALK_8m), + removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_W_OPENING_HOURS_8m)) + ); + + // Walk with opening hours can NOT dominate another access/egress without - even if it is + // faster. The reason is that it may not be allowed to time-shift it to the desired time. + assertElements( + List.of(WALK_10m, WALK_W_OPENING_HOURS_8m), + removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_W_OPENING_HOURS_8m)) + ); + + // If two paths both have opening hours, both should be accepted. + assertElements( + List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER), + removeNoneOptimalPathsForMcRaptor( + List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER) + ) + ); + } + @Test void groupByRoundTest() { // Map one element diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPathsTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPathsTest.java index 6d3b25c1d31..144904fa3e6 100644 --- a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPathsTest.java +++ b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPathsTest.java @@ -38,7 +38,7 @@ class EgressPathsTest { // Number of rides, smallest is better flex(STOP_D, D1m, 2), flex(STOP_D, D1m, 3), - // Opening Hours dominate each other(no check on overlapping) + // Opening Hours dominate each other (no check on overlapping) walk(STOP_E, D2m), walk(STOP_E, D1m).openingHours("10:00", "11:45"), walk(STOP_E, D1m).openingHours("11:30", "12:30"), @@ -83,18 +83,15 @@ void listAll() { """.strip(), subjectStd.listAll().stream().map(Object::toString).sorted().collect(Collectors.joining("\n")) ); + assertEquals( """ Flex 1m C₁120 1x ~ 3 Flex 1m C₁120 1x ~ 4 Flex 1m C₁120 2x ~ 5 - Flex 1m C₁120 3x ~ 5 - Flex 2m C₁240 1x ~ 3 - Flex+Walk 1m C₁120 1x ~ 4 Walk 1m C₁120 Open(10:00 11:45) ~ 6 Walk 1m C₁120 Open(11:30 12:30) ~ 6 Walk 1m C₁120 ~ 2 - Walk 2m C₁240 Open(14:00 14:00) ~ 6 Walk 2m C₁240 ~ 6 """.strip(), subjectMc.listAll().stream().map(Object::toString).sorted().collect(Collectors.joining("\n")) @@ -107,8 +104,10 @@ void walkToDestinationEgressStops() { toString(new int[] { STOP_A, STOP_E }), toString(subjectStd.egressesWitchStartByWalking()) ); + + //[2, 6] assertEquals( - toString(new int[] { STOP_A, STOP_C, STOP_E }), + toString(new int[] { STOP_A, STOP_E }), toString(subjectMc.egressesWitchStartByWalking()) ); } From 17d20c9d41a3d6345549c5fd8416fbdcd21d948e Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 3 Jan 2024 16:08:30 +0100 Subject: [PATCH 02/17] feature: Enable Raptor debug logging by event type --- .../transit/mappers/RaptorRequestMapper.java | 21 ++++++++++++++++--- .../routing/api/request/DebugEventType.java | 11 ++++++++++ .../routing/api/request/DebugRaptor.java | 19 ++++++++++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 src/main/java/org/opentripplanner/routing/api/request/DebugEventType.java diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java index 75c1bcf7214..e63546a87a2 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java @@ -13,6 +13,7 @@ import org.opentripplanner.raptor.api.model.RaptorAccessEgress; import org.opentripplanner.raptor.api.model.RaptorConstants; import org.opentripplanner.raptor.api.model.RelaxFunction; +import org.opentripplanner.raptor.api.request.DebugRequestBuilder; import org.opentripplanner.raptor.api.request.Optimization; import org.opentripplanner.raptor.api.request.PassThroughPoint; import org.opentripplanner.raptor.api.request.RaptorRequest; @@ -22,6 +23,7 @@ import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitPriorityGroup32n; +import org.opentripplanner.routing.api.request.DebugEventType; import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.request.framework.CostLinearFunction; import org.opentripplanner.transit.model.site.StopLocation; @@ -156,10 +158,11 @@ private RaptorRequest doMap() { .addStops(raptorDebugging.stops()) .setPath(raptorDebugging.path()) .debugPathFromStopIndex(raptorDebugging.debugPathFromStopIndex()) - .stopArrivalListener(debugLogger::stopArrivalLister) - .patternRideDebugListener(debugLogger::patternRideLister) - .pathFilteringListener(debugLogger::pathFilteringListener) .logger(debugLogger); + + for (var type : raptorDebugging.eventTypes()) { + addLogListenerForEachEventTypeRequested(debug, type, debugLogger); + } } if (!request.timetableView() && request.arriveBy()) { @@ -209,4 +212,16 @@ private int relativeTime(Instant time) { } return (int) (time.getEpochSecond() - transitSearchTimeZeroEpocSecond); } + + private static void addLogListenerForEachEventTypeRequested( + DebugRequestBuilder target, + DebugEventType type, + SystemErrDebugLogger logger + ) { + switch (type) { + case STOP_ARRIVALS -> target.stopArrivalListener(logger::stopArrivalLister); + case PATTERN_RIDES -> target.patternRideDebugListener(logger::patternRideLister); + case DESTINATION_ARRIVALS -> target.pathFilteringListener(logger::pathFilteringListener); + } + } } diff --git a/src/main/java/org/opentripplanner/routing/api/request/DebugEventType.java b/src/main/java/org/opentripplanner/routing/api/request/DebugEventType.java new file mode 100644 index 00000000000..09f9dbbe732 --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/api/request/DebugEventType.java @@ -0,0 +1,11 @@ +package org.opentripplanner.routing.api.request; + +/** + * Raptor check paths in 3 different places. The debugger can print events + * for each of these. + */ +public enum DebugEventType { + STOP_ARRIVALS, + PATTERN_RIDES, + DESTINATION_ARRIVALS, +} diff --git a/src/main/java/org/opentripplanner/routing/api/request/DebugRaptor.java b/src/main/java/org/opentripplanner/routing/api/request/DebugRaptor.java index 7d6c1472eee..0b9ff4f0c30 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/DebugRaptor.java +++ b/src/main/java/org/opentripplanner/routing/api/request/DebugRaptor.java @@ -3,7 +3,11 @@ import java.io.Serial; import java.io.Serializable; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; import java.util.List; +import java.util.Set; import java.util.regex.Pattern; import org.opentripplanner.framework.tostring.ToStringBuilder; import org.slf4j.Logger; @@ -46,14 +50,16 @@ public class DebugRaptor implements Serializable { private List stops = List.of(); private List path = List.of(); private int debugPathFromStopIndex = 0; + private Set eventTypes = EnumSet.noneOf(DebugEventType.class); public DebugRaptor() {} - /** Avoid using clone(), use copy-constructor instead(Josh Bloch). */ + /** Avoid using clone(), use copy-constructor instead (Josh Bloch). */ public DebugRaptor(DebugRaptor other) { this.stops = List.copyOf(other.stops); this.path = List.copyOf(other.path); this.debugPathFromStopIndex = other.debugPathFromStopIndex; + this.eventTypes = EnumSet.copyOf(other.eventTypes); } public boolean isEnabled() { @@ -90,12 +96,23 @@ public int debugPathFromStopIndex() { return debugPathFromStopIndex; } + public Set eventTypes() { + return Collections.unmodifiableSet(eventTypes); + } + + public DebugRaptor withEventTypes(Collection eventTypes) { + this.eventTypes.clear(); + this.eventTypes.addAll(eventTypes); + return this; + } + @Override public String toString() { return ToStringBuilder .of(DebugRaptor.class) .addObj("stops", toString(stops, FIRST_STOP_INDEX)) .addObj("path", toString(path, debugPathFromStopIndex)) + .addCol("eventType", eventTypes) .toString(); } From a3e426fce2e9f35e062a5d0e5c705f5113f95af2 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 3 Jan 2024 19:21:07 +0100 Subject: [PATCH 03/17] sandbox: Extract log config and rearrange OTP Startup View --- .../InteractiveOtpMain.java | 19 ++- .../ext/interactivelauncher/Model.java | 55 ++++---- .../OtpDebugController.java | 35 +++++ .../ext/interactivelauncher/SetupResult.java | 99 ------------- .../{ => logging}/DebugLoggingSupport.java | 33 ++--- .../interactivelauncher/logging/LogModel.java | 56 ++++++++ .../interactivelauncher/logging/LogView.java | 34 +++++ .../logging/OTPDebugLoggers.java | 19 +++ .../DataSourceRootView.java} | 17 ++- .../startup/DataSourcesView.java | 133 ++++++++++++++++++ .../{views => startup}/MainView.java | 127 +++++------------ .../{views => startup}/OptionsView.java | 64 ++++----- .../StartOtpButtonView.java | 4 +- .../startup/StatusBar.java | 15 ++ .../{ => support}/SearchForOtpConfig.java | 10 +- .../{views => support}/ViewUtils.java | 23 +-- .../views/DataSourcesView.java | 84 ----------- .../interactivelauncher/views/StatusBar.java | 15 -- 18 files changed, 434 insertions(+), 408 deletions(-) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java delete mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/SetupResult.java rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => logging}/DebugLoggingSupport.java (61%) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{views/SearchDirectoryView.java => startup/DataSourceRootView.java} (80%) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{views => startup}/MainView.java (55%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{views => startup}/OptionsView.java (63%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{views => startup}/StartOtpButtonView.java (82%) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StatusBar.java rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => support}/SearchForOtpConfig.java (85%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{views => support}/ViewUtils.java (53%) delete mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/views/DataSourcesView.java delete mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StatusBar.java diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java index 1f9227b3be6..d61ae6bfcd3 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java @@ -1,19 +1,18 @@ package org.opentripplanner.ext.interactivelauncher; -import static org.opentripplanner.ext.interactivelauncher.DebugLoggingSupport.configureDebugLogging; - -import org.opentripplanner.ext.interactivelauncher.views.MainView; +import org.opentripplanner.ext.interactivelauncher.startup.MainView; import org.opentripplanner.standalone.OTPMain; /** - * This class provide a main method to start a GUI which can start OTPMain. + * This class provides a main method to start a GUI which can start OTPMain. *

- * The UI allow the user to select a OTP configuration data set. The list of data location is - * created by searching the a root data source directory. + * The UI allows the user to select the OTP configuration dataset. The list of data locations is + * created by searching the root data source directory. *

- * The user then select what he/she want OTP to do. The settings are stored in the - * .interactive_otp_main.json file in the folder InteractiveOtpMain is started. The - * settings from the last run is loaded next time InteractiveOtpMain is started. + * The user then selects what he/she wants OTP to do. + * The settings are stored in the + * .interactive_otp_main.json file in the folder InteractiveOtpMain is started. + * The settings from the last run are loaded the next time InteractiveOtpMain is started. */ public class InteractiveOtpMain { @@ -32,7 +31,7 @@ private void run() { private void startOtp() { model.save(); - configureDebugLogging(model.getDebugLogging()); + new OtpDebugController(model).start(); System.out.println("Start OTP: " + model + "\n"); OTPMain.main(model.asOtpArgs()); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java index ee2dde3e684..1550f8bc1ab 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java @@ -6,11 +6,10 @@ import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import java.util.function.Consumer; +import org.opentripplanner.ext.interactivelauncher.logging.LogModel; +import org.opentripplanner.ext.interactivelauncher.support.SearchForOtpConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,8 +19,6 @@ public class Model implements Serializable { private static final File MODEL_FILE = new File("interactive_otp_main.json"); - private final Map debugLogging = new HashMap<>(); - @JsonIgnore private transient Consumer commandLineChange; @@ -32,13 +29,16 @@ public class Model implements Serializable { private boolean saveGraph = false; private boolean serveGraph = true; private boolean visualizer = false; + private LogModel logModel; - public Model() { - setupListOfDebugLoggers(); - } + public Model() {} public static Model load() { - return MODEL_FILE.exists() ? readFromFile() : new Model(); + var model = MODEL_FILE.exists() ? readFromFile() : createNew(); + // Setup callbacks + model.logModel.init(model::save); + + return model; } public void subscribeCmdLineUpdates(Consumer commandLineChange) { @@ -134,19 +134,6 @@ public void setVisualizer(boolean visualizer) { notifyChangeListener(); } - public Map getDebugLogging() { - return debugLogging; - } - - public void setDebugLogging(Map map) { - for (Entry e : map.entrySet()) { - // Only keep entries that exist in the log config - if (debugLogging.containsKey(e.getKey())) { - debugLogging.put(e.getKey(), e.getValue()); - } - } - } - @Override public String toString() { return ( @@ -174,6 +161,10 @@ public void save() { } } + public LogModel getLogModel() { + return logModel; + } + @JsonIgnore String getDataSourceDirectory() { if (dataSource == null) { @@ -210,16 +201,26 @@ String[] asOtpArgs() { return args.toArray(new String[0]); } + private static Model createNew() { + var model = new Model(); + model.logModel = new LogModel(); + model.logModel.initFromConfig(); + model.setupCallbacks(); + return model; + } + private static Model readFromFile() { try { - return new ObjectMapper().readValue(MODEL_FILE, Model.class); + var model = new ObjectMapper().readValue(MODEL_FILE, Model.class); + model.setupCallbacks(); + return model; } catch (IOException e) { System.err.println( "Unable to read the InteractiveOtpMain state cache. If the model changed this " + "is expected, and it will work next time. Cause: " + e.getMessage() ); - return new Model(); + return createNew(); } } @@ -237,9 +238,7 @@ private boolean buildStreetOnly() { return buildStreet && !buildTransit; } - private void setupListOfDebugLoggers() { - for (String log : DebugLoggingSupport.getLogs()) { - debugLogging.put(log, Boolean.FALSE); - } + private void setupCallbacks() { + logModel.init(this::save); } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java new file mode 100644 index 00000000000..95635c5bb0c --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java @@ -0,0 +1,35 @@ +package org.opentripplanner.ext.interactivelauncher; + +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BACKGROUND; + +import javax.swing.JComponent; +import javax.swing.JFrame; +import javax.swing.JPanel; +import javax.swing.JTabbedPane; +import org.opentripplanner.ext.interactivelauncher.logging.LogModel; +import org.opentripplanner.ext.interactivelauncher.logging.LogView; + +public class OtpDebugController { + + private final JFrame debugFrame = new JFrame("OTP Debug Controller"); + + public OtpDebugController(Model model) { + var tabPanel = new JTabbedPane(); + tabPanel.addTab("Logging", createLogPanel(model.getLogModel())); + tabPanel.addTab("Raptor", new JPanel()); + debugFrame.add(tabPanel); + debugFrame.getContentPane().setBackground(BACKGROUND); + start(); + } + + private static JComponent createLogPanel(LogModel logModel) { + return new LogView(logModel).panel(); + } + + public void start() { + debugFrame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE); + debugFrame.pack(); + debugFrame.setLocationRelativeTo(null); + debugFrame.setVisible(true); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/SetupResult.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/SetupResult.java deleted file mode 100644 index a6ecf5e7229..00000000000 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/SetupResult.java +++ /dev/null @@ -1,99 +0,0 @@ -package org.opentripplanner.ext.interactivelauncher; - -import java.io.File; -import java.util.ArrayList; -import java.util.List; - -public class SetupResult { - - private final File configDataDir; - private final boolean buildStreet; - private final boolean buildTransit; - private final boolean saveGraph; - private final boolean serveGraph; - - public SetupResult( - File configDataDir, - boolean buildStreet, - boolean buildTransit, - boolean saveGraph, - boolean serveGraph - ) { - this.configDataDir = configDataDir; - this.buildStreet = buildStreet; - this.buildTransit = buildTransit; - this.saveGraph = saveGraph; - this.serveGraph = serveGraph; - } - - @Override - public String toString() { - return ( - "SetupResult{" + - "configDataDir=" + - configDataDir.getAbsolutePath() + - (buildStreet ? ", buildStreet" : "") + - (buildTransit ? ", buildTransit" : "") + - (saveGraph ? ", saveGraph" : "") + - (serveGraph ? ", serveGraph" : "") + - '}' - ); - } - - public String toCliString() { - return String.join(" ", asOtpArgs()); - } - - File configDataDir() { - return configDataDir; - } - - boolean buildStreet() { - return buildStreet; - } - - boolean buildTransit() { - return buildTransit; - } - - boolean buildAll() { - return buildStreet && buildTransit; - } - - boolean buildStreetOnly() { - return buildStreet && !buildTransit; - } - - boolean saveGraph() { - return saveGraph; - } - - boolean serveGraph() { - return serveGraph; - } - - String[] asOtpArgs() { - List args = new ArrayList<>(); - - if (buildAll()) { - args.add("--build"); - } else if (buildStreet) { - args.add("--buildStreet"); - } else if (buildTransit) { - args.add("--loadStreet"); - } else { - args.add("--load"); - } - - if (saveGraph && (buildTransit || buildStreet)) { - args.add("--save"); - } - if (serveGraph && !buildStreetOnly()) { - args.add("--serve"); - } - - args.add(configDataDir.getAbsolutePath()); - - return args.toArray(new String[0]); - } -} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/DebugLoggingSupport.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java similarity index 61% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/DebugLoggingSupport.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java index e0b07a8c79e..fcfa323680f 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/DebugLoggingSupport.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher; +package org.opentripplanner.ext.interactivelauncher.logging; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; @@ -10,13 +10,13 @@ import org.slf4j.LoggerFactory; /** - * Responsible for integration with the OTP Debug log configuraton, reading loggers from the slf4j + * Responsible for integration with the OTP Debug log configuration, reading loggers from the slf4j * context and setting DEBUG level on selected loggers back. *

- * The log names are transformed to be more human readable: + * The log names are transformed to be more human-readable: * 

org.opentripplanner.routing.algorithm  -->  o.o.routing.algorithm
*/ -public class DebugLoggingSupport { +class DebugLoggingSupport { private static final String OTP = Pattern.quote("org.opentripplanner.") + ".*"; private static final String GRAPHQL = Pattern.quote("fea"); @@ -26,29 +26,26 @@ public class DebugLoggingSupport { "(" + OTP + "|" + GRAPHQL + "|" + NAMED_LOGGERS + ")" ); - public static List getLogs() { + static List getDebugLoggers() { List result = new ArrayList<>(); LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); for (Logger log : context.getLoggerList()) { var name = log.getName(); - if (!name.equals("ROOT") && LOG_MATCHER_PATTERN.matcher(name).matches()) { - result.add(logDisplayName(name)); + if (name.equals("ROOT") || log.getLevel() == null) { + continue; + } + if (log.getLevel().toInt() <= Level.DEBUG.toInt()) { + if (LOG_MATCHER_PATTERN.matcher(name).matches()) { + result.add(name); + } } } return result; } - public static void configureDebugLogging(Map loggers) { + static void configureDebugLogging(String logger, boolean debug) { LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); - for (Logger log : context.getLoggerList()) { - var name = logDisplayName(log.getName()); - if (loggers.getOrDefault(name, false)) { - log.setLevel(Level.DEBUG); - } - } - } - - private static String logDisplayName(String name) { - return name.replace("org.opentripplanner.", "o.o."); + var log = context.getLogger(logger); + log.setLevel(debug ? Level.DEBUG : Level.INFO); } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java new file mode 100644 index 00000000000..536c8d1dc87 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java @@ -0,0 +1,56 @@ +package org.opentripplanner.ext.interactivelauncher.logging; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import java.io.Serializable; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Responsible for storing the selected loggers to debug. This is + * serialized to store the user preferences between runs. + */ +public class LogModel implements Serializable { + + private final Set activeLoggers = new HashSet<>(); + + @JsonIgnore + private Runnable saveCallback; + + public LogModel() {} + + /** Need to set this manually to support JSON serialization. */ + public void init(Runnable saveCallback) { + this.saveCallback = saveCallback; + } + + /** Needed to do JSON serialization. */ + public Collection getActiveLoggers() { + return List.copyOf(activeLoggers); + } + + /** Needed to do JSON serialization. */ + public void setActiveLoggers(Collection loggers) { + this.activeLoggers.clear(); + this.activeLoggers.addAll(loggers); + } + + public void initFromConfig() { + activeLoggers.addAll(DebugLoggingSupport.getDebugLoggers()); + } + + boolean isLoggerEnabled(String name) { + return activeLoggers.contains(name); + } + + void turnLoggerOnOff(String name, boolean enable) { + if (enable) { + activeLoggers.add(name); + } else { + activeLoggers.remove(name); + } + DebugLoggingSupport.configureDebugLogging(name, enable); + saveCallback.run(); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java new file mode 100644 index 00000000000..a71f90eb697 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java @@ -0,0 +1,34 @@ +package org.opentripplanner.ext.interactivelauncher.logging; + +import javax.swing.Box; +import javax.swing.JCheckBox; +import javax.swing.JComponent; + +/** + * Display a list of loggers to turn on/off. + */ +public class LogView { + + private final Box panel = Box.createVerticalBox(); + private final LogModel model; + + public LogView(LogModel model) { + this.model = model; + OTPDebugLoggers.list().forEach(this::add); + } + + public JComponent panel() { + return panel; + } + + private void add(OTPDebugLoggers logger) { + var box = new JCheckBox(logger.label()); + box.setSelected(model.isLoggerEnabled(logger.logger())); + box.addActionListener(e -> selectLogger(logger.logger(), box.isSelected())); + panel.add(box); + } + + private void selectLogger(String logger, boolean selected) { + model.turnLoggerOnOff(logger, selected); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java new file mode 100644 index 00000000000..4663b6017de --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java @@ -0,0 +1,19 @@ +package org.opentripplanner.ext.interactivelauncher.logging; + +import java.util.List; + +record OTPDebugLoggers(String label, String logger) { + static List list() { + return List.of( + of("Data import issues", "DATA_IMPORT_ISSUES"), + of("All OTP debuggers", "org.opentripplanner"), + of("OTP request/response", "org.opentripplanner.routing.service.DefaultRoutingService"), + of("Raptor request/response", "org.opentripplanner.raptor.RaptorService"), + of("Transfer Optimization", "org.opentripplanner.routing.algorithm.transferoptimization") + ); + } + + private static OTPDebugLoggers of(String label, String logger) { + return new OTPDebugLoggers(label, logger); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/SearchDirectoryView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourceRootView.java similarity index 80% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/views/SearchDirectoryView.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourceRootView.java index ef054e2c879..e5ba9136e9d 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/SearchDirectoryView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourceRootView.java @@ -1,7 +1,7 @@ -package org.opentripplanner.ext.interactivelauncher.views; +package org.opentripplanner.ext.interactivelauncher.startup; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.BG_STATUS_BAR; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.FG_STATUS_BAR; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BG_STATUS_BAR; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.FG_STATUS_BAR; import java.awt.Component; import java.awt.Dimension; @@ -9,18 +9,20 @@ import java.util.function.Consumer; import javax.swing.Box; import javax.swing.JButton; +import javax.swing.JComponent; import javax.swing.JFileChooser; import javax.swing.JLabel; import javax.swing.JTextField; +import org.opentripplanner.ext.interactivelauncher.support.ViewUtils; -public class SearchDirectoryView { +class DataSourceRootView { private final Box panel; private final JTextField fileTxt = new JTextField(); private final JButton searchBtn = new JButton("Open"); private final Consumer rootDirChangedListener; - public SearchDirectoryView(String dir, Consumer rootDirChangedListener) { + DataSourceRootView(String dir, Consumer rootDirChangedListener) { this.fileTxt.setText(dir); this.rootDirChangedListener = rootDirChangedListener; @@ -35,9 +37,6 @@ public SearchDirectoryView(String dir, Consumer rootDirChangedListener) fileTxt.setEditable(false); fileTxt.setBackground(BG_STATUS_BAR); fileTxt.setForeground(FG_STATUS_BAR); - //var d = minWidth(fileTxt.getPreferredSize(), 460); - //fileTxt.setMinimumSize(d); - //fileTxt.setPreferredSize(d); // Add text field and open button Box box = Box.createHorizontalBox(); @@ -48,7 +47,7 @@ public SearchDirectoryView(String dir, Consumer rootDirChangedListener) panel.add(box); } - public Box panel() { + public JComponent panel() { return panel; } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java new file mode 100644 index 00000000000..07052b9cd29 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java @@ -0,0 +1,133 @@ +package org.opentripplanner.ext.interactivelauncher.startup; + +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addComp; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addHorizontalGlue; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addVerticalSectionSpace; + +import java.awt.Component; +import java.awt.event.ActionEvent; +import java.util.List; +import javax.swing.Box; +import javax.swing.ButtonGroup; +import javax.swing.JComponent; +import javax.swing.JLabel; +import javax.swing.JRadioButton; +import org.opentripplanner.ext.interactivelauncher.Model; +import org.opentripplanner.ext.interactivelauncher.support.ViewUtils; + +class DataSourcesView { + + /* + |-----------------------------------------------| + | Label | + |-----------------------------------------------| + | ( ) List 1 | ( ) List 2 | ( ) List 3 | + | ( ) List 1 | ( ) List 2 | ( ) List 3 | + |-----------------------------------------------| + */ + + private final Box mainPanel = Box.createVerticalBox(); + private final Box listPanel = Box.createHorizontalBox(); + private final Model model; + + public DataSourcesView(Model model) { + this.model = model; + setupDataSources(); + + JLabel label = new JLabel("Select data source"); + + listPanel.setAlignmentX(Component.LEFT_ALIGNMENT); + + addComp(label, mainPanel); + addVerticalSectionSpace(mainPanel); + addComp(listPanel, mainPanel); + } + + public JComponent panel() { + return mainPanel; + } + + public void onRootDirChange() { + model.setDataSource(null); + listPanel.removeAll(); + setupDataSources(); + listPanel.repaint(); + } + + public void onDataSourceChange(ActionEvent e) { + model.setDataSource(e.getActionCommand()); + } + + private void setupDataSources() { + final List values = model.getDataSourceOptions(); + + if (values.isEmpty()) { + model.setDataSource(null); + JLabel label = new JLabel(""); + label.setBackground(ViewUtils.BG_STATUS_BAR); + label.setForeground(ViewUtils.FG_STATUS_BAR); + addComp(label, listPanel); + return; + } + + String selectedValue = model.getDataSource(); + + if (selectedValue == null) { + selectedValue = values.get(0); + model.setDataSource(selectedValue); + } + + ButtonGroup selectDataSourceRadioGroup = new ButtonGroup(); + + List valuesSorted = values.stream().sorted().toList(); + int size = valuesSorted.size(); + + // Split the list of configuration in one, two or three columns depending on the + // number of configurations found. + if (size <= 10) { + addListPanel(valuesSorted, selectedValue, selectDataSourceRadioGroup); + } else if (size <= 20) { + int half = size / 2; + addListPanel(valuesSorted.subList(0, half), selectedValue, selectDataSourceRadioGroup); + addHorizontalGlue(listPanel); + addListPanel(valuesSorted.subList(half, size), selectedValue, selectDataSourceRadioGroup); + } else { + int third = size / 3; + addListPanel(valuesSorted.subList(0, third), selectedValue, selectDataSourceRadioGroup); + addHorizontalGlue(listPanel); + addListPanel( + valuesSorted.subList(third, third * 2), + selectedValue, + selectDataSourceRadioGroup + ); + addHorizontalGlue(listPanel); + addListPanel( + valuesSorted.subList(third * 2, size), + selectedValue, + selectDataSourceRadioGroup + ); + } + } + + private void addListPanel( + List values, + String selectedValue, + ButtonGroup selectDataSourceRadioGroup + ) { + Box column = Box.createVerticalBox(); + + for (String name : values) { + boolean selected = selectedValue.equals(name); + JRadioButton radioBtn = newRadioBtn(selectDataSourceRadioGroup, name, selected); + radioBtn.addActionListener(this::onDataSourceChange); + addComp(radioBtn, column); + } + addComp(column, listPanel); + } + + private static JRadioButton newRadioBtn(ButtonGroup group, String name, boolean selected) { + JRadioButton radioButton = new JRadioButton(name, selected); + group.add(radioButton); + return radioButton; + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/MainView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java similarity index 55% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/views/MainView.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java index 44ede02e3eb..d147624aa61 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/MainView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java @@ -1,11 +1,10 @@ -package org.opentripplanner.ext.interactivelauncher.views; +package org.opentripplanner.ext.interactivelauncher.startup; import static java.awt.GridBagConstraints.BOTH; import static java.awt.GridBagConstraints.CENTER; -import static java.awt.GridBagConstraints.NONE; -import static java.awt.GridBagConstraints.NORTH; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.BACKGROUND; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.debugLayout; +import static java.awt.GridBagConstraints.HORIZONTAL; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BACKGROUND; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.debugLayout; import java.awt.GridBagConstraints; import java.awt.GridBagLayout; @@ -18,101 +17,32 @@ public class MainView { - /** Margins between components (IN) */ private static final int M_IN = 10; - - /** Margins around frame boarder (OUT) */ private static final int M_OUT = 2 * M_IN; + private static final Insets DEFAULT_INSETS = new Insets(M_OUT, M_OUT, M_IN, M_OUT); + private static final Insets SMALL_INSETS = new Insets(M_OUT, M_OUT, M_IN, M_OUT); + private static int Y = 0; /* - The application have the following 4 panels: + The application have the following panels: + +-----------------------------------+ + | Root dir [Open] | + +-----------------------------------+ + | Config Dirs Panel | + +-----------------------------------+ + | Options Panel | +-----------------------------------+ - | Root dir [Open] | - +-------------------+---------------+ - | | | - | Config Dirs Panel | Options Panel | - | | | - +-------------------+---------------+ - | Start OTP Main Panel | + | [ Start OTP ] | +-----------------------------------+ | Status Bar | +-----------------------------------+ */ - // Root dir view - private static final GridBagConstraints CONFIG_SOURCE_DIR_PANEL_CONSTRAINTS = new GridBagConstraints( - 0, - 0, - 2, - 1, - 1.0, - 0.0, - NORTH, - BOTH, - new Insets(M_OUT, M_OUT, M_IN, M_IN), - 0, - 0 - ); - - // List of locations - private static final GridBagConstraints CONFIG_DIRS_PANEL_CONSTRAINTS = new GridBagConstraints( - 0, - 1, - 1, - 1, - 1.0, - 1.0, - NORTH, - NONE, - new Insets(M_OUT, M_OUT, M_IN, M_IN), - 0, - 0 - ); - - // Options panel - private static final GridBagConstraints OPTIONS_PANEL_CONSTRAINTS = new GridBagConstraints( - 1, - 1, - 1, - 1, - 1.0, - 1.0, - NORTH, - NONE, - new Insets(M_OUT, M_IN, M_IN, M_OUT), - 0, - 0 - ); - - // Run btn and status - private static final GridBagConstraints START_OTP_BUTTON_PANEL_CONSTRAINTS = new GridBagConstraints( - 0, - 2, - 2, - 1, - 1.0, - 1.0, - CENTER, - BOTH, - new Insets(M_IN, M_OUT, M_IN, M_OUT), - 0, - 0 - ); - - // Run btn and status - private static final GridBagConstraints STATUS_BAR_CONSTRAINTS = new GridBagConstraints( - 0, - 3, - 2, - 1, - 1.0, - 0.0, - CENTER, - BOTH, - new Insets(M_IN, 0, 0, 0), - 40, - 0 - ); + private static final GridBagConstraints DATA_SOURCE_ROOT_PANEL_CONSTRAINTS = gbc(0f); + private static final GridBagConstraints DATA_SOURCE_LIST_PANEL_CONSTRAINTS = gbc(1f); + private static final GridBagConstraints OPTIONS_PANEL_CONSTRAINTS = gbc(1f); + private static final GridBagConstraints START_BUTTON_PANEL_CONSTRAINTS = gbc(0f); + private static final GridBagConstraints STATUS_BAR_CONSTRAINTS = gbc(0f, SMALL_INSETS, 40); private final JFrame mainFrame = new JFrame("Setup and Run OTP Main"); @@ -134,7 +64,7 @@ public MainView(Runnable otpStarter, Model model) throws HeadlessException { innerPanel.setLayout(layout); innerPanel.setBackground(BACKGROUND); - var sourceDirectoryView = new SearchDirectoryView( + var sourceDirectoryView = new DataSourceRootView( model.getRootDirectory(), this::onRootDirChanged ); @@ -142,16 +72,17 @@ public MainView(Runnable otpStarter, Model model) throws HeadlessException { this.optionsView = new OptionsView(model); this.startOtpButtonView = new StartOtpButtonView(); - innerPanel.add(sourceDirectoryView.panel(), CONFIG_SOURCE_DIR_PANEL_CONSTRAINTS); - innerPanel.add(dataSourcesView.panel(), CONFIG_DIRS_PANEL_CONSTRAINTS); + innerPanel.add(sourceDirectoryView.panel(), DATA_SOURCE_ROOT_PANEL_CONSTRAINTS); + innerPanel.add(dataSourcesView.panel(), DATA_SOURCE_LIST_PANEL_CONSTRAINTS); innerPanel.add(optionsView.panel(), OPTIONS_PANEL_CONSTRAINTS); - innerPanel.add(startOtpButtonView.panel(), START_OTP_BUTTON_PANEL_CONSTRAINTS); + innerPanel.add(startOtpButtonView.panel(), START_BUTTON_PANEL_CONSTRAINTS); innerPanel.add(statusBarTxt, STATUS_BAR_CONSTRAINTS); // Setup action listeners startOtpButtonView.addActionListener(e -> startOtp()); debugLayout( + sourceDirectoryView.panel(), dataSourcesView.panel(), optionsView.panel(), startOtpButtonView.panel(), @@ -186,4 +117,12 @@ private void startOtp() { mainFrame.dispose(); otpStarter.run(); } + + private static GridBagConstraints gbc(float weighty) { + return gbc(weighty, DEFAULT_INSETS, 0); + } + + private static GridBagConstraints gbc(float weighty, Insets insets, int ipadx) { + return new GridBagConstraints(0, Y++, 1, 1, 1.0, weighty, CENTER, HORIZONTAL, insets, ipadx, 0); + } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/OptionsView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java similarity index 63% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/views/OptionsView.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java index 08923f3af88..3d98bbf0cfc 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/OptionsView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java @@ -1,20 +1,18 @@ -package org.opentripplanner.ext.interactivelauncher.views; +package org.opentripplanner.ext.interactivelauncher.startup; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addComp; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addSectionDoubleSpace; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addSectionSpace; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addComp; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addVerticalSectionSpace; -import java.util.List; import java.util.function.Consumer; -import java.util.stream.Collectors; import javax.swing.Box; import javax.swing.JCheckBox; +import javax.swing.JComponent; import javax.swing.JLabel; import org.opentripplanner.ext.interactivelauncher.Model; class OptionsView { - private final Box panel = Box.createVerticalBox(); + private final Box panel = Box.createHorizontalBox(); private final JCheckBox buildStreetGraphChk; private final JCheckBox buildTransitGraphChk; private final JCheckBox saveGraphChk; @@ -30,28 +28,41 @@ class OptionsView { this.startOptServerChk = new JCheckBox("Serve graph", model.isServeGraph()); this.startOptVisualizerChk = new JCheckBox("Visualizer", model.isVisualizer()); - addComp(new JLabel("Build graph"), panel); - addSectionSpace(panel); - addComp(buildStreetGraphChk, panel); - addComp(buildTransitGraphChk, panel); - addSectionDoubleSpace(panel); + panel.add(Box.createGlue()); + addComp(createBuildBox(), panel); + panel.add(Box.createGlue()); + addComp(createActionBox(), panel); + panel.add(Box.createGlue()); // Toggle [ ] save on/off buildStreetGraphChk.addActionListener(e -> onBuildGraphChkChanged()); buildTransitGraphChk.addActionListener(e -> onBuildGraphChkChanged()); startOptServerChk.addActionListener(e -> onStartOptServerChkChanged()); - addComp(new JLabel("Actions"), panel); - addSectionSpace(panel); - addComp(saveGraphChk, panel); - addComp(startOptServerChk, panel); - addComp(startOptVisualizerChk, panel); - - addDebugCheckBoxes(model); - addSectionDoubleSpace(panel); + //addSectionDoubleSpace(panel); bindCheckBoxesToModel(); } + private JComponent createBuildBox() { + var buildBox = Box.createVerticalBox(); + addComp(new JLabel("Build graph"), buildBox); + addVerticalSectionSpace(buildBox); + addComp(buildStreetGraphChk, buildBox); + addComp(buildTransitGraphChk, buildBox); + buildBox.add(Box.createVerticalGlue()); + return buildBox; + } + + private JComponent createActionBox() { + var actionBox = Box.createVerticalBox(); + addComp(new JLabel("Actions"), actionBox); + addVerticalSectionSpace(actionBox); + addComp(saveGraphChk, actionBox); + addComp(startOptServerChk, actionBox); + addComp(startOptVisualizerChk, actionBox); + return actionBox; + } + Box panel() { return panel; } @@ -64,19 +75,6 @@ void bind(JCheckBox box, Consumer modelUpdate) { box.addActionListener(l -> modelUpdate.accept(box.isSelected() && box.isEnabled())); } - private void addDebugCheckBoxes(Model model) { - addSectionSpace(panel); - addComp(new JLabel("Debug logging"), panel); - addSectionSpace(panel); - var entries = model.getDebugLogging(); - List keys = entries.keySet().stream().sorted().collect(Collectors.toList()); - for (String name : keys) { - JCheckBox box = new JCheckBox(name, entries.get(name)); - box.addActionListener(l -> model.getDebugLogging().put(name, box.isSelected())); - addComp(box, panel); - } - } - private void bindCheckBoxesToModel() { bind(buildStreetGraphChk, model::setBuildStreet); bind(buildTransitGraphChk, model::setBuildTransit); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StartOtpButtonView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartOtpButtonView.java similarity index 82% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StartOtpButtonView.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartOtpButtonView.java index 5c0c1e7a621..3050b7c5c62 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StartOtpButtonView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartOtpButtonView.java @@ -1,6 +1,6 @@ -package org.opentripplanner.ext.interactivelauncher.views; +package org.opentripplanner.ext.interactivelauncher.startup; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.adjustSize; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.adjustSize; import java.awt.event.ActionListener; import javax.swing.Box; diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StatusBar.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StatusBar.java new file mode 100644 index 00000000000..f88c77c9f75 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StatusBar.java @@ -0,0 +1,15 @@ +package org.opentripplanner.ext.interactivelauncher.startup; + +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BG_STATUS_BAR; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.FG_STATUS_BAR; + +import javax.swing.JTextField; + +class StatusBar extends JTextField { + + public StatusBar() { + setEditable(false); + setBackground(BG_STATUS_BAR); + setForeground(FG_STATUS_BAR); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/SearchForOtpConfig.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/SearchForOtpConfig.java similarity index 85% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/SearchForOtpConfig.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/support/SearchForOtpConfig.java index ccf6be72e20..6b0574f0fdf 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/SearchForOtpConfig.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/SearchForOtpConfig.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher; +package org.opentripplanner.ext.interactivelauncher.support; import java.io.File; import java.util.Arrays; @@ -10,17 +10,17 @@ /** * Search for directories containing OTP configuration files. The search is recursive and searches - * sub-directories 10 levels deep. + * subdirectories 10 levels deep. */ -class SearchForOtpConfig { +public class SearchForOtpConfig { private static final int DEPTH_LIMIT = 10; private static final Pattern EXCLUDE_DIR = Pattern.compile( "(otp1|archive|\\..*|te?mp|target|docs?|src|source|resource)" ); - static List search(File rootDir) { - return recursiveSearch(rootDir, DEPTH_LIMIT).collect(Collectors.toUnmodifiableList()); + public static List search(File rootDir) { + return recursiveSearch(rootDir, DEPTH_LIMIT).toList(); } @SuppressWarnings("ConstantConditions") diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/ViewUtils.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java similarity index 53% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/views/ViewUtils.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java index 14f6f589109..1e0c94fb588 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/ViewUtils.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java @@ -1,35 +1,36 @@ -package org.opentripplanner.ext.interactivelauncher.views; +package org.opentripplanner.ext.interactivelauncher.support; import java.awt.Color; +import java.awt.Container; import java.awt.Dimension; import javax.swing.BorderFactory; import javax.swing.Box; import javax.swing.JComponent; -final class ViewUtils { +public final class ViewUtils { private static final boolean DEBUG_LAYOUT = false; static final int SECTION_SPACE = 10; - static final Color BACKGROUND = new Color(0xe0, 0xf0, 0xff); - static final Color BG_STATUS_BAR = new Color(0xd0, 0xe0, 0xf0); - static final Color FG_STATUS_BAR = new Color(0, 0, 0x80); + public static final Color BACKGROUND = new Color(0xe0, 0xf0, 0xff); + public static final Color BG_STATUS_BAR = new Color(0xd0, 0xe0, 0xf0); + public static final Color FG_STATUS_BAR = new Color(0, 0, 0x80); - static void addSectionSpace(Box panel) { + public static void addVerticalSectionSpace(Box panel) { panel.add(Box.createVerticalStrut(SECTION_SPACE)); } - static void addSectionDoubleSpace(Box panel) { - panel.add(Box.createVerticalStrut(2 * SECTION_SPACE)); + public static void addHorizontalGlue(Box box) { + box.add(Box.createHorizontalGlue()); } - static void addComp(JComponent c, Box panel) { + public static void addComp(JComponent c, Container panel) { if (DEBUG_LAYOUT) { c.setBorder(BorderFactory.createLineBorder(Color.green)); } panel.add(c); } - static void debugLayout(JComponent... components) { + public static void debugLayout(JComponent... components) { if (DEBUG_LAYOUT) { for (JComponent c : components) { c.setBorder(BorderFactory.createLineBorder(Color.red)); @@ -37,7 +38,7 @@ static void debugLayout(JComponent... components) { } } - static void adjustSize(JComponent c, int dWidth, int dHeight) { + public static void adjustSize(JComponent c, int dWidth, int dHeight) { Dimension d0 = c.getPreferredSize(); Dimension d = new Dimension(d0.width + dWidth, d0.height + dHeight); c.setMinimumSize(d); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/DataSourcesView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/DataSourcesView.java deleted file mode 100644 index c7faa43779d..00000000000 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/DataSourcesView.java +++ /dev/null @@ -1,84 +0,0 @@ -package org.opentripplanner.ext.interactivelauncher.views; - -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addComp; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addSectionDoubleSpace; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.addSectionSpace; - -import java.awt.event.ActionEvent; -import java.util.List; -import java.util.stream.Collectors; -import javax.swing.Box; -import javax.swing.ButtonGroup; -import javax.swing.JLabel; -import javax.swing.JRadioButton; -import org.opentripplanner.ext.interactivelauncher.Model; - -class DataSourcesView { - - private final Box panel = Box.createVerticalBox(); - private final Box dataSourceSelectionPanel = Box.createVerticalBox(); - private final Model model; - - public DataSourcesView(Model model) { - this.model = model; - - setupDataSources(); - - addComp(new JLabel("Select data source"), panel); - addSectionSpace(panel); - addComp(dataSourceSelectionPanel, panel); - addSectionDoubleSpace(panel); - } - - public Box panel() { - return panel; - } - - public void onRootDirChange() { - model.setDataSource(null); - dataSourceSelectionPanel.removeAll(); - setupDataSources(); - panel.repaint(); - } - - public void onDataSourceChange(ActionEvent e) { - model.setDataSource(e.getActionCommand()); - } - - private static JRadioButton newRadioBtn(ButtonGroup group, String name, boolean selected) { - JRadioButton radioButton = new JRadioButton(name, selected); - group.add(radioButton); - return radioButton; - } - - private void setupDataSources() { - final List values = model.getDataSourceOptions(); - - if (values.isEmpty()) { - model.setDataSource(null); - JLabel label = new JLabel(""); - label.setBackground(ViewUtils.BG_STATUS_BAR); - label.setForeground(ViewUtils.FG_STATUS_BAR); - addComp(label, dataSourceSelectionPanel); - return; - } - - String selectedValue = model.getDataSource(); - - if (selectedValue == null) { - selectedValue = values.get(0); - model.setDataSource(selectedValue); - } - - ButtonGroup selectDataSourceRadioGroup = new ButtonGroup(); - - List valuesSorted = values.stream().sorted().collect(Collectors.toList()); - - for (String name : valuesSorted) { - boolean selected = selectedValue.equals(name); - JRadioButton radioBtn = newRadioBtn(selectDataSourceRadioGroup, name, selected); - radioBtn.addActionListener(this::onDataSourceChange); - addComp(radioBtn, dataSourceSelectionPanel); - } - } -} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StatusBar.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StatusBar.java deleted file mode 100644 index faffa9d87d2..00000000000 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/views/StatusBar.java +++ /dev/null @@ -1,15 +0,0 @@ -package org.opentripplanner.ext.interactivelauncher.views; - -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.BG_STATUS_BAR; -import static org.opentripplanner.ext.interactivelauncher.views.ViewUtils.FG_STATUS_BAR; - -import javax.swing.JTextField; - -public class StatusBar extends JTextField { - - public StatusBar() { - setEditable(false); - setBackground(BG_STATUS_BAR); - setForeground(FG_STATUS_BAR); - } -} From cb10208be9e800d93bb2b8c1621698d12740cc3e Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 01:38:18 +0100 Subject: [PATCH 04/17] refactor: Extend the main code, so the interactive launcher can intercept the request. --- .../api/LauncherRequestDecorator.java | 15 ++++++++++++++ .../InteractiveLauncherModule.java | 20 +++++++++++++++++++ .../ConstructApplicationFactory.java | 2 ++ .../configure/ConstructApplicationModule.java | 8 ++++++-- 4 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/api/LauncherRequestDecorator.java create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/api/LauncherRequestDecorator.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/api/LauncherRequestDecorator.java new file mode 100644 index 00000000000..99c28bad260 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/api/LauncherRequestDecorator.java @@ -0,0 +1,15 @@ +package org.opentripplanner.ext.interactivelauncher.api; + +import org.opentripplanner.routing.api.request.RouteRequest; + +/** + * Allow the interactive launcher intercept planing requests. + */ +public interface LauncherRequestDecorator { + /** + * The launcher may use this method to change the default plan request. Note! It is the DEFAULT + * request witch is passed in here, then the request-specific values are applied on top + * of that. + */ + RouteRequest intercept(RouteRequest defaultRequest); +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java new file mode 100644 index 00000000000..2df46b74127 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java @@ -0,0 +1,20 @@ +package org.opentripplanner.ext.interactivelauncher.configuration; + +import dagger.Module; +import dagger.Provides; +import org.opentripplanner.ext.interactivelauncher.api.LauncherRequestDecorator; + +@Module +public class InteractiveLauncherModule { + + static LauncherRequestDecorator decorator = request -> request; + + static void enable(LauncherRequestDecorator decorator) { + InteractiveLauncherModule.decorator = decorator; + } + + @Provides + LauncherRequestDecorator requestDecorator() { + return decorator; + } +} diff --git a/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java b/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java index 097fcfb4f7a..a4f0ee49652 100644 --- a/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java +++ b/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java @@ -6,6 +6,7 @@ import javax.annotation.Nullable; import org.opentripplanner.ext.emissions.EmissionsDataModel; import org.opentripplanner.ext.emissions.EmissionsServiceModule; +import org.opentripplanner.ext.interactivelauncher.configuration.InteractiveLauncherModule; import org.opentripplanner.ext.ridehailing.configure.RideHailingServicesModule; import org.opentripplanner.ext.stopconsolidation.StopConsolidationRepository; import org.opentripplanner.ext.stopconsolidation.configure.StopConsolidationServiceModule; @@ -50,6 +51,7 @@ RideHailingServicesModule.class, EmissionsServiceModule.class, StopConsolidationServiceModule.class, + InteractiveLauncherModule.class, } ) public interface ConstructApplicationFactory { diff --git a/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java b/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java index 2ff6de29b3d..c9d7253b0be 100644 --- a/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java +++ b/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java @@ -7,6 +7,7 @@ import javax.annotation.Nullable; import org.opentripplanner.astar.spi.TraverseVisitor; import org.opentripplanner.ext.emissions.EmissionsService; +import org.opentripplanner.ext.interactivelauncher.api.LauncherRequestDecorator; import org.opentripplanner.ext.ridehailing.RideHailingService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.raptor.configure.RaptorConfig; @@ -36,11 +37,14 @@ OtpServerRequestContext providesServerContext( List rideHailingServices, @Nullable StopConsolidationService stopConsolidationService, @Nullable TraverseVisitor traverseVisitor, - EmissionsService emissionsService + EmissionsService emissionsService, + LauncherRequestDecorator launcherRequestDecorator ) { + var defaultRequest = launcherRequestDecorator.intercept(routerConfig.routingRequestDefaults()); + return DefaultServerRequestContext.create( routerConfig.transitTuningConfig(), - routerConfig.routingRequestDefaults(), + defaultRequest, raptorConfig, graph, transitService, From bd16e4046bd85c00ca35e7113fde8b6fe0b588b6 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 14:07:19 +0100 Subject: [PATCH 05/17] refactor: Cleanup SystemErrDebugLogger --- .../raptor/rangeraptor/SystemErrDebugLogger.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java index 060d3a2e018..951721f81a7 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java @@ -43,14 +43,14 @@ public class SystemErrDebugLogger implements DebugLogger { private final Table arrivalTable = Table .of() .withAlights(Center, Center, Right, Right, Right, Right, Left, Left) - .withHeaders("ARRIVAL", "LEG", "RND", "STOP", "ARRIVE", "COST", "TRIP", "DETAILS") + .withHeaders("ARRIVAL", "LEG", "RND", "STOP", "ARRIVE", "C₁", "TRIP", "DETAILS") .withMinWidths(9, 7, 3, 5, 8, 9, 24, 0) .build(); private final Table pathTable = Table .of() .withAlights(Center, Center, Right, Right, Right, Right, Right, Right, Left) - .withHeaders(">>> PATH", "TR", "FROM", "TO", "START", "END", "DURATION", "COST", "DETAILS") - .withMinWidths(9, 2, 5, 5, 8, 8, 8, 6, 0) + .withHeaders(">>> PATH", "TR", "FROM", "TO", "START", "END", "DURATION", "C₁", "DETAILS") + .withMinWidths(9, 2, 5, 5, 8, 8, 8, 9, 0) .build(); private boolean forwardSearch = true; private int lastIterationTime = NOT_SET; @@ -112,6 +112,7 @@ public void pathFilteringListener(DebugEvent> e) { RaptorPath p = e.element(); var aLeg = p.accessLeg(); var eLeg = p.egressLeg(); + println( pathTable.rowAsText( e.action().toString(), From eb259ee3b247095206bfaff0876f00bd733250b9 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 15:59:41 +0100 Subject: [PATCH 06/17] refactor: Create a separate model for StartUp --- .../InteractiveOtpMain.java | 6 +- .../ext/interactivelauncher/Model.java | 187 +----------------- .../startup/DataSourcesView.java | 5 +- .../interactivelauncher/startup/MainView.java | 6 +- .../startup/OptionsView.java | 5 +- .../startup/StartupModel.java | 178 +++++++++++++++++ 6 files changed, 192 insertions(+), 195 deletions(-) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartupModel.java diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java index d61ae6bfcd3..f5621c45278 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java @@ -24,16 +24,14 @@ public static void main(String[] args) { private void run() { this.model = Model.load(); - MainView frame = new MainView(new Thread(this::startOtp)::start, model); + MainView frame = new MainView(new Thread(this::startOtp)::start, model.getStartupModel()); frame.start(); } private void startOtp() { model.save(); - new OtpDebugController(model).start(); - System.out.println("Start OTP: " + model + "\n"); - OTPMain.main(model.asOtpArgs()); + OTPMain.main(model.getStartupModel().asOtpArgs()); } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java index 1550f8bc1ab..8e0c2b353a7 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java @@ -1,34 +1,17 @@ package org.opentripplanner.ext.interactivelauncher; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.io.IOException; import java.io.Serializable; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Consumer; import org.opentripplanner.ext.interactivelauncher.logging.LogModel; -import org.opentripplanner.ext.interactivelauncher.support.SearchForOtpConfig; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.opentripplanner.ext.interactivelauncher.startup.StartupModel; public class Model implements Serializable { - private static final Logger LOG = LoggerFactory.getLogger(Model.class); - private static final File MODEL_FILE = new File("interactive_otp_main.json"); - @JsonIgnore - private transient Consumer commandLineChange; - - private String rootDirectory = null; - private String dataSource = null; - private boolean buildStreet = false; - private boolean buildTransit = true; - private boolean saveGraph = false; - private boolean serveGraph = true; - private boolean visualizer = false; + private StartupModel startupModel; private LogModel logModel; public Model() {} @@ -41,116 +24,12 @@ public static Model load() { return model; } - public void subscribeCmdLineUpdates(Consumer commandLineChange) { - this.commandLineChange = commandLineChange; - } - - @SuppressWarnings("AccessOfSystemProperties") - public String getRootDirectory() { - return rootDirectory == null ? System.getProperty("user.dir") : rootDirectory; - } - - public void setRootDirectory(String rootDirectory) { - // If the persisted JSON do not contain the rootDirectory, then avoid setting it - if (rootDirectory != null) { - this.rootDirectory = rootDirectory; - } - notifyChangeListener(); - } - - public String getDataSource() { - return dataSource; - } - - public void setDataSource(String dataSource) { - this.dataSource = dataSource; - notifyChangeListener(); - } - - @JsonIgnore - public List getDataSourceOptions() { - List dataSourceOptions = new ArrayList<>(); - File rootDir = new File(getRootDirectory()); - List dirs = SearchForOtpConfig.search(rootDir); - // Add 1 char for the path-separator-character - int length = rootDir.getAbsolutePath().length() + 1; - - for (File dir : dirs) { - var path = dir.getAbsolutePath(); - if (path.length() <= length) { - LOG.warn( - "The root directory contains a config file, choose " + - "the parent directory or delete the config file." - ); - continue; - } - dataSourceOptions.add(path.substring(length)); - } - return dataSourceOptions; - } - - public boolean isBuildStreet() { - return buildStreet; - } - - public void setBuildStreet(boolean buildStreet) { - this.buildStreet = buildStreet; - notifyChangeListener(); - } - - public boolean isBuildTransit() { - return buildTransit; - } - - public void setBuildTransit(boolean buildTransit) { - this.buildTransit = buildTransit; - notifyChangeListener(); - } - - public boolean isSaveGraph() { - return saveGraph; - } - - public void setSaveGraph(boolean saveGraph) { - this.saveGraph = saveGraph; - notifyChangeListener(); - } - - public boolean isServeGraph() { - return serveGraph; - } - - public void setServeGraph(boolean serveGraph) { - this.serveGraph = serveGraph; - notifyChangeListener(); + public StartupModel getStartupModel() { + return startupModel; } - public boolean isVisualizer() { - return visualizer; - } - - public void setVisualizer(boolean visualizer) { - this.visualizer = visualizer; - notifyChangeListener(); - } - - @Override - public String toString() { - return ( - "(" + - "data-source-dir: " + - getDataSourceDirectory() + - (buildStreet ? ", buildStreet" : "") + - (buildTransit ? ", buildTransit" : "") + - (saveGraph ? ", saveGraph" : "") + - (serveGraph ? ", serveGraph" : "") + - (visualizer ? ", visualizer" : "") + - ')' - ); - } - - public String toCliString() { - return String.join(" ", asOtpArgs()); + public LogModel getLogModel() { + return logModel; } public void save() { @@ -161,46 +40,6 @@ public void save() { } } - public LogModel getLogModel() { - return logModel; - } - - @JsonIgnore - String getDataSourceDirectory() { - if (dataSource == null) { - return "DATA_SOURCE_NOT_SET"; - } - return rootDirectory + File.separatorChar + dataSource; - } - - String[] asOtpArgs() { - List args = new ArrayList<>(); - - if (buildAll()) { - args.add("--build"); - } else if (buildStreet) { - args.add("--buildStreet"); - } else if (buildTransit) { - args.add("--loadStreet"); - } else { - args.add("--load"); - } - - if (saveGraph && (buildTransit || buildStreet)) { - args.add("--save"); - } - if (serveGraph && !buildStreetOnly()) { - args.add("--serve"); - } - if (serveGraph && !buildStreetOnly() && visualizer) { - args.add("--visualize"); - } - - args.add(getDataSourceDirectory()); - - return args.toArray(new String[0]); - } - private static Model createNew() { var model = new Model(); model.logModel = new LogModel(); @@ -224,20 +63,6 @@ private static Model readFromFile() { } } - private void notifyChangeListener() { - if (commandLineChange != null) { - commandLineChange.accept(toCliString()); - } - } - - private boolean buildAll() { - return buildStreet && buildTransit; - } - - private boolean buildStreetOnly() { - return buildStreet && !buildTransit; - } - private void setupCallbacks() { logModel.init(this::save); } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java index 07052b9cd29..d8787287681 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java @@ -12,7 +12,6 @@ import javax.swing.JComponent; import javax.swing.JLabel; import javax.swing.JRadioButton; -import org.opentripplanner.ext.interactivelauncher.Model; import org.opentripplanner.ext.interactivelauncher.support.ViewUtils; class DataSourcesView { @@ -28,9 +27,9 @@ class DataSourcesView { private final Box mainPanel = Box.createVerticalBox(); private final Box listPanel = Box.createHorizontalBox(); - private final Model model; + private final StartupModel model; - public DataSourcesView(Model model) { + public DataSourcesView(StartupModel model) { this.model = model; setupDataSources(); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java index d147624aa61..6db2508196c 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/MainView.java @@ -1,6 +1,5 @@ package org.opentripplanner.ext.interactivelauncher.startup; -import static java.awt.GridBagConstraints.BOTH; import static java.awt.GridBagConstraints.CENTER; import static java.awt.GridBagConstraints.HORIZONTAL; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BACKGROUND; @@ -13,7 +12,6 @@ import javax.swing.JFrame; import javax.swing.JPanel; import javax.swing.JScrollPane; -import org.opentripplanner.ext.interactivelauncher.Model; public class MainView { @@ -50,9 +48,9 @@ public class MainView { private final OptionsView optionsView; private final StartOtpButtonView startOtpButtonView; private final Runnable otpStarter; - private final Model model; + private final StartupModel model; - public MainView(Runnable otpStarter, Model model) throws HeadlessException { + public MainView(Runnable otpStarter, StartupModel model) throws HeadlessException { var innerPanel = new JPanel(); var statusBarTxt = new StatusBar(); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java index 3d98bbf0cfc..a8c4327a2d4 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java @@ -8,7 +8,6 @@ import javax.swing.JCheckBox; import javax.swing.JComponent; import javax.swing.JLabel; -import org.opentripplanner.ext.interactivelauncher.Model; class OptionsView { @@ -18,9 +17,9 @@ class OptionsView { private final JCheckBox saveGraphChk; private final JCheckBox startOptServerChk; private final JCheckBox startOptVisualizerChk; - private final Model model; + private final StartupModel model; - OptionsView(Model model) { + OptionsView(StartupModel model) { this.model = model; this.buildStreetGraphChk = new JCheckBox("Street graph", model.isBuildStreet()); this.buildTransitGraphChk = new JCheckBox("Transit graph", model.isBuildTransit()); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartupModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartupModel.java new file mode 100644 index 00000000000..5b803b2318c --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/StartupModel.java @@ -0,0 +1,178 @@ +package org.opentripplanner.ext.interactivelauncher.startup; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; +import org.opentripplanner.ext.interactivelauncher.support.SearchForOtpConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class StartupModel { + + private static final Logger LOG = LoggerFactory.getLogger(StartupModel.class); + + @JsonIgnore + private transient Consumer commandLineChange; + + private String rootDirectory = null; + private String dataSource = null; + private boolean buildStreet = false; + private boolean buildTransit = true; + private boolean saveGraph = false; + private boolean serveGraph = true; + private boolean visualizer = false; + + public void subscribeCmdLineUpdates(Consumer commandLineChange) { + this.commandLineChange = commandLineChange; + } + + @SuppressWarnings("AccessOfSystemProperties") + public String getRootDirectory() { + return rootDirectory == null ? System.getProperty("user.dir") : rootDirectory; + } + + public void setRootDirectory(String rootDirectory) { + // If the persisted JSON do not contain the rootDirectory, then avoid setting it + if (rootDirectory != null) { + this.rootDirectory = rootDirectory; + } + notifyChangeListener(); + } + + public String getDataSource() { + return dataSource; + } + + public void setDataSource(String dataSource) { + this.dataSource = dataSource; + notifyChangeListener(); + } + + @JsonIgnore + public List getDataSourceOptions() { + List dataSourceOptions = new ArrayList<>(); + File rootDir = new File(getRootDirectory()); + List dirs = SearchForOtpConfig.search(rootDir); + // Add 1 char for the path-separator-character + int length = rootDir.getAbsolutePath().length() + 1; + + for (File dir : dirs) { + var path = dir.getAbsolutePath(); + if (path.length() <= length) { + LOG.warn( + "The root directory contains a config file, choose " + + "the parent directory or delete the config file." + ); + continue; + } + dataSourceOptions.add(path.substring(length)); + } + return dataSourceOptions; + } + + public boolean isBuildStreet() { + return buildStreet; + } + + public void setBuildStreet(boolean buildStreet) { + this.buildStreet = buildStreet; + notifyChangeListener(); + } + + public boolean isBuildTransit() { + return buildTransit; + } + + public void setBuildTransit(boolean buildTransit) { + this.buildTransit = buildTransit; + notifyChangeListener(); + } + + public boolean isSaveGraph() { + return saveGraph; + } + + public void setSaveGraph(boolean saveGraph) { + this.saveGraph = saveGraph; + notifyChangeListener(); + } + + public boolean isServeGraph() { + return serveGraph; + } + + public void setServeGraph(boolean serveGraph) { + this.serveGraph = serveGraph; + notifyChangeListener(); + } + + public boolean isVisualizer() { + return visualizer; + } + + public void setVisualizer(boolean visualizer) { + this.visualizer = visualizer; + notifyChangeListener(); + } + + @Override + public String toString() { + return String.join("", asOtpArgs()); + } + + public String toCliString() { + return String.join(" ", asOtpArgs()); + } + + private void notifyChangeListener() { + if (commandLineChange != null) { + commandLineChange.accept(toCliString()); + } + } + + @JsonIgnore + String getDataSourceDirectory() { + if (dataSource == null) { + return "DATA_SOURCE_NOT_SET"; + } + return getRootDirectory() + File.separatorChar + dataSource; + } + + public String[] asOtpArgs() { + List args = new ArrayList<>(); + + if (buildAll()) { + args.add("--build"); + } else if (buildStreet) { + args.add("--buildStreet"); + } else if (buildTransit) { + args.add("--loadStreet"); + } else { + args.add("--load"); + } + + if (saveGraph && (buildTransit || buildStreet)) { + args.add("--save"); + } + if (serveGraph && !buildStreetOnly()) { + args.add("--serve"); + } + if (serveGraph && !buildStreetOnly() && visualizer) { + args.add("--visualize"); + } + + args.add(getDataSourceDirectory()); + + return args.toArray(new String[0]); + } + + private boolean buildAll() { + return buildStreet && buildTransit; + } + + private boolean buildStreetOnly() { + return buildStreet && !buildTransit; + } +} From 50cc04e15678ca3d74cc3931cf3c6ccb436a67c2 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:02:52 +0100 Subject: [PATCH 07/17] refactor: Create debug package in o.o.ext.interactivelauncher --- .../org/opentripplanner/ext/interactivelauncher/Model.java | 2 +- .../ext/interactivelauncher/OtpDebugController.java | 4 ++-- .../{ => debug}/logging/DebugLoggingSupport.java | 3 +-- .../ext/interactivelauncher/{ => debug}/logging/LogModel.java | 2 +- .../ext/interactivelauncher/{ => debug}/logging/LogView.java | 2 +- .../{ => debug}/logging/OTPDebugLoggers.java | 2 +- 6 files changed, 7 insertions(+), 8 deletions(-) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => debug}/logging/DebugLoggingSupport.java (95%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => debug}/logging/LogModel.java (95%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => debug}/logging/LogView.java (92%) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => debug}/logging/OTPDebugLoggers.java (90%) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java index 8e0c2b353a7..d44e7c0e340 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java @@ -4,7 +4,7 @@ import java.io.File; import java.io.IOException; import java.io.Serializable; -import org.opentripplanner.ext.interactivelauncher.logging.LogModel; +import org.opentripplanner.ext.interactivelauncher.debug.logging.LogModel; import org.opentripplanner.ext.interactivelauncher.startup.StartupModel; public class Model implements Serializable { diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java index 95635c5bb0c..57ef7f70644 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java @@ -6,8 +6,8 @@ import javax.swing.JFrame; import javax.swing.JPanel; import javax.swing.JTabbedPane; -import org.opentripplanner.ext.interactivelauncher.logging.LogModel; -import org.opentripplanner.ext.interactivelauncher.logging.LogView; +import org.opentripplanner.ext.interactivelauncher.debug.logging.LogModel; +import org.opentripplanner.ext.interactivelauncher.debug.logging.LogView; public class OtpDebugController { diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java similarity index 95% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java index fcfa323680f..9985f8eb079 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/DebugLoggingSupport.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java @@ -1,11 +1,10 @@ -package org.opentripplanner.ext.interactivelauncher.logging; +package org.opentripplanner.ext.interactivelauncher.debug.logging; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.regex.Pattern; import org.slf4j.LoggerFactory; diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java similarity index 95% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java index 536c8d1dc87..7f7695ad2e5 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogModel.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher.logging; +package org.opentripplanner.ext.interactivelauncher.debug.logging; import com.fasterxml.jackson.annotation.JsonIgnore; import java.io.Serializable; diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java similarity index 92% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java index a71f90eb697..d4cea4d0baf 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/LogView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher.logging; +package org.opentripplanner.ext.interactivelauncher.debug.logging; import javax.swing.Box; import javax.swing.JCheckBox; diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java similarity index 90% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java index 4663b6017de..a38cb5cab58 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/logging/OTPDebugLoggers.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher.logging; +package org.opentripplanner.ext.interactivelauncher.debug.logging; import java.util.List; From 6d890c774d5a3a8379f520e0a7b50b44c15d4de1 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:04:59 +0100 Subject: [PATCH 08/17] refactor: Rename OTPDebugLoggers to DebugLoggers --- .../logging/{OTPDebugLoggers.java => DebugLoggers.java} | 8 ++++---- .../ext/interactivelauncher/debug/logging/LogView.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/{OTPDebugLoggers.java => DebugLoggers.java} (71%) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java similarity index 71% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java index a38cb5cab58..0635b9a3675 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/OTPDebugLoggers.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java @@ -2,8 +2,8 @@ import java.util.List; -record OTPDebugLoggers(String label, String logger) { - static List list() { +record DebugLoggers(String label, String logger) { + static List list() { return List.of( of("Data import issues", "DATA_IMPORT_ISSUES"), of("All OTP debuggers", "org.opentripplanner"), @@ -13,7 +13,7 @@ static List list() { ); } - private static OTPDebugLoggers of(String label, String logger) { - return new OTPDebugLoggers(label, logger); + private static DebugLoggers of(String label, String logger) { + return new DebugLoggers(label, logger); } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java index d4cea4d0baf..6417e487ca7 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java @@ -14,14 +14,14 @@ public class LogView { public LogView(LogModel model) { this.model = model; - OTPDebugLoggers.list().forEach(this::add); + DebugLoggers.list().forEach(this::add); } public JComponent panel() { return panel; } - private void add(OTPDebugLoggers logger) { + private void add(DebugLoggers logger) { var box = new JCheckBox(logger.label()); box.setSelected(model.isLoggerEnabled(logger.logger())); box.addActionListener(e -> selectLogger(logger.logger(), box.isSelected())); From 578cc48867fea053193b9c37697022de23f49561 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:09:31 +0100 Subject: [PATCH 09/17] refactor: Extract addLabel utility method --- .../ext/interactivelauncher/startup/DataSourcesView.java | 9 ++++----- .../ext/interactivelauncher/startup/OptionsView.java | 6 +++--- .../ext/interactivelauncher/support/ViewUtils.java | 5 +++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java index d8787287681..e7b1814faa2 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/DataSourcesView.java @@ -2,6 +2,7 @@ import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addComp; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addHorizontalGlue; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addLabel; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addVerticalSectionSpace; import java.awt.Component; @@ -33,12 +34,10 @@ public DataSourcesView(StartupModel model) { this.model = model; setupDataSources(); - JLabel label = new JLabel("Select data source"); + addLabel("Select data source", mainPanel); + addVerticalSectionSpace(mainPanel); listPanel.setAlignmentX(Component.LEFT_ALIGNMENT); - - addComp(label, mainPanel); - addVerticalSectionSpace(mainPanel); addComp(listPanel, mainPanel); } @@ -62,7 +61,7 @@ private void setupDataSources() { if (values.isEmpty()) { model.setDataSource(null); - JLabel label = new JLabel(""); + var label = new JLabel(""); label.setBackground(ViewUtils.BG_STATUS_BAR); label.setForeground(ViewUtils.FG_STATUS_BAR); addComp(label, listPanel); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java index a8c4327a2d4..08907346a76 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/startup/OptionsView.java @@ -1,13 +1,13 @@ package org.opentripplanner.ext.interactivelauncher.startup; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addComp; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addLabel; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addVerticalSectionSpace; import java.util.function.Consumer; import javax.swing.Box; import javax.swing.JCheckBox; import javax.swing.JComponent; -import javax.swing.JLabel; class OptionsView { @@ -44,7 +44,7 @@ class OptionsView { private JComponent createBuildBox() { var buildBox = Box.createVerticalBox(); - addComp(new JLabel("Build graph"), buildBox); + addLabel("Build graph", buildBox); addVerticalSectionSpace(buildBox); addComp(buildStreetGraphChk, buildBox); addComp(buildTransitGraphChk, buildBox); @@ -54,7 +54,7 @@ private JComponent createBuildBox() { private JComponent createActionBox() { var actionBox = Box.createVerticalBox(); - addComp(new JLabel("Actions"), actionBox); + addLabel("Actions", actionBox); addVerticalSectionSpace(actionBox); addComp(saveGraphChk, actionBox); addComp(startOptServerChk, actionBox); diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java index 1e0c94fb588..f906f063058 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/support/ViewUtils.java @@ -6,6 +6,7 @@ import javax.swing.BorderFactory; import javax.swing.Box; import javax.swing.JComponent; +import javax.swing.JLabel; public final class ViewUtils { @@ -23,6 +24,10 @@ public static void addHorizontalGlue(Box box) { box.add(Box.createHorizontalGlue()); } + public static void addLabel(String label, Container panel) { + addComp(new JLabel(label), panel); + } + public static void addComp(JComponent c, Container panel) { if (DEBUG_LAYOUT) { c.setBorder(BorderFactory.createLineBorder(Color.green)); From 0756cad80168ce9ca6e43b464a4dfcb86382239e Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:13:21 +0100 Subject: [PATCH 10/17] InteractiveLauncherModule --- .../configuration/InteractiveLauncherModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java index 2df46b74127..9acd0298122 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/configuration/InteractiveLauncherModule.java @@ -9,7 +9,7 @@ public class InteractiveLauncherModule { static LauncherRequestDecorator decorator = request -> request; - static void enable(LauncherRequestDecorator decorator) { + public static void setRequestInterceptor(LauncherRequestDecorator decorator) { InteractiveLauncherModule.decorator = decorator; } From 5b131b2992348b3ebb5b308b0053803bfb2b6e55 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:31:25 +0100 Subject: [PATCH 11/17] refactor: Cleanup Model in interactive launcher This fixes a few small bugs/improve the JSON serialization --- .../ext/interactivelauncher/Model.java | 32 ++++++++++--------- .../debug/logging/LogModel.java | 14 +++++--- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java index d44e7c0e340..45d8583f66d 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java @@ -1,6 +1,8 @@ package org.opentripplanner.ext.interactivelauncher; +import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; import java.io.File; import java.io.IOException; import java.io.Serializable; @@ -17,11 +19,7 @@ public class Model implements Serializable { public Model() {} public static Model load() { - var model = MODEL_FILE.exists() ? readFromFile() : createNew(); - // Setup callbacks - model.logModel.init(model::save); - - return model; + return MODEL_FILE.exists() ? readFromFile() : createNew(); } public StartupModel getStartupModel() { @@ -34,25 +32,22 @@ public LogModel getLogModel() { public void save() { try { - new ObjectMapper().writeValue(MODEL_FILE, this); + var mapper = new ObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, true); + mapper.writeValue(MODEL_FILE, this); } catch (IOException e) { throw new RuntimeException(e.getMessage(), e); } } private static Model createNew() { - var model = new Model(); - model.logModel = new LogModel(); - model.logModel.initFromConfig(); - model.setupCallbacks(); - return model; + return new Model().initSubModels(); } private static Model readFromFile() { try { - var model = new ObjectMapper().readValue(MODEL_FILE, Model.class); - model.setupCallbacks(); - return model; + var mapper = new ObjectMapper() + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + return mapper.readValue(MODEL_FILE, Model.class).initSubModels(); } catch (IOException e) { System.err.println( "Unable to read the InteractiveOtpMain state cache. If the model changed this " + @@ -63,7 +58,14 @@ private static Model readFromFile() { } } - private void setupCallbacks() { + private Model initSubModels() { + if (startupModel == null) { + startupModel = new StartupModel(); + } + if (logModel == null) { + logModel = LogModel.createFromConfig(); + } logModel.init(this::save); + return this; } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java index 7f7695ad2e5..d6500fa060f 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java @@ -20,6 +20,12 @@ public class LogModel implements Serializable { public LogModel() {} + public static LogModel createFromConfig() { + var model = new LogModel(); + model.initFromConfig(); + return model; + } + /** Need to set this manually to support JSON serialization. */ public void init(Runnable saveCallback) { this.saveCallback = saveCallback; @@ -36,10 +42,6 @@ public void setActiveLoggers(Collection loggers) { this.activeLoggers.addAll(loggers); } - public void initFromConfig() { - activeLoggers.addAll(DebugLoggingSupport.getDebugLoggers()); - } - boolean isLoggerEnabled(String name) { return activeLoggers.contains(name); } @@ -53,4 +55,8 @@ void turnLoggerOnOff(String name, boolean enable) { DebugLoggingSupport.configureDebugLogging(name, enable); saveCallback.run(); } + + private void initFromConfig() { + activeLoggers.addAll(DebugLoggingSupport.getDebugLoggers()); + } } From dc06bb91ba827d3753ff1a41259e162d24ec398d Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:34:33 +0100 Subject: [PATCH 12/17] refactor: Cleanup interactive launcher debug loggers --- .../debug/logging/DebugLoggers.java | 15 +++++--- .../debug/logging/DebugLoggingSupport.java | 2 +- .../debug/logging/LogModel.java | 34 +++++++++++++++---- .../debug/logging/LogView.java | 8 ++--- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java index 0635b9a3675..48f87abf2ab 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggers.java @@ -2,8 +2,9 @@ import java.util.List; -record DebugLoggers(String label, String logger) { - static List list() { +class DebugLoggers { + + static List list() { return List.of( of("Data import issues", "DATA_IMPORT_ISSUES"), of("All OTP debuggers", "org.opentripplanner"), @@ -13,7 +14,13 @@ static List list() { ); } - private static DebugLoggers of(String label, String logger) { - return new DebugLoggers(label, logger); + static List listLoggers() { + return list().stream().map(Entry::logger).toList(); + } + + private static Entry of(String label, String logger) { + return new Entry(label, logger); } + + record Entry(String label, String logger) {} } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java index 9985f8eb079..09eddcfdf78 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/DebugLoggingSupport.java @@ -25,7 +25,7 @@ class DebugLoggingSupport { "(" + OTP + "|" + GRAPHQL + "|" + NAMED_LOGGERS + ")" ); - static List getDebugLoggers() { + static List listConfiguredDebugLoggers() { List result = new ArrayList<>(); LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); for (Logger log : context.getLoggerList()) { diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java index d6500fa060f..df59ccaa968 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogModel.java @@ -31,15 +31,18 @@ public void init(Runnable saveCallback) { this.saveCallback = saveCallback; } - /** Needed to do JSON serialization. */ + /** Used by JSON serialization. */ public Collection getActiveLoggers() { return List.copyOf(activeLoggers); } - /** Needed to do JSON serialization. */ + /** Used by JSON deserialization. */ public void setActiveLoggers(Collection loggers) { this.activeLoggers.clear(); this.activeLoggers.addAll(loggers); + for (var logger : activeLoggers) { + DebugLoggingSupport.configureDebugLogging(logger, true); + } } boolean isLoggerEnabled(String name) { @@ -48,15 +51,32 @@ boolean isLoggerEnabled(String name) { void turnLoggerOnOff(String name, boolean enable) { if (enable) { - activeLoggers.add(name); + if (!activeLoggers.contains(name)) { + activeLoggers.add(name); + DebugLoggingSupport.configureDebugLogging(name, enable); + save(); + } } else { - activeLoggers.remove(name); + if (activeLoggers.contains(name)) { + activeLoggers.remove(name); + DebugLoggingSupport.configureDebugLogging(name, enable); + save(); + } } - DebugLoggingSupport.configureDebugLogging(name, enable); - saveCallback.run(); } private void initFromConfig() { - activeLoggers.addAll(DebugLoggingSupport.getDebugLoggers()); + var debugLoggers = DebugLoggers.listLoggers(); + for (var logger : DebugLoggingSupport.listConfiguredDebugLoggers()) { + if (debugLoggers.contains(logger)) { + activeLoggers.add(logger); + } + } + } + + private void save() { + if (saveCallback != null) { + saveCallback.run(); + } } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java index 6417e487ca7..52c1774c019 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java @@ -21,10 +21,10 @@ public JComponent panel() { return panel; } - private void add(DebugLoggers logger) { - var box = new JCheckBox(logger.label()); - box.setSelected(model.isLoggerEnabled(logger.logger())); - box.addActionListener(e -> selectLogger(logger.logger(), box.isSelected())); + private void add(DebugLoggers.Entry entry) { + var box = new JCheckBox(entry.label()); + box.setSelected(model.isLoggerEnabled(entry.logger())); + box.addActionListener(e -> selectLogger(entry.logger(), box.isSelected())); panel.add(box); } From 61af2c9aa13b89b389743637444b2be8eb6b89e0 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 16:38:47 +0100 Subject: [PATCH 13/17] refactor: move OtpDebugController to o.o.ext.interactivelauncher.debug --- .../interactivelauncher/{ => debug}/OtpDebugController.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename src/ext/java/org/opentripplanner/ext/interactivelauncher/{ => debug}/OtpDebugController.java (90%) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java similarity index 90% rename from src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java rename to src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java index 57ef7f70644..25e3c8c7629 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/OtpDebugController.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java @@ -1,4 +1,4 @@ -package org.opentripplanner.ext.interactivelauncher; +package org.opentripplanner.ext.interactivelauncher.debug; import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BACKGROUND; @@ -6,6 +6,7 @@ import javax.swing.JFrame; import javax.swing.JPanel; import javax.swing.JTabbedPane; +import org.opentripplanner.ext.interactivelauncher.Model; import org.opentripplanner.ext.interactivelauncher.debug.logging.LogModel; import org.opentripplanner.ext.interactivelauncher.debug.logging.LogView; From 39f2c39ae43990712a81f7e526e6da2f9383eff9 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 17:03:17 +0100 Subject: [PATCH 14/17] feature: Ann UI to set Raptor debug parameters The UI is used to instrument the Raptor search and log events at decision points during routing. --- .../InteractiveOtpMain.java | 9 +- .../ext/interactivelauncher/Model.java | 24 +++-- .../debug/OtpDebugController.java | 25 ++--- .../debug/raptor/RaptorDebugModel.java | 97 +++++++++++++++++++ .../debug/raptor/RaptorDebugView.java | 87 +++++++++++++++++ 5 files changed, 222 insertions(+), 20 deletions(-) create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugModel.java create mode 100644 src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugView.java diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java index f5621c45278..43061ee2b62 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/InteractiveOtpMain.java @@ -1,5 +1,7 @@ package org.opentripplanner.ext.interactivelauncher; +import org.opentripplanner.ext.interactivelauncher.configuration.InteractiveLauncherModule; +import org.opentripplanner.ext.interactivelauncher.debug.OtpDebugController; import org.opentripplanner.ext.interactivelauncher.startup.MainView; import org.opentripplanner.standalone.OTPMain; @@ -29,9 +31,14 @@ private void run() { } private void startOtp() { - model.save(); + startDebugControllerAndSetupRequestInterceptor(); System.out.println("Start OTP: " + model + "\n"); OTPMain.main(model.getStartupModel().asOtpArgs()); } + + private void startDebugControllerAndSetupRequestInterceptor() { + new OtpDebugController(model).start(); + InteractiveLauncherModule.setRequestInterceptor(model.getRaptorDebugModel()); + } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java index 45d8583f66d..59682082b90 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/Model.java @@ -7,6 +7,7 @@ import java.io.IOException; import java.io.Serializable; import org.opentripplanner.ext.interactivelauncher.debug.logging.LogModel; +import org.opentripplanner.ext.interactivelauncher.debug.raptor.RaptorDebugModel; import org.opentripplanner.ext.interactivelauncher.startup.StartupModel; public class Model implements Serializable { @@ -15,6 +16,7 @@ public class Model implements Serializable { private StartupModel startupModel; private LogModel logModel; + private RaptorDebugModel raptorDebugModel; public Model() {} @@ -30,13 +32,8 @@ public LogModel getLogModel() { return logModel; } - public void save() { - try { - var mapper = new ObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, true); - mapper.writeValue(MODEL_FILE, this); - } catch (IOException e) { - throw new RuntimeException(e.getMessage(), e); - } + public RaptorDebugModel getRaptorDebugModel() { + return raptorDebugModel; } private static Model createNew() { @@ -58,6 +55,15 @@ private static Model readFromFile() { } } + private void save() { + try { + var mapper = new ObjectMapper().configure(SerializationFeature.INDENT_OUTPUT, true); + mapper.writeValue(MODEL_FILE, this); + } catch (IOException e) { + throw new RuntimeException(e.getMessage(), e); + } + } + private Model initSubModels() { if (startupModel == null) { startupModel = new StartupModel(); @@ -65,7 +71,11 @@ private Model initSubModels() { if (logModel == null) { logModel = LogModel.createFromConfig(); } + if (raptorDebugModel == null) { + raptorDebugModel = new RaptorDebugModel(); + } logModel.init(this::save); + raptorDebugModel.init(this::save); return this; } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java index 25e3c8c7629..9e41fe3412d 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/OtpDebugController.java @@ -2,29 +2,23 @@ import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.BACKGROUND; -import javax.swing.JComponent; import javax.swing.JFrame; -import javax.swing.JPanel; import javax.swing.JTabbedPane; import org.opentripplanner.ext.interactivelauncher.Model; -import org.opentripplanner.ext.interactivelauncher.debug.logging.LogModel; import org.opentripplanner.ext.interactivelauncher.debug.logging.LogView; +import org.opentripplanner.ext.interactivelauncher.debug.raptor.RaptorDebugView; +/** + * This controller/UI allows changing the debug loggers and setting the raptor + * debug parameters for incoming rute requests. + */ public class OtpDebugController { private final JFrame debugFrame = new JFrame("OTP Debug Controller"); public OtpDebugController(Model model) { - var tabPanel = new JTabbedPane(); - tabPanel.addTab("Logging", createLogPanel(model.getLogModel())); - tabPanel.addTab("Raptor", new JPanel()); - debugFrame.add(tabPanel); + debugFrame.add(createTabbedPane(model)); debugFrame.getContentPane().setBackground(BACKGROUND); - start(); - } - - private static JComponent createLogPanel(LogModel logModel) { - return new LogView(logModel).panel(); } public void start() { @@ -33,4 +27,11 @@ public void start() { debugFrame.setLocationRelativeTo(null); debugFrame.setVisible(true); } + + private static JTabbedPane createTabbedPane(Model model) { + var tabPanel = new JTabbedPane(); + tabPanel.addTab("Logging", new LogView(model.getLogModel()).panel()); + tabPanel.addTab("Raptor", new RaptorDebugView(model.getRaptorDebugModel()).panel()); + return tabPanel; + } } diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugModel.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugModel.java new file mode 100644 index 00000000000..41d6ed6be1a --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugModel.java @@ -0,0 +1,97 @@ +package org.opentripplanner.ext.interactivelauncher.debug.raptor; + +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Objects; +import java.util.Set; +import javax.annotation.Nullable; +import org.opentripplanner.ext.interactivelauncher.api.LauncherRequestDecorator; +import org.opentripplanner.framework.lang.StringUtils; +import org.opentripplanner.routing.api.request.DebugEventType; +import org.opentripplanner.routing.api.request.RouteRequest; + +public class RaptorDebugModel implements LauncherRequestDecorator { + + private final Set eventTypes = EnumSet.noneOf(DebugEventType.class); + private String stops = null; + private String path = null; + private Runnable saveCallback; + + public RaptorDebugModel() {} + + public void init(Runnable saveCallback) { + this.saveCallback = saveCallback; + } + + /** Used by JSON serialization */ + public Set getEventTypes() { + return Collections.unmodifiableSet(eventTypes); + } + + /** Used by JSON serialization */ + public void setEventTypes(Collection eventTypes) { + this.eventTypes.clear(); + this.eventTypes.addAll(eventTypes); + } + + public void enableEventTypes(DebugEventType eventType, boolean enable) { + if (enable) { + if (!isEventTypeSet(eventType)) { + this.eventTypes.add(eventType); + save(); + } + } else { + if (isEventTypeSet(eventType)) { + this.eventTypes.remove(eventType); + save(); + } + } + } + + @Nullable + public String getStops() { + return stops; + } + + public void setStops(@Nullable String stops) { + stops = StringUtils.hasValue(stops) ? stops : null; + if (!Objects.equals(this.stops, stops)) { + this.stops = stops; + save(); + } + } + + @Nullable + public String getPath() { + return path; + } + + public void setPath(@Nullable String path) { + path = StringUtils.hasValue(path) ? path : null; + if (!Objects.equals(this.path, path)) { + this.path = path; + save(); + } + } + + public boolean isEventTypeSet(DebugEventType eventType) { + return eventTypes.contains(eventType); + } + + @Override + public RouteRequest intercept(RouteRequest defaultRequest) { + var newRequest = defaultRequest.clone(); + var debug = newRequest.journey().transit().raptorDebugging(); + debug.withEventTypes(eventTypes); + debug.withStops(stops); + debug.withPath(path); + return newRequest; + } + + private void save() { + if (saveCallback != null) { + saveCallback.run(); + } + } +} diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugView.java new file mode 100644 index 00000000000..186b8d14837 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/raptor/RaptorDebugView.java @@ -0,0 +1,87 @@ +package org.opentripplanner.ext.interactivelauncher.debug.raptor; + +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addComp; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addLabel; +import static org.opentripplanner.ext.interactivelauncher.support.ViewUtils.addVerticalSectionSpace; +import static org.opentripplanner.routing.api.request.DebugEventType.DESTINATION_ARRIVALS; +import static org.opentripplanner.routing.api.request.DebugEventType.PATTERN_RIDES; +import static org.opentripplanner.routing.api.request.DebugEventType.STOP_ARRIVALS; + +import java.awt.Component; +import java.awt.event.FocusAdapter; +import java.awt.event.FocusEvent; +import java.util.function.Consumer; +import javax.swing.Box; +import javax.swing.JCheckBox; +import javax.swing.JTextField; +import org.opentripplanner.routing.api.request.DebugEventType; + +/** + * This UI is used to set Raptor debug parameters, instrument the Raptor + * search, and log event at decision points during routing. + */ +public class RaptorDebugView { + + private final RaptorDebugModel model; + private final Box panel = Box.createVerticalBox(); + private final JCheckBox logStopArrivalsChk = new JCheckBox("Stop arrivals"); + private final JCheckBox logPatternRidesChk = new JCheckBox("Pattern rides"); + private final JCheckBox logDestinationArrivalsChk = new JCheckBox("Destination arrivals"); + private final JTextField stopsTxt = new JTextField(40); + private final JTextField pathTxt = new JTextField(40); + + public RaptorDebugView(RaptorDebugModel model) { + this.model = model; + + addLabel("Log Raptor events for", panel); + addComp(logStopArrivalsChk, panel); + addComp(logPatternRidesChk, panel); + addComp(logDestinationArrivalsChk, panel); + addVerticalSectionSpace(panel); + + addLabel("A list of stops to debug", panel); + addComp(stopsTxt, panel); + addVerticalSectionSpace(panel); + addLabel("A a path (as a list of stops) to debug", panel); + addComp(pathTxt, panel); + addVerticalSectionSpace(panel); + + initValues(); + setupActionListeners(); + } + + private void initValues() { + logStopArrivalsChk.setSelected(model.isEventTypeSet(STOP_ARRIVALS)); + logPatternRidesChk.setSelected(model.isEventTypeSet(PATTERN_RIDES)); + logDestinationArrivalsChk.setSelected(model.isEventTypeSet(DESTINATION_ARRIVALS)); + stopsTxt.setText(model.getStops()); + pathTxt.setText(model.getPath()); + } + + private void setupActionListeners() { + setupActionListenerChkBox(logStopArrivalsChk, STOP_ARRIVALS); + setupActionListenerChkBox(logPatternRidesChk, PATTERN_RIDES); + setupActionListenerChkBox(logDestinationArrivalsChk, DESTINATION_ARRIVALS); + setupActionListenerTextField(stopsTxt, model::setStops); + setupActionListenerTextField(pathTxt, model::setPath); + } + + public Component panel() { + return panel; + } + + private void setupActionListenerChkBox(JCheckBox box, DebugEventType type) { + box.addActionListener(l -> model.enableEventTypes(type, box.isSelected())); + } + + private static void setupActionListenerTextField(JTextField txtField, Consumer model) { + txtField.addFocusListener( + new FocusAdapter() { + @Override + public void focusLost(FocusEvent e) { + model.accept(txtField.getText()); + } + } + ); + } +} From c4d2e6ea7d74e73e7ec9a397b597068764f98fa3 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 4 Jan 2024 18:13:41 +0100 Subject: [PATCH 15/17] refactor: Add tooltip to debug loggers --- .../ext/interactivelauncher/debug/logging/LogView.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java index 52c1774c019..ae8be59b07d 100644 --- a/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java +++ b/src/ext/java/org/opentripplanner/ext/interactivelauncher/debug/logging/LogView.java @@ -23,6 +23,7 @@ public JComponent panel() { private void add(DebugLoggers.Entry entry) { var box = new JCheckBox(entry.label()); + box.setToolTipText("Logger: " + entry.logger()); box.setSelected(model.isLoggerEnabled(entry.logger())); box.addActionListener(e -> selectLogger(entry.logger(), box.isSelected())); panel.add(box); From 3c8fd3ed0696b0a41e1f3e0a6999c8f372ee25b8 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 9 Jan 2024 20:26:26 +0100 Subject: [PATCH 16/17] review: Six spelling removeNone -> removeNon --- .../transit/AccessEgressFunctions.java | 10 ++-- .../rangeraptor/transit/AccessPaths.java | 8 +-- .../rangeraptor/transit/EgressPaths.java | 8 +-- .../transit/AccessEgressFunctionsTest.java | 50 +++++++++---------- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java index 275a0f85cf3..d4bf4d1ef92 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java @@ -81,10 +81,10 @@ private AccessEgressFunctions() {} * Filter non-optimal paths away for the standard search. This method does not * look at the c1 value. */ - static Collection removeNoneOptimalPathsForStandardRaptor( + static Collection removeNonOptimalPathsForStandardRaptor( Collection paths ) { - return removeNoneOptimalPaths(paths, STANDARD_COMPARATOR); + return removeNonOptimalPaths(paths, STANDARD_COMPARATOR); } /** @@ -92,10 +92,10 @@ static Collection removeNoneOptimalPathsForStandardRaptor( * not remove any paths since the caller should not pass in duplicates, but it turns out that * this happens, so we do it. */ - static Collection removeNoneOptimalPathsForMcRaptor( + static Collection removeNonOptimalPathsForMcRaptor( Collection paths ) { - var result = removeNoneOptimalPaths(paths, MC_COMPARATOR); + var result = removeNonOptimalPaths(paths, MC_COMPARATOR); if (LOG.isDebugEnabled() && result.size() < paths.size()) { var duplicates = new ArrayList<>(paths); duplicates.removeAll(result); @@ -137,7 +137,7 @@ static TIntObjectMap> groupByStop(Collection removeNoneOptimalPaths( + static Collection removeNonOptimalPaths( Collection paths, ParetoComparator comparator ) { diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java index dd757ac0415..5c26a10db23 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessPaths.java @@ -1,8 +1,8 @@ package org.opentripplanner.raptor.rangeraptor.transit; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByRound; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForMcRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForStandardRaptor; import gnu.trove.map.TIntObjectMap; import java.util.Arrays; @@ -59,9 +59,9 @@ public int calculateMaxNumberOfRides() { */ public static AccessPaths create(Collection paths, RaptorProfile profile) { if (profile.is(RaptorProfile.MULTI_CRITERIA)) { - paths = removeNoneOptimalPathsForMcRaptor(paths); + paths = removeNonOptimalPathsForMcRaptor(paths); } else { - paths = removeNoneOptimalPathsForStandardRaptor(paths); + paths = removeNonOptimalPathsForStandardRaptor(paths); } return new AccessPaths( groupByRound(paths, RaptorAccessEgress::stopReachedByWalking), diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java index a1688ff8f5b..374fc050782 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/EgressPaths.java @@ -2,8 +2,8 @@ import static org.opentripplanner.raptor.api.request.RaptorProfile.MULTI_CRITERIA; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByStop; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForMcRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForStandardRaptor; import gnu.trove.map.TIntObjectMap; import java.util.Collection; @@ -33,9 +33,9 @@ private EgressPaths(TIntObjectMap> pathsByStop) { */ public static EgressPaths create(Collection paths, RaptorProfile profile) { if (MULTI_CRITERIA.is(profile)) { - paths = removeNoneOptimalPathsForMcRaptor(paths); + paths = removeNonOptimalPathsForMcRaptor(paths); } else { - paths = removeNoneOptimalPathsForStandardRaptor(paths); + paths = removeNonOptimalPathsForStandardRaptor(paths); } return new EgressPaths(groupByStop(paths)); } diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java index 041d74786d7..1d4f90557b0 100644 --- a/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java +++ b/src/test/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctionsTest.java @@ -6,8 +6,8 @@ import static org.opentripplanner.raptor._data.transit.TestAccessEgress.flexAndWalk; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByRound; import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.groupByStop; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForMcRaptor; -import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNoneOptimalPathsForStandardRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForMcRaptor; +import static org.opentripplanner.raptor.rangeraptor.transit.AccessEgressFunctions.removeNonOptimalPathsForStandardRaptor; import java.util.Arrays; import java.util.Collection; @@ -48,127 +48,127 @@ class AccessEgressFunctionsTest implements RaptorTestConstants { .openingHours(T00_10, T01_00); @Test - void removeNoneOptimalPathsForStandardRaptorTest() { + void removeNonOptimalPathsForStandardRaptorTest() { // Empty set - assertElements(List.of(), removeNoneOptimalPathsForStandardRaptor(List.of())); + assertElements(List.of(), removeNonOptimalPathsForStandardRaptor(List.of())); // One element - assertElements(List.of(WALK_8m), removeNoneOptimalPathsForStandardRaptor(List.of(WALK_8m))); + assertElements(List.of(WALK_8m), removeNonOptimalPathsForStandardRaptor(List.of(WALK_8m))); // Shortest duration assertElements( List.of(WALK_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(WALK_8m, WALK_10m)) + removeNonOptimalPathsForStandardRaptor(List.of(WALK_8m, WALK_10m)) ); // Fewest rides assertElements( List.of(FLEX_1x_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(FLEX_1x_8m, FLEX_2x_8m)) + removeNonOptimalPathsForStandardRaptor(List.of(FLEX_1x_8m, FLEX_2x_8m)) ); // Arriving at the stop on-board, and by-foot. // OnBoard is better because we can do a transfer walk to nearby stops. assertElements( List.of(FLEX_1x_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_8m)) + removeNonOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_8m)) ); // Flex+walk is faster, flex arrive on-board, both is optimal assertElements( List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m), - removeNoneOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m)) + removeNonOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m)) ); // Walk has few rides, and Flex is faster - both is optimal assertElements( List.of(WALK_10m, FLEX_1x_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(WALK_10m, FLEX_1x_8m)) + removeNonOptimalPathsForStandardRaptor(List.of(WALK_10m, FLEX_1x_8m)) ); // Walk without opening hours is better than with, because it can be time-shifted without // any constraints assertElements( List.of(WALK_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(WALK_8m, WALK_W_OPENING_HOURS_8m)) + removeNonOptimalPathsForStandardRaptor(List.of(WALK_8m, WALK_W_OPENING_HOURS_8m)) ); // Walk with opening hours can NOT dominate another access/egress without - even if it is // faster. The reason is that it may not be allowed to time-shift it to the desired time. assertElements( List.of(WALK_10m, WALK_W_OPENING_HOURS_8m), - removeNoneOptimalPathsForStandardRaptor(List.of(WALK_10m, WALK_W_OPENING_HOURS_8m)) + removeNonOptimalPathsForStandardRaptor(List.of(WALK_10m, WALK_W_OPENING_HOURS_8m)) ); // If two paths both have opening hours, both should be accepted. assertElements( List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER), - removeNoneOptimalPathsForStandardRaptor( + removeNonOptimalPathsForStandardRaptor( List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER) ) ); } @Test - void removeNoneOptimalPathsForMcRaptorTest() { + void removeNonOptimalPathsForMcRaptorTest() { // Empty set - assertElements(List.of(), removeNoneOptimalPathsForMcRaptor(List.of())); + assertElements(List.of(), removeNonOptimalPathsForMcRaptor(List.of())); // One element - assertElements(List.of(WALK_8m), removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m))); + assertElements(List.of(WALK_8m), removeNonOptimalPathsForMcRaptor(List.of(WALK_8m))); // Lowest cost assertElements( List.of(WALK_10m_C1_LOW), - removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_10m_C1_LOW)) + removeNonOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_10m_C1_LOW)) ); // Shortest duration - assertElements(List.of(WALK_8m), removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_10m))); + assertElements(List.of(WALK_8m), removeNonOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_10m))); // Fewest rides assertElements( List.of(FLEX_1x_8m), - removeNoneOptimalPathsForMcRaptor(List.of(FLEX_1x_8m, FLEX_2x_8m)) + removeNonOptimalPathsForMcRaptor(List.of(FLEX_1x_8m, FLEX_2x_8m)) ); // Arriving at the stop on-board, and by-foot. // OnBoard is better because we can do a transfer walk to nearby stops. assertElements( List.of(FLEX_1x_8m), - removeNoneOptimalPathsForMcRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_8m)) + removeNonOptimalPathsForMcRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_8m)) ); // Flex+walk is faster, flex arrive on-board, both is optimal assertElements( List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m), - removeNoneOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m)) + removeNonOptimalPathsForStandardRaptor(List.of(FLEX_AND_WALK_1x_8m, FLEX_1x_10m)) ); // Walk has few rides, and Flex is faster - both is optimal assertElements( List.of(WALK_10m, FLEX_1x_8m), - removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, FLEX_1x_8m)) + removeNonOptimalPathsForMcRaptor(List.of(WALK_10m, FLEX_1x_8m)) ); // Walk without opening hours is better than with, because it can be time-shifted without // any constraints assertElements( List.of(WALK_8m), - removeNoneOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_W_OPENING_HOURS_8m)) + removeNonOptimalPathsForMcRaptor(List.of(WALK_8m, WALK_W_OPENING_HOURS_8m)) ); // Walk with opening hours can NOT dominate another access/egress without - even if it is // faster. The reason is that it may not be allowed to time-shift it to the desired time. assertElements( List.of(WALK_10m, WALK_W_OPENING_HOURS_8m), - removeNoneOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_W_OPENING_HOURS_8m)) + removeNonOptimalPathsForMcRaptor(List.of(WALK_10m, WALK_W_OPENING_HOURS_8m)) ); // If two paths both have opening hours, both should be accepted. assertElements( List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER), - removeNoneOptimalPathsForMcRaptor( + removeNonOptimalPathsForMcRaptor( List.of(WALK_W_OPENING_HOURS_8m, WALK_W_OPENING_HOURS_8m_OTHER) ) ); From de319cdf178a35f85e114d28945ab5c51e9cb2ad Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 9 Jan 2024 20:30:21 +0100 Subject: [PATCH 17/17] review: Make sure ParetoComparator is as readable as possible using white-space. --- .../rangeraptor/transit/AccessEgressFunctions.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java index d4bf4d1ef92..1ab737ae907 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/transit/AccessEgressFunctions.java @@ -29,7 +29,7 @@ public final class AccessEgressFunctions { private static final Logger LOG = LoggerFactory.getLogger(AccessEgressFunctions.class); /** - * Filter standard(not multi-criteria) Raptor access and egress paths. A path is pareto optimal + * Filter standard (not multi-criteria) Raptor access and egress paths. A path is pareto optimal * for a given stop if *
    *
  1. @@ -53,8 +53,7 @@ public final class AccessEgressFunctions { */ private static final ParetoComparator STANDARD_COMPARATOR = (l, r) -> ( - l.stopReachedOnBoard() && - !r.stopReachedOnBoard() || + (l.stopReachedOnBoard() && !r.stopReachedOnBoard()) || r.hasOpeningHours() || l.numberOfRides() < r.numberOfRides() || l.durationInSeconds() < r.durationInSeconds() @@ -66,13 +65,7 @@ public final class AccessEgressFunctions { * Raptor - it is a bug. */ private static final ParetoComparator MC_COMPARATOR = (l, r) -> - ( - (l.stopReachedOnBoard() && !r.stopReachedOnBoard()) || - r.hasOpeningHours() || - l.numberOfRides() < r.numberOfRides() || - l.durationInSeconds() < r.durationInSeconds() || - l.c1() < r.c1() - ); + STANDARD_COMPARATOR.leftDominanceExist(l, r) || l.c1() < r.c1(); /** private constructor to prevent instantiation of utils class. */ private AccessEgressFunctions() {}