From dc61eb8c8c2137fbe1a448f1f0688e84b12f5278 Mon Sep 17 00:00:00 2001 From: Brian Major <brian.major@nrc-cnrc.gc.ca> Date: Wed, 24 Sep 2014 13:57:08 -0700 Subject: [PATCH] s1651 - Search runner doesn't update job after execution in case in-memory job disappears --- .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 2 +- .../cadc/ac/server/web/ACSearchRunner.java | 173 +++++++++--------- 2 files changed, 86 insertions(+), 89 deletions(-) diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java index f0a205a4..ef2b5ebe 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java @@ -72,6 +72,7 @@ import java.security.AccessControlException; import java.security.Principal; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -104,7 +105,6 @@ import com.unboundid.ldap.sdk.SearchResult; import com.unboundid.ldap.sdk.SearchResultEntry; import com.unboundid.ldap.sdk.SearchScope; import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV2RequestControl; -import java.util.HashSet; public class LdapGroupDAO<T extends Principal> extends LdapDAO { diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java index dad14550..81601acb 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/ACSearchRunner.java @@ -68,38 +68,30 @@ */ package ca.nrc.cadc.ac.server.web; +import java.security.AccessControlException; +import java.security.Principal; +import java.util.Collection; +import java.util.Date; + +import javax.servlet.http.HttpServletResponse; + +import org.apache.log4j.Logger; + import ca.nrc.cadc.ac.Group; import ca.nrc.cadc.ac.GroupNotFoundException; import ca.nrc.cadc.ac.GroupsWriter; -import ca.nrc.cadc.ac.IdentityType; import ca.nrc.cadc.ac.UserNotFoundException; import ca.nrc.cadc.ac.server.GroupPersistence; import ca.nrc.cadc.ac.server.PluginFactory; import ca.nrc.cadc.ac.server.RequestValidator; -import ca.nrc.cadc.ac.server.UserPersistence; import ca.nrc.cadc.auth.AuthenticationUtil; -import ca.nrc.cadc.auth.HttpPrincipal; -import ca.nrc.cadc.auth.NumericPrincipal; -import ca.nrc.cadc.auth.OpenIdPrincipal; import ca.nrc.cadc.net.TransientException; -import ca.nrc.cadc.uws.ErrorSummary; -import ca.nrc.cadc.uws.ErrorType; import ca.nrc.cadc.uws.ExecutionPhase; import ca.nrc.cadc.uws.Job; -import ca.nrc.cadc.uws.server.JobNotFoundException; -import ca.nrc.cadc.uws.server.JobPersistenceException; import ca.nrc.cadc.uws.server.JobRunner; import ca.nrc.cadc.uws.server.JobUpdater; import ca.nrc.cadc.uws.server.SyncOutput; import ca.nrc.cadc.uws.util.JobLogInfo; -import java.io.IOException; -import java.security.AccessControlException; -import java.security.Principal; -import java.util.Collection; -import java.util.Date; -import javax.security.auth.x500.X500Principal; -import javax.servlet.http.HttpServletResponse; -import org.apache.log4j.Logger; public class ACSearchRunner implements JobRunner @@ -151,6 +143,15 @@ public class ACSearchRunner private void search() { + + // Note: This search runner is customized to run with + // InMemoryJobPersistence, and synchronous POST requests are + // dealt with immediately, rather than returning results via + // a redirect. + // Jobs in this runner are never updated after execution begins + // in case the in-memory job has gone away. Error reporting + // is done directly through the response on both POST and GET + try { ExecutionPhase ep = @@ -158,11 +159,7 @@ public class ACSearchRunner ExecutionPhase.EXECUTING, new Date()); if ( !ExecutionPhase.EXECUTING.equals(ep) ) { - String message = job.getID() + - ": QUEUED -> EXECUTING [FAILED] -- DONE"; - logInfo.setSuccess(false); - logInfo.setMessage(message); - return; + throw new IllegalStateException("QUEUED -> EXECUTING [FAILED]"); } log.debug(job.getID() + ": QUEUED -> EXECUTING [OK]"); @@ -181,29 +178,29 @@ public class ACSearchRunner GroupsWriter.write(groups, syncOut.getOutputStream()); // Mark the Job as completed. - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.COMPLETED, new Date()); +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.COMPLETED, new Date()); } catch (TransientException t) { logInfo.setSuccess(false); logInfo.setMessage(t.getMessage()); - log.debug("FAIL", t); + log.error("FAIL", t); - syncOut.setResponseCode(400); + syncOut.setResponseCode(503); - ErrorSummary errorSummary = - new ErrorSummary(t.getMessage(), ErrorType.FATAL); - try - { - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.ERROR, errorSummary, - new Date()); - } - catch(Throwable oops) - { - log.debug("failed to set final error status after " + t, oops); - } +// ErrorSummary errorSummary = +// new ErrorSummary(t.getMessage(), ErrorType.FATAL); +// try +// { +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.ERROR, errorSummary, +// new Date()); +// } +// catch(Throwable oops) +// { +// log.debug("failed to set final error status after " + t, oops); +// } } catch (UserNotFoundException t) { @@ -213,18 +210,18 @@ public class ACSearchRunner syncOut.setResponseCode(404); - ErrorSummary errorSummary = - new ErrorSummary(t.getMessage(), ErrorType.FATAL); - try - { - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.ERROR, errorSummary, - new Date()); - } - catch(Throwable oops) - { - log.debug("failed to set final error status after " + t, oops); - } +// ErrorSummary errorSummary = +// new ErrorSummary(t.getMessage(), ErrorType.FATAL); +// try +// { +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.ERROR, errorSummary, +// new Date()); +// } +// catch(Throwable oops) +// { +// log.debug("failed to set final error status after " + t, oops); +// } } catch (GroupNotFoundException t) { @@ -234,18 +231,18 @@ public class ACSearchRunner syncOut.setResponseCode(404); - ErrorSummary errorSummary = - new ErrorSummary(t.getMessage(), ErrorType.FATAL); - try - { - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.ERROR, errorSummary, - new Date()); - } - catch(Throwable oops) - { - log.debug("failed to set final error status after " + t, oops); - } +// ErrorSummary errorSummary = +// new ErrorSummary(t.getMessage(), ErrorType.FATAL); +// try +// { +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.ERROR, errorSummary, +// new Date()); +// } +// catch(Throwable oops) +// { +// log.debug("failed to set final error status after " + t, oops); +// } } catch (AccessControlException t) { @@ -255,39 +252,39 @@ public class ACSearchRunner syncOut.setResponseCode(401); - ErrorSummary errorSummary = - new ErrorSummary(t.getMessage(), ErrorType.FATAL); - try - { - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.ERROR, errorSummary, - new Date()); - } - catch(Throwable oops) - { - log.debug("failed to set final error status after " + t, oops); - } +// ErrorSummary errorSummary = +// new ErrorSummary(t.getMessage(), ErrorType.FATAL); +// try +// { +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.ERROR, errorSummary, +// new Date()); +// } +// catch(Throwable oops) +// { +// log.debug("failed to set final error status after " + t, oops); +// } } catch (Throwable t) { logInfo.setSuccess(false); logInfo.setMessage(t.getMessage()); - log.debug("FAIL", t); + log.error("FAIL", t); - syncOut.setResponseCode(400); + syncOut.setResponseCode(500); - ErrorSummary errorSummary = - new ErrorSummary(t.getMessage(), ErrorType.FATAL); - try - { - jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, - ExecutionPhase.ERROR, errorSummary, - new Date()); - } - catch(Throwable oops) - { - log.debug("failed to set final error status after " + t, oops); - } +// ErrorSummary errorSummary = +// new ErrorSummary(t.getMessage(), ErrorType.FATAL); +// try +// { +// jobUpdater.setPhase(job.getID(), ExecutionPhase.EXECUTING, +// ExecutionPhase.ERROR, errorSummary, +// new Date()); +// } +// catch(Throwable oops) +// { +// log.debug("failed to set final error status after " + t, oops); +// } } } -- GitLab