Repository: ibi-group/datatools-server
Issue: MTC feed merge shapes issue

## Observed behavior (please include a screenshot if possible)

MTC has reported an issue with shapes.txt during an MTC feed merge (future + active). Essentially, identical shape_ids found in both feeds are causing the merged dataset to contain shape_id:shape_pt_sequence values from both datasets (e.g., if future dataset contains sequences 1,2,3,10 and active contains 1,2,7,9,10; the merged set will contain 1,2,3,7,9,10). So the output shape will be a frankenstein of both input shape point records.

## Expected behavior

If a `shape_id` is found in both the future and active datasets, all shape points from the active dataset must be feed-scoped.

## Steps to reproduce the problem

Run merge BART feeds test that now fails here: https://github.com/ibi-group/datatools-server/commit/17bf199705e7765ef9bb70766b03339fb5a1f32a

## Any special notes on configuration used

n/a

## Version of datatools-ui and datatools-server if applicable (exact commit hash or branch name)

latest dev

Changed files:
src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java, src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java

diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java b/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
index 829224d2..30e147c5 100644
--- a/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
+++ b/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java
@@ -332,6 +332,8 @@ public class MergeFeedsJob extends MonitorableJob {
         // Set up objects for tracking the rows encountered
         Map<String, String[]> rowValuesForStopOrRouteId = new HashMap<>();
         Set<String> rowStrings = new HashSet<>();
+        // Track shape_ids found in future feed in order to check for conflicts with active feed (MTC only).
+        Set<String> shapeIdsInFutureFeed = new HashSet<>();
         int mergedLineNumber = 0;
         // Get the spec fields to export
         List<Field> specFields = table.specFields();
@@ -582,6 +584,37 @@ public class MergeFeedsJob extends MonitorableJob {
                                         }
                                     }
                                     break;
+                                case "shapes":
+                                    // If a shape_id is found in both future and active datasets, all shape points from
+                                    // the active dataset must be feed-scoped. Otherwise, the merged dataset may contain
+                                    // shape_id:shape_pt_sequence values from both datasets (e.g., if future dataset contains
+                                    // sequences 1,2,3,10 and active contains 1,2,7,9,10; the merged set will contain
+                                    // 1,2,3,7,9,10).
+                                    if (field.name.equals("shape_id")) {
+                                        if (feedIndex == 0) {
+                                            // Track shape_id if working on future feed.
+                                            shapeIdsInFutureFeed.add(val);
+                                        } else if (shapeIdsInFutureFeed.contains(val)) {
+                                            // For the active feed, if the shape_id was already processed from the
+                                            // future feed, we need to add the feed-scope to avoid weird, hybrid shapes
+                                            // with points from both feeds.
+                                            valueToWrite = String.join(":", idScope, val);
+                                            // Update key value for subsequent ID conflict checks for this row.
+                                            keyValue = valueToWrite;
+                                            mergeFeedsResult.remappedIds.put(
+                                                getTableScopedValue(table, idScope, val),
+                                                valueToWrite
+                                            );
+                                            // Re-check refs and uniqueness after changing shape_id value. (Note: this
+                                            // probably won't have any impact, but there's not much harm in including it.)
+                                            idErrors = referenceTracker
+                                                .checkReferencesAndUniqueness(keyValue, lineNumber, field, valueToWrite,
+                                                    table, keyField, orderField);
+                                        }
+                                    }
+                                    // Skip record if normal duplicate errors are found.
+                                    if (hasDuplicateError(idErrors)) skipRecord = true;
+                                    break;
                                 case "trips":
                                     // trip_ids between active and future datasets must not match. If any trip_id is found
                                     // to be matching, the merge should fail with appropriate notification to user with the
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 87701d1e..e2c37dda 100644
--- a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
+++ b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
@@ -186,6 +186,12 @@ public class MergeFeedsJobTest extends UnitTest {
             9, // Magic number represents the number of routes in the merged BART feed.
             mergeFeedsJob.mergedVersion.feedLoadResult.routes.rowCount
         );
+        assertEquals(
+            "Merged feed shapes count should equal expected value.",
+            // During merge, if identical shape_id is found in both feeds, active feed shape_id should be feed-scoped.
+            bartVersion1.feedLoadResult.shapes.rowCount + bartVersion2.feedLoadResult.shapes.rowCount,
+            mergeFeedsJob.mergedVersion.feedLoadResult.shapes.rowCount
+        );
         // Ensure there are no referential integrity errors or duplicate ID errors.
         TestUtils.assertThatFeedHasNoErrorsOfType(
             mergeFeedsJob.mergedVersion.namespace,