diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java index d09a9a7ff..2229968ec 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java @@ -176,8 +176,7 @@ private TableLoadResult copy (Table table, boolean createIndexes) { */ private TableLoadResult createScheduleExceptionsTable() { // check to see if the schedule_exceptions table exists - boolean scheduleExceptionsTableExists = feedIdToSnapshot != null && - tableExists(feedIdToSnapshot, "schedule_exceptions"); + boolean scheduleExceptionsTableExists = tableExists(feedIdToSnapshot, "schedule_exceptions"); String scheduleExceptionsTableName = tablePrefix + "schedule_exceptions"; if (scheduleExceptionsTableExists) { @@ -271,7 +270,11 @@ private TableLoadResult createScheduleExceptionsTable() { // determine if we appear to be working with a calendar_dates-only feed. // If so, we must also add dummy entries to the calendar table - if (!tableExists(feedIdToSnapshot, "calendar") && calendarDatesReader.getRowCount() > 0) { + if ( + feedIdToSnapshot != null && + !tableExists(feedIdToSnapshot, "calendar") && + calendarDatesReader.getRowCount() > 0 + ) { sql = String.format( "insert into %s (service_id, description, start_date, end_date, " + "monday, tuesday, wednesday, thursday, friday, saturday, sunday)" + @@ -322,6 +325,8 @@ private TableLoadResult createScheduleExceptionsTable() { * Helper method to determine if a table exists within a namespace. */ private boolean tableExists(String namespace, String tableName) { + // Preempt SQL check with null check of either namespace or table name. + if (namespace == null || tableName == null) return false; try { // This statement is postgres-specific. PreparedStatement tableExistsStatement = connection.prepareStatement( diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index f9bfefbf9..94f77418d 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -292,10 +292,13 @@ private void setStatementParameters(ObjectNode jsonObject, Table table, Prepared continue; } JsonNode value = jsonObject.get(field.name); -// LOG.info("{}={}", field.name, value); + LOG.debug("{}={}", field.name, value); try { if (value == null || value.isNull()) { - if (field.isRequired()) { + if (field.isRequired() && !field.isEmptyValuePermitted()) { + // Only register the field as missing if the value is null, the field is required, and empty + // values are not permitted. For example, a null value for fare_attributes#transfers should not + // trigger a missing field exception. missingFieldNames.add(field.name); continue; } @@ -318,7 +321,7 @@ private void setStatementParameters(ObjectNode jsonObject, Table table, Prepared // FIXME: This is a hack to get arrival and departure time into the right format. Because the UI // currently returns them as seconds since midnight rather than the Field-defined format HH:MM:SS. try { - if (value == null ||value.isNull()) { + if (value == null || value.isNull()) { if (field.isRequired()) { missingFieldNames.add(field.name); continue; @@ -421,7 +424,11 @@ private void updateChildTable(ArrayNode subEntities, Integer id, boolean isCreat // field statement above. for (Field field : subTable.specFields()) { if (field.referenceTable != null && !field.referenceTable.name.equals(specTable.name)) { - referencesPerTable.put(field.referenceTable, subEntity.get(field.name).asText()); + JsonNode refValueNode = subEntity.get(field.name); + // Skip over references that are null but not required (e.g., route_id in fare_rules). + if (refValueNode.isNull() && !field.isRequired()) continue; + String refValue = refValueNode.asText(); + referencesPerTable.put(field.referenceTable, refValue); } } // Insert new sub-entity. diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index a8c94a6ee..47ab05086 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -33,7 +33,7 @@ import static org.junit.Assert.assertThat; /** - * A test suite for the GTFS Class + * A test suite for the {@link GTFS} Class. */ public class GTFSTest { private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); @@ -570,7 +570,9 @@ private void assertThatPersistenceExpectationRecordWasFound(int numRecordsSearch ); } - // a helper class to verify that data got stored in a particular table + /** + * A helper class to verify that data got stored in a particular table. + */ private class PersistenceExpectation { public String tableName; // each persistence expectation has an array of record expectations which all must reference a single row @@ -590,7 +592,9 @@ private enum ExpectedFieldType { DOUBLE, STRING } - // a helper class to verify that data got stored in a particular record + /** + * A helper class to verify that data got stored in a particular record. + */ private class RecordExpectation { public double acceptedDelta; public double doubleExpectation; diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index a713c4e66..44a418b4a 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -1,8 +1,11 @@ package com.conveyal.gtfs.loader; import com.conveyal.gtfs.TestUtils; +import com.conveyal.gtfs.util.FareDTO; +import com.conveyal.gtfs.util.FareRuleDTO; import com.conveyal.gtfs.util.FeedInfoDTO; import com.conveyal.gtfs.util.InvalidNamespaceException; +import com.conveyal.gtfs.util.RouteDTO; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -28,6 +31,11 @@ public class JDBCTableWriterTest { private static String testDBName; private static DataSource testDataSource; private static String testNamespace; + private static final ObjectMapper mapper = new ObjectMapper(); + + private static JdbcTableWriter createTestTableWriter (Table table) throws InvalidNamespaceException { + return new JdbcTableWriter(table, testDataSource, testNamespace); + } @BeforeClass public static void setUpClass() throws SQLException { @@ -35,7 +43,7 @@ public static void setUpClass() throws SQLException { testDBName = TestUtils.generateNewDB(); String dbConnectionUrl = String.format("jdbc:postgresql://localhost/%s", testDBName); testDataSource = createDataSource (dbConnectionUrl, null, null); - LOG.info("creating feeds table because it isn't automtically generated unless you import a feed"); + LOG.info("creating feeds table because it isn't automatically generated unless you import a feed"); Connection connection = testDataSource.getConnection(); connection.createStatement() .execute("create table if not exists feeds (namespace varchar primary key, md5 varchar, " + @@ -50,8 +58,10 @@ public static void setUpClass() throws SQLException { } @Test - public void canCreateUpdateAndDeleteFeedinfoEntities () throws IOException, SQLException, InvalidNamespaceException { - ObjectMapper mapper = new ObjectMapper(); + public void canCreateUpdateAndDeleteFeedInfoEntities() throws IOException, SQLException, InvalidNamespaceException { + // Store Table and Class values for use in test. + final Table feedInfoTable = Table.FEED_INFO; + final Class feedInfoDTOClass = FeedInfoDTO.class; // create new object to be saved FeedInfoDTO feedInfoInput = new FeedInfoDTO(); @@ -63,13 +73,13 @@ public void canCreateUpdateAndDeleteFeedinfoEntities () throws IOException, SQLE feedInfoInput.default_route_type = "3"; // convert object to json and save it - JdbcTableWriter createTableWriter = new JdbcTableWriter(Table.FEED_INFO, testDataSource, testNamespace); + JdbcTableWriter createTableWriter = createTestTableWriter(feedInfoTable); String createOutput = createTableWriter.create(mapper.writeValueAsString(feedInfoInput), true); - LOG.info("create output:"); + LOG.info("create {} output:", feedInfoTable.name); LOG.info(createOutput); // parse output - FeedInfoDTO createdFeedInfo = mapper.readValue(createOutput, FeedInfoDTO.class); + FeedInfoDTO createdFeedInfo = mapper.readValue(createOutput, feedInfoDTOClass); // make sure saved data matches expected data assertThat(createdFeedInfo.feed_publisher_name, equalTo(publisherName)); @@ -79,45 +89,39 @@ public void canCreateUpdateAndDeleteFeedinfoEntities () throws IOException, SQLE createdFeedInfo.feed_publisher_name = updatedPublisherName; // covert object to json and save it - JdbcTableWriter updateTableWriter = new JdbcTableWriter(Table.FEED_INFO, testDataSource, testNamespace); + JdbcTableWriter updateTableWriter = createTestTableWriter(feedInfoTable); String updateOutput = updateTableWriter.update( createdFeedInfo.id, mapper.writeValueAsString(createdFeedInfo), true ); - LOG.info("update output:"); + LOG.info("update {} output:", feedInfoTable.name); LOG.info(updateOutput); - FeedInfoDTO updatedFeedInfoDTO = mapper.readValue(updateOutput, FeedInfoDTO.class); + FeedInfoDTO updatedFeedInfoDTO = mapper.readValue(updateOutput, feedInfoDTOClass); // make sure saved data matches expected data assertThat(updatedFeedInfoDTO.feed_publisher_name, equalTo(updatedPublisherName)); // try to delete record - JdbcTableWriter deleteTableWriter = new JdbcTableWriter(Table.FEED_INFO, testDataSource, testNamespace); + JdbcTableWriter deleteTableWriter = createTestTableWriter(feedInfoTable); int deleteOutput = deleteTableWriter.delete( createdFeedInfo.id, true ); - LOG.info("delete output:"); - LOG.info(updateOutput); + LOG.info("deleted {} records from {}", deleteOutput, feedInfoTable.name); // make sure record does not exist in DB - String sql = String.format( + assertThatSqlQueryYieldsZeroRows(String.format( "select * from %s.%s where id=%d", testNamespace, - Table.FEED_INFO.name, + feedInfoTable.name, createdFeedInfo.id - ); - LOG.info(sql); - ResultSet rs = testDataSource.getConnection().prepareStatement(sql).executeQuery(); - assertThat(rs.getFetchSize(), equalTo(0)); + )); } @Test public void canPreventSQLInjection() throws IOException, SQLException, InvalidNamespaceException { - ObjectMapper mapper = new ObjectMapper(); - // create new object to be saved FeedInfoDTO feedInfoInput = new FeedInfoDTO(); String publisherName = "' OR 1 = 1; SELECT '1"; @@ -128,7 +132,7 @@ public void canPreventSQLInjection() throws IOException, SQLException, InvalidNa feedInfoInput.default_route_type = "3"; // convert object to json and save it - JdbcTableWriter createTableWriter = new JdbcTableWriter(Table.FEED_INFO, testDataSource, testNamespace); + JdbcTableWriter createTableWriter = createTestTableWriter(Table.FEED_INFO); String createOutput = createTableWriter.create(mapper.writeValueAsString(feedInfoInput), true); LOG.info("create output:"); LOG.info(createOutput); @@ -140,6 +144,169 @@ public void canPreventSQLInjection() throws IOException, SQLException, InvalidNa assertThat(createdFeedInfo.feed_publisher_name, equalTo(publisherName)); } + @Test + public void canCreateUpdateAndDeleteFares() throws IOException, SQLException, InvalidNamespaceException { + // Store Table and Class values for use in test. + final Table fareTable = Table.FARE_ATTRIBUTES; + final Class fareDTOClass = FareDTO.class; + + // create new object to be saved + FareDTO fareInput = new FareDTO(); + String fareId = "2A"; + fareInput.fare_id = fareId; + fareInput.currency_type = "USD"; + fareInput.price = 2.50; + fareInput.agency_id = "RTA"; + fareInput.payment_method = 0; + // Empty value should be permitted for transfers and transfer_duration + fareInput.transfers = null; + fareInput.transfer_duration = null; + FareRuleDTO fareRuleInput = new FareRuleDTO(); + // Fare ID should be assigned to "child entity" by editor automatically. + fareRuleInput.fare_id = null; + fareRuleInput.route_id = null; + // FIXME There is currently no check for valid zone_id values in contains_id, origin_id, and destination_id. + fareRuleInput.contains_id = "any"; + fareRuleInput.origin_id = "value"; + fareRuleInput.destination_id = "permitted"; + fareInput.fare_rules = new FareRuleDTO[]{fareRuleInput}; + + // convert object to json and save it + JdbcTableWriter createTableWriter = createTestTableWriter(fareTable); + String createOutput = createTableWriter.create(mapper.writeValueAsString(fareInput), true); + LOG.info("create {} output:", fareTable.name); + LOG.info(createOutput); + + // parse output + FareDTO createdFare = mapper.readValue(createOutput, fareDTOClass); + + // make sure saved data matches expected data + assertThat(createdFare.fare_id, equalTo(fareId)); + assertThat(createdFare.fare_rules[0].fare_id, equalTo(fareId)); + + // try to update record + String updatedFareId = "3B"; + createdFare.fare_id = updatedFareId; + + // covert object to json and save it + JdbcTableWriter updateTableWriter = createTestTableWriter(fareTable); + String updateOutput = updateTableWriter.update( + createdFare.id, + mapper.writeValueAsString(createdFare), + true + ); + LOG.info("update {} output:", fareTable.name); + LOG.info(updateOutput); + + FareDTO updatedFareDTO = mapper.readValue(updateOutput, fareDTOClass); + + // make sure saved data matches expected data + assertThat(updatedFareDTO.fare_id, equalTo(updatedFareId)); + assertThat(updatedFareDTO.fare_rules[0].fare_id, equalTo(updatedFareId)); + + // try to delete record + JdbcTableWriter deleteTableWriter = createTestTableWriter(fareTable); + int deleteOutput = deleteTableWriter.delete( + createdFare.id, + true + ); + LOG.info("deleted {} records from {}", deleteOutput, fareTable.name); + + // make sure fare_attributes record does not exist in DB + assertThatSqlQueryYieldsZeroRows(String.format( + "select * from %s.%s where id=%d", + testNamespace, + fareTable.name, + createdFare.id + )); + + // make sure fare_rules record does not exist in DB + assertThatSqlQueryYieldsZeroRows(String.format( + "select * from %s.%s where id=%d", + testNamespace, + Table.FARE_RULES.name, + createdFare.fare_rules[0].id + )); + } + + private void assertThatSqlQueryYieldsZeroRows(String sql) throws SQLException { + assertThatSqlQueryYieldsRowCount(sql, 0); + } + + private void assertThatSqlQueryYieldsRowCount(String sql, int expectedRowCount) throws SQLException { + LOG.info(sql); + ResultSet resultSet = testDataSource.getConnection().prepareStatement(sql).executeQuery(); + assertThat(resultSet.getFetchSize(), equalTo(expectedRowCount)); + } + + @Test + public void canCreateUpdateAndDeleteRoutes() throws IOException, SQLException, InvalidNamespaceException { + // Store Table and Class values for use in test. + final Table routeTable = Table.ROUTES; + final Class routeDTOClass = RouteDTO.class; + + // create new object to be saved + RouteDTO routeInput = new RouteDTO(); + String routeId = "500"; + routeInput.route_id = routeId; + routeInput.agency_id = "RTA"; + // Empty value should be permitted for transfers and transfer_duration + routeInput.route_short_name = "500"; + routeInput.route_long_name = "Hollingsworth"; + routeInput.route_type = 3; + + // convert object to json and save it + JdbcTableWriter createTableWriter = createTestTableWriter(routeTable); + String createOutput = createTableWriter.create(mapper.writeValueAsString(routeInput), true); + LOG.info("create {} output:", routeTable.name); + LOG.info(createOutput); + + // parse output + RouteDTO createdRoute = mapper.readValue(createOutput, routeDTOClass); + + // make sure saved data matches expected data + assertThat(createdRoute.route_id, equalTo(routeId)); + // TODO: Verify with a SQL query that the database now contains the created data (we may need to use the same + // db connection to do this successfully?) + + // try to update record + String updatedRouteId = "600"; + createdRoute.route_id = updatedRouteId; + + // covert object to json and save it + JdbcTableWriter updateTableWriter = createTestTableWriter(routeTable); + String updateOutput = updateTableWriter.update( + createdRoute.id, + mapper.writeValueAsString(createdRoute), + true + ); + LOG.info("update {} output:", routeTable.name); + LOG.info(updateOutput); + + RouteDTO updatedRouteDTO = mapper.readValue(updateOutput, routeDTOClass); + + // make sure saved data matches expected data + assertThat(updatedRouteDTO.route_id, equalTo(updatedRouteId)); + // TODO: Verify with a SQL query that the database now contains the updated data (we may need to use the same + // db connection to do this successfully?) + + // try to delete record + JdbcTableWriter deleteTableWriter = createTestTableWriter(routeTable); + int deleteOutput = deleteTableWriter.delete( + createdRoute.id, + true + ); + LOG.info("deleted {} records from {}", deleteOutput, routeTable.name); + + // make sure route record does not exist in DB + assertThatSqlQueryYieldsZeroRows(String.format( + "select * from %s.%s where id=%d", + testNamespace, + routeTable.name, + createdRoute.id + )); + } + @AfterClass public static void tearDownClass() { TestUtils.dropDB(testDBName); diff --git a/src/test/java/com/conveyal/gtfs/util/FareDTO.java b/src/test/java/com/conveyal/gtfs/util/FareDTO.java new file mode 100644 index 000000000..889e328e3 --- /dev/null +++ b/src/test/java/com/conveyal/gtfs/util/FareDTO.java @@ -0,0 +1,20 @@ +package com.conveyal.gtfs.util; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * DTO used to model expected {@link com.conveyal.gtfs.model.Fare} JSON structure. NOTE: reference types (e.g., Integer + * and Double) are used here in order to model null/empty values in JSON object. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class FareDTO { + public int id; + public String fare_id; + public Double price; + public String currency_type; + public Integer payment_method; + public Integer transfers; + public String agency_id; + public Integer transfer_duration; + public FareRuleDTO[] fare_rules; +} diff --git a/src/test/java/com/conveyal/gtfs/util/FareRuleDTO.java b/src/test/java/com/conveyal/gtfs/util/FareRuleDTO.java new file mode 100644 index 000000000..a5fb475d1 --- /dev/null +++ b/src/test/java/com/conveyal/gtfs/util/FareRuleDTO.java @@ -0,0 +1,16 @@ +package com.conveyal.gtfs.util; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * DTO used to model expected {@link com.conveyal.gtfs.model.FareRule} JSON structure. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class FareRuleDTO { + public int id; + public String fare_id; + public String route_id; + public String contains_id; + public String origin_id; + public String destination_id; +} diff --git a/src/test/java/com/conveyal/gtfs/util/RouteDTO.java b/src/test/java/com/conveyal/gtfs/util/RouteDTO.java new file mode 100644 index 000000000..c5868f592 --- /dev/null +++ b/src/test/java/com/conveyal/gtfs/util/RouteDTO.java @@ -0,0 +1,26 @@ +package com.conveyal.gtfs.util; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * DTO used to model expected {@link com.conveyal.gtfs.model.Route} JSON structure for the editor. NOTE: reference types + * (e.g., Integer and Double) are used here in order to model null/empty values in JSON object. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class RouteDTO { + public int id; + public String route_id; + public String agency_id; + public String route_short_name; + public String route_long_name; + public String route_desc; + public Integer route_type; + public String route_url; + public String route_branding_url; + public String route_color; + public String route_text_color; + public Integer publicly_visible; + public Integer wheelchair_accessible; + public Integer route_sort_order; + public Integer status; +}