From 453d9b86559eba62a65af66db34460ae97854080 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 11 Jul 2023 11:59:09 +0100 Subject: [PATCH 01/10] refactor(Flex unit test update): export of areas was missed. Updated unit test to cover this omissio --- .../gtfs/loader/JdbcGtfsExporter.java | 1 + .../java/com/conveyal/gtfs/GTFSFeedTest.java | 103 +++++++++++++++++- src/test/java/com/conveyal/gtfs/GTFSTest.java | 24 +++- 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index b55861e9c..9d08c35d8 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -102,6 +102,7 @@ public FeedLoadResult exportTables() { String whereRouteIsApproved = String.format("where %s.%s.status = 2", feedIdToExport, Table.ROUTES.name); // Export each table in turn (by placing entry in zip output stream). result.agency = export(Table.AGENCY, connection); + result.area = export(Table.AREA, connection); result.bookingRules = export(Table.BOOKING_RULES, connection); result.stopAreas = exportStopAreas(); if (fromEditor) { diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index 079941ba5..4d7bb8142 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -26,20 +26,23 @@ public class GTFSFeedTest { private static final Logger LOG = LoggerFactory.getLogger(GTFSFeedTest.class); private static String simpleGtfsZipFileName; + private static String simpleFlexGtfsZipFileName; @BeforeAll public static void setUpClass() { //executed only once, before the first test simpleGtfsZipFileName = null; + simpleFlexGtfsZipFileName = null; try { simpleGtfsZipFileName = TestUtils.zipFolderFiles("fake-agency", true); + simpleFlexGtfsZipFileName = TestUtils.zipFolderFiles("fake-agency-with-flex", true); } catch (IOException e) { e.printStackTrace(); } } /** - * Make sure a roundtrip of loading a GTFS zip file and then writing another zip file can be performed. + * Make sure a round-trip of loading a GTFS zip file and then writing another zip file can be performed. */ @Test public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { @@ -114,6 +117,104 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { outZip.delete(); } + /** + * Make sure a round-trip of loading a GTFS flex zip file and then writing another zip file can be performed. + */ + @Test + void canDoRoundTripLoadAndWriteToFlexZipFile() throws IOException { + // create a temp file for this test + File outZip = File.createTempFile("fake-agency-with-flex-output", ".zip"); + + // delete file to make sure we can assert that this program created the file + outZip.delete(); + + GTFSFeed feed = GTFSFeed.fromFile(simpleFlexGtfsZipFileName); + feed.toFile(outZip.getAbsolutePath()); + feed.close(); + assertThat(outZip.exists(), is(true)); + + // assert that rows of data were written to files within the zip file. + ZipFile zip = new ZipFile(outZip); + + FileTestCase[] fileTestCases = { + // agency.txt + new FileTestCase( + "agency.txt", + new TestUtils.DataExpectation[]{ + new TestUtils.DataExpectation("agency_id", "1"), + new TestUtils.DataExpectation("agency_name", "Fake Transit") + } + ), + new FileTestCase( + "areas.txt", + new TestUtils.DataExpectation[]{ + new TestUtils.DataExpectation("area_id", "1"), + new TestUtils.DataExpectation("area_name", "Area referencing a stop") + } + ), + new FileTestCase( + "booking_rules.txt", + new TestUtils.DataExpectation[]{ + new TestUtils.DataExpectation("booking_rule_id", "1"), + new TestUtils.DataExpectation("booking_type", "1"), + new TestUtils.DataExpectation("pickup_message", "This is a pickup message") + } + ), + new FileTestCase( + "calendar.txt", + new DataExpectation[]{ + new DataExpectation("service_id", "04100312-8fe1-46a5-a9f2-556f39478f57"), + new DataExpectation("start_date", "20170915"), + new DataExpectation("end_date", "20170917") + } + ), + new FileTestCase( + "routes.txt", + new DataExpectation[]{ + new DataExpectation("agency_id", "1"), + new DataExpectation("route_id", "1"), + new DataExpectation("route_long_name", "Route 1") + } + ), + new FileTestCase( + "shapes.txt", + new DataExpectation[]{ + new DataExpectation("shape_id", "5820f377-f947-4728-ac29-ac0102cbc34e"), + new DataExpectation("shape_pt_lat", "37.0612132"), + new DataExpectation("shape_pt_lon", "-122.0074332") + } + ), + new FileTestCase( + "stop_times.txt", + new DataExpectation[]{ + new DataExpectation("trip_id", "a30277f8-e50a-4a85-9141-b1e0da9d429d"), + new DataExpectation("departure_time", "07:00:00"), + new DataExpectation("stop_id", "4u6g") + } + ), + new FileTestCase( + "stop_areas.txt", + new DataExpectation[]{ + new DataExpectation("area_id", "2"), + new DataExpectation("stop_id", "area-999") + } + ), + new FileTestCase( + "trips.txt", + new DataExpectation[]{ + new DataExpectation("route_id", "1"), + new DataExpectation("trip_id", "a30277f8-e50a-4a85-9141-b1e0da9d429d"), + new DataExpectation("service_id", "04100312-8fe1-46a5-a9f2-556f39478f57") + } + ) + }; + TestUtils.lookThroughFiles(fileTestCases, zip); + // Close the zip file so it can be deleted. + zip.close(); + // delete file to make sure we can assert that this program created the file + outZip.delete(); + } + /** * Make sure that a GTFS feed with interpolated stop times have calculated times after feed processing * @throws GTFSFeed.FirstAndLastStopsDoNotHaveTimes diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 7f724cb79..88d5de1c1 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -1203,7 +1203,7 @@ private void assertThatPersistenceExpectationRecordWasFound( /** * Persistence expectations for use with the GTFS contained within the "fake-agency" resources folder. */ - private PersistenceExpectation[] fakeAgencyPersistenceExpectations = new PersistenceExpectation[]{ + private final PersistenceExpectation[] fakeAgencyPersistenceExpectations = new PersistenceExpectation[]{ new PersistenceExpectation( "agency", new RecordExpectation[]{ @@ -1212,6 +1212,21 @@ private void assertThatPersistenceExpectationRecordWasFound( new RecordExpectation("agency_timezone", "America/Los_Angeles") } ), + new PersistenceExpectation( + "areas", + new RecordExpectation[]{ + new RecordExpectation("area_id", "area1"), + new RecordExpectation("area_name", "This is the area name") + } + ), + new PersistenceExpectation( + "booking_rules", + new RecordExpectation[]{ + new RecordExpectation("booking_rule_id", "1"), + new RecordExpectation("booking_type", "1"), + new RecordExpectation("pickup_message", "This is a pickup message") + } + ), new PersistenceExpectation( "calendar", new RecordExpectation[]{ @@ -1315,6 +1330,13 @@ private void assertThatPersistenceExpectationRecordWasFound( new RecordExpectation("shape_dist_traveled", 0.0, 0.01) } ), + new PersistenceExpectation( + "stop_areas", + new RecordExpectation[]{ + new RecordExpectation("area_id", "area1"), + new RecordExpectation("stop_id", "123") + } + ), new PersistenceExpectation( "trips", new RecordExpectation[]{ From 0a5fb981d59ef775a10faf5f3f4bfac596090cfe Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 12 Jul 2023 10:59:11 +0100 Subject: [PATCH 02/10] refactor(Table.java): Update file name check for feeds in sub directories --- src/main/java/com/conveyal/gtfs/loader/Table.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index b418081d0..0088c3e7d 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -777,7 +777,9 @@ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) Enumeration entries = zipFile.entries(); while (entries.hasMoreElements()) { ZipEntry e = entries.nextElement(); - if (e.getName().endsWith(tableFileName)) { + // Ordering of entries cannot be guaranteed across OS. Using a `\` prefix forces the complete file name + // to be considered. This prevents stop_areas.txt being loaded instead of areas.txt! + if (e.getName().endsWith(String.format("\\%s", tableFileName))) { entry = e; if (sqlErrorStorage != null) sqlErrorStorage.storeError(NewGTFSError.forTable(this, TABLE_IN_SUBDIRECTORY)); break; From 087aaa59dc578840092868ca4fac34c22a9f307e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 12 Jul 2023 11:23:09 +0100 Subject: [PATCH 03/10] refactor(Table.java): Updated to use OS specific file separator --- src/main/java/com/conveyal/gtfs/loader/Table.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index 0088c3e7d..84cc2d59b 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -43,6 +43,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -779,7 +780,7 @@ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) ZipEntry e = entries.nextElement(); // Ordering of entries cannot be guaranteed across OS. Using a `\` prefix forces the complete file name // to be considered. This prevents stop_areas.txt being loaded instead of areas.txt! - if (e.getName().endsWith(String.format("\\%s", tableFileName))) { + if (e.getName().endsWith(String.format("%s%s", File.separator, tableFileName))) { entry = e; if (sqlErrorStorage != null) sqlErrorStorage.storeError(NewGTFSError.forTable(this, TABLE_IN_SUBDIRECTORY)); break; From 600862a14f9887de2b4d774af0f0693793173c28 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 14 Jul 2023 11:07:37 +0100 Subject: [PATCH 04/10] refactor(New unit test): Unit test to cover normalizing flex stop times for patterns --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 2 +- .../java/com/conveyal/gtfs/loader/Table.java | 4 +- .../conveyal/gtfs/dto/PatternStopAreaDTO.java | 8 +- .../com/conveyal/gtfs/dto/StopTimeDTO.java | 9 + .../gtfs/loader/JDBCTableWriterTest.java | 171 ++++++++++++++++++ 5 files changed, 187 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 48f27905c..75d8bfa25 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -1017,7 +1017,7 @@ private int updateStopTimesForPatternLocationOrPatternStopArea( patternId, stopSequence ); - LOG.info("{} stop_time flex service arrivals/departures updated", entitiesUpdated); + LOG.info("{} stop_time flex service start/end pickup drop off window updated.", entitiesUpdated); return travelTime + dwellTime; } diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index 84cc2d59b..b4aa238cf 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -778,8 +778,8 @@ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) Enumeration entries = zipFile.entries(); while (entries.hasMoreElements()) { ZipEntry e = entries.nextElement(); - // Ordering of entries cannot be guaranteed across OS. Using a `\` prefix forces the complete file name - // to be considered. This prevents stop_areas.txt being loaded instead of areas.txt! + // Order of entries cannot be guaranteed across OS. Including a file separator prefix forces the + // complete file name to be considered. This prevents stop_areas.txt being loaded instead of areas.txt. if (e.getName().endsWith(String.format("%s%s", File.separator, tableFileName))) { entry = e; if (sqlErrorStorage != null) sqlErrorStorage.storeError(NewGTFSError.forTable(this, TABLE_IN_SUBDIRECTORY)); diff --git a/src/test/java/com/conveyal/gtfs/dto/PatternStopAreaDTO.java b/src/test/java/com/conveyal/gtfs/dto/PatternStopAreaDTO.java index bd8aa4ef8..5de7e6437 100644 --- a/src/test/java/com/conveyal/gtfs/dto/PatternStopAreaDTO.java +++ b/src/test/java/com/conveyal/gtfs/dto/PatternStopAreaDTO.java @@ -50,9 +50,9 @@ public PatternStopAreaDTO( this.flex_default_zone_time = flexDefaultZoneTime; } - public PatternStopAreaDTO(String pattern_id, String area_id, int stop_sequence) { - this.pattern_id = pattern_id; - this.area_id = area_id; - this.stop_sequence = stop_sequence; + public PatternStopAreaDTO(String patternId, String areaId, int stopSequence) { + this.pattern_id = patternId; + this.area_id = areaId; + this.stop_sequence = stopSequence; } } diff --git a/src/test/java/com/conveyal/gtfs/dto/StopTimeDTO.java b/src/test/java/com/conveyal/gtfs/dto/StopTimeDTO.java index 22e300e6f..b2ba9c411 100644 --- a/src/test/java/com/conveyal/gtfs/dto/StopTimeDTO.java +++ b/src/test/java/com/conveyal/gtfs/dto/StopTimeDTO.java @@ -39,4 +39,13 @@ public StopTimeDTO(String stopId, Integer arrivalTime, Integer departureTime, In departure_time = departureTime; stop_sequence = stopSequence; } + + public static StopTimeDTO flexStopTime(String stopId, Integer startPickupDropOffWindow, Integer endPickupDropOffWindow, Integer stopSequence) { + StopTimeDTO stopTimeDTO = new StopTimeDTO(); + stopTimeDTO.stop_id = stopId; + stopTimeDTO.start_pickup_drop_off_window = startPickupDropOffWindow; + stopTimeDTO.end_pickup_drop_off_window = endPickupDropOffWindow; + stopTimeDTO.stop_sequence = stopSequence; + return stopTimeDTO; + } } diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index e540e1222..6374f2ba0 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -1266,6 +1266,141 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv assertThat(index, equalTo(patternStops.length)); } + /** + * Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int)} can normalize stop times for flex + * patterns. + */ + @Test + void canNormalizePatternStopTimesForFlex() throws IOException, SQLException, InvalidNamespaceException { + int startTime = 6 * 60 * 60; // 6AM + String patternId = newUUID(); + + StopDTO stopOne = createSimpleStop(newUUID(), "Stop One", 0.0, 0.0); + StopDTO stopTwo = createSimpleStop(newUUID(), "Stop Two", 0.0, 0.0); + LocationDTO locationOne = createSimpleTestLocation(newUUID()); + LocationDTO locationTwo = createSimpleTestLocation(newUUID()); + StopAreaDTO stopAreaOne = createSimpleTestStopArea(newUUID()); + StopAreaDTO stopAreaTwo = createSimpleTestStopArea(newUUID()); + + PatternStopDTO[] patternStops = new PatternStopDTO[] { + new PatternStopDTO(patternId, stopOne.stop_id, 0), + new PatternStopDTO(patternId, stopTwo.stop_id, 1) + }; + PatternLocationDTO[] patternLocations = new PatternLocationDTO[] { + new PatternLocationDTO(patternId, locationOne.location_id, 2), + new PatternLocationDTO(patternId, locationTwo.location_id, 3) + }; + PatternStopAreaDTO[] patternStopAreas = new PatternStopAreaDTO[] { + new PatternStopAreaDTO(patternId, stopAreaOne.area_id, 4), + new PatternStopAreaDTO(patternId, stopAreaTwo.area_id, 5) + }; + + int travelTime = 60; + patternStops[0].default_travel_time = 0; + patternStops[1].default_travel_time = travelTime; + patternLocations[0].flex_default_travel_time = travelTime; + patternLocations[1].flex_default_travel_time = travelTime; + patternStopAreas[0].flex_default_travel_time = travelTime; + patternStopAreas[1].flex_default_travel_time = travelTime; + PatternDTO pattern = createRouteAndPattern(newUUID(), + patternId, + "Pattern A", + null, + new ShapePointDTO[] {}, + patternStops, + patternLocations, + patternStopAreas, + 0 + ); + + int cumulativeTravelTime = startTime + travelTime; + StopTimeDTO[] stopTimes = new StopTimeDTO[] { + new StopTimeDTO(stopOne.stop_id, startTime, startTime, 0), + new StopTimeDTO(stopTwo.stop_id, startTime, startTime, 1), + StopTimeDTO.flexStopTime(locationOne.location_id, cumulativeTravelTime, cumulativeTravelTime, 2), + StopTimeDTO.flexStopTime(locationTwo.location_id, (cumulativeTravelTime += travelTime), cumulativeTravelTime, 3), + StopTimeDTO.flexStopTime(stopAreaOne.area_id, (cumulativeTravelTime += travelTime), cumulativeTravelTime, 4), + StopTimeDTO.flexStopTime(stopAreaTwo.area_id, (cumulativeTravelTime += travelTime), cumulativeTravelTime, 5) + }; + + // Create trip with travel times that match pattern stops. + TripDTO tripInput = new TripDTO(); + tripInput.pattern_id = patternId; + tripInput.route_id = pattern.route_id; + tripInput.service_id = simpleServiceId; + tripInput.stop_times = stopTimes; + tripInput.frequencies = new FrequencyDTO[] {}; + JdbcTableWriter createTripWriter = createTestTableWriter(Table.TRIPS); + String createTripOutput = createTripWriter.create(mapper.writeValueAsString(tripInput), true); + LOG.info(createTripOutput); + TripDTO createdTrip = mapper.readValue(createTripOutput, TripDTO.class); + + checkStopArrivalAndDepartures( + createdTrip.trip_id, + startTime, + 0, + travelTime, + patternStops.length + patternLocations.length + patternStopAreas.length + ); + + // Update pattern stop with new travel time. + JdbcTableWriter patternUpdater = createTestTableWriter(Table.PATTERNS); + int updatedTravelTime = 3600; // one hour + pattern.pattern_stops[1].default_travel_time = updatedTravelTime; + String updatedPatternOutput = patternUpdater.update(pattern.id, mapper.writeValueAsString(pattern), true); + LOG.info("Updated pattern output: {}", updatedPatternOutput); + // Normalize stop times. + JdbcTableWriter updateTripWriter = createTestTableWriter(Table.TRIPS); + updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0); + checkStopArrivalAndDepartures( + createdTrip.trip_id, + startTime, + updatedTravelTime, + travelTime, + patternStops.length + patternLocations.length + patternStopAreas.length + ); + } + + /** + * Read stop times from the database and check that the arrivals/departures have been set correctly. + */ + private void checkStopArrivalAndDepartures( + String tripId, + int startTime, + int updatedTravelTime, + int travelTime, + int totalNumberOfPatterns + ) { + JDBCTableReader stopTimesTable = new JDBCTableReader(Table.STOP_TIMES, + testDataSource, + testNamespace + ".", + EntityPopulator.STOP_TIME + ); + int index = 0; + for (StopTime stopTime : stopTimesTable.getOrdered(tripId)) { + if (stopTime.stop_sequence < 2) { + LOG.info("stop times i={} arrival={} departure={}", + index, + stopTime.arrival_time, + stopTime.departure_time + ); + assertEquals(stopTime.arrival_time, startTime + index * updatedTravelTime); + assertEquals(stopTime.departure_time, startTime + index * updatedTravelTime); + } else { + LOG.info("stop times i={} start_pickup_drop_off_window={} end_pickup_drop_off_window={}", + index, + stopTime.start_pickup_drop_off_window, + stopTime.end_pickup_drop_off_window + ); + assertEquals(stopTime.start_pickup_drop_off_window, startTime + updatedTravelTime + (index-1) * travelTime); + assertEquals(stopTime.end_pickup_drop_off_window, startTime + updatedTravelTime + (index-1) * travelTime); + } + index++; + } + // Ensure that updated stop times equals pattern stops length + assertEquals(index, totalNumberOfPatterns); + } + /** * This test makes sure that updated the service_id will properly update affected referenced entities properly. * This test case was initially developed to prove that https://github.com/conveyal/gtfs-lib/issues/203 is @@ -1765,6 +1900,42 @@ private static PatternDTO createRouteAndPattern( return mapper.readValue(output, PatternDTO.class); } + /** + * Creates a pattern by first creating a route and then a pattern for that route. + */ + private static PatternDTO createRouteAndPattern( + String routeId, + String patternId, + String name, + String shapeId, + ShapePointDTO[] shapes, + PatternStopDTO[] patternStops, + PatternLocationDTO[] patternLocations, + PatternStopAreaDTO[] patternStopAreas, + int useFrequency + ) throws InvalidNamespaceException, SQLException, IOException { + // Create new route + createSimpleTestRoute(routeId, "RTA", "500", "Hollingsworth", 3); + // Create new pattern for route + PatternDTO input = new PatternDTO(); + input.pattern_id = patternId; + input.route_id = routeId; + input.name = name; + input.use_frequency = useFrequency; + input.shape_id = shapeId; + input.shapes = shapes; + input.pattern_stops = patternStops; + input.pattern_locations = patternLocations; + input.pattern_stop_areas = patternStopAreas; + // Write the pattern to the database + JdbcTableWriter createPatternWriter = createTestTableWriter(Table.PATTERNS); + String output = createPatternWriter.create(mapper.writeValueAsString(input), true); + LOG.info("create {} output:", Table.PATTERNS.name); + LOG.info(output); + // Parse output + return mapper.readValue(output, PatternDTO.class); + } + /** * Creates a pattern by first creating a route and then a pattern for that route. */ From 86dcb3bbf56f43f5261ea3a9877f041456df825d Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 14 Jul 2023 11:19:43 +0100 Subject: [PATCH 05/10] refactor(JdbcGtfsExporter.java): Restored merge of dev branch --- .../gtfs/loader/JdbcGtfsExporter.java | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index 0cfd7d2da..a19984ee0 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -35,7 +35,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -75,17 +74,6 @@ public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, bo this.fromEditor = fromEditor; } - /** - * Utility method to check if an exception uses a specific service. - */ - public Boolean exceptionInvolvesService(ScheduleException ex, String serviceId) { - return ( - ex.addedService.contains(serviceId) || - ex.removedService.contains(serviceId) || - ex.customSchedule.contains(serviceId) - ); - } - /** * Export primary entity tables as well as Pattern and PatternStops tables. * @@ -136,11 +124,11 @@ public FeedLoadResult exportTables() { GTFSFeed feed = new GTFSFeed(); // FIXME: The below table readers should probably just share a connection with the exporter. JDBCTableReader exceptionsReader = - new JDBCTableReader(Table.SCHEDULE_EXCEPTIONS, dataSource, feedIdToExport + ".", - EntityPopulator.SCHEDULE_EXCEPTION); + new JDBCTableReader(Table.SCHEDULE_EXCEPTIONS, dataSource, feedIdToExport + ".", + EntityPopulator.SCHEDULE_EXCEPTION); JDBCTableReader calendarsReader = - new JDBCTableReader(Table.CALENDAR, dataSource, feedIdToExport + ".", - EntityPopulator.CALENDAR); + new JDBCTableReader(Table.CALENDAR, dataSource, feedIdToExport + ".", + EntityPopulator.CALENDAR); Iterable calendars = calendarsReader.getAll(); Iterable exceptionsIterator = exceptionsReader.getAll(); List exceptions = new ArrayList<>(); @@ -157,10 +145,7 @@ public FeedLoadResult exportTables() { for (Calendar cal : calendars) { Service service = new Service(cal.service_id); service.calendar = cal; - for (ScheduleException ex : exceptions.stream() - .filter(ex -> exceptionInvolvesService(ex, cal.service_id)) - .collect(Collectors.toList()) - ) { + for (ScheduleException ex : exceptions) { if (ex.exemplar.equals(ScheduleException.ExemplarServiceDescriptor.SWAP) && (!ex.addedService.contains(cal.service_id) && !ex.removedService.contains(cal.service_id))) { // Skip swap exception if cal is not referenced by added or removed service. @@ -178,7 +163,7 @@ public FeedLoadResult exportTables() { calendarDate.date = date; calendarDate.service_id = cal.service_id; calendarDate.exception_type = ex.serviceRunsOn(cal) ? 1 : 2; - LOG.info("Adding exception {} (type={}) for calendar {} on date {}", ex.name, calendarDate.exception_type, cal.service_id, date); + LOG.info("Adding exception {} (type={}) for calendar {} on date {}", ex.name, calendarDate.exception_type, cal.service_id, date.toString()); if (service.calendar_dates.containsKey(date)) throw new IllegalArgumentException("Duplicate schedule exceptions on " + date.toString()); From 60677c4d4291bbb6f25895c20661020ca8cb8da2 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 14 Jul 2023 11:45:47 +0100 Subject: [PATCH 06/10] refactor(JDBCTableWriterTest.java): Tidied up helper methods --- .../gtfs/loader/JDBCTableWriterTest.java | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 6374f2ba0..9fb9a8275 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -1880,24 +1880,17 @@ private static PatternDTO createRouteAndPattern( PatternStopDTO[] patternStops, int useFrequency ) throws InvalidNamespaceException, SQLException, IOException { - // Create new route - createSimpleTestRoute(routeId, "RTA", "500", "Hollingsworth", 3); - // Create new pattern for route - PatternDTO input = new PatternDTO(); - input.pattern_id = patternId; - input.route_id = routeId; - input.name = name; - input.use_frequency = useFrequency; - input.shape_id = shapeId; - input.shapes = shapes; - input.pattern_stops = patternStops; - // Write the pattern to the database - JdbcTableWriter createPatternWriter = createTestTableWriter(Table.PATTERNS); - String output = createPatternWriter.create(mapper.writeValueAsString(input), true); - LOG.info("create {} output:", Table.PATTERNS.name); - LOG.info(output); - // Parse output - return mapper.readValue(output, PatternDTO.class); + return createRouteAndPattern( + routeId, + patternId, + name, + shapeId, + shapes, + patternStops, + null, + null, + useFrequency + ); } /** @@ -1925,8 +1918,8 @@ private static PatternDTO createRouteAndPattern( input.shape_id = shapeId; input.shapes = shapes; input.pattern_stops = patternStops; - input.pattern_locations = patternLocations; - input.pattern_stop_areas = patternStopAreas; + if (patternLocations != null) input.pattern_locations = patternLocations; + if (patternStopAreas != null) input.pattern_stop_areas = patternStopAreas; // Write the pattern to the database JdbcTableWriter createPatternWriter = createTestTableWriter(Table.PATTERNS); String output = createPatternWriter.create(mapper.writeValueAsString(input), true); From 890bb2939a7265f69274c027387db4ba54759f87 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 14 Jul 2023 11:52:57 +0100 Subject: [PATCH 07/10] refactor(JdbcGtfsExporter.java): Reinstated merged updates which were previously removed --- .../gtfs/loader/JdbcGtfsExporter.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index a19984ee0..0d61f7365 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -35,6 +35,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -74,6 +75,17 @@ public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, bo this.fromEditor = fromEditor; } + /** + * Utility method to check if an exception uses a specific service. + */ + public Boolean exceptionInvolvesService(ScheduleException ex, String serviceId) { + return ( + ex.addedService.contains(serviceId) || + ex.removedService.contains(serviceId) || + ex.customSchedule.contains(serviceId) + ); + } + /** * Export primary entity tables as well as Pattern and PatternStops tables. * @@ -145,7 +157,10 @@ public FeedLoadResult exportTables() { for (Calendar cal : calendars) { Service service = new Service(cal.service_id); service.calendar = cal; - for (ScheduleException ex : exceptions) { + for (ScheduleException ex : exceptions.stream() + .filter(ex -> exceptionInvolvesService(ex, cal.service_id)) + .collect(Collectors.toList()) + ) { if (ex.exemplar.equals(ScheduleException.ExemplarServiceDescriptor.SWAP) && (!ex.addedService.contains(cal.service_id) && !ex.removedService.contains(cal.service_id))) { // Skip swap exception if cal is not referenced by added or removed service. @@ -163,7 +178,7 @@ public FeedLoadResult exportTables() { calendarDate.date = date; calendarDate.service_id = cal.service_id; calendarDate.exception_type = ex.serviceRunsOn(cal) ? 1 : 2; - LOG.info("Adding exception {} (type={}) for calendar {} on date {}", ex.name, calendarDate.exception_type, cal.service_id, date.toString()); + LOG.info("Adding exception {} (type={}) for calendar {} on date {}", ex.name, calendarDate.exception_type, cal.service_id, date); if (service.calendar_dates.containsKey(date)) throw new IllegalArgumentException("Duplicate schedule exceptions on " + date.toString()); From 4a8026807224d2d855c8f1338c779ac9c008da81 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 8 Aug 2023 14:23:41 +0100 Subject: [PATCH 08/10] Update src/main/java/com/conveyal/gtfs/loader/Table.java Co-authored-by: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> --- src/main/java/com/conveyal/gtfs/loader/Table.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index b4aa238cf..dd107784a 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -778,8 +778,8 @@ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) Enumeration entries = zipFile.entries(); while (entries.hasMoreElements()) { ZipEntry e = entries.nextElement(); - // Order of entries cannot be guaranteed across OS. Including a file separator prefix forces the - // complete file name to be considered. This prevents stop_areas.txt being loaded instead of areas.txt. + // Include the file separator prefix to force the complete file name to be considered. + // This prevents stop_areas.txt from being loaded instead of areas.txt. if (e.getName().endsWith(String.format("%s%s", File.separator, tableFileName))) { entry = e; if (sqlErrorStorage != null) sqlErrorStorage.storeError(NewGTFSError.forTable(this, TABLE_IN_SUBDIRECTORY)); From 6b1d9a1e56222a09c2363673fc3d40057c2b756b Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 8 Aug 2023 14:53:13 +0100 Subject: [PATCH 09/10] refactor(GTFSFeedTest.java): Updated to move common code to generic method --- .../java/com/conveyal/gtfs/GTFSFeedTest.java | 56 ++++++++----------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index 4d7bb8142..ef012a822 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -46,20 +46,6 @@ public static void setUpClass() { */ @Test public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { - // create a temp file for this test - File outZip = File.createTempFile("fake-agency-output", ".zip"); - - // delete file to make sure we can assert that this program created the file - outZip.delete(); - - GTFSFeed feed = GTFSFeed.fromFile(simpleGtfsZipFileName); - feed.toFile(outZip.getAbsolutePath()); - feed.close(); - assertThat(outZip.exists(), is(true)); - - // assert that rows of data were written to files within the zipfile - ZipFile zip = new ZipFile(outZip); - FileTestCase[] fileTestCases = { // agency.txt new FileTestCase( @@ -110,11 +96,8 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { } ) }; - TestUtils.lookThroughFiles(fileTestCases, zip); - // Close the zip file so it can be deleted. - zip.close(); - // delete file to make sure we can assert that this program created the file - outZip.delete(); + loadAndWriteToZipFile(simpleGtfsZipFileName, fileTestCases); + } /** @@ -122,20 +105,6 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { */ @Test void canDoRoundTripLoadAndWriteToFlexZipFile() throws IOException { - // create a temp file for this test - File outZip = File.createTempFile("fake-agency-with-flex-output", ".zip"); - - // delete file to make sure we can assert that this program created the file - outZip.delete(); - - GTFSFeed feed = GTFSFeed.fromFile(simpleFlexGtfsZipFileName); - feed.toFile(outZip.getAbsolutePath()); - feed.close(); - assertThat(outZip.exists(), is(true)); - - // assert that rows of data were written to files within the zip file. - ZipFile zip = new ZipFile(outZip); - FileTestCase[] fileTestCases = { // agency.txt new FileTestCase( @@ -208,6 +177,27 @@ void canDoRoundTripLoadAndWriteToFlexZipFile() throws IOException { } ) }; + loadAndWriteToZipFile(simpleFlexGtfsZipFileName, fileTestCases); + } + + /** + * Load feed and then write to zip file. Once complete, perform tests. + */ + void loadAndWriteToZipFile(String zipFileName, FileTestCase[] fileTestCases) throws IOException { + // create a temp file for this test + File outZip = File.createTempFile(zipFileName, ".zip"); + + // delete file to make sure we can assert that this program created the file + outZip.delete(); + + GTFSFeed feed = GTFSFeed.fromFile(zipFileName); + feed.toFile(outZip.getAbsolutePath()); + feed.close(); + assertThat(outZip.exists(), is(true)); + + // assert that rows of data were written to files within the zip file. + ZipFile zip = new ZipFile(outZip); + TestUtils.lookThroughFiles(fileTestCases, zip); // Close the zip file so it can be deleted. zip.close(); From 9332c419ec56eac75a622ac62fca612ce65302b2 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 10 Aug 2023 11:21:50 +0100 Subject: [PATCH 10/10] refactor(GTFSFeedTest.java): Removed redundant comment --- src/test/java/com/conveyal/gtfs/GTFSFeedTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index ef012a822..5aebac91c 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -106,7 +106,6 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { @Test void canDoRoundTripLoadAndWriteToFlexZipFile() throws IOException { FileTestCase[] fileTestCases = { - // agency.txt new FileTestCase( "agency.txt", new TestUtils.DataExpectation[]{