fix(delete): do a cascade delete for project/feed source

Rather than simply deleting the single document when a feed source or project is deleted, use the
object's delete method to delete related documents (for example, project feed sources or
deployments) so that the database does not become cluttered with extra floating documents.

Author: landonreed

src/main/java/com/conveyal/datatools/manager/controllers/api/DeploymentController.java

@@ -68,7 +68,7 @@ private static Deployment getDeployment (Request req, Response res) {
 
     private static Deployment deleteDeployment (Request req, Response res) {
         Deployment deployment = checkDeploymentPermissions(req, res);
-        Persistence.deployments.removeById(deployment.id);
+        deployment.delete();
         return deployment;
     }
 

src/main/java/com/conveyal/datatools/manager/controllers/api/FeedSourceController.java

@@ -207,15 +207,15 @@ public static FeedSource updateExternalFeedResource(Request req, Response res) t
      *
      * FIXME: Should this just set a "deleted" flag instead of removing from the database entirely?
      */
-    public static FeedSource deleteFeedSource(Request req, Response res) {
+    private static FeedSource deleteFeedSource(Request req, Response res) {
         FeedSource source = requestFeedSourceById(req, "manage");
 
         try {
-            Persistence.feedSources.removeById(source.id);
+            source.delete();
             return source;
         } catch (Exception e) {
-            e.printStackTrace();
-            halt(400, "Unknown error deleting feed source.");
+            LOG.error("Could not delete feed source", e);
+            haltWithMessage(400, "Unknown error deleting feed source.");
             return null;
         }
     }

src/main/java/com/conveyal/datatools/manager/controllers/api/FeedVersionController.java

@@ -54,6 +54,7 @@
 
 import javax.servlet.ServletInputStream;
 import javax.servlet.ServletRequestWrapper;
+import javax.servlet.http.HttpServletResponse;
 
 import static com.conveyal.datatools.common.status.MonitorableJob.JobType.BUILD_TRANSPORT_NETWORK;
 import static com.conveyal.datatools.common.utils.S3Utils.downloadFromS3;
@@ -80,19 +81,17 @@ enum Permission {
      * Grab the feed version for the ID supplied in the request.
      * If you pass in ?summarized=true, don't include the full tree of validation results, only the counts.
      */
-    public static FeedVersion getFeedVersion (Request req, Response res) {
-        FeedVersion feedVersion = requestFeedVersion(req, "view");
-        return feedVersion;
+    private static FeedVersion getFeedVersion (Request req, Response res) {
+        return requestFeedVersion(req, "view");
     }
 
     /**
      * Get all feed versions for a given feedSource (whose ID is specified in the request).
      */
-    public static Collection<FeedVersion> getAllFeedVersionsForFeedSource(Request req, Response res) {
+    private static Collection<FeedVersion> getAllFeedVersionsForFeedSource(Request req, Response res) {
         // Check permissions and get the FeedSource whose FeedVersions we want.
         FeedSource feedSource = requestFeedSourceById(req, "view");
-        Collection<FeedVersion> feedVersions = feedSource.retrieveFeedVersions();
-        return feedVersions;
+        return feedSource.retrieveFeedVersions();
     }
 
     public static FeedSource requestFeedSourceById(Request req, String action, String paramName) {
@@ -103,7 +102,7 @@ public static FeedSource requestFeedSourceById(Request req, String action, Strin
         return checkFeedSourcePermissions(req, Persistence.feedSources.getById(id), action);
     }
 
-    public static FeedSource requestFeedSourceById(Request req, String action) {
+    private static FeedSource requestFeedSourceById(Request req, String action) {
         return requestFeedSourceById(req, action, "feedSourceId");
     }
 
@@ -196,7 +195,7 @@ public static String createFeedVersionViaUpload(Request req, Response res) {
      *
      *  OR we could just export the feed to a file and then re-import it per usual. This seems like it's wasting time/energy.
      */
-    public static boolean createFeedVersionFromSnapshot (Request req, Response res) {
+    private static boolean createFeedVersionFromSnapshot (Request req, Response res) {
 
         Auth0UserProfile userProfile = req.attribute("user");
         // TODO: Should the ability to create a feedVersion from snapshot be controlled by the 'edit-gtfs' privilege?
@@ -216,13 +215,13 @@ public static boolean createFeedVersionFromSnapshot (Request req, Response res)
     /**
      * Spark HTTP API handler that deletes a single feed version based on the ID in the request.
      */
-    public static FeedVersion deleteFeedVersion(Request req, Response res) {
+    private static FeedVersion deleteFeedVersion(Request req, Response res) {
         FeedVersion version = requestFeedVersion(req, "manage");
         version.delete();
         return version;
     }
 
-    public static FeedVersion requestFeedVersion(Request req, String action) {
+    private static FeedVersion requestFeedVersion(Request req, String action) {
         return requestFeedVersion(req, action, req.params("id"));
     }
 
@@ -241,7 +240,7 @@ public static FeedVersion requestFeedVersion(Request req, String action, String
      * constructed in {@link #buildProfileRequest}). If a transport network does not exist for the feed version, an async
      * build job is kicked off. Otherwise, the transport network cache is checked for the network.
      */
-    public static String getIsochrones(Request req, Response res) {
+    private static String getIsochrones(Request req, Response res) {
         if (!DataManager.isModuleEnabled("r5_network")) {
             haltWithMessage(400, "Isochrone generation not enabled in this application.");
         }
@@ -357,7 +356,7 @@ private static AnalystClusterRequest buildProfileRequest(Request req) {
         return clusterRequest;
     }
 
-    public static Boolean renameFeedVersion (Request req, Response res) {
+    private static boolean renameFeedVersion (Request req, Response res) {
         FeedVersion v = requestFeedVersion(req, "manage");
 
         String name = req.queryParams("name");
@@ -378,7 +377,7 @@ private static Object downloadFeedVersionDirectly(Request req, Response res) {
      * Returns credentials that a client may use to then download a feed version. Functionality
      * changes depending on whether application.data.use_s3_storage config property is true.
      */
-    public static Object getFeedDownloadCredentials(Request req, Response res) {
+    private static Object getFeedDownloadCredentials(Request req, Response res) {
         FeedVersion version = requestFeedVersion(req, "view");
 
         if (DataManager.useS3) {
@@ -398,7 +397,7 @@ public static Object getFeedDownloadCredentials(Request req, Response res) {
      */
     private static JsonNode validate (Request req, Response res) {
         FeedVersion version = requestFeedVersion(req, "manage");
-
+        haltWithMessage(400, "Validate endpoint not currently configured!");
         // FIXME: Update for sql-loader validation process?
         return null;
 //        return version.retrieveValidationResult(true);
@@ -426,7 +425,7 @@ private static FeedVersion publishToExternalResource (Request req, Response res)
      * Download locally stored feed version with token supplied by this application. This method is only used when
      * useS3 is set to false. Otherwise, a direct download from s3 should be used.
      */
-    private static Object downloadFeedVersionWithToken (Request req, Response res) {
+    private static HttpServletResponse downloadFeedVersionWithToken (Request req, Response res) {
         String tokenValue = req.params("token");
         FeedDownloadToken token = Persistence.tokens.getById(tokenValue);
 

src/main/java/com/conveyal/datatools/manager/controllers/api/ProjectController.java

@@ -7,6 +7,7 @@
 import com.conveyal.datatools.manager.jobs.MakePublicJob;
 import com.conveyal.datatools.manager.jobs.MergeProjectFeedsJob;
 import com.conveyal.datatools.manager.models.FeedDownloadToken;
+import com.conveyal.datatools.manager.models.FeedSource;
 import com.conveyal.datatools.manager.models.FeedVersion;
 import com.conveyal.datatools.manager.models.JsonViews;
 import com.conveyal.datatools.manager.models.Project;
@@ -135,10 +136,10 @@ private static Project updateProject(Request req, Response res) throws IOExcepti
     /**
      * Delete the project for the UUID given in the request.
      */
-    private static Project deleteProject(Request req, Response res) throws IOException {
+    private static Project deleteProject(Request req, Response res) {
         // Fetch project first to check permissions, and so we can return the deleted project after deletion.
         Project project = requestProjectById(req, "manage");
-        boolean successfullyDeleted = Persistence.projects.removeById(req.params("id"));
+        boolean successfullyDeleted = project.delete();
         if (!successfullyDeleted) {
             haltWithMessage(400, "Did not delete project.");
         }

src/main/java/com/conveyal/datatools/manager/jobs/FeedUpdater.java

@@ -125,7 +125,9 @@ private Map<String, String> checkForUpdatedFeeds() {
                                 properties.stream().map(p -> p.feedSourceId).collect(Collectors.joining(",")));
                     }
                     for (ExternalFeedSourceProperty prop : properties) {
-                        // FIXME: What if there are multiple props found for different feed sources.
+                        // FIXME: What if there are multiple props found for different feed sources. This could happen if
+                        // multiple projects have been synced with MTC or if the ExternalFeedSourceProperty for a feed
+                        // source is not deleted properly when the feed source is deleted.
                         feedSource = Persistence.feedSources.getById(prop.feedSourceId);
                     }
                     if (feedSource == null) {

src/main/java/com/conveyal/datatools/manager/models/Deployment.java

@@ -434,6 +434,10 @@ public String organizationId() {
         return project == null ? null : project.organizationId;
     }
 
+    public boolean delete() {
+        return Persistence.deployments.removeById(this.id);
+    }
+
     /**
      * A summary of a FeedVersion, leaving out all of the individual validation errors.
      */

src/main/java/com/conveyal/datatools/manager/models/FeedSource.java

@@ -375,16 +375,6 @@ public Map<String, Map<String, String>> externalProperties() {
         return resourceTable;
     }
 
-    public static FeedSource retrieve(String id) {
-//        return sourceStore.getById(id);
-        return null;
-    }
-
-    public static Collection<FeedSource> retrieveAll() {
-//        return sourceStore.getAll();
-        return null;
-    }
-
     /**
      * Get all of the feed versions for this source
      * @return collection of feed versions
@@ -517,35 +507,33 @@ public enum FeedRetrievalMethod {
 
     /**
      * Delete this feed source and everything that it contains.
+     *
+     * FIXME: Use a Mongo transaction to handle the deletion of these related objects.
      */
-    public void delete() {
-        retrieveFeedVersions().forEach(FeedVersion::delete);
+    public boolean delete() {
+        try {
+            retrieveFeedVersions().forEach(FeedVersion::delete);
 
-        // delete latest copy of feed source
-        if (DataManager.useS3) {
-            DeleteObjectsRequest delete = new DeleteObjectsRequest(DataManager.feedBucket);
-            delete.withKeys("public/" + this.name + ".zip", FeedStore.s3Prefix + this.id + ".zip");
-            FeedStore.s3Client.deleteObjects(delete);
-        }
+            // Delete latest copy of feed source on S3.
+            if (DataManager.useS3) {
+                DeleteObjectsRequest delete = new DeleteObjectsRequest(DataManager.feedBucket);
+                delete.withKeys("public/" + this.name + ".zip", FeedStore.s3Prefix + this.id + ".zip");
+                FeedStore.s3Client.deleteObjects(delete);
+            }
+            // Remove all external properties for this feed source.
+            Persistence.externalFeedSourceProperties.removeFiltered(eq("feedSourceId", this.id));
 
-        // Delete editor feed mapdb
-        // TODO: does the mapdb folder need to be deleted separately?
-        GlobalTx gtx = VersionedDataStore.getGlobalTx();
-        if (!gtx.feeds.containsKey(id)) {
-            gtx.rollback();
-        }
-        else {
-            gtx.feeds.remove(id);
-            gtx.commit();
-        }
+            // TODO: add delete for osm extract and r5 network (maybe that goes with version)
 
-        // FIXME use Mongo filters instead
-        Persistence.externalFeedSourceProperties.getAll().stream()
-                .filter(prop -> prop.feedSourceId.equals(this.id))
-                .forEach(prop -> Persistence.externalFeedSourceProperties.removeById(prop.id));
+            // FIXME: Should this delete related feed versions from the SQL database (for both published versions and
+            // editor snapshots)?
 
-        // TODO: add delete for osm extract and r5 network (maybe that goes with version)
-        Persistence.feedSources.removeById(this.id);
+            // Finally, delete the feed source mongo document.
+            return Persistence.feedSources.removeById(this.id);
+        } catch (Exception e) {
+            LOG.error("Could not delete feed source", e);
+            return false;
+        }
     }
 
     public FeedSource clone () throws CloneNotSupportedException {

src/main/java/com/conveyal/datatools/manager/models/Project.java

@@ -2,26 +2,24 @@
 
 import com.conveyal.datatools.manager.persistence.Persistence;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.annotation.JsonInclude.Include;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.*;
+import java.util.Collection;
+import java.util.List;
 import java.util.stream.Collectors;
 
 import static com.mongodb.client.model.Filters.eq;
 
 /**
  * Represents a collection of feed sources that can be made into a deployment.
  * Generally, this would represent one agency that is managing the data.
- * For now, there is one FeedCollection per instance of GTFS data manager, but
+ * For now, there is one Project per instance of GTFS data manager, but
  * we're trying to write the code in such a way that this is not necessary.
  *
  * @author mattwigway
  *
  */
-@JsonInclude(Include.ALWAYS)
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class Project extends Model {
     private static final long serialVersionUID = 1L;
@@ -101,4 +99,17 @@ public Organization retrieveOrganization() {
             return null;
         }
     }
+
+    public boolean delete() {
+        // FIXME: Handle this in a Mongo transaction. See https://docs.mongodb.com/master/core/transactions/#transactions-and-mongodb-drivers
+//        ClientSession clientSession = Persistence.startSession();
+//        clientSession.startTransaction();
+
+        // Delete each feed source in the project (which in turn deletes each feed version).
+        retrieveProjectFeedSources().forEach(FeedSource::delete);
+        // Delete each deployment in the project.
+        retrieveDeployments().forEach(Deployment::delete);
+        // Finally, delete the project.
+        return Persistence.projects.removeById(this.id);
+    }
 }