From 5baff84efb3987a5cc148ca2aae8e87f850709c9 Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Thu, 9 Mar 2017 20:45:43 +0100
Subject: [PATCH] [TAP] Fix incorrect abortion handling in SYNChronous mode. It
 is also now recommended to make DBConnection.executeQuery(ADQLQuery) return
 NULL if the query has been aborted (indeed, the DBConnection is the only one
 that can reliably know that fact). JDBCConnection has been adapted
 consequently.

---
 src/tap/ADQLExecutor.java      |  2 +-
 src/tap/TAPSyncJob.java        | 22 +++++++++++++++-------
 src/tap/db/DBConnection.java   |  6 +++---
 src/tap/db/JDBCConnection.java | 16 +++++++++++++---
 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/tap/ADQLExecutor.java b/src/tap/ADQLExecutor.java
index fe7102a..2be9b57 100644
--- a/src/tap/ADQLExecutor.java
+++ b/src/tap/ADQLExecutor.java
@@ -381,7 +381,7 @@ public class ADQLExecutor {
 			queryResult = executeADQL(adqlQuery);
 			endStep();
 
-			if (thread.isInterrupted())
+			if (queryResult == null || thread.isInterrupted())
 				throw new InterruptedException();
 
 			// 4. WRITE RESULT:
diff --git a/src/tap/TAPSyncJob.java b/src/tap/TAPSyncJob.java
index a1c3c82..abcbb59 100644
--- a/src/tap/TAPSyncJob.java
+++ b/src/tap/TAPSyncJob.java
@@ -16,7 +16,7 @@ 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-2016 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
+ * Copyright 2012-2017 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
  *                       Astronomisches Rechen Institut (ARI)
  */
 
@@ -48,7 +48,7 @@ import uws.service.log.UWSLog.LogLevel;
  * </p>
  * 
  * @author Gr&eacute;gory Mantelet (CDS;ARI)
- * @version 2.1 (09/2016)
+ * @version 2.1 (03/2017)
  */
 public class TAPSyncJob {
 
@@ -248,12 +248,12 @@ public class TAPSyncJob {
 		if (timeout && error != null && error instanceof InterruptedException){
 			// Log the timeout:
 			if (thread.isAlive())
-				service.getLogger().logTAP(LogLevel.WARNING, this, "TIME_OUT", "Time out (after " + tapParams.getExecutionDuration() + "seconds) for the synchonous job " + ID + ", but the thread can not be interrupted!", null);
+				service.getLogger().logTAP(LogLevel.WARNING, this, "TIME_OUT", "Time out (after " + tapParams.getExecutionDuration() + " seconds) for the synchonous job " + ID + ", but the thread can not be interrupted!", null);
 			else
-				service.getLogger().logTAP(LogLevel.INFO, this, "TIME_OUT", "Time out (after " + tapParams.getExecutionDuration() + "seconds) for the synchonous job " + ID + ".", null);
+				service.getLogger().logTAP(LogLevel.INFO, this, "TIME_OUT", "Time out (after " + tapParams.getExecutionDuration() + " seconds) for the synchonous job " + ID + ".", null);
 
 			// Report the timeout to the user:
-			throw new TAPException("Time out! The execution of this synchronous TAP query was limited to " + tapParams.getExecutionDuration() + "seconds. You should try again but in asynchronous execution.", UWSException.ACCEPTED_BUT_NOT_COMPLETE);
+			throw new TAPException("Time out! The execution of this synchronous TAP query was limited to " + tapParams.getExecutionDuration() + " seconds. You should try again but in asynchronous mode.", UWSException.ACCEPTED_BUT_NOT_COMPLETE);
 		}
 		// CASE: ERRORS
 		else if (!thread.isSuccess()){
@@ -283,7 +283,9 @@ public class TAPSyncJob {
 				// log the error:
 				service.getLogger().logTAP(LogLevel.FATAL, this, "END", "The following GRAVE error interrupted the execution of the synchronous job " + ID + ".", error);
 				// report the error to the user:
-				if (error instanceof Error)
+				if (error == null)
+					throw new TAPException("This query (" + ID + ") stopped unexpectedly without any result! No error/exception has been caught and no execution report has been registered. You should contact the service administrator to investigate this.", UWSException.INTERNAL_SERVER_ERROR);
+				else if (error instanceof Error)
 					throw (Error)error;
 				else
 					throw new TAPException(error);
@@ -303,7 +305,7 @@ public class TAPSyncJob {
 	 * </p>
 	 * 
 	 * @author Gr&eacute;gory Mantelet (CDS;ARI)
-	 * @version 2.0 (04/2015)
+	 * @version 2.1 (03/2017)
 	 */
 	protected class SyncThread extends Thread {
 
@@ -368,6 +370,12 @@ public class TAPSyncJob {
 			return report;
 		}
 
+		@Override
+		public void interrupt(){
+			super.interrupt();
+			executor.cancelQuery();
+		}
+
 		@Override
 		public void run(){
 			// Log the start of this thread:
diff --git a/src/tap/db/DBConnection.java b/src/tap/db/DBConnection.java
index af21338..d46e98f 100644
--- a/src/tap/db/DBConnection.java
+++ b/src/tap/db/DBConnection.java
@@ -16,7 +16,7 @@ package tap.db;
  * 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-2016 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
+ * Copyright 2012-2017 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
  *                       Astronomisches Rechen Institut (ARI)
  */
 
@@ -45,7 +45,7 @@ import tap.metadata.TAPTable;
  * </p>
  * 
  * @author Gr&eacute;gory Mantelet (CDS;ARI)
- * @version 2.1 (04/2016)
+ * @version 2.1 (03/2017)
  */
 public interface DBConnection {
 
@@ -237,7 +237,7 @@ public interface DBConnection {
 	 * 
 	 * @param adqlQuery	ADQL query to execute.
 	 * 
-	 * @return	The table result.
+	 * @return	The table result or NULL if the query has been aborted.
 	 * 
 	 * @throws DBException	If any errors occurs while executing the query.
 	 * 
diff --git a/src/tap/db/JDBCConnection.java b/src/tap/db/JDBCConnection.java
index 89351d9..8de20cd 100644
--- a/src/tap/db/JDBCConnection.java
+++ b/src/tap/db/JDBCConnection.java
@@ -720,6 +720,10 @@ public class JDBCConnection implements DBConnection {
 				logger.logDB(LogLevel.INFO, this, "EXECUTE", "SQL query: " + sql.replaceAll("(\t|\r?\n)+", " "), null);
 			result = stmt.executeQuery(sql);
 
+			// If the query has been aborted and no result is provided, return immediately:
+			if (cancelled)
+				return null;
+
 			// 4. Return the result through a TableIterator object:
 			if (logger != null)
 				logger.logDB(LogLevel.INFO, this, "RESULT", "Returning result (" + (supportsFetchSize ? "fetch size = " + fetchSize : "all in once") + ").", null);
@@ -731,9 +735,15 @@ public class JDBCConnection implements DBConnection {
 			// End properly the query:
 			endQuery();
 			// Propagate the exception with an appropriate error message:
-			if (ex instanceof SQLException)
-				throw new DBException("Unexpected error while executing a SQL query: " + ex.getMessage(), ex);
-			else if (ex instanceof TranslationException)
+			if (ex instanceof SQLException){
+				/* ...except if the query has been aborted:
+				 * then, it is normal to receive an SQLException
+				 * and NULL should be returned: */
+				if (isCancelled())
+					return null;
+				else
+					throw new DBException("Unexpected error while executing a SQL query: " + ex.getMessage(), ex);
+			}else if (ex instanceof TranslationException)
 				throw new DBException("Unexpected error while translating ADQL into SQL: " + ex.getMessage(), ex);
 			else if (ex instanceof DataReadException)
 				throw new DBException("Impossible to read the query result, because: " + ex.getMessage(), ex);
-- 
GitLab