diff --git a/.gitignore b/.gitignore index dc693fe7..7a707e02 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,6 @@ .idea/ target/ -lambda$*.json \ No newline at end of file +lambda$*.json + +.DS_Store \ No newline at end of file diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index f7300661..cb02c57f 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -257,13 +257,6 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce } } - /** - * Deprecated method to normalize stop times before stop time interpolation. Defaults to - * false for interpolation. - */ - public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQLException { - return normalizeStopTimesForPattern(id, beginWithSequence, false); - } /** * For a given pattern id and starting stop sequence (inclusive), normalize all stop times to match the pattern @@ -271,7 +264,7 @@ public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQ * * @return number of stop times updated */ - public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean interpolateStopTimes) throws SQLException { + public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean interpolateStopTimes, boolean ignoreNonBlankStopTimes) throws SQLException { try { JDBCTableReader patternStops = new JDBCTableReader( Table.PATTERN_STOP, @@ -288,7 +281,7 @@ public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean i patternStopsToNormalize.add(patternStop); } } - int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize, interpolateStopTimes); + int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize, interpolateStopTimes, ignoreNonBlankStopTimes); connection.commit(); return stopTimesUpdated; } catch (Exception e) { @@ -788,27 +781,47 @@ private int interpolateTimesFromTimepoints( PatternStop patternStop, List timepoints, Integer timepointNumber, - double previousShapeDistTraveled + PatternStop prevPatternStop ) { if (timepointNumber == 0 || timepoints.size() == 1 || timepointNumber >= timepoints.size()) { - throw new IllegalStateException("Issue in pattern stops which prevents interpolation (e.g. less than 2 timepoints)"); + throw new IllegalStateException("Issue in pattern stops which prevents interpolation (e.g. less than 2 stoptimes to interpolate between)"); } PatternStop nextTimepoint = timepoints.get(timepointNumber); - PatternStop lastTimepoint = timepoints.get(timepointNumber-1); + PatternStop prevTimepoint = timepoints.get(timepointNumber - 1); if ( nextTimepoint == null || nextTimepoint.default_travel_time == Entity.INT_MISSING || nextTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING || - lastTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING + prevTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING || + (prevPatternStop != null && prevPatternStop.shape_dist_traveled == Entity.DOUBLE_MISSING) ) { throw new IllegalStateException("Error with stop time interpolation: timepoint or shape_dist_traveled is null"); } - double timepointSpeed = (nextTimepoint.shape_dist_traveled - lastTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time; - return (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed); + double prevSdt = prevPatternStop == null ? 0 : prevPatternStop.shape_dist_traveled; + + double timepointSpeed = (nextTimepoint.shape_dist_traveled - prevTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time; + return (int) Math.round((patternStop.shape_dist_traveled - prevSdt) / timepointSpeed); } + private boolean checkIfArrivalTimeExists(int stopSequence, String trip_id) throws SQLException { + String sql = String.format("select arrival_time from %s.stop_times where stop_sequence = ? and trip_id = ?", tablePrefix); + PreparedStatement statement = connection.prepareStatement(sql); + statement.setInt(1, stopSequence); + statement.setString(2, trip_id); + try (ResultSet rs = statement.executeQuery()) { + if (!rs.next()) { + return false; + } + int arrivalTime = rs.getInt(1); + if (rs.wasNull()) { + return false; + } + return arrivalTime > 0; + } + + } /** * Normalizes all stop times' arrivals and departures for an ordered set of pattern stops. This set can be the full * set of stops for a pattern or just a subset. Typical usage for this method would be to overwrite the arrival and @@ -819,7 +832,11 @@ private int interpolateTimesFromTimepoints( * * TODO? add param Set serviceIdFilters service_id values to filter trips on */ - private int updateStopTimesForPatternStops(List patternStops, boolean interpolateStopTimes) throws SQLException { + private int updateStopTimesForPatternStops( + List patternStops, + boolean interpolateStopTimes, + boolean ignoreNonBlankStopTimes + ) throws SQLException { PatternStop firstPatternStop = patternStops.iterator().next(); List timepoints = patternStops.stream().filter(ps -> ps.timepoint == 1).collect(Collectors.toList()); int firstStopSequence = firstPatternStop.stop_sequence; @@ -854,43 +871,64 @@ private int updateStopTimesForPatternStops(List patternStops, boole for (String tripId : timesForTripIds.keySet()) { // Initialize travel time with previous stop time value. int cumulativeTravelTime = timesForTripIds.get(tripId); - int cumulativeInterpolatedTime = cumulativeTravelTime; + int timepointCumulativeTravelTime = cumulativeTravelTime; int timepointNumber = 0; - double previousShapeDistTraveled = 0; // Used for calculating timepoint speed for interpolation + PatternStop prevPatternStop = null; for (PatternStop patternStop : patternStops) { boolean isTimepoint = patternStop.timepoint == 1; if (isTimepoint) timepointNumber++; + boolean hasStopTime = !ignoreNonBlankStopTimes || checkIfArrivalTimeExists(patternStop.stop_sequence, tripId); // Gather travel/dwell time for pattern stop (being sure to check for missing values). int travelTime = patternStop.default_travel_time == Entity.INT_MISSING ? 0 : patternStop.default_travel_time; - if (interpolateStopTimes) { + if (interpolateStopTimes || ignoreNonBlankStopTimes) { if (patternStop.shape_dist_traveled == Entity.DOUBLE_MISSING) { throw new IllegalStateException("Shape_dist_traveled must be defined for all stops in order to perform interpolation"); } - // Override travel time if we're interpolating between timepoints. - if (!isTimepoint) travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, previousShapeDistTraveled); - previousShapeDistTraveled += patternStop.shape_dist_traveled; + // Override travel time if we're interpolating between timepoints, or generating blank stop times + if ((!ignoreNonBlankStopTimes && !isTimepoint) || (ignoreNonBlankStopTimes && !hasStopTime && patternStop.stop_sequence != 0)) { + travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, prevPatternStop); + } } - int dwellTime = patternStop.default_dwell_time == Entity.INT_MISSING ? 0 : patternStop.default_dwell_time; - int oneBasedIndex = 1; - // Increase travel time by current pattern stop's travel and dwell times (and set values for update). - if (!isTimepoint && interpolateStopTimes) { - // We don't want to increment the true cumulative travel time because that adjusts the timepoint - // times later in the pattern. - // Dwell times are ignored right now as they do not fit the typical use case for interpolation. - // They may be incorporated by accounting for all dwell times in intermediate stops when calculating - // the timepoint speed. - cumulativeInterpolatedTime += travelTime; - updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime); - updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime); - } else { - cumulativeTravelTime += travelTime; - updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime); - cumulativeTravelTime += dwellTime; - updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime); + // When interpolating, assume that *all* non-timepoint data is incorrect/missing + // The travel and dwell times defined for stops between timepoints will be ignored. + int dwellTime = patternStop.default_dwell_time == Entity.INT_MISSING || (interpolateStopTimes && !isTimepoint) ? + 0 : patternStop.default_dwell_time; + + // In non-blank stop time mode, only update if our stop time is blank + if (ignoreNonBlankStopTimes) { + if (hasStopTime) { + cumulativeTravelTime += travelTime + dwellTime; + prevPatternStop = patternStop; + continue; + } + + updateStopTimeStatement.setInt(1, cumulativeTravelTime + travelTime); + updateStopTimeStatement.setInt(2, cumulativeTravelTime + travelTime); + updateStopTimeStatement.setString(3, tripId); + updateStopTimeStatement.setInt(4, patternStop.stop_sequence); + stopTimesTracker.addBatch(); } - updateStopTimeStatement.setString(oneBasedIndex++, tripId); - updateStopTimeStatement.setInt(oneBasedIndex++, patternStop.stop_sequence); + + + // In interpolation mode, don't write changes to db for timepoints + // We don't write the travel time, since it is interpolated + if (interpolateStopTimes && isTimepoint) { + timepointCumulativeTravelTime += travelTime + dwellTime; + // Reset the cumulativeTravelTime to the schedule truth + cumulativeTravelTime = timepointCumulativeTravelTime; + prevPatternStop = patternStop; + continue; + } + // Increase travel time by current pattern stop's travel and dwell times (and set values for update). + cumulativeTravelTime += travelTime; + updateStopTimeStatement.setInt(1, cumulativeTravelTime); + cumulativeTravelTime += dwellTime; + updateStopTimeStatement.setInt(2, cumulativeTravelTime); + updateStopTimeStatement.setString(3, tripId); + updateStopTimeStatement.setInt(4, patternStop.stop_sequence); stopTimesTracker.addBatch(); + + prevPatternStop = patternStop; } } return stopTimesTracker.executeRemaining(); @@ -1621,7 +1659,7 @@ private String[] parseExceptionListField(int id, String namespace, Table table, return parsedString.replaceAll("[{}]", "").split("[,]", 0); } - private String getResultSetString(int column, ResultSet resultSet) throws java.sql.SQLException { + private String getResultSetString(int column, ResultSet resultSet) throws SQLException { String resultSetString = resultSet.getString(column); return resultSetString == null ? "" : resultSetString; } diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 171278a1..df2d2fc5 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -42,6 +42,7 @@ import static com.conveyal.gtfs.TestUtils.getResourceFileName; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -62,6 +63,7 @@ public class JDBCTableWriterTest { private static DataSource testDataSource; private static String testNamespace; private static String testGtfsGLSnapshotNamespace; + private static String testGtfsBlankStoptimesSnapshotNamespace; private static String simpleServiceId = "1"; private static String firstStopId = "1"; private static String secondStopId= "1.5"; @@ -109,6 +111,16 @@ public static void setUpClass() throws SQLException, IOException, InvalidNamespa JdbcGtfsSnapshotter snapshotter = new JdbcGtfsSnapshotter(testGtfsGLNamespace, testDataSource, false); SnapshotResult snapshotResult = snapshotter.copyTables(); testGtfsGLSnapshotNamespace = snapshotResult.uniqueIdentifier; + + // Load fake-agency-blank-stoptimes feed for use with blank stoptimes insertion test + FeedLoadResult blankStoptimesFeedLoadResult = load(TestUtils.zipFolderFiles("fake-agency-blank-stoptimes", true), testDataSource); + String testGtfsBlankStoptimesNamespace = blankStoptimesFeedLoadResult.uniqueIdentifier; + // validate feed to create additional tables + validate(testGtfsBlankStoptimesNamespace, testDataSource); + // load into editor via snapshot + JdbcGtfsSnapshotter blankStoptimesSnapshotter = new JdbcGtfsSnapshotter(testGtfsBlankStoptimesNamespace, testDataSource, false); + SnapshotResult blankStoptimesSnapshotResult = blankStoptimesSnapshotter.copyTables(); + testGtfsBlankStoptimesSnapshotNamespace = blankStoptimesSnapshotResult.uniqueIdentifier; } @AfterAll @@ -874,7 +886,7 @@ private static String normalizeStopsForPattern( LOG.info("Updated pattern output: {}", updatedPatternOutput); // Normalize stop times. JdbcTableWriter updateTripWriter = createTestTableWriter(tripsTable); - updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, interpolateStopTimes); + updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, interpolateStopTimes, false); return createdTrip.trip_id; } @@ -950,6 +962,25 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv assertThat(index, equalTo(patternStops.length)); } + @Test + public void canInterpolateMissingStoptimes() throws IOException, SQLException, InvalidNamespaceException { + JdbcTableWriter tableWriter = new JdbcTableWriter(Table.PATTERN_STOP, testDataSource, testGtfsBlankStoptimesSnapshotNamespace); + tableWriter.normalizeStopTimesForPattern(1, 0, false, true); + + JDBCTableReader stopTimesTable = new JDBCTableReader(Table.STOP_TIMES, + testDataSource, + testGtfsBlankStoptimesSnapshotNamespace + ".", + EntityPopulator.STOP_TIME); + int index = 0; + for (StopTime stopTime : stopTimesTable.getOrdered("1720")) { + LOG.info("stop times i={} arrival={} departure={}", index, stopTime.arrival_time, stopTime.departure_time); + // These outcomes will be nonsensical, but correct given the nonsensical shape-dist-traveled values + assertThat(stopTime.arrival_time, greaterThan(0)); + assertThat(stopTime.departure_time, greaterThan(0)); + index++; + } + } + /** * 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 diff --git a/src/test/resources/fake-agency-blank-stoptimes/agency.txt b/src/test/resources/fake-agency-blank-stoptimes/agency.txt new file mode 100644 index 00000000..b492e4ae --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/agency.txt @@ -0,0 +1,2 @@ +agency_id,agency_name,agency_url,agency_phone,agency_timezone,agency_lang +1,"Tri Delta Transit","http://trideltatransit.com","(925) 754-4040","America/Los_Angeles","en" diff --git a/src/test/resources/fake-agency-blank-stoptimes/calendar.txt b/src/test/resources/fake-agency-blank-stoptimes/calendar.txt new file mode 100644 index 00000000..3de7f395 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/calendar.txt @@ -0,0 +1,2 @@ +service_id,monday,tuesday,wednesday,thursday,friday,saturday,sunday,start_date,end_date +1,1,1,1,1,1,1,1,20210828,20221231 diff --git a/src/test/resources/fake-agency-blank-stoptimes/fare_attributes.txt b/src/test/resources/fake-agency-blank-stoptimes/fare_attributes.txt new file mode 100644 index 00000000..3db783ec --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/fare_attributes.txt @@ -0,0 +1,2 @@ +fare_id,price,currency_type,payment_method,transfers,transfer_duration,agency_id +"1",2.00,USD,0,0,,1 diff --git a/src/test/resources/fake-agency-blank-stoptimes/fare_rules.txt b/src/test/resources/fake-agency-blank-stoptimes/fare_rules.txt new file mode 100644 index 00000000..33ebe988 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/fare_rules.txt @@ -0,0 +1,2 @@ +fare_id,route_id +"1","300" diff --git a/src/test/resources/fake-agency-blank-stoptimes/routes.txt b/src/test/resources/fake-agency-blank-stoptimes/routes.txt new file mode 100644 index 00000000..6c514bf3 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/routes.txt @@ -0,0 +1,2 @@ +route_id,agency_id,route_short_name,route_long_name,route_type,route_color,route_text_color,route_url +"300",1,"300","Brentwood Park & Ride / Antioch BART",3,800080,FFFFFF,"http://12.155.17.20/RTT/Public/Schedule.aspx?RouteNo=300" diff --git a/src/test/resources/fake-agency-blank-stoptimes/shapes.txt b/src/test/resources/fake-agency-blank-stoptimes/shapes.txt new file mode 100644 index 00000000..cf807663 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/shapes.txt @@ -0,0 +1,3 @@ +"shape_id","shape_pt_lon","shape_pt_lat","shape_pt_sequence","shape_dist_traveled" +"27","-121.780946","37.996068","1","0" +"27","-121.69877","37.933812","2","200" \ No newline at end of file diff --git a/src/test/resources/fake-agency-blank-stoptimes/stop_times.txt b/src/test/resources/fake-agency-blank-stoptimes/stop_times.txt new file mode 100644 index 00000000..38d10131 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/stop_times.txt @@ -0,0 +1,7 @@ +"trip_id","arrival_time","departure_time","stop_id","stop_sequence","stop_headsign","timepoint","shape_dist_traveled" +"1720","17:46:00","17:46:00","722","0","","1","0" +"1720","17:59:00","17:59:00","33","2","","1","50" +"1720","","","900","3","","","150" +"1720","18:02:01","18:02:01","443","4","","0","200" +"1720","18:20:06","18:20:06","421","5","","0","250" +"1720","18:24:00","18:24:00","610","6","","1","300" \ No newline at end of file diff --git a/src/test/resources/fake-agency-blank-stoptimes/stops.txt b/src/test/resources/fake-agency-blank-stoptimes/stops.txt new file mode 100644 index 00000000..6fab9348 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/stops.txt @@ -0,0 +1,7 @@ +"stop_id","stop_code","stop_name","stop_lat","stop_lon","stop_url","zone_id" +"33","813644","Main St & Empire Ave","37.997839","-121.730492","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=33","" +"421","810930","Brentwood Blvd & Village Dr","37.941524","-121.696341","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=421","" +"443","811043","Main St & Norcross Ln","37.997716","-121.716376","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=443","" +"610","811425","Brentwood Park n Ride","37.933838","-121.698755","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=610","" +"722","817754","Antioch BART","37.995572","-121.778137","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=722","" +"900","817755","test statin","37.995578","-121.778139","","" \ No newline at end of file diff --git a/src/test/resources/fake-agency-blank-stoptimes/trips.txt b/src/test/resources/fake-agency-blank-stoptimes/trips.txt new file mode 100644 index 00000000..9c50e4f5 --- /dev/null +++ b/src/test/resources/fake-agency-blank-stoptimes/trips.txt @@ -0,0 +1,2 @@ +route_id,service_id,trip_id,trip_headsign,direction_id,block_id,shape_id +"300",1,1720,"Eastbound Brentwood Park & Ride",0,167,27