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

Fix bad date NPE #143

Merged
merged 11 commits into from
Jan 4, 2019
5 changes: 3 additions & 2 deletions src/main/java/com/conveyal/gtfs/GTFS.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.conveyal.gtfs.loader.JdbcGtfsExporter;
import com.conveyal.gtfs.loader.JdbcGtfsLoader;
import com.conveyal.gtfs.loader.JdbcGtfsSnapshotter;
import com.conveyal.gtfs.loader.SnapshotResult;
import com.conveyal.gtfs.util.InvalidNamespaceException;
import com.conveyal.gtfs.validator.ValidationResult;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -83,9 +84,9 @@ public static FeedLoadResult load (String filePath, DataSource dataSource) {
* @param dataSource JDBC connection to existing database
* @return FIXME should this be a separate SnapshotResult object?
*/
public static FeedLoadResult makeSnapshot (String feedId, DataSource dataSource) {
public static SnapshotResult makeSnapshot (String feedId, DataSource dataSource) {
JdbcGtfsSnapshotter snapshotter = new JdbcGtfsSnapshotter(feedId, dataSource);
FeedLoadResult result = snapshotter.copyTables();
SnapshotResult result = snapshotter.copyTables();
return result;
}

Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/conveyal/gtfs/error/SQLErrorStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ private void commit() {
*/
public void commitAndClose() {
try {
LOG.info("Committing errors and closing SQL connection.");
this.commit();
// Close the connection permanently (should be called only after errorStorage instance no longer needed).
connection.close();
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/conveyal/gtfs/loader/Feed.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ public ValidationResult validate () {
SQLErrorStorage errorStorage = null;
try {
errorStorage = new SQLErrorStorage(dataSource.getConnection(), tablePrefix, false);
} catch (SQLException ex) {
throw new StorageException(ex);
} catch (InvalidNamespaceException ex) {
} catch (SQLException | InvalidNamespaceException ex) {
throw new StorageException(ex);
}
int errorCountBeforeValidation = errorStorage.getErrorCount();
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ public FeedLoadResult exportTables() {
// TableLoadResult.
LOG.error("Exception while creating snapshot: {}", ex.toString());
ex.printStackTrace();
result.fatalException = ex.getMessage();
result.fatalException = ex.toString();
}
return result;
}
Expand Down Expand Up @@ -354,7 +354,7 @@ private TableLoadResult export (Table table, Connection connection) {
} catch (SQLException e) {
LOG.error("failed to generate select statement for existing fields");
TableLoadResult tableLoadResult = new TableLoadResult();
tableLoadResult.fatalException = e.getMessage();
tableLoadResult.fatalException = e.toString();
e.printStackTrace();
return tableLoadResult;
}
Expand Down Expand Up @@ -402,7 +402,7 @@ private TableLoadResult export (Table table, String filterSql) {
} catch (SQLException ex) {
ex.printStackTrace();
}
tableLoadResult.fatalException = e.getMessage();
tableLoadResult.fatalException = e.toString();
LOG.error("Exception while exporting tables", e);
}
return tableLoadResult;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public FeedLoadResult loadTables () {
// TODO catch exceptions separately while loading each table so load can continue, store in TableLoadResult
LOG.error("Exception while loading GTFS file: {}", ex.toString());
ex.printStackTrace();
result.fatalException = ex.getMessage();
result.fatalException = ex.toString();
}
return result;
}
Expand Down Expand Up @@ -251,7 +251,7 @@ private void registerFeed (File gtfsFile) {
connection.commit();
LOG.info("Created new feed namespace: {}", insertStatement);
} catch (Exception ex) {
LOG.error("Exception while registering new feed namespace in feeds table: {}", ex.getMessage());
LOG.error("Exception while registering new feed namespace in feeds table", ex);
DbUtils.closeQuietly(connection);
}
}
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ public JdbcGtfsSnapshotter(String feedId, DataSource dataSource) {
/**
* Copy primary entity tables as well as Pattern and PatternStops tables.
*/
public FeedLoadResult copyTables() {
public SnapshotResult copyTables() {
// This result object will be returned to the caller to summarize the feed and report any critical errors.
FeedLoadResult result = new FeedLoadResult();
SnapshotResult result = new SnapshotResult();

try {
long startTime = System.currentTimeMillis();
Expand Down Expand Up @@ -89,7 +89,7 @@ public FeedLoadResult copyTables() {
copy(Table.PATTERNS, true);
copy(Table.PATTERN_STOP, true);
// see method comments fo why different logic is needed for this table
createScheduleExceptionsTable();
result.scheduleExceptions = createScheduleExceptionsTable();
result.shapes = copy(Table.SHAPES, true);
result.stops = copy(Table.STOPS, true);
// TODO: Should we defer index creation on stop times?
Expand All @@ -106,7 +106,7 @@ public FeedLoadResult copyTables() {
// TableLoadResult.
LOG.error("Exception while creating snapshot: {}", ex.toString());
ex.printStackTrace();
result.fatalException = ex.getMessage();
result.fatalException = ex.toString();
}
return result;
}
Expand Down Expand Up @@ -145,7 +145,7 @@ private TableLoadResult copy (Table table, boolean createIndexes) {
connection.commit();
LOG.info("Done.");
} catch (Exception ex) {
tableLoadResult.fatalException = ex.getMessage();
tableLoadResult.fatalException = ex.toString();
LOG.error("Error: ", ex);
try {
connection.rollback();
Expand Down Expand Up @@ -217,6 +217,11 @@ private TableLoadResult createScheduleExceptionsTable() {
Multimap<String, String> removedServiceForDate = HashMultimap.create();
Multimap<String, String> addedServiceForDate = HashMultimap.create();
for (CalendarDate calendarDate : calendarDates) {
// Skip any null dates
if (calendarDate.date == null) {
LOG.warn("Encountered calendar date record with null value for date field. Skipping.");
continue;
}
String date = calendarDate.date.format(DateTimeFormatter.BASIC_ISO_DATE);
if (calendarDate.exception_type == 1) {
addedServiceForDate.put(date, calendarDate.service_id);
Expand Down Expand Up @@ -293,8 +298,8 @@ private TableLoadResult createScheduleExceptionsTable() {
}

connection.commit();
} catch (SQLException e) {
tableLoadResult.fatalException = e.getMessage();
} catch (Exception e) {
tableLoadResult.fatalException = e.toString();
LOG.error("Error creating schedule Exceptions: ", e);
e.printStackTrace();
try {
Expand Down Expand Up @@ -427,7 +432,7 @@ private void registerSnapshot () {
connection.commit();
LOG.info("Created new snapshot namespace: {}", insertStatement);
} catch (Exception ex) {
LOG.error("Exception while registering snapshot namespace in feeds table: {}", ex.getMessage());
LOG.error("Exception while registering snapshot namespace in feeds table", ex);
DbUtils.closeQuietly(connection);
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/conveyal/gtfs/loader/SnapshotResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.conveyal.gtfs.loader;

/**
* This contains the result of a feed snapshot operation. It is nearly identical to {@link FeedLoadResult} except that
* it has some additional tables that only exist for snapshots/editor feeds.
*/
public class SnapshotResult extends FeedLoadResult {
private static final long serialVersionUID = 1L;

public TableLoadResult scheduleExceptions;
}
11 changes: 8 additions & 3 deletions src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ public void complete(ValidationResult validationResult) {
}
}
} catch (Exception ex) {
LOG.error(ex.getMessage());
ex.printStackTrace();
LOG.error("Error validating service entries (merging calendars and calendar_dates)", ex);
// Continue on to next calendar entry.
}
}
Expand Down Expand Up @@ -181,6 +180,11 @@ select durations.service_id, duration_seconds, days_active from (
LocalDate firstDate = LocalDate.MAX;
LocalDate lastDate = LocalDate.MIN;
for (LocalDate date : dateInfoForDate.keySet()) {
// If the date is invalid, skip.
if (date == null) {
LOG.error("Encountered null date. Did something go wrong with computeIfAbsent?");
continue;
}
if (date.isBefore(firstDate)) firstDate = date;
if (date.isAfter(lastDate)) lastDate = date;
}
Expand All @@ -191,7 +195,8 @@ select durations.service_id, duration_seconds, days_active from (
validationResult.firstCalendarDate = firstDate;
validationResult.lastCalendarDate = lastDate;
// Is this any different? firstDate.until(lastDate, ChronoUnit.DAYS);
int nDays = (int) ChronoUnit.DAYS.between(firstDate, lastDate) + 1;
// If no days were found in the dateInfoForDate, nDays is a very large negative number, so we default to 0.
int nDays = Math.max(0, (int) ChronoUnit.DAYS.between(firstDate, lastDate) + 1);
validationResult.dailyBusSeconds = new int[nDays];
validationResult.dailyTramSeconds = new int[nDays];
validationResult.dailyMetroSeconds = new int[nDays];
Expand Down
Loading