Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve paging - avoid duplicates and missed itineraries when paging #5551

Merged
merged 49 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
c0fc502
feature: Add support for enums in tokens
t2gran Nov 22, 2023
09f15d9
feature: Remove sort-order duplicate from ItineraryPageCut
t2gran Nov 22, 2023
50a414d
refactor: Add test that test all combinations for sorting itineraries
t2gran Nov 23, 2023
5229361
refactor: Remove search-window pruning from PagingFilter
t2gran Nov 23, 2023
bbe8b22
refactor: Move ListSection to framework, so it can be reused
t2gran Nov 24, 2023
e2a1246
refactor: Cleanup code: PageType and RouteRequest
t2gran Nov 24, 2023
f97861c
refactor: Calculate deduplicationSection instead of including it in t…
t2gran Nov 24, 2023
f08a7ad
refactor: Cleanup PageCursor, do not repeat ItinerarySortKey
t2gran Nov 24, 2023
043c512
refactor: Rename ItineraryPageCut to DeduplicationPageCut
t2gran Nov 24, 2023
f636e66
refactor: Make DeduplicationPageCut package local
t2gran Nov 24, 2023
28e7466
feature: Add boolean type to token serializer
t2gran Nov 24, 2023
6461290
feature: Add CharacterEscapeFormatter to escape a special character.
t2gran Nov 26, 2023
180243f
feature: Make tokens readable after Base64 decode
t2gran Nov 27, 2023
2b50ac7
refactor: Use Token framwork to serialize page-cursor
t2gran Nov 28, 2023
cf7f5ed
refactor: Cleanup NumItinerariesFilterResults
t2gran Nov 28, 2023
595dc2d
refactor: Move PagingSearchWindowAdjuster to paging package
t2gran Nov 29, 2023
f07a15e
refactor: Move 'pagecursor' to 'paging.cursor' package
t2gran Nov 29, 2023
c5afe6c
refactor: Improve naming: withMaxNumberOfItinerariesCropSection(..)
t2gran Nov 29, 2023
5db6775
refactor: Extract paging logic into PagingService
t2gran Nov 29, 2023
e52c370
refactor: Cleanup PagingFilterTest
t2gran Nov 29, 2023
1618774
feature: Add a mutable Box to pass a reference to a method.
t2gran Dec 2, 2023
bce86f1
refactor: Push logic from request class to PageCursor(feature envy)
t2gran Dec 2, 2023
23f6b43
feature: Add a utility method to simplify using remove-filters in tes…
t2gran Dec 2, 2023
0fbfbcc
feature: Add a TestDebug class to simplify "System.out" debugging in …
t2gran Dec 2, 2023
a5037ca
refactor: Extract Paging functionality into a PagingService and mappi…
t2gran Dec 2, 2023
cf6a299
feature: Add a paging module test.
t2gran Dec 2, 2023
28e473f
refactor: Remove duplicate code in ItineraryListFilterChainBuilder
t2gran Dec 2, 2023
9bbee78
refactor: Do paging related filtering at the end of the filter process.
t2gran Dec 2, 2023
044af91
feature: Add 'first()' and 'last()' function to ListUtils.
t2gran Dec 2, 2023
2109205
refactor: SortOrderComparator switch
t2gran Dec 3, 2023
7951888
refactor: Rename PageCursorInput firstRemoved() to pageCut(), add Jav…
t2gran Dec 3, 2023
4eafcf3
test: Improve paging module tests
t2gran Dec 5, 2023
8ed052e
refactor: Order filters: OutsideSearchWindowFilter -> PagingFilter
t2gran Dec 6, 2023
5d0c8f2
bug: Improve OutsideSearchWindow, make it [inclusive, exclusive]
t2gran Dec 6, 2023
7e00e09
bug: The latest-arrive-time should not change for arriveBy search whe…
t2gran Dec 6, 2023
0b350c6
refactor: Use interface ItineraryDeletionFlagger default method inste…
t2gran Dec 6, 2023
83940e1
refactor: Use last kept itinerary as page-cut, not first removed.
t2gran Dec 6, 2023
51a7c9b
refactor: Use last Remove unused methods in PageCursorInput.
t2gran Dec 6, 2023
65801fa
Apply suggestions from code review
t2gran Dec 7, 2023
bdebc41
Merge remote-tracking branch 'otp/dev-2.x' into serialize_page_token
t2gran Dec 8, 2023
081fc0c
Apply suggestions from code review
t2gran Dec 11, 2023
ddac7a7
review: Rename TokenFormat to TokenFormatterConfiguration
t2gran Dec 12, 2023
dadbd04
review: Improve TestDebug implementation
t2gran Dec 12, 2023
2b2ab93
Apply suggestions from code review
t2gran Dec 12, 2023
251a85c
review: Fix bugs in legacy calculation of prev/next request-time
t2gran Dec 12, 2023
a5caefb
Update src/main/java/org/opentripplanner/service/paging/PagingService…
t2gran Dec 12, 2023
81cbba3
fix: Fix NPE in DefaultRoutingService debug logging.
t2gran Dec 12, 2023
ad2e35d
fix: NPE in PagingService
t2gran Dec 13, 2023
e4b1fd0
Update src/main/java/org/opentripplanner/routing/algorithm/filterchai…
t2gran Dec 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.util.List;
import org.opentripplanner.api.resource.DebugOutput;
import org.opentripplanner.model.plan.TripPlan;
import org.opentripplanner.model.plan.pagecursor.PageCursor;
import org.opentripplanner.model.plan.paging.cursor.PageCursor;
import org.opentripplanner.routing.api.response.RoutingError;
import org.opentripplanner.routing.api.response.TripSearchMetadata;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.opentripplanner.ext.transmodelapi.model.PlanResponse;
import org.opentripplanner.ext.transmodelapi.support.GqlUtil;
import org.opentripplanner.framework.graphql.GraphQLUtils;
import org.opentripplanner.model.plan.pagecursor.PageCursor;
import org.opentripplanner.model.plan.paging.cursor.PageCursor;

public class TripType {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import org.opentripplanner.apis.gtfs.generated.GraphQLDataFetchers;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.StopArrival;
import org.opentripplanner.model.plan.pagecursor.PageCursor;
import org.opentripplanner.model.plan.paging.cursor.PageCursor;
import org.opentripplanner.routing.api.response.RoutingError;
import org.opentripplanner.routing.api.response.RoutingResponse;
import org.opentripplanner.routing.api.response.TripSearchMetadata;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.opentripplanner.routing.algorithm.filterchain;
package org.opentripplanner.framework.collection;

/**
* This enum is used to signal which part of a list an operation apply to. You may remove elements
Expand All @@ -9,5 +9,16 @@ public enum ListSection {
HEAD,

/** The end of the list */
TAIL,
TAIL;

public boolean isHead() {
return this == HEAD;
}

public ListSection invert() {
return switch (this) {
case HEAD -> TAIL;
case TAIL -> HEAD;
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@

public class ListUtils {

/**
* Return the first element in the list. {@code null} is returned if the list is
* null or empty.
*/
public static <T> T first(List<T> list) {
return list == null || list.isEmpty() ? null : list.getFirst();
}

/**
* Return the last element in the list. {@code null} is returned if the list is
* null or empty.
*/
public static <T> T last(List<T> list) {
return list == null || list.isEmpty() ? null : list.getLast();
}

/**
* Combine a number of collections into a single list.
*/
Expand Down
61 changes: 61 additions & 0 deletions src/main/java/org/opentripplanner/framework/lang/Box.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package org.opentripplanner.framework.lang;

import java.util.Objects;
import javax.annotation.Nullable;

/**
* A box around a mutable value reference. This can be used inside a lambda or passed into
* a function.
* @param <T> the type of the wrapped value.
*/
public class Box<T> {

private T value;

private Box(T value) {
this.value = value;
}

public Box() {
this(null);
}

public static <T> Box<T> empty() {
return new Box<>();
}

public static <T> Box<T> of(T value) {
return new Box<>(value);
}

@Nullable
public T get() {
return value;
}

public void set(@Nullable T value) {
this.value = value;
}

public boolean isEmpty() {
return value == null;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Box<?> box = (Box<?>) o;
return Objects.equals(value, box.value);
}

@Override
public int hashCode() {
return Objects.hash(value);
}

@Override
public String toString() {
return "[" + value + ']';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package org.opentripplanner.framework.text;

/**
* This class is used to escape characters in a string, removing a special character from
* the string. For example, if you want to make sure a string does not contain {@code ';'},
* the {@code ';'} can be replaced with {@code '\+'}. The slash({@code '\'}) is used as an
* escape character, so we need to escape all {@code '\'} as well. Now, the escaped string
* does not contain the special character anymore. The original string can be computed by
* reversing the process.
* <p>
* A "special-character" is removed from a text using an escape character and
* a substitution character. For example, if:
* <ul>
* <li>the escape char is '\'</li>
* <li>the special char is ';'</li>
* <li>and the substitution char is '+'</li>
* </ul>
*
* then replace:
* <ul>
* <li>'\' with '\\' and</li>
* <li>';' with '\;'</li>
* </ul>
* To get back the original text, the reverse process using {@link #decode(String)}.
* <pre>
*
* Original: "\tThis;is;an;example\+"
* Encoded: "\\tThis\+is\+an\+example\\+"
* Decoded: "\tThis;is;an;example\+"
* </pre>
*/
public class CharacterEscapeFormatter {

private final char escapeChar;
private final char specialChar;
private final char substitutionChar;

/**
* @param escapeChar the character used as an escape character.
* @param specialChar the character to be removed/replaced in the encoded text.
* @param substitutionChar the character used together with the escape character to put in the
* encoded text as a placeholder for the special character.
*/
public CharacterEscapeFormatter(char escapeChar, char specialChar, char substitutionChar) {
this.escapeChar = escapeChar;
this.specialChar = specialChar;
this.substitutionChar = substitutionChar;
}

/**
* Encode the given text and replace the {@code specialChar} with a placeholder. The original
* text can be retrieved by using {@link #decode(String)}.
* @param text the text to encode.
* @return the encoded text without the {@code specialChar}.
*/
public String encode(String text) {
final var buf = new StringBuilder();
for (int i = 0; i < text.length(); ++i) {
char ch = text.charAt(i);
if (ch == escapeChar) {
buf.append(escapeChar).append(escapeChar);
} else if (ch == specialChar) {
buf.append(escapeChar).append(substitutionChar);
} else {
buf.append(ch);
}
}
return buf.toString();
}

/**
* Return the original text by decoding the encoded text.
* @see #encode(String)
*/
public String decode(String encodedText) {
if (encodedText.length() < 2) {
return encodedText;
}
final var buf = new StringBuilder();
boolean prevEsc = false;
for (int i = 0; i < encodedText.length(); ++i) {
char ch = encodedText.charAt(i);
if (prevEsc) {
if (ch == escapeChar) {
buf.append(escapeChar);
} else if (ch == substitutionChar) {
buf.append(specialChar);
} else {
throw new IllegalStateException(
"Unexpected combination of escape-char '%c' and '%c' character at position %d. Text: '%s'.".formatted(
escapeChar,
ch,
i,
encodedText
)
);
}
prevEsc = false;
} else if (ch == escapeChar) {
prevEsc = true;
} else {
buf.append(ch);
}
}
return buf.toString();
}
}
Original file line number Diff line number Diff line change
@@ -1,97 +1,74 @@
package org.opentripplanner.framework.token;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Base64;
import java.util.List;
import org.opentripplanner.framework.time.DurationUtils;
import java.util.regex.Pattern;
import java.util.stream.Stream;

class Deserializer {

private final ByteArrayInputStream input;
private static final Pattern SPLIT_PATTERN = Pattern.compile(
"[" + TokenFormatterConfiguration.fieldSeparator() + "]"
);

private final List<String> values;

Deserializer(String token) {
this.input = new ByteArrayInputStream(Base64.getUrlDecoder().decode(token));
byte[] bytes = Base64.getUrlDecoder().decode(token);
var tokenFormatter = TokenFormatterConfiguration.tokenFormatter();
this.values =
Stream.of(SPLIT_PATTERN.split(new String(bytes), -1)).map(tokenFormatter::decode).toList();
}

List<Object> deserialize(TokenDefinition definition) throws IOException {
List<Object> deserialize(TokenDefinition definition) {
try {
// Assume deprecated fields are included in the token
return readFields(definition, false);
} catch (IOException ignore) {
} catch (Exception ignore) {
// If the token is the next version, then deprecated field are removed. Try
// skipping the deprecated tokens
return readFields(definition, true);
}
}

private List<Object> readFields(TokenDefinition definition, boolean matchNewVersionPlusOne)
throws IOException {
input.reset();
private List<Object> readFields(TokenDefinition definition, boolean matchNewVersionPlusOne) {
List<Object> result = new ArrayList<>();

var in = new ObjectInputStream(input);

readAndMatchVersion(in, definition, matchNewVersionPlusOne);
matchVersion(definition, matchNewVersionPlusOne);
int index = 1;

for (FieldDefinition field : definition.listFields()) {
if (matchNewVersionPlusOne && field.deprecated()) {
continue;
}
var v = read(in, field);
var v = read(field, index);
if (!field.deprecated()) {
result.add(v);
}
++index;
}
return result;
}

private void readAndMatchVersion(
ObjectInputStream in,
TokenDefinition definition,
boolean matchVersionPlusOne
) throws IOException {
private void matchVersion(TokenDefinition definition, boolean matchVersionPlusOne) {
int matchVersion = (matchVersionPlusOne ? 1 : 0) + definition.version();

int v = readInt(in);
if (v != matchVersion) {
throw new IOException(
"Version does not match. Token version: " + v + ", schema version: " + definition.version()
int version = readVersion();
if (version != matchVersion) {
throw new IllegalStateException(
"Version does not match. Token version: " +
version +
", schema version: " +
definition.version()
);
}
}

private Object read(ObjectInputStream in, FieldDefinition field) throws IOException {
return switch (field.type()) {
case BYTE -> readByte(in);
case DURATION -> readDuration(in);
case INT -> readInt(in);
case STRING -> readString(in);
case TIME_INSTANT -> readTimeInstant(in);
};
}

private static byte readByte(ObjectInputStream in) throws IOException {
return in.readByte();
}

private static int readInt(ObjectInputStream in) throws IOException {
return Integer.parseInt(in.readUTF());
}

private static String readString(ObjectInputStream in) throws IOException {
return in.readUTF();
}

private static Duration readDuration(ObjectInputStream in) throws IOException {
return DurationUtils.duration(in.readUTF());
private Object read(FieldDefinition field, int index) {
return field.type().stringToValue(values.get(index));
}

private static Instant readTimeInstant(ObjectInputStream in) throws IOException {
return Instant.parse(in.readUTF());
private int readVersion() {
return Integer.parseInt(values.get(0));
}
}
Loading
Loading