From 46c2b8ecaa796e268a20d8fae95aeda7ddfab349 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 22 Jan 2020 13:07:54 -0800 Subject: [PATCH 1/4] feat(graphql): allow fetching of the priority of a validation error type --- .../gtfs/graphql/GraphQLGtfsSchema.java | 2 ++ .../fetchers/ErrorPriorityFetcher.java | 31 +++++++++++++++++++ src/test/resources/graphql/feedErrors.txt | 1 + .../GTFSGraphQLTest/canFetchErrors-0.json | 5 +++ 4 files changed, 39 insertions(+) create mode 100644 src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java diff --git a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java index f72a0ef50..0bc92d958 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java +++ b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java @@ -1,6 +1,7 @@ package com.conveyal.gtfs.graphql; import com.conveyal.gtfs.graphql.fetchers.ErrorCountFetcher; +import com.conveyal.gtfs.graphql.fetchers.ErrorPriorityFetcher; import com.conveyal.gtfs.graphql.fetchers.FeedFetcher; import com.conveyal.gtfs.graphql.fetchers.JDBCFetcher; import com.conveyal.gtfs.graphql.fetchers.MapFetcher; @@ -421,6 +422,7 @@ public class GraphQLGtfsSchema { .field(MapFetcher.field("entity_id")) .field(MapFetcher.field("entity_sequence", GraphQLInt)) .field(MapFetcher.field("bad_value")) + .field(ErrorPriorityFetcher.field("error_type_priority")) .build(); /** diff --git a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java new file mode 100644 index 000000000..9205928f0 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java @@ -0,0 +1,31 @@ +package com.conveyal.gtfs.graphql.fetchers; + +import com.conveyal.gtfs.error.NewGTFSErrorType; +import graphql.schema.DataFetcher; +import graphql.schema.DataFetchingEnvironment; +import graphql.schema.GraphQLFieldDefinition; + +import java.util.Map; + +import static graphql.Scalars.GraphQLString; +import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition; + +/** + * A fetcher that is specifically used for looking up and returning the priority of a specific error_type + */ +public class ErrorPriorityFetcher implements DataFetcher { + public static GraphQLFieldDefinition field(String name) { + return newFieldDefinition() + .name(name) + .type(GraphQLString) + .dataFetcher(new ErrorPriorityFetcher()) + .build(); + } + + @Override + public Object get(DataFetchingEnvironment dataFetchingEnvironment) { + Object source = dataFetchingEnvironment.getSource(); + String errorType = (String) ((Map)source).get("error_type"); + return NewGTFSErrorType.valueOf(errorType).priority; + } +} diff --git a/src/test/resources/graphql/feedErrors.txt b/src/test/resources/graphql/feedErrors.txt index 5c65394e8..a31a9026b 100644 --- a/src/test/resources/graphql/feedErrors.txt +++ b/src/test/resources/graphql/feedErrors.txt @@ -5,6 +5,7 @@ query($namespace: String) { bad_value error_id error_type + error_type_priority entity_id entity_sequence entity_type diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json index 4184bed16..70e3858a0 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json +++ b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json @@ -8,6 +8,7 @@ "entity_type" : "Agency", "error_id" : 0, "error_type" : "MISSING_FIELD", + "error_type_priority" : "MEDIUM", "line_number" : 2 }, { "bad_value" : "route 1", @@ -16,6 +17,7 @@ "entity_type" : "Route", "error_id" : 1, "error_type" : "ROUTE_LONG_NAME_CONTAINS_SHORT_NAME", + "error_type_priority" : "LOW", "line_number" : 2 }, { "bad_value" : null, @@ -24,6 +26,7 @@ "entity_type" : null, "error_id" : 2, "error_type" : "FEED_TRAVEL_TIMES_ROUNDED", + "error_type_priority" : "LOW", "line_number" : null }, { "bad_value" : "1234567", @@ -32,6 +35,7 @@ "entity_type" : "Stop", "error_id" : 3, "error_type" : "STOP_UNUSED", + "error_type_priority" : "MEDIUM", "line_number" : 6 }, { "bad_value" : "20170916", @@ -40,6 +44,7 @@ "entity_type" : null, "error_id" : 4, "error_type" : "DATE_NO_SERVICE", + "error_type_priority" : "MEDIUM", "line_number" : null } ], "feed_version" : "1.0" From b85a669333102788a7e3bc0d208f667ea34e4b82 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 22 Jan 2020 16:56:02 -0800 Subject: [PATCH 2/4] feat(graphql): include error priority in feed error_counts --- .../gtfs/graphql/GraphQLGtfsSchema.java | 1 + .../graphql/fetchers/ErrorCountFetcher.java | 3 +++ src/test/resources/graphql/feedErrors.txt | 6 +++++ .../GTFSGraphQLTest/canFetchErrors-0.json | 26 +++++++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java index 0bc92d958..181a80fbf 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java +++ b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java @@ -459,6 +459,7 @@ public class GraphQLGtfsSchema { .field(string("type")) .field(intt("count")) .field(string("message")) + .field(string("priority")) .build(); /** diff --git a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java index 98f43551f..b0ce06798 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java +++ b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java @@ -2,6 +2,7 @@ import com.conveyal.gtfs.error.NewGTFSErrorType; import com.conveyal.gtfs.graphql.GTFSGraphQL; +import com.conveyal.gtfs.validator.model.Priority; import graphql.schema.DataFetcher; import graphql.schema.DataFetchingEnvironment; import org.apache.commons.dbutils.DbUtils; @@ -61,11 +62,13 @@ public static class ErrorCount { public NewGTFSErrorType type; public int count; public String message; + public Priority priority; public ErrorCount(NewGTFSErrorType errorType, int count) { this.type = errorType; this.count = count; this.message = errorType.englishMessage; + this.priority = errorType.priority; } } diff --git a/src/test/resources/graphql/feedErrors.txt b/src/test/resources/graphql/feedErrors.txt index a31a9026b..6a923e35f 100644 --- a/src/test/resources/graphql/feedErrors.txt +++ b/src/test/resources/graphql/feedErrors.txt @@ -1,6 +1,12 @@ query($namespace: String) { feed(namespace: $namespace) { feed_version + error_counts { + count + message + priority + type + } errors { bad_value error_id diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json index 70e3858a0..d496ab3a7 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json +++ b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json @@ -1,6 +1,32 @@ { "data" : { "feed" : { + "error_counts" : [ { + "count" : 1, + "message" : "This stop is not referenced by any trips.", + "priority" : "MEDIUM", + "type" : "STOP_UNUSED" + }, { + "count" : 1, + "message" : "A required field was missing or empty in a particular row.", + "priority" : "MEDIUM", + "type" : "MISSING_FIELD" + }, { + "count" : 1, + "message" : "No service_ids were active on a date within the range of dates with defined service.", + "priority" : "MEDIUM", + "type" : "DATE_NO_SERVICE" + }, { + "count" : 1, + "message" : "The long name of a route should complement the short name, not include it.", + "priority" : "LOW", + "type" : "ROUTE_LONG_NAME_CONTAINS_SHORT_NAME" + }, { + "count" : 1, + "message" : "All travel times in the feed are rounded to the minute, which may cause unexpected results in routing applications where travel times are zero.", + "priority" : "LOW", + "type" : "FEED_TRAVEL_TIMES_ROUNDED" + } ], "errors" : [ { "bad_value" : "agency_url", "entity_id" : null, From ddd59281b28fcec7eabd55c68643cf281336be00 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 22 Jan 2020 23:00:10 -0800 Subject: [PATCH 3/4] test(graphql): order error counts for consistent snapshotting --- .../graphql/fetchers/ErrorCountFetcher.java | 7 ++++++- .../GTFSGraphQLTest/canFetchErrors-0.json | 20 +++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java index b0ce06798..c72ac658b 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java +++ b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorCountFetcher.java @@ -42,7 +42,12 @@ public Object get(DataFetchingEnvironment environment) { try { connection = GTFSGraphQL.getConnection(); Statement statement = connection.createStatement(); - String sql = String.format("select error_type, count(*) from %s.errors group by error_type", namespace); + String sql = String.format( + // this order_by is only needed to make sure that the testing snapshots are consistently in the same + // order during every test + "select error_type, count(*) from %s.errors group by error_type order by error_type", + namespace + ); LOG.info("SQL: {}", sql); if (statement.execute(sql)) { ResultSet resultSet = statement.getResultSet(); diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json index d496ab3a7..ee032da32 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json +++ b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json @@ -3,19 +3,19 @@ "feed" : { "error_counts" : [ { "count" : 1, - "message" : "This stop is not referenced by any trips.", + "message" : "No service_ids were active on a date within the range of dates with defined service.", "priority" : "MEDIUM", - "type" : "STOP_UNUSED" + "type" : "DATE_NO_SERVICE" }, { "count" : 1, - "message" : "A required field was missing or empty in a particular row.", - "priority" : "MEDIUM", - "type" : "MISSING_FIELD" + "message" : "All travel times in the feed are rounded to the minute, which may cause unexpected results in routing applications where travel times are zero.", + "priority" : "LOW", + "type" : "FEED_TRAVEL_TIMES_ROUNDED" }, { "count" : 1, - "message" : "No service_ids were active on a date within the range of dates with defined service.", + "message" : "A required field was missing or empty in a particular row.", "priority" : "MEDIUM", - "type" : "DATE_NO_SERVICE" + "type" : "MISSING_FIELD" }, { "count" : 1, "message" : "The long name of a route should complement the short name, not include it.", @@ -23,9 +23,9 @@ "type" : "ROUTE_LONG_NAME_CONTAINS_SHORT_NAME" }, { "count" : 1, - "message" : "All travel times in the feed are rounded to the minute, which may cause unexpected results in routing applications where travel times are zero.", - "priority" : "LOW", - "type" : "FEED_TRAVEL_TIMES_ROUNDED" + "message" : "This stop is not referenced by any trips.", + "priority" : "MEDIUM", + "type" : "STOP_UNUSED" } ], "errors" : [ { "bad_value" : "agency_url", From 77790978723f038d18e8ae823de9ae804e146f73 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 27 Jan 2020 11:47:36 -0800 Subject: [PATCH 4/4] refactor(graphql): remove validation error priority field and fetcher --- .../gtfs/graphql/GraphQLGtfsSchema.java | 2 -- .../fetchers/ErrorPriorityFetcher.java | 31 ------------------- src/test/resources/graphql/feedErrors.txt | 1 - .../GTFSGraphQLTest/canFetchErrors-0.json | 5 --- 4 files changed, 39 deletions(-) delete mode 100644 src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java diff --git a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java index 181a80fbf..4bde76b85 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java +++ b/src/main/java/com/conveyal/gtfs/graphql/GraphQLGtfsSchema.java @@ -1,7 +1,6 @@ package com.conveyal.gtfs.graphql; import com.conveyal.gtfs.graphql.fetchers.ErrorCountFetcher; -import com.conveyal.gtfs.graphql.fetchers.ErrorPriorityFetcher; import com.conveyal.gtfs.graphql.fetchers.FeedFetcher; import com.conveyal.gtfs.graphql.fetchers.JDBCFetcher; import com.conveyal.gtfs.graphql.fetchers.MapFetcher; @@ -422,7 +421,6 @@ public class GraphQLGtfsSchema { .field(MapFetcher.field("entity_id")) .field(MapFetcher.field("entity_sequence", GraphQLInt)) .field(MapFetcher.field("bad_value")) - .field(ErrorPriorityFetcher.field("error_type_priority")) .build(); /** diff --git a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java b/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java deleted file mode 100644 index 9205928f0..000000000 --- a/src/main/java/com/conveyal/gtfs/graphql/fetchers/ErrorPriorityFetcher.java +++ /dev/null @@ -1,31 +0,0 @@ -package com.conveyal.gtfs.graphql.fetchers; - -import com.conveyal.gtfs.error.NewGTFSErrorType; -import graphql.schema.DataFetcher; -import graphql.schema.DataFetchingEnvironment; -import graphql.schema.GraphQLFieldDefinition; - -import java.util.Map; - -import static graphql.Scalars.GraphQLString; -import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition; - -/** - * A fetcher that is specifically used for looking up and returning the priority of a specific error_type - */ -public class ErrorPriorityFetcher implements DataFetcher { - public static GraphQLFieldDefinition field(String name) { - return newFieldDefinition() - .name(name) - .type(GraphQLString) - .dataFetcher(new ErrorPriorityFetcher()) - .build(); - } - - @Override - public Object get(DataFetchingEnvironment dataFetchingEnvironment) { - Object source = dataFetchingEnvironment.getSource(); - String errorType = (String) ((Map)source).get("error_type"); - return NewGTFSErrorType.valueOf(errorType).priority; - } -} diff --git a/src/test/resources/graphql/feedErrors.txt b/src/test/resources/graphql/feedErrors.txt index 6a923e35f..9487d7bf6 100644 --- a/src/test/resources/graphql/feedErrors.txt +++ b/src/test/resources/graphql/feedErrors.txt @@ -11,7 +11,6 @@ query($namespace: String) { bad_value error_id error_type - error_type_priority entity_id entity_sequence entity_type diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json index ee032da32..d49e48ca4 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json +++ b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json @@ -34,7 +34,6 @@ "entity_type" : "Agency", "error_id" : 0, "error_type" : "MISSING_FIELD", - "error_type_priority" : "MEDIUM", "line_number" : 2 }, { "bad_value" : "route 1", @@ -43,7 +42,6 @@ "entity_type" : "Route", "error_id" : 1, "error_type" : "ROUTE_LONG_NAME_CONTAINS_SHORT_NAME", - "error_type_priority" : "LOW", "line_number" : 2 }, { "bad_value" : null, @@ -52,7 +50,6 @@ "entity_type" : null, "error_id" : 2, "error_type" : "FEED_TRAVEL_TIMES_ROUNDED", - "error_type_priority" : "LOW", "line_number" : null }, { "bad_value" : "1234567", @@ -61,7 +58,6 @@ "entity_type" : "Stop", "error_id" : 3, "error_type" : "STOP_UNUSED", - "error_type_priority" : "MEDIUM", "line_number" : 6 }, { "bad_value" : "20170916", @@ -70,7 +66,6 @@ "entity_type" : null, "error_id" : 4, "error_type" : "DATE_NO_SERVICE", - "error_type_priority" : "MEDIUM", "line_number" : null } ], "feed_version" : "1.0"