Don't delete the webui job if the engine job is not deleted. fixes #62

daisy · Mar 15, 2016 · 4258259 · 4258259
1 parent 43ee84a
commit 4258259
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 11 deletions.
diff --git a/app/Global.java b/app/Global.java
@@ -138,7 +138,7 @@ public void run() {
 							for (Job job : jobs) {
 								if (job.getFinished() != null && job.getFinished().before(timeoutDate)) {
 									Logger.info("Deleting old job: "+job.getId()+" ("+job.getNicename()+")");
-									job.delete();
+									job.deleteFromEngineAndWebUi();
 								}
 							}
 						} catch (javax.persistence.PersistenceException e) {
@@ -175,7 +175,7 @@ public void run() {
 									Date lastAccessed = Job.lastAccessed.get(webUiJob.getId());
 									final int deleteNewJobsAfterSeconds = 600;
 									if (lastAccessed == null || (new Date().getTime() - lastAccessed.getTime())/1000 > deleteNewJobsAfterSeconds) {
-										webUiJob.delete();
+										webUiJob.deleteFromEngineAndWebUi();
 									}
 
 									continue;
@@ -197,7 +197,7 @@ public void run() {
 										 */
 										//if ( should delete job ) {
 											//Logger.info("Deleting job that no longer exists in the Pipeline engine: "+webUiJob.getId()+" ("+webUiJob.getEngineId()+" - "+webUiJob.getNicename()+")");
-											//webUiJob.delete();
+											//webUiJob.deleteFromEngineAndWebUi();
 										//}
 									}
 								}

diff --git a/app/controllers/Jobs.java b/app/controllers/Jobs.java
@@ -220,7 +220,15 @@ public static Result restart(Long jobId) {
 
 		if (webuiJob.getEngineId() != null) {
 			Logger.info("deleting old job: "+webuiJob.getEngineId());
-			Application.ws.deleteJob(webuiJob.getEngineId());
+			org.daisy.pipeline.client.models.Job engineJob = Application.ws.getJob(webuiJob.getEngineId(), 0);
+			if (engineJob != null) {
+				boolean deleted = Application.ws.deleteJob(webuiJob.getEngineId());
+				if (!deleted) {
+					Logger.info("unable to delete old job: "+webuiJob.getEngineId());
+					flash("error", "An error occured while trying to delete the previous job. Please try creating a new job instead.");
+					return redirect(routes.Jobs.getJob(jobId));
+				}
+			}
 			webuiJob.setEngineId(null);
 		}
 
@@ -724,8 +732,13 @@ public static Result delete(Long jobId) {
 		}
 
     	Logger.debug("deleting "+jobId);
-		webuiJob.delete();
-		return ok();
+		boolean deletedSuccessfully = webuiJob.deleteFromEngineAndWebUi();
+		if (deletedSuccessfully) {
+			return ok();
+		} else {
+			flash("error", "An error occured while trying to delete the job. Please try creating a new job instead.");
+			return internalServerError();
+		}
     }
 
     public static Result postUpload(Long jobId) {

diff --git a/app/controllers/Templates.java b/app/controllers/Templates.java
@@ -90,7 +90,7 @@ public static Result postTemplate(User user, Job job, org.daisy.pipeline.client.
 		Template template = Template.create(clientlibJob, user);
 
 		if ("NEW".equals(job.getStatus())) {
-			job.delete();
+			job.deleteFromEngineAndWebUi();
 		}
 
 		String highlightTemplateName = template.name == null ? "" : ""+template.name;

diff --git a/app/models/Job.java b/app/models/Job.java
@@ -29,6 +29,7 @@
 import com.avaje.ebean.Model;
 
 import controllers.Application;
+import controllers.routes;
 
 @Entity
 public class Job extends Model implements Comparable<Job> {
@@ -277,14 +278,15 @@ public static Object jsonifiableResults(org.daisy.pipeline.client.models.Job job
 		return jsonResults;
 	}
 
-	@Override
-	public void delete() {
+	/** Same as `delete` but with boolean return value. Returns false if unable to delete the job in the engine. */
+	public boolean deleteFromEngineAndWebUi() {
 		try {
-			if (Status.valueOf(status) != null) {
+			if (Status.valueOf(status) != null && Application.ws.getJob(this.engineId, 0) != null) {
 				Logger.debug("deleting "+this.id+" (sending DELETE request)");
 				boolean success = Application.ws.deleteJob(this.engineId);
 				if (!success) {
 					Pipeline2Logger.logger().error("An error occured when trying to delete job "+this.id+" ("+this.engineId+") from the Pipeline 2 Engine");
+					return false; // don't delete Web UI job when an error occured attempting to delete the engine job
 				}
 			}
 
@@ -294,6 +296,12 @@ public void delete() {
 		asJob().getJobStorage().delete();
 		lastAccessed.remove(id);
 		super.delete();
+		return true;
+	}
+
+	@Override
+	public void delete() {
+		deleteFromEngineAndWebUi();
 	}
 
 	/*

diff --git a/app/models/User.java b/app/models/User.java
@@ -348,7 +348,7 @@ public void delete() {
 		try {
 			List<Job> jobs = getJobs();
 			for (Job job : jobs)
-				job.delete();
+				job.deleteFromEngineAndWebUi();
 			super.delete();
 		} catch (javax.persistence.OptimisticLockException e) {
 			Logger.warn("Could not delete user "+this.id+" ("+this.name+" / "+this.email+")", e);

diff --git a/app/views/Jobs/getJobs.scala.html b/app/views/Jobs/getJobs.scala.html
@@ -150,6 +150,7 @@ <h1>Jobs</h1>
 				},
 				error: function() {
 					$("#job-delete-"+jobId).show();
+					// TODO: show alert somehow about the failed job deletion?
 				},
 				context: this
 			});