Skip to content

Commit

Permalink
Merge pull request #5519 from entur/otp2_cleanup_paging_filter
Browse files Browse the repository at this point in the history
Code cleanup paging filter
  • Loading branch information
t2gran authored Nov 21, 2023
2 parents a498d21 + dad7d74 commit c9f9d56
Show file tree
Hide file tree
Showing 20 changed files with 354 additions and 115 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package org.opentripplanner.framework.tostring;

import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.opentripplanner.framework.time.DurationUtils;

/**
* When debug logging it is much more readable if the logging is nicely formatted with line-breaks.
* This builder can be used to create a nice list of key/values. It also allows breaking a
* collection of values into lines. It only allows for two nested levels, see {@code PageCursor}
* below.
* <p>
* Example output:
* <pre>
* Response {
* SearchWindowUsed : 50m
* NextPage........ : PageCursor{type: NEXT_PAGE, ...}
* PreviousPage.... : PageCursor{type: PREVIOUS_PAGE, ...}
* Itineraries..... : [
* Origin ~ Walk 2h20m49s ~ Destination [$20186]
* Origin ~ Walk 1m47s ~ Malmö C ~ RAIL Pågatåg 5:55 6:03 ~ ... ~ Destination [$1587]
* Origin ~ Walk 2m31s ~ Malmö C ~ RAIL Ö-tåg 5:08 5:28 ~ ... ~ Destination [$4142]
* ]
* }
* </pre>
* The {@code '...'} is just to shorten the doc, this class does not truncate lines.
*/
public class MultiLineToStringBuilder {

private static final String NL_INDENT_1 = "\n ";
private static final String NL_INDENT_2 = "\n ";

private final String name;
private final List<Item> items = new ArrayList<>();

private MultiLineToStringBuilder(String name) {
this.name = name;
}

public static MultiLineToStringBuilder of(String name) {
return new MultiLineToStringBuilder(name);
}

public MultiLineToStringBuilder add(String label, Object value) {
return addIf(label, value, Objects::nonNull, Object::toString);
}

public MultiLineToStringBuilder addDuration(String label, Duration value) {
return addIf(label, value, Objects::nonNull, DurationUtils::durationToStr);
}

public MultiLineToStringBuilder addColNl(String label, Collection<?> value) {
return addIf(label, value, this::colExist, this::colToString);
}

private <T> MultiLineToStringBuilder addIf(
String label,
T value,
Predicate<T> ignoreValue,
Function<T, String> toString
) {
if (ignoreValue.test(value)) {
items.add(new Item(label, toString.apply(value)));
}
return this;
}

private boolean colExist(Collection<?> c) {
return !(c == null || c.isEmpty());
}

private String colToString(Collection<?> c) {
return c
.stream()
.map(Object::toString)
.collect(Collectors.joining(NL_INDENT_2, "[" + NL_INDENT_2, NL_INDENT_1 + "]"));
}

public String toString() {
var buf = new StringBuilder(name).append(" {");
int labelSize = items.stream().mapToInt(it -> it.key.length()).max().orElse(0);

for (Item item : items) {
var labelTxt = padRight(item.key(), labelSize);
buf.append(NL_INDENT_1).append(labelTxt).append(" : ").append(item.value());
}

return buf.append("\n}").toString();
}

private String padRight(String value, int size) {
var buf = new StringBuilder(value);
while (buf.length() < size) {
buf.append('.');
}
return buf.toString();
}

private record Item(String key, Object value) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,21 +186,27 @@ public <T> ToStringBuilder addCol(String name, Collection<T> c, Function<T, Stri
}

/** Add the collection, truncate the number of elements at given maxLimit. */
public ToStringBuilder addCollection(String name, Collection<?> c, int maxLimit) {
public <T> ToStringBuilder addCollection(
String name,
Collection<T> c,
int maxLimit,
Function<T, String> toString
) {
if (c == null) {
return this;
}
if (c.size() > maxLimit + 1) {
String value = c
.stream()
.limit(maxLimit)
.map(Object::toString)
.collect(Collectors.joining(", "));
String value = c.stream().limit(maxLimit).map(toString).collect(Collectors.joining(", "));
return addIt(name + "(" + maxLimit + "/" + c.size() + ")", "[" + value + ", ..]");
}
return addIfNotNull(name, c);
}

/** Add the collection, truncate the number of elements at given maxLimit. */
public <T> ToStringBuilder addCollection(String name, Collection<T> c, int maxLimit) {
return addCollection(name, c, maxLimit, Object::toString);
}

public ToStringBuilder addColSize(String name, Collection<?> c) {
return addIfNotNull(name, c, x -> String.format("%d items", x.size()));
}
Expand Down Expand Up @@ -372,7 +378,7 @@ private void addValue(@Nonnull String value) {
* Map the given object to a String. If the input object is {@code null} the string
* {@code "null"} is returned if not the {@link Object#toString()} method is called.
*/
public static String nullSafeToString(@Nullable Object object) {
static String nullSafeToString(@Nullable Object object) {
if (object == null) {
return NULL_VALUE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ public ScheduledTransitLeg withAccessibilityScore(Float score) {

/**
* Should be used for debug logging only
* <p>
* The {@code legGeometry} and {@code transitAlerts} are skipped to avoid
* spamming logs. Explicit access should be used if needed.
*/
@Override
public String toString() {
Expand All @@ -407,8 +410,7 @@ public String toString() {
.addObjOp("tripId", getTrip(), AbstractTransitEntity::getId)
.addObj("headsign", getHeadsign())
.addObj("serviceDate", serviceDate)
.addObj("legGeometry", legGeometry)
.addCol("transitAlerts", transitAlerts)
.addColSize("transitAlerts", transitAlerts)
.addEnum("boardRule", getBoardRule())
.addEnum("alightRule", getAlightRule())
.addObj("transferFromPrevLeg", transferFromPrevLeg)
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/opentripplanner/model/plan/StreetLeg.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ public StreetLeg withAccessibilityScore(float accessibilityScore) {
}

/**
* Should be used for debug logging only
* Should be used for debug logging only.
* <p>
* The {@code legGeometry}, {@code elevationProfile}, and {@code walkSteps} are skipped to avoid
* spamming logs. Explicit access should be used if needed.
*/
@Override
public String toString() {
Expand All @@ -193,9 +196,6 @@ public String toString() {
.addEnum("mode", mode)
.addNum("distance", distanceMeters, "m")
.addNum("cost", generalizedCost)
.addObj("legGeometry", legGeometry)
.addObj("legElevation", elevationProfile)
.addCol("walkSteps", walkSteps)
.addCol("streetNotes", streetNotes)
.addBool("walkingBike", walkingBike)
.addBool("rentedVehicle", rentedVehicle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public boolean containsItineraryPageCut() {
return itineraryPageCut != null;
}

@Nullable
public String encode() {
return PageCursorSerializer.encode(this);
}

@Nullable
public static PageCursor decode(String cursor) {
return PageCursorSerializer.decode(cursor);
Expand All @@ -66,9 +71,4 @@ public String toString() {
.addObj("itineraryPageCut", itineraryPageCut)
.toString();
}

@Nullable
public String encode() {
return PageCursorSerializer.encode(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class PageCursorFactory {
private Duration currentSearchWindow = null;
private boolean wholeSwUsed = true;
private ItineraryPageCut itineraryPageCut = null;
private PageCursorFactoryParameters pageCursorFactoryParams = null;
private PageCursorInput pageCursorInput = null;

private PageCursor nextCursor = null;
private PageCursor prevCursor = null;
Expand Down Expand Up @@ -56,11 +56,9 @@ public PageCursorFactory withOriginalSearch(
*
* @param pageCursorFactoryParams contains the result from the {@code PagingDuplicateFilter}
*/
public PageCursorFactory withRemovedItineraries(
PageCursorFactoryParameters pageCursorFactoryParams
) {
public PageCursorFactory withRemovedItineraries(PageCursorInput pageCursorFactoryParams) {
this.wholeSwUsed = false;
this.pageCursorFactoryParams = pageCursorFactoryParams;
this.pageCursorInput = pageCursorFactoryParams;
this.itineraryPageCut =
new ItineraryPageCut(
pageCursorFactoryParams.earliestRemovedDeparture().truncatedTo(ChronoUnit.SECONDS),
Expand Down Expand Up @@ -98,7 +96,7 @@ public String toString() {
.addDuration("currentSearchWindow", currentSearchWindow)
.addDuration("newSearchWindow", newSearchWindow)
.addBoolIfTrue("searchWindowCropped", !wholeSwUsed)
.addObj("pageCursorFactoryParams", pageCursorFactoryParams)
.addObj("pageCursorFactoryParams", pageCursorInput)
.addObj("nextCursor", nextCursor)
.addObj("prevCursor", prevCursor)
.toString();
Expand Down Expand Up @@ -131,20 +129,19 @@ private void createPageCursors() {
} else { // If the whole search window was not used (i.e. if there were removed itineraries)
if (currentPageType == NEXT_PAGE) {
prev.edt = edtBeforeNewSw();
next.edt = pageCursorFactoryParams.earliestRemovedDeparture();
next.edt = pageCursorInput.earliestRemovedDeparture();
if (sortOrder.isSortedByArrivalTimeAscending()) {
prev.lat = pageCursorFactoryParams.earliestKeptArrival().truncatedTo(ChronoUnit.MINUTES);
prev.lat = pageCursorInput.earliestKeptArrival().truncatedTo(ChronoUnit.MINUTES);
} else {
prev.lat = current.lat;
}
} else {
// The search-window start and end is [inclusive, exclusive], so to calculate the start of the
// search-window from the last time included in the search window we need to include one extra
// minute at the end.
prev.edt =
pageCursorFactoryParams.latestRemovedDeparture().minus(newSearchWindow).plusSeconds(60);
prev.edt = pageCursorInput.latestRemovedDeparture().minus(newSearchWindow).plusSeconds(60);
next.edt = edtAfterUsedSw();
prev.lat = pageCursorFactoryParams.latestRemovedArrival();
prev.lat = pageCursorInput.latestRemovedArrival();
}
}
prevCursor = new PageCursor(PREVIOUS_PAGE, sortOrder, prev.edt, prev.lat, newSearchWindow);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* The Instant fields come from the sets of itineraries that were removed and the ones that were
* kept as a result of using the numItineraries parameter.
*/
public interface PageCursorFactoryParameters {
public interface PageCursorInput {
Instant earliestRemovedDeparture();
Instant earliestKeptArrival();
Instant latestRemovedDeparture();
Expand Down
29 changes: 20 additions & 9 deletions src/main/java/org/opentripplanner/raptor/RaptorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,15 @@ public RaptorResponse<T> route(
RaptorRequest<T> request,
RaptorTransitDataProvider<T> transitData
) {
LOG.debug("Original request: {}", request);
logRequest(request);
RaptorResponse<T> response;

if (request.isDynamicSearch()) {
response = new RangeRaptorDynamicSearch<>(config, transitData, request).route();
} else {
response = routeUsingStdWorker(transitData, request);
}
if (LOG.isDebugEnabled()) {
var pathsAsText = response
.paths()
.stream()
.map(p -> "\t\n" + p.toString(transitData.stopNameResolver()))
.collect(Collectors.joining());
LOG.debug("Result: {}", pathsAsText);
}
logResponse(transitData, response);
return response;
}

Expand Down Expand Up @@ -80,4 +73,22 @@ private RaptorResponse<T> routeUsingStdWorker(
var arrivals = new DefaultStopArrivals(result);
return new RaptorResponse<>(result.extractPaths(), arrivals, request, request);
}

private static <T extends RaptorTripSchedule> void logRequest(RaptorRequest<T> request) {
LOG.debug("Original request: {}", request);
}

private static <T extends RaptorTripSchedule> void logResponse(
RaptorTransitDataProvider<T> transitData,
RaptorResponse<T> response
) {
if (LOG.isDebugEnabled()) {
var pathsAsText = response
.paths()
.stream()
.map(p -> "\t\n" + p.toString(transitData.stopNameResolver()))
.collect(Collectors.joining());
LOG.debug("Result: {}", pathsAsText);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,12 @@ default boolean isFree() {
return durationInSeconds() == 0;
}

/** Call this from toString */
/** Call this from toString or {@link #asString(boolean, boolean, String)}*/
default String defaultToString() {
return asString(true, true, null);
}

/** Call this from toString or {@link #defaultToString()} */
default String asString(boolean includeStop, boolean includeCost, @Nullable String summary) {
StringBuilder buf = new StringBuilder();
if (isFree()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,11 @@ public String toString() {
.of(SearchParams.class)
.addServiceTime("earliestDepartureTime", earliestDepartureTime, RaptorConstants.TIME_NOT_SET)
.addServiceTime("latestArrivalTime", latestArrivalTime, RaptorConstants.TIME_NOT_SET)
.addDurationSec("searchWindow", searchWindowInSeconds)
.addDurationSec("searchWindow", searchWindowInSeconds, RaptorConstants.NOT_SET)
.addBoolIfTrue("departAsLateAsPossible", preferLateArrival)
.addNum("numberOfAdditionalTransfers", numberOfAdditionalTransfers)
.addCollection("accessPaths", accessPaths, 5)
.addCollection("egressPaths", egressPaths, 5)
.addNum("numberOfAdditionalTransfers", numberOfAdditionalTransfers, RaptorConstants.NOT_SET)
.addCollection("accessPaths", accessPaths, 5, RaptorAccessEgress::defaultToString)
.addCollection("egressPaths", egressPaths, 5, RaptorAccessEgress::defaultToString)
.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.NumItinerariesFilterResults;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.OtherThanSameLegsMaxGeneralizedCostFilter;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.OutsideSearchWindowFilter;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.PagingDuplicateFilter;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.PagingFilter;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.RemoveBikerentalWithMostlyWalkingFilter;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.RemoveItinerariesWithShortStreetLeg;
import org.opentripplanner.routing.algorithm.filterchain.deletionflagger.RemoveParkAndRideWithMostlyWalkingFilter;
Expand Down Expand Up @@ -347,12 +347,11 @@ public ItineraryListFilterChainBuilder withStopConsolidationFilter(
return this;
}

@SuppressWarnings("CollectionAddAllCanBeReplacedWithConstructor")
public ItineraryListFilterChain build() {
List<ItineraryListFilter> filters = new ArrayList<>();

if (itineraryPageCut != null) {
filters.add(new DeletionFlaggingFilter(new PagingDuplicateFilter(itineraryPageCut)));
filters.add(new DeletionFlaggingFilter(new PagingFilter(itineraryPageCut)));
}

filters.addAll(buildGroupByTripIdAndDistanceFilters());
Expand Down Expand Up @@ -380,8 +379,8 @@ public ItineraryListFilterChain build() {
filters.add(faresFilter);
}

if (this.emissionsFilter != null) {
filters.add(this.emissionsFilter);
if (emissionsFilter != null) {
filters.add(emissionsFilter);
}

if (transitAlertService != null) {
Expand Down
Loading

0 comments on commit c9f9d56

Please sign in to comment.