Skip to content

Commit

Permalink
refactor(Added FLEX_OPTIONAL to allow for missing fields when updatin…
Browse files Browse the repository at this point in the history 
…g linked fields):
  • Loading branch information
@br648
br648 committed Jul 8, 2024
1 parent 458639c commit 0dd268b
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 29 deletions.
71 changes: 51 additions & 20 deletions src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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.
Expand Down Expand Up @@ -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<String> 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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<String> 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
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/conveyal/gtfs/loader/Requirement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 18 additions & 9 deletions src/main/java/com/conveyal/gtfs/loader/Table.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -363,11 +364,13 @@ public Table (String name, Class<? extends Entity> 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,
Expand Down Expand Up @@ -612,8 +615,10 @@ private Table withParentTable(Table parentTable) {
*/
public List<Field> editorFields() {
List<Field> 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;
}
Expand All @@ -634,7 +639,11 @@ public List<Field> requiredFields () {
*/
public List<Field> specFields () {
List<Field> 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;
}

Expand Down Expand Up @@ -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;
}

/**
Expand Down

0 comments on commit 0dd268b

Please sign in to comment.