From c16503a484a8898965829f590e451ffa4cc2d1bb Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 17 Jan 2025 13:40:29 +0100 Subject: [PATCH 1/2] Extract mapper for bookWhen, add test --- .../transmodel/mapping/BookingInfoMapper.java | 38 +++++++++ .../timetable/BookingArrangementType.java | 30 +------ .../mapping/BookingInfoMapperTest.java | 83 +++++++++++++++++++ 3 files changed, 123 insertions(+), 28 deletions(-) create mode 100644 application/src/main/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapper.java create mode 100644 application/src/test/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapperTest.java diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapper.java b/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapper.java new file mode 100644 index 00000000000..de59794ea18 --- /dev/null +++ b/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapper.java @@ -0,0 +1,38 @@ +package org.opentripplanner.apis.transmodel.mapping; + +import org.opentripplanner.transit.model.timetable.booking.BookingInfo; +import org.opentripplanner.transit.model.timetable.booking.BookingTime; + +/** + * Maps the {@link BookingInfo} to enum value (as a string) returned by the API. + */ +public class BookingInfoMapper { + + public static String mapToBookWhen(BookingInfo bookingInfo) { + if (bookingInfo.getMinimumBookingNotice().isPresent()) { + return null; + } + BookingTime latestBookingTime = bookingInfo.getLatestBookingTime(); + BookingTime earliestBookingTime = bookingInfo.getEarliestBookingTime(); + + // Try to deduce the original enum from stored values + if (earliestBookingTime == null) { + if (latestBookingTime == null) { + return "timeOfTravelOnly"; + } else if (latestBookingTime.getDaysPrior() == 1) { + return "untilPreviousDay"; + } else if (latestBookingTime.getDaysPrior() == 0) { + return "advanceAndDayOfTravel"; + } else { + return "other"; + } + } else if ( + earliestBookingTime.getDaysPrior() == 0 && + (latestBookingTime == null || latestBookingTime.getDaysPrior() == 0) + ) { + return "dayOfTravelOnly"; + } else { + return "other"; + } + } +} diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/model/timetable/BookingArrangementType.java b/application/src/main/java/org/opentripplanner/apis/transmodel/model/timetable/BookingArrangementType.java index 911ec8d9b0c..097fa92baca 100644 --- a/application/src/main/java/org/opentripplanner/apis/transmodel/model/timetable/BookingArrangementType.java +++ b/application/src/main/java/org/opentripplanner/apis/transmodel/model/timetable/BookingArrangementType.java @@ -6,6 +6,7 @@ import graphql.schema.GraphQLList; import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLOutputType; +import org.opentripplanner.apis.transmodel.mapping.BookingInfoMapper; import org.opentripplanner.apis.transmodel.model.EnumTypes; import org.opentripplanner.apis.transmodel.model.framework.TransmodelScalars; import org.opentripplanner.transit.model.organization.ContactInfo; @@ -107,34 +108,7 @@ public static GraphQLObjectType create() { .name("bookWhen") .description("Time constraints for booking") .type(EnumTypes.PURCHASE_WHEN) - .dataFetcher(environment -> { - BookingInfo bookingInfo = bookingInfo(environment); - if (bookingInfo.getMinimumBookingNotice().isPresent()) { - return null; - } - BookingTime latestBookingTime = bookingInfo.getLatestBookingTime(); - BookingTime earliestBookingTime = bookingInfo.getEarliestBookingTime(); - - // Try to deduce the original enum from stored values - if (earliestBookingTime == null) { - if (latestBookingTime == null) { - return "timeOfTravelOnly"; - } else if (latestBookingTime.getDaysPrior() == 1) { - return "untilPreviousDay"; - } else if (latestBookingTime.getDaysPrior() == 0) { - return "advanceAndDayOfTravel"; - } else { - return "other"; - } - } else if ( - earliestBookingTime.getDaysPrior() == 0 && - (latestBookingTime == null || latestBookingTime.getDaysPrior() == 0) - ) { - return "dayOfTravelOnly"; - } else { - return "other"; - } - }) + .dataFetcher(environment -> BookingInfoMapper.mapToBookWhen(bookingInfo(environment))) .build() ) .field( diff --git a/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapperTest.java b/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapperTest.java new file mode 100644 index 00000000000..76af5f76d37 --- /dev/null +++ b/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/BookingInfoMapperTest.java @@ -0,0 +1,83 @@ +package org.opentripplanner.apis.transmodel.mapping; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.opentripplanner.apis.transmodel.mapping.BookingInfoMapper.mapToBookWhen; + +import java.time.Duration; +import java.time.LocalTime; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.opentripplanner.transit.model.timetable.booking.BookingInfo; +import org.opentripplanner.transit.model.timetable.booking.BookingTime; + +class BookingInfoMapperTest { + + private static final Duration TEN_MINUTES = Duration.ofMinutes(10); + private static final BookingTime BOOKING_TIME_ZERO_DAYS_PRIOR = new BookingTime( + LocalTime.of(10, 0), + 0 + ); + + @Test + void bookingNotice() { + assertNull(mapToBookWhen(BookingInfo.of().withMinimumBookingNotice(TEN_MINUTES).build())); + } + + @Test + void timeOfTravelOnly() { + assertEquals("timeOfTravelOnly", mapToBookWhen(BookingInfo.of().build())); + } + + @Test + void untilPreviousDay() { + var info = daysPrior(1); + assertEquals("untilPreviousDay", mapToBookWhen(info)); + } + + @Test + void advanceAndDayOfTravel() { + var info = daysPrior(0); + assertEquals("advanceAndDayOfTravel", mapToBookWhen(info)); + } + + @ParameterizedTest + @ValueSource(ints = { 2, 3, 4, 14, 28 }) + void other(int days) { + var info = daysPrior(days); + assertEquals("other", mapToBookWhen(info)); + } + + @Test + void dayOfTravelOnly() { + var info = BookingInfo.of().withEarliestBookingTime(BOOKING_TIME_ZERO_DAYS_PRIOR).build(); + assertEquals("dayOfTravelOnly", mapToBookWhen(info)); + } + + @Test + void latestBookingTime() { + var info = BookingInfo + .of() + .withEarliestBookingTime(BOOKING_TIME_ZERO_DAYS_PRIOR) + .withLatestBookingTime(BOOKING_TIME_ZERO_DAYS_PRIOR) + .build(); + assertEquals("dayOfTravelOnly", mapToBookWhen(info)); + } + + @Test + void earliestBookingTimeZero() { + var info = BookingInfo + .of() + .withEarliestBookingTime(new BookingTime(LocalTime.of(10, 0), 10)) + .build(); + assertEquals("other", mapToBookWhen(info)); + } + + private static BookingInfo daysPrior(int daysPrior) { + return BookingInfo + .of() + .withLatestBookingTime(new BookingTime(LocalTime.of(10, 0), daysPrior)) + .build(); + } +} From 5639c3181955c665aa3a55866cd4fedc93caa5c7 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 17 Jan 2025 14:05:28 +0100 Subject: [PATCH 2/2] Add enum values for RelativeDirection in Transmodel API --- .../transmodel/mapping/RelativeDirectionMapper.java | 10 +++++----- .../apis/transmodel/model/EnumTypes.java | 3 +++ .../org/opentripplanner/apis/transmodel/schema.graphql | 3 +++ .../apis/transmodel/model/EnumTypesTest.java | 3 ++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/RelativeDirectionMapper.java b/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/RelativeDirectionMapper.java index 3228cb914df..1787515e0c7 100644 --- a/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/RelativeDirectionMapper.java +++ b/application/src/main/java/org/opentripplanner/apis/transmodel/mapping/RelativeDirectionMapper.java @@ -22,12 +22,12 @@ public static RelativeDirection map(RelativeDirection relativeDirection) { CIRCLE_COUNTERCLOCKWISE, ELEVATOR, UTURN_LEFT, - UTURN_RIGHT -> relativeDirection; - // for these the Transmodel API doesn't have a mapping. should it? - case ENTER_STATION, + UTURN_RIGHT, + ENTER_STATION, EXIT_STATION, - ENTER_OR_EXIT_STATION, - FOLLOW_SIGNS -> RelativeDirection.CONTINUE; + FOLLOW_SIGNS -> relativeDirection; + // this type should never be exposed by an API + case ENTER_OR_EXIT_STATION -> RelativeDirection.CONTINUE; }; } } diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/model/EnumTypes.java b/application/src/main/java/org/opentripplanner/apis/transmodel/model/EnumTypes.java index 2f8e69cc593..fba1a8f637a 100644 --- a/application/src/main/java/org/opentripplanner/apis/transmodel/model/EnumTypes.java +++ b/application/src/main/java/org/opentripplanner/apis/transmodel/model/EnumTypes.java @@ -289,6 +289,9 @@ public class EnumTypes { .value("elevator", RelativeDirection.ELEVATOR) .value("uturnLeft", RelativeDirection.UTURN_LEFT) .value("uturnRight", RelativeDirection.UTURN_RIGHT) + .value("enterStation", RelativeDirection.ENTER_STATION) + .value("exitStation", RelativeDirection.EXIT_STATION) + .value("followSigns", RelativeDirection.FOLLOW_SIGNS) .build(); public static final GraphQLEnumType REPORT_TYPE = GraphQLEnumType diff --git a/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql b/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql index 6834d375bf1..c01115a2120 100644 --- a/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql +++ b/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql @@ -1719,6 +1719,9 @@ enum RelativeDirection { continue depart elevator + enterStation + exitStation + followSigns hardLeft hardRight left diff --git a/application/src/test/java/org/opentripplanner/apis/transmodel/model/EnumTypesTest.java b/application/src/test/java/org/opentripplanner/apis/transmodel/model/EnumTypesTest.java index 9090cd1bdc5..5a83ec66bb8 100644 --- a/application/src/test/java/org/opentripplanner/apis/transmodel/model/EnumTypesTest.java +++ b/application/src/test/java/org/opentripplanner/apis/transmodel/model/EnumTypesTest.java @@ -1,6 +1,7 @@ package org.opentripplanner.apis.transmodel.model; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -93,7 +94,7 @@ void serializeRelativeDirection(RelativeDirection direction) { Locale.ENGLISH ); assertInstanceOf(String.class, value); - assertNotNull(value); + assertFalse(((String) value).isEmpty()); } @Test