refactor(Multiple reference tables lookup): Feed merge updated to han…

…dle multi ref tables

pom.xml

@@ -268,7 +268,7 @@
             <groupId>com.github.conveyal</groupId>
             <artifactId>gtfs-lib</artifactId>
             <!-- Latest dev build on jitpack.io -->
-            <version>ba747df</version>
+            <version>59b568a</version>
             <!-- Exclusions added in order to silence SLF4J warnings about multiple bindings:
                 http://www.slf4j.org/codes.html#multiple_bindings
             -->

src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/MergeLineContext.java

@@ -237,31 +237,6 @@ public void startNewRow() throws IOException {
             .collect(Collectors.toList());
     }
 
-    public enum ReferenceTableLookup {
-        TRIP_SERVICE_ID_KEY(
-            String.join(":", Table.TRIPS.name, SERVICE_ID, Table.CALENDAR.name, Table.CALENDAR_DATES.name)
-        );
-
-        private final String value;
-
-        ReferenceTableLookup(String value) {
-            this.value = value;
-        }
-
-        public String getValue() {
-            return value;
-        }
-
-        public static ReferenceTableLookup fromValue(String key) {
-            for (ReferenceTableLookup ref: ReferenceTableLookup.values()) {
-                if (ref.getValue().equals(key)) {
-                    return ref;
-                }
-            }
-            throw new UnsupportedOperationException(String.format("Unsupported reference table combination: %s.", key));
-        }
-    }
-
     /**
      * 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.
@@ -271,41 +246,27 @@ private Table getReferenceTable(FieldContext fieldContext, Field field) {
             return field.referenceTables.iterator().next();
         }
 
-        // A table reference is still required even if no match is found.
-        Table defaultTable = null;
-        switch (ReferenceTableLookup.fromValue(createKey(field))) {
+        switch (ReferenceTableDiscovery.getReferenceTableKey(field, table)) {
             case TRIP_SERVICE_ID_KEY:
-                boolean isCalendarServiceId = mergeFeedsResult.calendarServiceIds.contains(fieldContext.getValueToWrite());
-                boolean isCalendarDatesServiceId = mergeFeedsResult.calendarDatesServiceIds.contains(fieldContext.getValueToWrite());
-                if (isCalendarServiceId && !isCalendarDatesServiceId) {
-                    return Table.CALENDAR;
-                } else if (!isCalendarServiceId && isCalendarDatesServiceId) {
-                    return Table.CALENDAR_DATES;
-                } else {
-                    // For this case it doesn't seem to matter which is returned so going with calendar.
-                    defaultTable = Table.CALENDAR;
-                }
-            // Add other cases as multiple references are added e.g. flex!.
+                return ReferenceTableDiscovery.getTripServiceIdReferenceTable(
+                    fieldContext,
+                    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;
         }
-        return defaultTable;
-    }
-
-    /**
-     * Create a unique key for this table, field and reference tables.
-     */
-    private String createKey(Field field) {
-        return String.format(
-            "%s:%s:%s",
-            table.name,
-            field.name,
-            field.referenceTables.stream().map(r -> r.name).collect(Collectors.joining(":"))
-        );
     }
 
     public boolean checkForeignReferences(FieldContext fieldContext) throws IOException {
         Field field = fieldContext.getField();
         if (field.isForeignReference()) {
-            String key = getTableScopedValue(getReferenceTable(fieldContext, field), 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());

src/main/java/com/conveyal/datatools/manager/jobs/feedmerge/ReferenceTableDiscovery.java

@@ -0,0 +1,83 @@
+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)
+        );
+
+        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(
+        FieldContext fieldContext,
+        MergeFeedsResult mergeFeedsResult,
+        String calendarKey,
+        String calendarDatesKey
+    ) {
+        if (
+            mergeFeedsResult.calendarServiceIds.contains(fieldContext.getValueToWrite()) ||
+            mergeFeedsResult.skippedIds.contains(calendarKey)
+        ) {
+            return Table.CALENDAR;
+        } else if (
+            mergeFeedsResult.calendarDatesServiceIds.contains(fieldContext.getValueToWrite()) ||
+            mergeFeedsResult.skippedIds.contains(calendarDatesKey)
+        ) {
+            return Table.CALENDAR_DATES;
+        }
+        return null;
+    }
+}

src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java

@@ -765,7 +765,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.
@@ -775,12 +775,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'");