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

Bug fix release #175

Merged
merged 33 commits into from
Jan 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
87faf8b
fix(load): skip bad (null) dates when validating services
landonreed Sep 26, 2018
d673e36
test: add test with bad calendar dates
landonreed Nov 8, 2018
b552fb2
Merge branch 'dev' into fix-bad-date-npe
landonreed Nov 8, 2018
cd2627e
test: fix class name misspelling
landonreed Nov 8, 2018
77d29c7
Merge branch 'dev' into fix-bad-date-npe
landonreed Nov 9, 2018
62eb74e
test(gtfs): improve SQL GTFS integration tests
landonreed Nov 9, 2018
a3623fb
test: fix bad method signature
landonreed Nov 12, 2018
c874bc2
test: handle null date on snapshot
landonreed Nov 12, 2018
a4d43a2
fix(shape-edits): update trip#shape_id if pattern value changes
landonreed Dec 18, 2018
07e0e29
style: add new lines to reduce line width
landonreed Dec 18, 2018
f773972
fix(editor): update stop_time travel times when frequency pattern is …
landonreed Dec 18, 2018
903aebd
refactor(editor): fix NPE
landonreed Dec 18, 2018
a9ad0f6
refactor: change method arg from Integer -> int
landonreed Dec 18, 2018
b96299a
feat(validator): validate stop_time#shape_dist_traveled for increasin…
landonreed Dec 19, 2018
e07f019
fix(frequencies): increase headway seconds max to 6 hours
landonreed Dec 19, 2018
57566ec
feat(validator): validate stop names and trip headsigns
landonreed Dec 19, 2018
040a9f8
test(JdbcTableWriter): add CRUD tests for trips and basic create test…
landonreed Jan 3, 2019
667d3fc
refactor: address PR comments about inlining string literals
landonreed Jan 3, 2019
55f2d87
refactor(writer): fix check for useFrequency when updating patterns o…
landonreed Jan 3, 2019
6e9042f
test(JdbcTableWriter): improve writer tests for frequencies
landonreed Jan 3, 2019
0fcf662
refactor: fix comment language from bad copypasta
landonreed Jan 3, 2019
67a5d56
test: refactor vague assertion into countValidationErrorsOfType
landonreed Jan 3, 2019
71b5536
Merge branch 'dev' into fix-bad-date-npe
landonreed Jan 3, 2019
0311a68
test(writer): fix failure message in assertion
landonreed Jan 4, 2019
44bdd4b
Merge pull request #143 from conveyal/fix-bad-date-npe
Jan 4, 2019
37fb468
test(writer): rename frequency trip crud test
landonreed Jan 4, 2019
be32a87
Merge pull request #169 from conveyal/improve-name-validator
Jan 4, 2019
7062076
Merge pull request #164 from conveyal/update-trip-shape_id
Jan 4, 2019
f9839e1
Merge pull request #166 from conveyal/validate-stop_time-shape_dist_t…
Jan 4, 2019
7cbaf3d
Merge pull request #168 from conveyal/increase-max-headway-secs
Jan 4, 2019
5fc8bd3
ci(codecov): run codecov after semantic-release
evansiroky Jan 4, 2019
6d3997e
ci(codecov): add comment explaining placement of codecov
evansiroky Jan 4, 2019
444b3ce
Merge pull request #176 from conveyal/codecov-fix
Jan 4, 2019
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
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,7 @@ notifications:

# Push results to codecov.io and run semantic-release (releases only created on pushes to the master branch).
after_success:
- bash <(curl -s https://codecov.io/bash)
- semantic-release --prepare @conveyal/maven-semantic-release --publish @semantic-release/github,@conveyal/maven-semantic-release --verify-conditions @semantic-release/github,@conveyal/maven-semantic-release --verify-release @conveyal/maven-semantic-release --use-conveyal-workflow --dev-branch=dev
# run codecov after semantic-release because maven-semantic-release creates extra commits that
# codecov will need a report on to reference in future PRs to the release branch
- bash <(curl -s https://codecov.io/bash)
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
4 changes: 4 additions & 0 deletions src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@ public enum NewGTFSErrorType {
SERVICE_NEVER_ACTIVE(Priority.MEDIUM, "A service code was defined, but is never active on any date."),
SERVICE_UNUSED(Priority.MEDIUM, "A service code was defined, but is never referenced by any trips."),
SHAPE_DIST_TRAVELED_NOT_INCREASING(Priority.MEDIUM, "Shape distance traveled must increase with stop times."),
STOP_DESCRIPTION_SAME_AS_NAME(Priority.LOW, "The description of a stop is identical to its name, so does not add any information."),
STOP_LOW_POPULATION_DENSITY(Priority.HIGH, "A stop is located in a geographic area with very low human population density."),
STOP_NAME_MISSING(Priority.MEDIUM, "A stop does not have a name."),
STOP_GEOGRAPHIC_OUTLIER(Priority.HIGH, "This stop is located very far from the middle 90% of stops in this feed."),
STOP_UNUSED(Priority.MEDIUM, "This stop is not referenced by any trips."),
TRIP_EMPTY(Priority.HIGH, "This trip is defined but has no stop times."),
TRIP_HEADSIGN_CONTAINS_ROUTE_NAME(Priority.LOW, "A trip headsign contains the route name, but should only contain information to distinguish it from other trips for the route."),
TRIP_HEADSIGN_SHOULD_DESCRIBE_DESTINATION_OR_WAYPOINTS(Priority.LOW, "A trip headsign begins with 'to' or 'towards', but should begin with destination or direction and optionally include waypoints with 'via'"),
TRIP_NEVER_ACTIVE(Priority.MEDIUM, "A trip is defined, but its service is never running on any date."),
ROUTE_UNUSED(Priority.HIGH, "This route is defined but has no trips."),
TRAVEL_DISTANCE_ZERO(Priority.MEDIUM, "The vehicle does not cover any distance between the last stop and this one."),
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
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ public class GraphQLGtfsSchema {
.field(MapFetcher.field("shape_dist_traveled", GraphQLFloat))
.field(MapFetcher.field("drop_off_type", GraphQLInt))
.field(MapFetcher.field("pickup_type", GraphQLInt))
.field(MapFetcher.field("stop_sequence", GraphQLInt))
.field(MapFetcher.field("timepoint", GraphQLInt))
// FIXME: This will only returns a list with one stop entity (unless there is a referential integrity issue)
// Should this be modified to be an object, rather than a list?
Expand All @@ -390,7 +391,6 @@ public class GraphQLGtfsSchema {
.dataFetcher(new JDBCFetcher("stops", "stop_id"))
.build()
)
.field(MapFetcher.field("stop_sequence", GraphQLInt))
.build();

/**
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
Loading