From c1224bf3b163826e0c5d9b38a76311512a82d4d4 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 15 Nov 2021 18:43:31 -0500 Subject: [PATCH] fix(MergeFeedUtils): Use simplified stop times comparison (MTD req) --- .../datatools/manager/jobs/MergeFeedsJob.java | 25 +++++++++++-------- .../jobs/feedmerge/MergeLineContext.java | 2 +- .../manager/utils/MergeFeedUtils.java | 16 +++++++++--- 3 files changed, 29 insertions(+), 14 deletions(-) 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 90050f7d0..192892178 100644 --- a/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java +++ b/src/main/java/com/conveyal/datatools/manager/jobs/MergeFeedsJob.java @@ -136,7 +136,7 @@ public class MergeFeedsJob extends FeedSourceJob { @JsonIgnore @BsonIgnore public Set sharedTripIdsWithConsistentSignature = new HashSet<>(); @JsonIgnore @BsonIgnore - public Set serviceIdsToCloneAndRename = new HashSet<>(); + public Set serviceIdsToCloneRenameAndExtend = new HashSet<>(); @JsonIgnore @BsonIgnore public Set serviceIdsToTerminateEarly = new HashSet<>(); @@ -507,24 +507,29 @@ private void determineMergeStrategy() { // Build the set of calendars to be cloned/renamed/extended from trip ids present // in both active/future feeds and that have consistent signature. // These trips will be linked to the new service_ids. - serviceIdsToCloneAndRename.addAll( - sharedTripIdsWithConsistentSignature.stream() - .map(tripId -> activeFeed.trips.get(tripId).service_id) - .collect(Collectors.toList()) + serviceIdsToCloneRenameAndExtend.addAll( + getActiveServiceIds(this.sharedTripIdsWithConsistentSignature) ); // Build the set of calendars to be shortened to the day before the future feed start date // from trips in the active feed but not in the future feed. serviceIdsToTerminateEarly.addAll( - feedMergeContext.getActiveTripIdsNotInFutureFeed().stream() - .map(tripId -> activeFeed.trips.get(tripId).service_id) - .collect(Collectors.toList()) + getActiveServiceIds(feedMergeContext.getActiveTripIdsNotInFutureFeed()) ); mergeFeedsResult.mergeStrategy = CHECK_STOP_TIMES; } } + /** + * Obtains the service ids corresponding to the provided trip ids. + */ + private List getActiveServiceIds(Set tripIds) { + return tripIds.stream() + .map(tripId -> feedMergeContext.activeFeed.trips.get(tripId).service_id) + .collect(Collectors.toList()); + } + /** * Compare stop times for the given tripId between the future and active feeds. The comparison will inform whether * trip and/or service IDs should be modified in the output merged feed. @@ -537,7 +542,7 @@ private void compareStopTimesAndCollectTripAndServiceIds(String tripId, Feed fut List activeStopTimes = Lists.newArrayList(activeFeed.stopTimes.getOrdered(tripId)); String activeServiceId = activeFeed.trips.get(tripId).service_id; String futureServiceId = futureFeed.trips.get(tripId).service_id; - if (!stopTimesMatch(futureStopTimes, activeStopTimes)) { + if (!stopTimesMatchSimplified(futureStopTimes, activeStopTimes)) { // If stop_times or services do not match, merge will fail and no other action will be taken. sharedTripIdsWithInconsistentSignature.add(tripId); } else { @@ -545,7 +550,7 @@ private void compareStopTimesAndCollectTripAndServiceIds(String tripId, Feed fut // future trip and exclude the active one. Also, mark the service_id for cloning, // the cloned service id will need to be extended to the full time range. sharedTripIdsWithConsistentSignature.add(tripId); - serviceIdsToCloneAndRename.add(futureServiceId); + serviceIdsToCloneRenameAndExtend.add(futureServiceId); sharedConsistentTripAndCalendarIds.add(new TripAndCalendars(tripId, activeServiceId, futureServiceId)); } } 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 0ae018f36..127e2f860 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 @@ -463,7 +463,7 @@ public void initializeRowValues() { public void addClonedServiceId() throws IOException { if (table.name.equals("calendar")) { String originalServiceId = rowValues[keyFieldIndex]; - if (job.serviceIdsToCloneAndRename.contains(originalServiceId)) { + if (job.serviceIdsToCloneRenameAndExtend.contains(originalServiceId)) { // FIXME: Do we need to worry about calendar_dates? String[] clonedValues = rowValues.clone(); String newServiceId = clonedValues[keyFieldIndex] = String.join(":", idScope, originalServiceId); diff --git a/src/main/java/com/conveyal/datatools/manager/utils/MergeFeedUtils.java b/src/main/java/com/conveyal/datatools/manager/utils/MergeFeedUtils.java index fa8fe6252..87c856037 100644 --- a/src/main/java/com/conveyal/datatools/manager/utils/MergeFeedUtils.java +++ b/src/main/java/com/conveyal/datatools/manager/utils/MergeFeedUtils.java @@ -150,14 +150,24 @@ public static String getTableScopedValue(Table table, String prefix, String id) } /** - * Checks whether the future and active stop_times for a particular trip_id are an exact match. + * Checks whether the future and active stop_times for a particular trip_id are an exact match, + * using these criteria only: arrival_time, departure_time, stop_id, and stop_sequence + * instead of StopTime::equals (Revised MTC feed merge requirement). */ - public static boolean stopTimesMatch(List futureStopTimes, List activeStopTimes) { + public static boolean stopTimesMatchSimplified(List futureStopTimes, List activeStopTimes) { if (futureStopTimes.size() != activeStopTimes.size()) { return false; } for (int i = 0; i < activeStopTimes.size(); i++) { - if (!activeStopTimes.get(i).equals(futureStopTimes.get(i))) { + StopTime activeTime = activeStopTimes.get(i); + StopTime futureTime = futureStopTimes.get(i); + + if ( + activeTime.arrival_time != futureTime.arrival_time || + activeTime.departure_time != futureTime.departure_time || + activeTime.stop_sequence != futureTime.stop_sequence || + !activeTime.stop_id.equals(futureTime.stop_id) + ) { return false; } }