From 9aa13889e2ebb4447c34351bccdf73997d85596e Mon Sep 17 00:00:00 2001 From: gmantele <gmantele@ari.uni-heidelberg.de> Date: Tue, 23 Sep 2014 17:41:52 +0200 Subject: [PATCH] [TAP,UWS] Fix a problem with the format of the error details returned by the parameter .../error/details. In TAP a VOTable document is expected, but a text/plain description was returned (default behavior in the UWS lib.). Now, a ServiceErrorWriter is used to format the error details correctly. --- src/tap/AbstractTAPFactory.java | 35 +++++++- src/tap/AsyncThread.java | 29 ++++++- src/tap/TAPFactory.java | 16 +++- src/tap/error/DefaultTAPErrorWriter.java | 37 ++++++++- src/tap/resource/TAP.java | 1 - src/uws/job/JobThread.java | 82 ++++++++++++++++--- src/uws/service/actions/GetJobParam.java | 4 +- .../service/error/DefaultUWSErrorWriter.java | 14 ++++ src/uws/service/error/ServiceErrorWriter.java | 50 ++++++++++- 9 files changed, 240 insertions(+), 28 deletions(-) diff --git a/src/tap/AbstractTAPFactory.java b/src/tap/AbstractTAPFactory.java index 1241c92..678d6fd 100644 --- a/src/tap/AbstractTAPFactory.java +++ b/src/tap/AbstractTAPFactory.java @@ -29,6 +29,7 @@ import java.util.Map; import javax.servlet.http.HttpServletRequest; import tap.db.DBConnection; +import tap.error.DefaultTAPErrorWriter; import tap.metadata.TAPMetadata; import tap.metadata.TAPSchema; import tap.metadata.TAPTable; @@ -40,6 +41,7 @@ import uws.job.Result; import uws.job.user.JobOwner; import uws.service.UWSService; import uws.service.backup.UWSBackupManager; +import uws.service.error.ServiceErrorWriter; import adql.db.DBChecker; import adql.parser.ADQLQueryFactory; import adql.parser.QueryChecker; @@ -50,10 +52,12 @@ import adql.query.ADQLQuery; * Only the functions related with the database connection stay abstract. * * @author Grégory Mantelet (CDS;ARI) - * @version 2.0 (08/2014) + * @version 2.0 (09/2014) */ public abstract class AbstractTAPFactory extends TAPFactory { + protected final ServiceErrorWriter errorWriter; + /** * Build a basic TAPFactory. * Nothing is done except setting the service connection. @@ -62,10 +66,33 @@ public abstract class AbstractTAPFactory extends TAPFactory { * * @throws NullPointerException If the given {@link ServiceConnection} is NULL. * - * @see {@link TAPFactory#TAPFactory(ServiceConnection)} + * @see AbstractTAPFactory#AbstractTAPFactory(ServiceConnection, ServiceErrorWriter) */ protected AbstractTAPFactory(ServiceConnection service) throws NullPointerException{ + this(service, new DefaultTAPErrorWriter(service)); + } + + /** + * <p>Build a basic TAPFactory. + * Nothing is done except setting the service connection and the given error writer.</p> + * + * <p>Then the error writer will be used when creating a UWS service and a job thread.</p> + * + * @param service Configuration of the TAP service. <i>MUST NOT be NULL</i> + * @param errorWriter Object to use to format and write the errors for the user. + * + * @throws NullPointerException If the given {@link ServiceConnection} is NULL. + * + * @see {@link TAPFactory#TAPFactory(ServiceConnection)} + */ + protected AbstractTAPFactory(final ServiceConnection service, final ServiceErrorWriter errorWriter) throws NullPointerException{ super(service); + this.errorWriter = errorWriter; + } + + @Override + public final ServiceErrorWriter getErrorWriter(){ + return errorWriter; } /* *************** */ @@ -192,7 +219,9 @@ public abstract class AbstractTAPFactory extends TAPFactory { */ @Override public UWSService createUWS() throws TAPException{ - return new UWSService(this, this.service.getFileManager(), this.service.getLogger()); + UWSService uws = new UWSService(this, this.service.getFileManager(), this.service.getLogger()); + uws.setErrorWriter(errorWriter); + return uws; } /** diff --git a/src/tap/AsyncThread.java b/src/tap/AsyncThread.java index 00f1f2d..deb7a3d 100644 --- a/src/tap/AsyncThread.java +++ b/src/tap/AsyncThread.java @@ -16,18 +16,36 @@ package tap; * You should have received a copy of the GNU Lesser General Public License * along with TAPLibrary. If not, see <http://www.gnu.org/licenses/>. * - * Copyright 2012 - UDS/Centre de DonnĂ©es astronomiques de Strasbourg (CDS) + * Copyright 2012,2014 - UDS/Centre de DonnĂ©es astronomiques de Strasbourg (CDS), + * Astronomisches Rechen Institut (ARI) */ import uws.UWSException; import uws.job.JobThread; +import uws.service.error.ServiceErrorWriter; +/** + * Thread in charge of a TAP job execution. + * + * @author Grégory Mantelet (CDS;ARI) + * @version 2.0 (09/2014) + */ public class AsyncThread extends JobThread { + /** The only object which knows how to execute an ADQL query. */ protected final ADQLExecutor executor; - public AsyncThread(TAPJob j, ADQLExecutor executor) throws UWSException{ - super(j, "Execute the ADQL query of the TAP request " + j.getJobId()); + /** + * Build a TAP asynchronous job execution. + * + * @param j Description of the job to execute. + * @param executor The object to use for the ADQL execution itself. + * @param errorWriter The object to use to format and to write an execution error for the user. + * + * @throws NullPointerException If the job parameter is missing. + */ + public AsyncThread(final TAPJob j, final ADQLExecutor executor, final ServiceErrorWriter errorWriter) throws NullPointerException{ + super(j, "Execute the ADQL query of the TAP request " + j.getJobId(), errorWriter); this.executor = executor; } @@ -46,6 +64,11 @@ public class AsyncThread extends JobThread { } } + /** + * Get the description of the job that this thread is executing. + * + * @return The executed job. + */ public final TAPJob getTAPJob(){ return (TAPJob)job; } diff --git a/src/tap/TAPFactory.java b/src/tap/TAPFactory.java index b8c0aab..de00df9 100644 --- a/src/tap/TAPFactory.java +++ b/src/tap/TAPFactory.java @@ -40,6 +40,7 @@ import uws.job.user.JobOwner; import uws.service.AbstractUWSFactory; import uws.service.UWSService; import uws.service.backup.UWSBackupManager; +import uws.service.error.ServiceErrorWriter; import adql.parser.ADQLQueryFactory; import adql.parser.QueryChecker; import adql.query.ADQLQuery; @@ -80,6 +81,17 @@ public abstract class TAPFactory extends AbstractUWSFactory { this.service = service; } + /** + * <p>Get the object to use when an error must be formatted and written to the user.</p> + * + * <p>This formatted error will be either written in an HTTP response or in a job error summary.</p> + * + * @return The error writer to use. + * + * @since 4.1 + */ + public abstract ServiceErrorWriter getErrorWriter(); + /* ******************* */ /* DATABASE CONNECTION */ /* ******************* */ @@ -343,7 +355,7 @@ public abstract class TAPFactory extends AbstractUWSFactory { * <p>Create the thread which will execute the task described by the given UWSJob instance.</p> * * <p> - * This function is definitely implemented here and can not be overrided. The processing of + * This function is definitely implemented here and can not be overridden. The processing of * an ADQL query must always be the same in a TAP service ; it is completely done by {@link AsyncThread}. * </p> * @@ -353,7 +365,7 @@ public abstract class TAPFactory extends AbstractUWSFactory { @Override public final JobThread createJobThread(final UWSJob job) throws UWSException{ try{ - return new AsyncThread((TAPJob)job, createADQLExecutor()); + return new AsyncThread((TAPJob)job, createADQLExecutor(), getErrorWriter()); }catch(TAPException te){ throw new UWSException(UWSException.INTERNAL_SERVER_ERROR, te, "Impossible to create an AsyncThread !"); } diff --git a/src/tap/error/DefaultTAPErrorWriter.java b/src/tap/error/DefaultTAPErrorWriter.java index 064d778..d347c67 100644 --- a/src/tap/error/DefaultTAPErrorWriter.java +++ b/src/tap/error/DefaultTAPErrorWriter.java @@ -21,6 +21,8 @@ package tap.error; */ import java.io.IOException; +import java.io.OutputStream; +import java.io.PrintWriter; import java.util.HashMap; import javax.servlet.http.HttpServletRequest; @@ -32,7 +34,9 @@ import tap.formatter.VOTableFormat; import tap.log.DefaultTAPLog; import tap.log.TAPLog; import uws.UWSException; +import uws.job.ErrorSummary; import uws.job.ErrorType; +import uws.job.UWSJob; import uws.job.user.JobOwner; import uws.service.error.DefaultUWSErrorWriter; import uws.service.error.ServiceErrorWriter; @@ -125,7 +129,7 @@ public class DefaultTAPErrorWriter implements ServiceErrorWriter { response.setStatus((httpErrorCode <= 0) ? 500 : httpErrorCode); // Set the MIME type of the answer (XML for a VOTable document): - response.setContentType("text/xml"); + response.setContentType("application/xml"); // List any additional information useful to report to the user: HashMap<String,String> addInfos = new HashMap<String,String>(); @@ -142,4 +146,35 @@ public class DefaultTAPErrorWriter implements ServiceErrorWriter { formatter.writeError(message, addInfos, response.getWriter()); } + @Override + public void writeError(Throwable t, ErrorSummary error, UWSJob job, OutputStream output) throws IOException{ + // Get the error message: + String message; + if (error != null && error.getMessage() != null) + message = error.getMessage(); + else if (t != null) + message = (t.getMessage() == null) ? t.getClass().getName() : t.getMessage(); + else + message = "{NO MESSAGE}"; + + // List any additional information useful to report to the user: + HashMap<String,String> addInfos = new HashMap<String,String>(); + if (job != null){ + addInfos.put("JOB_ID", job.getJobId()); + if (job.getOwner() != null) + addInfos.put("USER", job.getOwner().getID() + ((job.getOwner().getPseudo() == null) ? "" : " (" + job.getOwner().getPseudo() + ")")); + } + if (error != null && error.getType() != null) + addInfos.put("ERROR_TYPE", error.getType().toString()); + addInfos.put("ACTION", "EXECUTING"); + + // Format the error in VOTable and write the document in the given HTTP response: + formatter.writeError(message, addInfos, new PrintWriter(output)); + } + + @Override + public String getErrorDetailsMIMEType(){ + return "application/xml"; + } + } diff --git a/src/tap/resource/TAP.java b/src/tap/resource/TAP.java index c83bd5d..65ab29e 100644 --- a/src/tap/resource/TAP.java +++ b/src/tap/resource/TAP.java @@ -111,7 +111,6 @@ public class TAP implements VOSIResource { res = new ASync(service); resources.put(res.getName(), res); - getUWS().setErrorWriter(errorWriter); TAPMetadata metadata = service.getTAPMetadata(); if (metadata != null) diff --git a/src/uws/job/JobThread.java b/src/uws/job/JobThread.java index 8f8a4af..538e0ba 100644 --- a/src/uws/job/JobThread.java +++ b/src/uws/job/JobThread.java @@ -26,6 +26,7 @@ import java.util.Date; import uws.UWSException; import uws.UWSToolBox; +import uws.service.error.ServiceErrorWriter; import uws.service.file.UWSFileManager; import uws.service.log.UWSLog; import uws.service.log.UWSLog.LogLevel; @@ -59,7 +60,7 @@ import uws.service.log.UWSLog.LogLevel; * </ul> * * @author Grégory Mantelet (CDS;ARI) - * @version 4.1 (08/2014) + * @version 4.1 (09/2014) * * @see UWSJob#start() * @see UWSJob#abort() @@ -80,36 +81,80 @@ public abstract class JobThread extends Thread { /** Description of what is done by this thread. */ protected final String taskDescription; + /** + * Object to use in order to write the content of an error/exception in any output stream. + * If NULL, the content will be written by {@link UWSToolBox#writeErrorFile(Exception, ErrorSummary, UWSJob, OutputStream)} + * (in text/plain with stack-trace). + * Otherwise the content and the MIME type are determined by the error writer. + * @since 4.1 + */ + protected final ServiceErrorWriter errorWriter; + + /** Group of threads in which this job thread will run. */ public final static ThreadGroup tg = new ThreadGroup("UWS_GROUP"); /** * Builds the JobThread instance which will be used by the given job to execute its task. * * @param j The associated job. - * @param fileManager An object to get access to UWS files (particularly: error and results file). * - * @throws UWSException If the given job or the given file manager is null. + * @throws NullPointerException If the given job or the given file manager is null. * * @see #getDefaultTaskDescription(UWSJob) */ - public JobThread(UWSJob j) throws UWSException{ - this(j, getDefaultTaskDescription(j)); + public JobThread(final UWSJob j) throws NullPointerException{ + this(j, getDefaultTaskDescription(j), null); + } + + /** + * Builds the JobThread instance which will be used by the given job to execute its task. + * + * @param j The associated job. + * @param errorWriter Object to use in case of error in order to format the details of the error for the .../error/details parameter. + * + * @throws NullPointerException If the given job is null. + * + * @see #getDefaultTaskDescription(UWSJob) + * + * @since 4.1 + */ + public JobThread(final UWSJob j, final ServiceErrorWriter errorWriter) throws NullPointerException{ + this(j, getDefaultTaskDescription(j), errorWriter); + } + + /** + * Builds the JobThread instance which will be used by the given job to execute its task. + * + * @param j The associated job. + * @param task Description of the task executed by this thread. + * + * @throws NullPointerException If the given job is null. + */ + public JobThread(final UWSJob j, final String task) throws NullPointerException{ + super(tg, j.getJobId()); + + job = j; + taskDescription = task; + errorWriter = null; } /** * Builds the JobThread instance which will be used by the given job to execute its task. * * @param j The associated job. - * @param fileManager An object to get access to UWS files (particularly: error and results file). * @param task Description of the task executed by this thread. + * @param errorWriter Object to use in case of error in order to format the details of the error for the .../error/details parameter. * - * @throws UWSException If the given job or the given file manager is null. + * @throws NullPointerException If the given job is null. + * + * @since 4.1 */ - public JobThread(UWSJob j, String task) throws UWSException{ + public JobThread(final UWSJob j, final String task, final ServiceErrorWriter errorWriter) throws NullPointerException{ super(tg, j.getJobId()); job = j; taskDescription = task; + this.errorWriter = errorWriter; } /** @@ -211,6 +256,7 @@ public abstract class JobThread extends Thread { * * @throws UWSException If there is an error while publishing the given exception. * + * {@link ServiceErrorWriter#writeError(UWSJob, Throwable, String, ErrorType, OutputStream)} * {@link UWSToolBox#writeErrorFile(Exception, ErrorSummary, UWSJob, OutputStream)} */ public void setError(final UWSException ue) throws UWSException{ @@ -218,11 +264,21 @@ public abstract class JobThread extends Thread { return; try{ + // Set the error summary: ErrorSummary error = new ErrorSummary(ue, ue.getUWSErrorType(), job.getUrl() + "/" + UWSJob.PARAM_ERROR_SUMMARY + "/details"); + + // Prepare the output stream: OutputStream output = getFileManager().getErrorOutput(error, job); - UWSToolBox.writeErrorFile(ue, error, job, output); + // Format and write the error... + // ...using the error writer, if any: + if (errorWriter != null) + errorWriter.writeError(ue, error, job, output); + // ...or write a default output: + else + UWSToolBox.writeErrorFile(ue, error, job, output); + // Set the error summary inside the job: setError(error); }catch(IOException ioe){ @@ -365,7 +421,7 @@ public abstract class JobThread extends Thread { UWSLog logger = job.getLogger(); // Log the start of this thread: - logger.logThread(LogLevel.INFO, this, "START", "Thread \"" + getId() + "\" started.", null); + logger.logThread(LogLevel.INFO, this, "START", "Thread \"" + getName() + "\" started.", null); try{ try{ @@ -381,7 +437,7 @@ public abstract class JobThread extends Thread { if (!job.stopping) job.abort(); // Log the abortion: - logger.logThread(LogLevel.INFO, this, "END", "Thread \"" + getId() + "\" cancelled.", null); + logger.logThread(LogLevel.INFO, this, "END", "Thread \"" + getName() + "\" cancelled.", null); } return; @@ -405,7 +461,7 @@ public abstract class JobThread extends Thread { if (lastError != null){ // Log the error: LogLevel logLevel = (lastError.getCause() != null && lastError.getCause() instanceof Error) ? LogLevel.FATAL : LogLevel.ERROR; - logger.logThread(logLevel, this, "END", "Thread \"" + getId() + "\" ended with an error.", lastError); + logger.logThread(logLevel, this, "END", "Thread \"" + getName() + "\" ended with an error.", lastError); // Set the error into the job: try{ setError(lastError); @@ -420,7 +476,7 @@ public abstract class JobThread extends Thread { } } }else - logger.logThread(LogLevel.INFO, this, "END", "Thread \"" + getId() + "\" successfully ended.", null); + logger.logThread(LogLevel.INFO, this, "END", "Thread \"" + getName() + "\" successfully ended.", null); } } } diff --git a/src/uws/service/actions/GetJobParam.java b/src/uws/service/actions/GetJobParam.java index 0df5a2b..e261a92 100644 --- a/src/uws/service/actions/GetJobParam.java +++ b/src/uws/service/actions/GetJobParam.java @@ -49,7 +49,7 @@ import uws.service.log.UWSLog.LogLevel; * The serializer is choosen in function of the HTTP Accept header.</p> * * @author Grégory Mantelet (CDS;ARI) - * @version 4.1 (08/2014) + * @version 4.1 (09/2014) */ public class GetJobParam extends UWSAction { private static final long serialVersionUID = 1L; @@ -137,7 +137,7 @@ public class GetJobParam extends UWSAction { InputStream input = null; try{ input = uws.getFileManager().getErrorInput(error, job); - UWSToolBox.write(input, "text/plain", uws.getFileManager().getErrorSize(error, job), response); + UWSToolBox.write(input, getUWS().getErrorWriter().getErrorDetailsMIMEType(), uws.getFileManager().getErrorSize(error, job), response); }catch(IOException ioe){ getLogger().logUWS(LogLevel.ERROR, error, "GET_ERROR", "Can not read the details of the error summary of the job \"" + job.getJobId() + "\"!", ioe); throw new UWSException(UWSException.INTERNAL_SERVER_ERROR, ioe, "Can not read the error details (job ID: " + job.getJobId() + ")."); diff --git a/src/uws/service/error/DefaultUWSErrorWriter.java b/src/uws/service/error/DefaultUWSErrorWriter.java index 50386f4..70c5135 100644 --- a/src/uws/service/error/DefaultUWSErrorWriter.java +++ b/src/uws/service/error/DefaultUWSErrorWriter.java @@ -21,6 +21,7 @@ package uws.service.error; */ import java.io.IOException; +import java.io.OutputStream; import java.io.PrintWriter; import java.util.ArrayList; @@ -33,7 +34,10 @@ import org.json.JSONWriter; import tap.TAPException; import uws.AcceptHeader; import uws.UWSException; +import uws.UWSToolBox; +import uws.job.ErrorSummary; import uws.job.ErrorType; +import uws.job.UWSJob; import uws.job.serializer.UWSSerializer; import uws.job.user.JobOwner; import uws.service.log.UWSLog; @@ -113,6 +117,16 @@ public class DefaultUWSErrorWriter implements ServiceErrorWriter { formatError(message, type, httpErrorCode, reqID, action, user, response, (request != null) ? request.getHeader("Accept") : null); } + @Override + public void writeError(Throwable t, ErrorSummary error, UWSJob job, OutputStream output) throws IOException{ + UWSToolBox.writeErrorFile((t instanceof Exception) ? (Exception)t : new UWSException(t), error, job, output); + } + + @Override + public String getErrorDetailsMIMEType(){ + return "text/plain"; + } + /** * Parses the header "Accept", splits it in a list of MIME type and compare each one to each managed formats ({@link #managedFormats}). * If there is a match (not case sensitive), return the corresponding managed format immediately. diff --git a/src/uws/service/error/ServiceErrorWriter.java b/src/uws/service/error/ServiceErrorWriter.java index 42b9a08..965e171 100644 --- a/src/uws/service/error/ServiceErrorWriter.java +++ b/src/uws/service/error/ServiceErrorWriter.java @@ -21,15 +21,18 @@ package uws.service.error; */ import java.io.IOException; +import java.io.OutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import uws.job.ErrorSummary; import uws.job.ErrorType; +import uws.job.UWSJob; import uws.job.user.JobOwner; /** - * Let's writing/formatting any Exception/Throwable in a {@link HttpServletResponse}. + * Let's writing/formatting any Exception/Throwable in an {@link HttpServletResponse} or in an error summary. * * @author Grégory Mantelet (CDS;ARI) * @version 4.1 (09/2014) @@ -37,7 +40,7 @@ import uws.job.user.JobOwner; public interface ServiceErrorWriter { /** - * <p>Writes the given exception in the given response.</p> + * <p>Write the given exception in the given response.</p> * * <p><i>Note: * If this function is called without at least an exception and an HTTP response, nothing should be done. @@ -56,7 +59,7 @@ public interface ServiceErrorWriter { public void writeError(final Throwable t, final HttpServletResponse response, final HttpServletRequest request, final String reqID, final JobOwner user, final String action) throws IOException; /** - * <p>Writes the described error in the given response.</p> + * <p>Write the described error in the given response.</p> * * <p><i>Note: * If this function is called without at least a message and an HTTP response, nothing should be done. @@ -76,4 +79,45 @@ public interface ServiceErrorWriter { */ public void writeError(final String message, final ErrorType type, final int httpErrorCode, final HttpServletResponse response, final HttpServletRequest request, final String reqID, final JobOwner user, final String action) throws IOException; + /** + * <p>Write the given error in the given output stream.</p> + * + * <p> + * This function is used only for the error summary of a job (that's to say to report in the + * ../error/details parameter any error which occurs while executing a job). + * </p> + * + * <p><i><b>Important note:</b> + * The error details written in the given output MUST always have the same MIME type. + * This latter MUST be returned by {@link #getErrorDetailsMIMEType()}. + * </i></p> + * + * @param t Error to write. If <i>error</i> is not null, it will be displayed instead of the message of this throwable. + * @param error Summary of the error. It may particularly contain a message different from the one of the given exception. In this case, it will displayed instead of the exception's message. + * @param job The job which fails. + * @param output Stream in which the error must be written. + * + * @throws IOException If there an error while writing the error in the given stream. + * + * @see #getErrorDetailsMIMEType() + * + * @since 4.1 + */ + public void writeError(final Throwable t, final ErrorSummary error, final UWSJob job, final OutputStream output) throws IOException; + + /** + * <p>Get the MIME type of the error details written by {@link #writeError(UWSJob, Throwable, ErrorSummary, OutputStream)} in the error summary.</p> + * + * <p><i><b>Important note:</b> + * If NULL is returned, the MIME type will be considered as <i>text/plain</i>. + * </i></p> + * + * @return MIME type of the error details document. If NULL, it will be considered as text/plain. + * + * @see #writeError(UWSJob, Throwable, ErrorSummary, OutputStream) + * + * @since 4.1 + */ + public String getErrorDetailsMIMEType(); + } -- GitLab