From 0dd268bf27afc5efa925d06bdbfab17bfc14d832 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 8 Jul 2024 16:19:48 +0100 Subject: [PATCH] refactor(Added FLEX_OPTIONAL to allow for missing fields when updating linked fields): --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 71 +++++++++++++------ .../com/conveyal/gtfs/loader/Requirement.java | 1 + .../java/com/conveyal/gtfs/loader/Table.java | 27 ++++--- 3 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index dda7f96d8..6d023132a 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -212,7 +212,7 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce jsonObject, "trips", "route_id", - "wheelchair_accessible" + new String[]{"wheelchair_accessible"} ); break; case "patterns": @@ -221,7 +221,7 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce jsonObject, "trips", "pattern_id", - "direction_id", "shape_id" + new String[]{"direction_id", "shape_id"} ); break; default: @@ -334,7 +334,7 @@ private void updateLinkedFields( ObjectNode exemplarEntity, String linkedTableName, String keyField, - String... linkedFieldsToUpdate + String[] linkedFieldsToUpdate ) throws SQLException { boolean updatingStopTimes = "stop_times".equals(linkedTableName); // Collect fields, the JSON values for these fields, and the strings to add to the prepared statement into Lists. @@ -367,8 +367,11 @@ private void updateLinkedFields( for (int i = 0; i < fields.size(); i++) { Field field = fields.get(i); String newValue = values.get(i).isNull() ? null : values.get(i).asText(); - if (newValue == null) field.setNull(statement, oneBasedIndex++); - else field.setParameter(statement, oneBasedIndex++, newValue); + if (newValue == null) { + field.setNull(statement, oneBasedIndex++); + } else { + field.setParameter(statement, oneBasedIndex++, newValue); + } } // Set "where clause" with value for key field (e.g., set values where pattern_id = '3') statement.setString(oneBasedIndex++, exemplarEntity.get(keyField).asText()); @@ -438,7 +441,7 @@ private void setStatementParameters( // have all of the required fields, yet this would prohibit such an update. Further, an update on such // a table that DID have all of the spec table fields would fail because they might be missing from // the actual database table. - missingFieldNames.add(field.name); + updateMissingFields(missingFieldNames, field); continue; } JsonNode value = jsonObject.get(field.name); @@ -450,7 +453,7 @@ private void setStatementParameters( // 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); + updateMissingFields(missingFieldNames, field); continue; } // Set value to null if empty value is OK and update JSON. @@ -516,17 +519,27 @@ private void setStatementParameters( // Increment index for next field. index += 1; } - if (missingFieldNames.size() > 0) { + if (!missingFieldNames.isEmpty()) { throw new SQLException( String.format( "The following field(s) are missing from JSON %s object: %s", table.name, - missingFieldNames.toString() + missingFieldNames ) ); } } + /** + * If a field is an optional flex field don't add it to the list of missing field names. This then covers normal + * non-flex feeds where these fields are not defined. + */ + private void updateMissingFields(List missingFieldNames, Field field) { + if (field.requirement != Requirement.FLEX_OPTIONAL) { + missingFieldNames.add(field.name); + } + } + /** * Set field to null in prepared statement and update JSON object that is ultimately returned (see return value of * {@link #update}.) in response to reflect actual database value that will be persisted. This method should be @@ -647,17 +660,7 @@ private String updateChildTable( subEntity, "stop_times", "pattern_id", - "timepoint", - "drop_off_type", - "stop_headsign", - "pickup_type", - "continuous_pickup", - "continuous_drop_off", - "shape_dist_traveled", - "pickup_booking_rule_id", - "drop_off_booking_rule_id", - "start_pickup_drop_off_window", - "end_pickup_drop_off_window" + getLinkedFields(subEntity) ); } setStatementParameters(subEntity, subTable, insertStatement, connection); @@ -713,6 +716,34 @@ private String updateChildTable( return keyValue; } + /** + * Defined the linked fields to be updated. This will depend on the fields that are provided. Optional, editor and + * extension fields can be undefined and therefore not provided. To prevent a mismatch between provided and expected + * a linked field is only defined if the entity provides a value for it. + */ + private String[] getLinkedFields(ObjectNode entity) { + Set linkedFields = new HashSet<>(); + String[] linkedFieldsToCheck = { + "timepoint", + "drop_off_type", + "stop_headsign", + "pickup_type", + "continuous_pickup", + "continuous_drop_off", + "shape_dist_traveled", + "pickup_booking_rule_id", + "drop_off_booking_rule_id", + "start_pickup_drop_off_window", + "end_pickup_drop_off_window" + }; + for (String field : linkedFieldsToCheck) { + if (entity.get(field) != null) { + linkedFields.add(field); + } + } + return linkedFields.toArray(new String[0]); + } + /** * Check any references the sub entity might have. For example, this checks that a service_id defined in a trip * refers to a calendar or calendar date. NOTE: This skips the "specTable", i.e., for pattern stops it will not diff --git a/src/main/java/com/conveyal/gtfs/loader/Requirement.java b/src/main/java/com/conveyal/gtfs/loader/Requirement.java index 641f401f7..73dfeb74d 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Requirement.java +++ b/src/main/java/com/conveyal/gtfs/loader/Requirement.java @@ -12,6 +12,7 @@ public enum Requirement { REQUIRED, // Required by the GTFS spec OPTIONAL, // Optional according to the GTFS spec + FLEX_OPTIONAL, // Optional according to the Flex GTFS spec EXTENSION, // Extension proposed and documented on gtfs-changes PROPRIETARY, // Known proprietary extension that is not yet an official proposal UNKNOWN, // Undocumented proprietary extension diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index f4f90f5bc..18591e840 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -71,6 +71,7 @@ import static com.conveyal.gtfs.loader.JdbcGtfsLoader.sanitize; import static com.conveyal.gtfs.loader.Requirement.EDITOR; import static com.conveyal.gtfs.loader.Requirement.EXTENSION; +import static com.conveyal.gtfs.loader.Requirement.FLEX_OPTIONAL; import static com.conveyal.gtfs.loader.Requirement.OPTIONAL; import static com.conveyal.gtfs.loader.Requirement.REQUIRED; import static com.conveyal.gtfs.loader.Requirement.UNKNOWN; @@ -363,11 +364,13 @@ public Table (String name, Class entityClass, Requirement requ new ShortField("timepoint", EDITOR, 1), new ShortField("continuous_pickup", OPTIONAL,3), new ShortField("continuous_drop_off", OPTIONAL,3), - new StringField("pickup_booking_rule_id", OPTIONAL), - new StringField("drop_off_booking_rule_id", OPTIONAL), - new TimeField("start_pickup_drop_off_window", OPTIONAL), - new TimeField("end_pickup_drop_off_window", OPTIONAL) -).withParentTable(PATTERNS) + // These flex fields are defined as "flex_optional" and not "optional" to avoid the missing field exception + // when updating linked fields. + new StringField("pickup_booking_rule_id", FLEX_OPTIONAL), + new StringField("drop_off_booking_rule_id", FLEX_OPTIONAL), + new TimeField("start_pickup_drop_off_window", FLEX_OPTIONAL), + new TimeField("end_pickup_drop_off_window", FLEX_OPTIONAL) + ).withParentTable(PATTERNS) .addPrimaryKeyNames("pattern_id", "stop_sequence"); public static final Table TRIPS = new Table("trips", Trip.class, REQUIRED, @@ -612,8 +615,10 @@ private Table withParentTable(Table parentTable) { */ public List editorFields() { List editorFields = new ArrayList<>(); - for (Field f : fields) if (f.requirement == REQUIRED || f.requirement == OPTIONAL || f.requirement == EDITOR) { - editorFields.add(f); + for (Field f : fields) { + if (f.requirement == REQUIRED || f.requirement == OPTIONAL || f.requirement == FLEX_OPTIONAL || f.requirement == EDITOR) { + editorFields.add(f); + } } return editorFields; } @@ -634,7 +639,11 @@ public List requiredFields () { */ public List specFields () { List specFields = new ArrayList<>(); - for (Field f : fields) if (f.requirement == REQUIRED || f.requirement == OPTIONAL) specFields.add(f); + for (Field f : fields) { + if (f.requirement == REQUIRED || f.requirement == OPTIONAL || f.requirement == FLEX_OPTIONAL) { + specFields.add(f); + } + } return specFields; } @@ -1061,7 +1070,7 @@ public boolean isRequired () { * (e.g., Patterns or PatternStops). */ public boolean isSpecTable() { - return required == REQUIRED || required == OPTIONAL; + return required == REQUIRED || required == OPTIONAL || required == FLEX_OPTIONAL; } /**