[TAP,UWS] Fix a problem with the format of the error details returned by the...

[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

@@ -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;
 	}
 
 	/**

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&eacute;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;
 	}

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 !");
 		}

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";
+	}
+
 }

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)

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&eacute;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);
 
+			// 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);
 		}
 	}
 }

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&eacute;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() + ").");

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.

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();
+
 }