diff --git a/pom.xml b/pom.xml index 41ab42ff8..bec80e21d 100644 --- a/pom.xml +++ b/pom.xml @@ -272,7 +272,7 @@ com.github.conveyal gtfs-lib - 10307d43c6 + 9bc752d3ef diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarDatesMergeLineContext.java b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarDatesMergeLineContext.java index 649e0c088..d65548e23 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarDatesMergeLineContext.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarDatesMergeLineContext.java @@ -87,6 +87,7 @@ private boolean checkCalendarDatesIds(FieldContext fieldContext) throws IOExcept // the valid date range, i.e., before the future feed's first date. if (!shouldSkipRecord && fieldContext.nameEquals(SERVICE_ID)) { mergeFeedsResult.serviceIds.add(fieldContext.getValueToWrite()); + mergeFeedsResult.calendarDatesServiceIds.add(fieldContext.getValueToWrite()); } return !shouldSkipRecord; diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarMergeLineContext.java b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarMergeLineContext.java index ab8902531..1f6cd27aa 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarMergeLineContext.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/CalendarMergeLineContext.java @@ -111,6 +111,7 @@ private boolean checkCalendarIds(Set idErrors, FieldContext fieldC // If service is going to be cloned, add to the output service ids. if (!shouldSkipRecord && fieldContext.nameEquals(SERVICE_ID)) { mergeFeedsResult.serviceIds.add(fieldContext.getValueToWrite()); + mergeFeedsResult.calendarServiceIds.add(fieldContext.getValueToWrite()); } return !shouldSkipRecord; diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeFeedsResult.java b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeFeedsResult.java index 1adadbb91..881c78051 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeFeedsResult.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeFeedsResult.java @@ -21,10 +21,21 @@ public class MergeFeedsResult implements Serializable { /** Contains the set of IDs for records that were excluded in the merged feed */ public Set skippedIds = new HashSet<>(); /** - * Track the set of service IDs to end up in the merged feed in order to determine which calendar_dates and trips - * records should be retained in the merged result. + * Track the set of service IDs to end up in the merged feed in order to determine which calendar, calendar_dates and + * trip records should be retained in the merged result. */ public Set serviceIds = new HashSet<>(); + + /** + * Track the set of service IDs obtained from calendar records. + */ + public Set calendarServiceIds = new HashSet<>(); + + /** + * Track the set of service IDs obtained from calendar date records. + */ + public Set calendarDatesServiceIds = new HashSet<>(); + /** * Track the set of route IDs to end up in the merged feed in order to determine which route_attributes * records should be retained in the merged result. diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeLineContext.java b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeLineContext.java index bbcdcbb9c..320ba54dd 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeLineContext.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeLineContext.java @@ -237,10 +237,36 @@ public void startNewRow() throws IOException { .collect(Collectors.toList()); } + /** + * Determine which reference table to use. If there is only one reference use this. If there are multiple references + * determine the context and then the correct reference table to use. + */ + private Table getReferenceTable(FieldContext fieldContext, Field field) { + if (field.referenceTables.size() == 1) { + return field.referenceTables.iterator().next(); + } + + switch (ReferenceTableDiscovery.getReferenceTableKey(field, table)) { + case TRIP_SERVICE_ID_KEY: + return ReferenceTableDiscovery.getTripServiceIdReferenceTable( + fieldContext.getValueToWrite(), + mergeFeedsResult, + getTableScopedValue(Table.CALENDAR, fieldContext.getValue()), + getTableScopedValue(Table.CALENDAR_DATES, fieldContext.getValue()) + ); + // Include other cases as multiple references are added e.g. flex!. + default: + return null; + } + } + public boolean checkForeignReferences(FieldContext fieldContext) throws IOException { Field field = fieldContext.getField(); if (field.isForeignReference()) { - String key = getTableScopedValue(field.referenceTable, fieldContext.getValue()); + Table refTable = getReferenceTable(fieldContext, field); + String key = (refTable != null) + ? getTableScopedValue(refTable, fieldContext.getValue()) + : "unknown"; // Check if we're performing a service period merge, this ref field is a service_id, and it // is not found in the list of service_ids (e.g., it was removed). boolean isValidServiceId = mergeFeedsResult.serviceIds.contains(fieldContext.getValueToWrite()); diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/ReferenceTableDiscovery.java b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/ReferenceTableDiscovery.java new file mode 100644 index 000000000..a972dbc25 --- /dev/null +++ b/src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/ReferenceTableDiscovery.java @@ -0,0 +1,90 @@ +package com.conveyal.datatools.manager.jobs.feedmerge; + +import com.conveyal.gtfs.loader.Field; +import com.conveyal.gtfs.loader.Table; + +import java.util.stream.Collectors; + +import static com.conveyal.datatools.manager.jobs.feedmerge.MergeLineContext.SERVICE_ID; + +public class ReferenceTableDiscovery { + + public static final String REF_TABLE_SEPARATOR = "#~#"; + + public enum ReferenceTableKey { + + TRIP_SERVICE_ID_KEY( + String.join( + REF_TABLE_SEPARATOR, + Table.TRIPS.name, + SERVICE_ID, + Table.CALENDAR.name, + Table.CALENDAR_DATES.name, + Table.SCHEDULE_EXCEPTIONS.name + ) + ); + + private final String value; + + ReferenceTableKey(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + public static ReferenceTableKey fromValue(String key) { + for (ReferenceTableKey ref: ReferenceTableKey.values()) { + if (ref.getValue().equals(key)) { + return ref; + } + } + throw new UnsupportedOperationException(String.format("Unsupported reference table key: %s.", key)); + } + } + + /** + * Get reference table key by matching the provided values to predefined reference table keys. + */ + public static ReferenceTableKey getReferenceTableKey(Field field, Table table) { + return ReferenceTableKey.fromValue(createKey(field, table)); + } + + /** + * Create a unique key for this table, field and reference tables. + */ + public static String createKey(Field field, Table table) { + return String.format( + "%s%s%s%s%s", + table.name, + REF_TABLE_SEPARATOR, + field.name, + REF_TABLE_SEPARATOR, + field.referenceTables.stream().map(r -> r.name).collect(Collectors.joining(REF_TABLE_SEPARATOR)) + ); + } + + /** + * Define the reference table for a trip service id. + */ + public static Table getTripServiceIdReferenceTable( + String fieldValue, + MergeFeedsResult mergeFeedsResult, + String calendarKey, + String calendarDatesKey + ) { + if ( + mergeFeedsResult.calendarServiceIds.contains(fieldValue) || + mergeFeedsResult.skippedIds.contains(calendarKey) + ) { + return Table.CALENDAR; + } else if ( + mergeFeedsResult.calendarDatesServiceIds.contains(fieldValue) || + mergeFeedsResult.skippedIds.contains(calendarDatesKey) + ) { + return Table.CALENDAR_DATES; + } + return null; + } +} diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java index 50ae60586..1be702b30 100644 --- a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java +++ b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java @@ -768,7 +768,7 @@ void canMergeFeedsWithMTCForServiceIds4 () throws SQLException { mergeFeedsJob.run(); assertFeedMergeSucceeded(mergeFeedsJob); SqlAssert sqlAssert = new SqlAssert(mergeFeedsJob.mergedVersion); - // FIXME: "version3" contains ref integrity errors... was hat intentional? + // FIXME: "version3" contains ref integrity errors... was that intentional? // sqlAssert.assertNoRefIntegrityErrors(); // - calendar table should have 3 records. @@ -778,12 +778,18 @@ void canMergeFeedsWithMTCForServiceIds4 () throws SQLException { // all records from future feed and keep_one from the active feed. sqlAssert.calendarDates.assertCount(3); + // Calendar dates service exception should still be available. + sqlAssert.calendarDates.assertCount(1, "service_id='Fake_Agency5:keep_one'"); + // - trips table should have 3 records. sqlAssert.trips.assertCount(3); // common_id service_id should be scoped for earlier feed version. sqlAssert.trips.assertCount(1, "service_id='Fake_Agency5:common_id'"); + // service_id should still reference calendar dates service exception. + sqlAssert.trips.assertCount(1, "service_id='Fake_Agency5:keep_one'"); + // Amended calendar record from earlier feed version should also have a modified end date (one day before the // earliest start_date from the future feed). sqlAssert.calendar.assertCount(1, "service_id='Fake_Agency5:common_id' AND end_date='20170914'");