diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index bc5fbac84..89674bff9 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -424,7 +424,11 @@ private void setStatementParameters( } field.setParameter(preparedStatement, index, String.join(",", values)); } else { - field.setParameter(preparedStatement, index, value.asText()); + String text = value.asText(); + // If the field is a ShortField and the string is empty, set value to null. Otherwise, set + // parameter with string. + if (field instanceof ShortField && text.isEmpty()) field.setNull(preparedStatement, index); + else field.setParameter(preparedStatement, index, text); } } } catch (StorageException e) { diff --git a/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java b/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java index 41a29273b..303f55c9a 100644 --- a/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java +++ b/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java @@ -116,13 +116,16 @@ public Set checkReferencesAndUniqueness(String keyValue, int lineN if (isOrderField) { duplicateIdError.setSequence(value); } errors.add(duplicateIdError); } - } else if (field.name.equals(keyField) && !field.isForeignReference()) { + } else if ( + field.name.equals(keyField) && + (!field.isForeignReference() || Table.CALENDAR_DATES.name.equals(table.name)) + ) { // We arrive here if the field is not a foreign reference and not the unique key field // on the table (e.g., shape_pt_sequence), but is still a key on the table. For // example, this is where we add shape_id from the shapes table, so that when we // check the referential integrity of trips#shape_id, we know that the shape_id // exists in the shapes table. It also handles tracking calendar_dates#service_id values. - this.transitIds.add(uniqueId); + listOfUniqueIds.add(uniqueId); } return errors; } diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index d744fae63..4866640d7 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -286,12 +286,12 @@ public Table (String name, Class entityClass, Requirement requ // FIXME: Do we need an index on stop_id new StringField("stop_id", REQUIRED).isReferenceTo(STOPS), // .indexThisColumn(), - // TODO verify that we have a special check for arrival and departure times first and last stop_time in a trip, which are reqiured + // TODO verify that we have a special check for arrival and departure times first and last stop_time in a trip, which are required new TimeField("arrival_time", OPTIONAL), new TimeField("departure_time", OPTIONAL), new StringField("stop_headsign", OPTIONAL), - new ShortField("pickup_type", OPTIONAL, 2), - new ShortField("drop_off_type", OPTIONAL, 2), + new ShortField("pickup_type", OPTIONAL, 3), + new ShortField("drop_off_type", OPTIONAL, 3), new DoubleField("shape_dist_traveled", OPTIONAL, 0, Double.POSITIVE_INFINITY, 2), new ShortField("timepoint", OPTIONAL, 1), new IntegerField("fare_units_traveled", EXTENSION) // OpenOV NL extension diff --git a/src/test/java/com/conveyal/gtfs/dto/FareDTO.java b/src/test/java/com/conveyal/gtfs/dto/FareDTO.java index 5aec354c3..081e385fa 100644 --- a/src/test/java/com/conveyal/gtfs/dto/FareDTO.java +++ b/src/test/java/com/conveyal/gtfs/dto/FareDTO.java @@ -13,7 +13,8 @@ public class FareDTO { public Double price; public String currency_type; public Integer payment_method; - public Integer transfers; + // transfers is a string because we need to be able to pass empty strings to the JdbcTableWriter + public String transfers; public String agency_id; public Integer transfer_duration; public FareRuleDTO[] fare_rules; diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 12649e636..5ca031da1 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -18,6 +18,8 @@ import com.conveyal.gtfs.dto.StopTimeDTO; import com.conveyal.gtfs.dto.TripDTO; import com.fasterxml.jackson.databind.ObjectMapper; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -164,7 +166,7 @@ public void canCreateUpdateAndDeleteFeedInfoEntities() throws IOException, SQLEx LOG.info("deleted {} records from {}", deleteOutput, feedInfoTable.name); // make sure record does not exist in DB - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(createdFeedInfo.id, feedInfoTable)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(createdFeedInfo.id, feedInfoTable)); } /** @@ -209,8 +211,8 @@ public void canCreateUpdateAndDeleteFares() throws IOException, SQLException, In 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; + // Empty string value or null should be permitted for transfers and transfer_duration + fareInput.transfers = ""; fareInput.transfer_duration = null; FareRuleDTO fareRuleInput = new FareRuleDTO(); // Fare ID should be assigned to "child entity" by editor automatically. @@ -235,9 +237,24 @@ public void canCreateUpdateAndDeleteFares() throws IOException, SQLException, In assertThat(createdFare.fare_id, equalTo(fareId)); assertThat(createdFare.fare_rules[0].fare_id, equalTo(fareId)); + // Ensure transfers value is null to check database integrity. + ResultSet resultSet = getResultSetForId(createdFare.id, Table.FARE_ATTRIBUTES); + while (resultSet.next()) { + // We must match against null value for transfers because the database stored value will + // not be an empty string, but null. + assertResultValue(resultSet, "transfers", Matchers.nullValue()); + assertResultValue(resultSet, "fare_id", equalTo(fareInput.fare_id)); + assertResultValue(resultSet, "currency_type", equalTo(fareInput.currency_type)); + assertResultValue(resultSet, "price", equalTo(fareInput.price)); + assertResultValue(resultSet, "agency_id", equalTo(fareInput.agency_id)); + assertResultValue(resultSet, "payment_method", equalTo(fareInput.payment_method)); + assertResultValue(resultSet, "transfer_duration", equalTo(fareInput.transfer_duration)); + } + // try to update record String updatedFareId = "3B"; createdFare.fare_id = updatedFareId; + createdFare.transfers = "0"; // covert object to json and save it JdbcTableWriter updateTableWriter = createTestTableWriter(fareTable); @@ -255,6 +272,13 @@ public void canCreateUpdateAndDeleteFares() throws IOException, SQLException, In assertThat(updatedFareDTO.fare_id, equalTo(updatedFareId)); assertThat(updatedFareDTO.fare_rules[0].fare_id, equalTo(updatedFareId)); + // Ensure transfers value is updated correctly to check database integrity. + ResultSet updatedResult = getResultSetForId(createdFare.id, Table.FARE_ATTRIBUTES); + while (updatedResult.next()) { + assertResultValue(updatedResult, "transfers", equalTo(0)); + assertResultValue(updatedResult, "fare_id", equalTo(createdFare.fare_id)); + } + // try to delete record JdbcTableWriter deleteTableWriter = createTestTableWriter(fareTable); int deleteOutput = deleteTableWriter.delete( @@ -264,10 +288,10 @@ public void canCreateUpdateAndDeleteFares() throws IOException, SQLException, In LOG.info("deleted {} records from {}", deleteOutput, fareTable.name); // make sure fare_attributes record does not exist in DB - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(createdFare.id, fareTable)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(createdFare.id, fareTable)); // make sure fare_rules record does not exist in DB - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(createdFare.fare_rules[0].id, Table.FARE_RULES)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(createdFare.fare_rules[0].id, Table.FARE_RULES)); } @Test @@ -315,7 +339,7 @@ public void canCreateUpdateAndDeleteRoutes() throws IOException, SQLException, I LOG.info("deleted {} records from {}", deleteOutput, routeTable.name); // make sure route record does not exist in DB - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(createdRoute.id, routeTable)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(createdRoute.id, routeTable)); } /** @@ -376,7 +400,7 @@ public void canCreateUpdateAndDeleteScheduleExceptions() throws IOException, SQL ); LOG.info("deleted {} records from {}", deleteOutput, scheduleExceptionTable.name); // Make sure route record does not exist in DB. - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(scheduleException.id, scheduleExceptionTable)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(scheduleException.id, scheduleExceptionTable)); } /** @@ -444,13 +468,13 @@ public void shouldDeleteReferencingTripsAndStopTimesOnPatternDelete() throws IOE JdbcTableWriter createTripWriter = createTestTableWriter(Table.TRIPS); String createdTripOutput = createTripWriter.create(mapper.writeValueAsString(tripInput), true); TripDTO createdTrip = mapper.readValue(createdTripOutput, TripDTO.class); - assertThatSqlQueryYieldsRowCount(getAllColumnsForId(createdTrip.id, Table.TRIPS), 1); + assertThatSqlQueryYieldsRowCount(getColumnsForId(createdTrip.id, Table.TRIPS), 1); // Delete pattern record JdbcTableWriter deletePatternWriter = createTestTableWriter(Table.PATTERNS); int deleteOutput = deletePatternWriter.delete(pattern.id, true); LOG.info("deleted {} records from {}", deleteOutput, Table.PATTERNS.name); // Check that pattern record does not exist in DB - assertThatSqlQueryYieldsZeroRows(getAllColumnsForId(pattern.id, Table.PATTERNS)); + assertThatSqlQueryYieldsZeroRows(getColumnsForId(pattern.id, Table.PATTERNS)); // Check that trip records for pattern do not exist in DB assertThatSqlQueryYieldsZeroRows( String.format( @@ -676,7 +700,7 @@ private static String newUUID() { private String getColumnsForId(int id, Table table, String... columns) throws SQLException { String sql = String.format( "select %s from %s.%s where id=%d", - String.join(", ", columns), + columns.length > 0 ? String.join(", ", columns) : "*", testNamespace, table.name, id @@ -685,13 +709,6 @@ private String getColumnsForId(int id, Table table, String... columns) throws SQ return sql; } - /** - * Constructs SQL query for the specified ID for all table columns and returns the resulting result set. - */ - private String getAllColumnsForId(int id, Table table) throws SQLException { - return getColumnsForId(id, table, "*"); - } - /** * Executes SQL query for the specified ID and columns and returns the resulting result set. */ @@ -700,6 +717,13 @@ private ResultSet getResultSetForId(int id, Table table, String... columns) thro return testDataSource.getConnection().prepareStatement(sql).executeQuery(); } + /** + * Asserts that a given value for the specified field in result set matches provided matcher. + */ + private void assertResultValue(ResultSet resultSet, String field, Matcher matcher) throws SQLException { + assertThat(resultSet.getObject(field), matcher); + } + private void assertThatSqlQueryYieldsRowCount(String sql, int expectedRowCount) throws SQLException { LOG.info(sql); int recordCount = 0;