refactor(MergeFeedsJob): add storeNewVersion option

ibi-group · Mar 16, 2020 · 12f8dd5 · 12f8dd5
1 parent bea6caf
commit 12f8dd5
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 29 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
@@ -5,7 +5,6 @@
 import com.conveyal.datatools.manager.auth.Auth0UserProfile;
 import com.conveyal.datatools.manager.gtfsplus.tables.GtfsPlusTable;
 import com.conveyal.datatools.manager.models.FeedSource;
-import com.conveyal.datatools.manager.models.FeedSource.FeedRetrievalMethod;
 import com.conveyal.datatools.manager.models.FeedVersion;
 import com.conveyal.datatools.manager.models.Project;
 import com.conveyal.datatools.manager.persistence.FeedStore;
@@ -136,14 +135,19 @@ public class MergeFeedsJob extends MonitorableJob {
     final FeedVersion mergedVersion;
     public boolean failOnDuplicateTripId = true;
 
+    public MergeFeedsJob(Auth0UserProfile owner, Set<FeedVersion> feedVersions, String file, MergeFeedsType mergeType) {
+        this(owner, feedVersions, file, mergeType, true);
+    }
+
     /**
-     * @param owner        user ID that initiated job
-     * @param feedVersions set of feed versions to merge
-     * @param file         resulting merge filename (without .zip)
-     * @param mergeType    the type of merge to perform (@link MergeFeedsType)
+     * @param owner             user ID that initiated job
+     * @param feedVersions      set of feed versions to merge
+     * @param file              resulting merge filename (without .zip)
+     * @param mergeType         the type of merge to perform {@link MergeFeedsType}
+     * @param storeNewVersion   whether to store merged feed as new version
      */
     public MergeFeedsJob(Auth0UserProfile owner, Set<FeedVersion> feedVersions, String file,
-                         MergeFeedsType mergeType) {
+                         MergeFeedsType mergeType, boolean storeNewVersion) {
         super(owner, mergeType.equals(REGIONAL) ? "Merging project feeds" : "Merging feed versions",
             JobType.MERGE_FEED_VERSIONS);
         this.feedVersions = feedVersions;
@@ -158,7 +162,7 @@ public MergeFeedsJob(Auth0UserProfile owner, Set<FeedVersion> feedVersions, Stri
         // Grab parent feed source depending on merge type.
         FeedSource regionalFeedSource = null;
         // If storing a regional merge as a new version, find the feed source designated by the project.
-        if (mergeType.equals(REGIONAL)) {
+        if (mergeType.equals(REGIONAL) && storeNewVersion) {
             regionalFeedSource = Persistence.feedSources.getById(project.regionalFeedSourceId);
             // Create new feed source if this is the first regional merge.
             if (regionalFeedSource == null) {
@@ -175,11 +179,13 @@ public MergeFeedsJob(Auth0UserProfile owner, Set<FeedVersion> feedVersions, Stri
         this.feedSource = mergeType.equals(REGIONAL)
             ? regionalFeedSource
             : feedVersions.iterator().next().parentFeedSource();
-        // Set feed source for merged version.
-        this.mergedVersion = new FeedVersion(this.feedSource);
-        this.mergedVersion.retrievalMethod = mergeType.equals(REGIONAL)
-            ? FeedRetrievalMethod.REGIONAL_MERGE
-            : FeedRetrievalMethod.SERVICE_PERIOD_MERGE;
+        // Merged version will be null if the new version should not be stored.
+        this.mergedVersion = storeNewVersion ? new FeedVersion(this.feedSource) : null;
+        if (this.mergedVersion != null) {
+            this.mergedVersion.retrievalMethod = mergeType.equals(REGIONAL)
+                ? REGIONAL_MERGE
+                : FeedSource.FeedRetrievalMethod.SERVICE_PERIOD_MERGE;
+        }
         this.mergeFeedsResult = new MergeFeedsResult(mergeType);
     }
 

diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/MergeFeedsJobTest.java
@@ -26,6 +26,7 @@
 import static com.conveyal.datatools.TestUtils.createFeedVersion;
 import static com.conveyal.datatools.TestUtils.createFeedVersionFromGtfsZip;
 import static com.conveyal.datatools.TestUtils.zipFolderFiles;
+import static com.conveyal.datatools.manager.models.FeedSource.FeedRetrievalMethod.MANUALLY_UPLOADED;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -60,27 +61,23 @@ public static void setUp() throws IOException {
         Persistence.projects.create(project);
 
         // Bart
-        FeedSource bart = new FeedSource("BART");
-        bart.projectId = project.id;
+        FeedSource bart = new FeedSource("BART", project.id, MANUALLY_UPLOADED);
         Persistence.feedSources.create(bart);
         bartVersion1 = createFeedVersionFromGtfsZip(bart, "bart_old.zip");
         bartVersion2 = createFeedVersionFromGtfsZip(bart, "bart_new.zip");
 
         // Caltrain
-        FeedSource caltrain = new FeedSource("Caltrain");
-        caltrain.projectId = project.id;
+        FeedSource caltrain = new FeedSource("Caltrain", project.id, MANUALLY_UPLOADED);
         Persistence.feedSources.create(caltrain);
         calTrainVersion = createFeedVersionFromGtfsZip(caltrain, "caltrain_gtfs.zip");
 
         // Napa
-        FeedSource napa = new FeedSource("Napa");
-        napa.projectId = project.id;
+        FeedSource napa = new FeedSource("Napa", project.id, MANUALLY_UPLOADED);
         Persistence.feedSources.create(napa);
         napaVersion = createFeedVersionFromGtfsZip(napa, "napa-no-agency-id.zip");
 
         // Fake agencies (for testing calendar service_id merges with MTC strategy).
-        FeedSource fakeAgency = new FeedSource("Fake Agency");
-        fakeAgency.projectId = project.id;
+        FeedSource fakeAgency = new FeedSource("Fake Agency", project.id, MANUALLY_UPLOADED);
         Persistence.feedSources.create(fakeAgency);
         bothCalendarFilesVersion = createFeedVersion(
             fakeAgency,
@@ -589,16 +586,10 @@ private void assertFeedMergeSucceeded(MergeFeedsJob mergeFeedsJob) {
      * Merges a set of FeedVersions and then creates a new FeedSource and FeedVersion of the merged feed.
      */
     private FeedVersion regionallyMergeVersions(Set<FeedVersion> versions) {
-        String mergeName = UUID.randomUUID().toString();
-        MergeFeedsJob mergeFeedsJob = new MergeFeedsJob(user, versions, mergeName, MergeFeedsType.REGIONAL);
+        MergeFeedsJob mergeFeedsJob = new MergeFeedsJob(user, versions, project.id, MergeFeedsType.REGIONAL);
         // Run the job in this thread (we're not concerned about concurrency here).
         mergeFeedsJob.run();
-        // Create a new feed source/version for the merged feed, so we can easily analyze its contents.
-        FeedSource source = new FeedSource(mergeName);
-        source.projectId = project.id;
-        Persistence.feedSources.create(source);
-        File feed = FeedVersion.feedStore.getFeed(mergeName + ".zip");
-        LOG.info("Regional merged file: {}", feed.getAbsolutePath());
-        return createFeedVersion(source, feed);
+        LOG.info("Regional merged file: {}", mergeFeedsJob.mergedVersion.retrieveGtfsFile().getAbsolutePath());
+        return mergeFeedsJob.mergedVersion;
     }
 }