From b1a279987ccd69091cf4d5f96962d9cfa64568f2 Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Mon, 9 Feb 2015 16:28:34 +0100
Subject: [PATCH] [TAP] Add support for user identification in the
 configuration file. Besides, each time a property expected as an integer is
 not an integer, an exception is thrown.

---
 src/tap/config/DefaultServiceConnection.java  | 53 ++++++++++----
 src/tap/config/DefaultTAPFactory.java         |  4 +-
 src/tap/config/TAPConfiguration.java          |  5 +-
 src/tap/config/tap_configuration_file.html    | 16 +++++
 src/tap/config/tap_full.properties            | 11 +++
 .../config/TestDefaultServiceConnection.java  | 69 +++++++++++++++++--
 6 files changed, 140 insertions(+), 18 deletions(-)

diff --git a/src/tap/config/DefaultServiceConnection.java b/src/tap/config/DefaultServiceConnection.java
index c6aa84c..7dfcf6f 100644
--- a/src/tap/config/DefaultServiceConnection.java
+++ b/src/tap/config/DefaultServiceConnection.java
@@ -26,6 +26,7 @@ import static tap.config.TAPConfiguration.KEY_PROVIDER_NAME;
 import static tap.config.TAPConfiguration.KEY_SERVICE_DESCRIPTION;
 import static tap.config.TAPConfiguration.KEY_UPLOAD_ENABLED;
 import static tap.config.TAPConfiguration.KEY_UPLOAD_MAX_FILE_SIZE;
+import static tap.config.TAPConfiguration.KEY_USER_IDENTIFIER;
 import static tap.config.TAPConfiguration.VALUE_CSV;
 import static tap.config.TAPConfiguration.VALUE_DB;
 import static tap.config.TAPConfiguration.VALUE_JSON;
@@ -94,6 +95,8 @@ public final class DefaultServiceConnection implements ServiceConnection {
 	private LimitUnit[] uploadLimitTypes = new LimitUnit[2];
 	private int maxUploadSize = DEFAULT_UPLOAD_MAX_FILE_SIZE;
 
+	private UserIdentifier userIdentifier = null;
+
 	private final Collection<FunctionDef> udfs = new ArrayList<FunctionDef>(0);
 
 	public DefaultServiceConnection(final Properties tapConfig) throws NullPointerException, TAPException, UWSException{
@@ -133,7 +136,10 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		// set the maximum upload file size:
 		initMaxUploadSize(tapConfig);
 
-		// 8. MAKE THE SERVICE AVAILABLE:
+		// 8. SET A USER IDENTIFIER:
+		initUserIdentifier(tapConfig);
+
+		// 9. MAKE THE SERVICE AVAILABLE:
 		setAvailable(true, "TAP service available.");
 	}
 
@@ -235,19 +241,18 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		return metadata;
 	}
 
-	private void initMaxAsyncJobs(final Properties tapConfig){
+	private void initMaxAsyncJobs(final Properties tapConfig) throws TAPException{
 		// Get the property value:
 		String propValue = getProperty(tapConfig, KEY_MAX_ASYNC_JOBS);
 		try{
 			// If a value is provided, cast it into an integer and set the attribute:
 			maxAsyncJobs = (propValue == null) ? DEFAULT_MAX_ASYNC_JOBS : Integer.parseInt(propValue);
 		}catch(NumberFormatException nfe){
-			// If the value given in the Property file is not an integer, set the default value:
-			maxAsyncJobs = DEFAULT_MAX_ASYNC_JOBS;
+			throw new TAPException("Integer expected for the property \"" + KEY_MAX_ASYNC_JOBS + "\", instead of: \"" + propValue + "\"!");
 		}
 	}
 
-	private void initRetentionPeriod(final Properties tapConfig){
+	private void initRetentionPeriod(final Properties tapConfig) throws TAPException{
 		retentionPeriod = new int[2];
 
 		// Set the default period:
@@ -255,7 +260,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		try{
 			retentionPeriod[0] = (propValue == null) ? DEFAULT_RETENTION_PERIOD : Integer.parseInt(propValue);
 		}catch(NumberFormatException nfe){
-			retentionPeriod[0] = DEFAULT_RETENTION_PERIOD;
+			throw new TAPException("Integer expected for the property \"" + KEY_DEFAULT_RETENTION_PERIOD + "\", instead of: \"" + propValue + "\"!");
 		}
 
 		// Set the maximum period:
@@ -263,7 +268,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		try{
 			retentionPeriod[1] = (propValue == null) ? DEFAULT_RETENTION_PERIOD : Integer.parseInt(propValue);
 		}catch(NumberFormatException nfe){
-			retentionPeriod[1] = DEFAULT_RETENTION_PERIOD;
+			throw new TAPException("Integer expected for the property \"" + KEY_MAX_RETENTION_PERIOD + "\", instead of: \"" + propValue + "\"!");
 		}
 
 		// The maximum period MUST be greater or equals than the default period.
@@ -272,7 +277,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 			retentionPeriod[0] = retentionPeriod[1];
 	}
 
-	private void initExecutionDuration(final Properties tapConfig){
+	private void initExecutionDuration(final Properties tapConfig) throws TAPException{
 		executionDuration = new int[2];
 
 		// Set the default duration:
@@ -280,7 +285,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		try{
 			executionDuration[0] = (propValue == null) ? DEFAULT_EXECUTION_DURATION : Integer.parseInt(propValue);
 		}catch(NumberFormatException nfe){
-			executionDuration[0] = DEFAULT_EXECUTION_DURATION;
+			throw new TAPException("Integer expected for the property \"" + KEY_DEFAULT_EXECUTION_DURATION + "\", instead of: \"" + propValue + "\"!");
 		}
 
 		// Set the maximum duration:
@@ -288,7 +293,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		try{
 			executionDuration[1] = (propValue == null) ? DEFAULT_EXECUTION_DURATION : Integer.parseInt(propValue);
 		}catch(NumberFormatException nfe){
-			executionDuration[1] = DEFAULT_EXECUTION_DURATION;
+			throw new TAPException("Integer expected for the property \"" + KEY_MAX_EXECUTION_DURATION + "\", instead of: \"" + propValue + "\"!");
 		}
 
 		// The maximum duration MUST be greater or equals than the default duration.
@@ -365,7 +370,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 					if (e instanceof TAPException)
 						throw (TAPException)e;
 					else
-						throw new TAPException("Impossible to create an OutputFormat<ResultSet> instance with the constructor (ServiceConnection<ResultSet>) of \"" + userOutputFormatClass.getName() + "\" (see the property output_add_format) for the following reason: " + e.getMessage());
+						throw new TAPException("Impossible to create an OutputFormat instance with the constructor (ServiceConnection) of \"" + userOutputFormatClass.getName() + "\" (see the property output_add_format) for the following reason: " + e.getMessage());
 				}
 			}
 			// unknown format
@@ -416,6 +421,30 @@ public final class DefaultServiceConnection implements ServiceConnection {
 		}
 	}
 
+	private void initUserIdentifier(final Properties tapConfig) throws TAPException{
+		// Get the property value:
+		String propValue = getProperty(tapConfig, KEY_USER_IDENTIFIER);
+		if (propValue == null)
+			return;
+
+		// Check the value is a class path:
+		if (!isClassPath(propValue))
+			throw new TAPException("Class path expected for the property \"" + KEY_USER_IDENTIFIER + "\", instead of: \"" + propValue + "\"!");
+
+		// Fetch the class:
+		Class<? extends UserIdentifier> c = fetchClass(propValue, KEY_USER_IDENTIFIER, UserIdentifier.class);
+
+		// Create an instance with the empty constructor:
+		try{
+			userIdentifier = c.getConstructor().newInstance();
+		}catch(Exception e){
+			if (e instanceof TAPException)
+				throw (TAPException)e;
+			else
+				throw new TAPException("Impossible to create a UserIdentifier instance with the empty constructor of \"" + c.getName() + "\" (see the property user_identifier) for the following reason: " + e.getMessage());
+		}
+	}
+
 	@Override
 	public String getProviderName(){
 		return providerName;
@@ -629,7 +658,7 @@ public final class DefaultServiceConnection implements ServiceConnection {
 
 	@Override
 	public UserIdentifier getUserIdentifier(){
-		return null;	// NO USER IDENTIFICATION
+		return userIdentifier;
 	}
 
 	@Override
diff --git a/src/tap/config/DefaultTAPFactory.java b/src/tap/config/DefaultTAPFactory.java
index 399c891..1242917 100644
--- a/src/tap/config/DefaultTAPFactory.java
+++ b/src/tap/config/DefaultTAPFactory.java
@@ -112,7 +112,9 @@ public final class DefaultTAPFactory extends AbstractTAPFactory {
 				backupFrequency = Long.parseLong(propValue);
 				if (backupFrequency > 0)
 					isTime = true;
-			}catch(NumberFormatException nfe){}
+			}catch(NumberFormatException nfe){
+				throw new TAPException("Long expected for the property \"" + KEY_BACKUP_FREQUENCY + "\", instead of: \"" + propValue + "\"!");
+			}
 		}
 		// if the value was not a valid numeric time period, try to identify the different textual options:
 		if (!isTime){
diff --git a/src/tap/config/TAPConfiguration.java b/src/tap/config/TAPConfiguration.java
index c6522b2..8cb44b6 100644
--- a/src/tap/config/TAPConfiguration.java
+++ b/src/tap/config/TAPConfiguration.java
@@ -83,6 +83,9 @@ public final class TAPConfiguration {
 	public final static String KEY_DEFAULT_OUTPUT_LIMIT = "output_default_limit";
 	public final static String KEY_MAX_OUTPUT_LIMIT = "output_max_limit";
 
+	/* USER IDENTIFICATION */
+	public final static String KEY_USER_IDENTIFIER = "user_identifier";
+
 	/**
 	 * <p>Read the asked property from the given Properties object.</p>
 	 * <ul>
@@ -194,7 +197,7 @@ public final class TAPConfiguration {
 					numValue = Integer.parseInt(value.substring(0, i + 1));
 					break;
 				}catch(NumberFormatException nfe){
-					throw new TAPException("Numeric value expected for the property " + propertyName + " for the substring \"" + value.substring(0, i + 1) + "\" of the whole value: \"" + value + "\"!");
+					throw new TAPException("Integer expected for the property " + propertyName + " for the substring \"" + value.substring(0, i + 1) + "\" of the whole value: \"" + value + "\"!");
 				}
 			}
 			// if a character, store it for later processing:
diff --git a/src/tap/config/tap_configuration_file.html b/src/tap/config/tap_configuration_file.html
index 84fed73..429d2fd 100644
--- a/src/tap/config/tap_configuration_file.html
+++ b/src/tap/config/tap_configuration_file.html
@@ -438,6 +438,22 @@
 				</td>
 				<td><ul><li>2147483647B <em>(default)</em></li><li>2MB</li></ul></td>
 			</tr>
+			
+			<tr><td colspan="5">User identification</td></tr>
+			<tr>
+				<td class="done">user_identifier</td>
+				<td></td>
+				<td>text</td>
+				<td>
+					<p>Class to use in order to identify a user of the TAP service. The same instance of this class will be used for every request sent to the service.</p>
+					<p>
+						The value of this property MUST be a class path (with brackets: {...}) toward a class implementing the interface uws.service.UserIdentifier.
+						This class MUST have one of its constructors with no parameter.
+					</p>
+					<p><em>By default, no identification is performed ; all users are then anonymous and their jobs can be seen by everybody.</em></p>
+				</td>
+				<td>{apackage.FooUserIdentifier}</td>
+			</tr>
 		</table>
 	</body>
 </html>
\ No newline at end of file
diff --git a/src/tap/config/tap_full.properties b/src/tap/config/tap_full.properties
index b4b147b..09a2a82 100644
--- a/src/tap/config/tap_full.properties
+++ b/src/tap/config/tap_full.properties
@@ -224,3 +224,14 @@ upload_max_db_limit = 0
 # In function of the chosen upload_max_db_limit type, upload_max_file_size should be greater in order to figure out the metadata part.
 # By default, there is no restriction: upload_max_file_size=0
 upload_max_file_size = 0
+
+#######################
+# USER IDENTIFICATION #
+#######################
+
+# [OPTIONAL]
+# Class to use in order to identify a user of the TAP service. The same instance of this class will be used for every request sent to the service.
+# The value of this property MUST be a class path (with brackets: {...}) toward a class implementing the interface uws.service.UserIdentifier.
+# This class MUST have one of its constructors with no parameter.
+# By default, no identification is performed ; all users are then anonymous and their jobs can be seen by everybody.
+user_identifier = 
diff --git a/test/tap/config/TestDefaultServiceConnection.java b/test/tap/config/TestDefaultServiceConnection.java
index e45239d..54c3e95 100644
--- a/test/tap/config/TestDefaultServiceConnection.java
+++ b/test/tap/config/TestDefaultServiceConnection.java
@@ -2,6 +2,7 @@ package tap.config;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static tap.config.TAPConfiguration.DEFAULT_MAX_ASYNC_JOBS;
@@ -12,6 +13,7 @@ import static tap.config.TAPConfiguration.KEY_MAX_OUTPUT_LIMIT;
 import static tap.config.TAPConfiguration.KEY_METADATA;
 import static tap.config.TAPConfiguration.KEY_METADATA_FILE;
 import static tap.config.TAPConfiguration.KEY_OUTPUT_FORMATS;
+import static tap.config.TAPConfiguration.KEY_USER_IDENTIFIER;
 import static tap.config.TAPConfiguration.VALUE_CSV;
 import static tap.config.TAPConfiguration.VALUE_DB;
 import static tap.config.TAPConfiguration.VALUE_JSON;
@@ -22,8 +24,11 @@ import static tap.config.TAPConfiguration.VALUE_XML;
 
 import java.io.File;
 import java.io.PrintWriter;
+import java.util.Map;
 import java.util.Properties;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.junit.Before;
 import org.junit.Test;
 
@@ -31,6 +36,10 @@ import tap.ServiceConnection;
 import tap.ServiceConnection.LimitUnit;
 import tap.TAPException;
 import uws.UWSException;
+import uws.job.user.DefaultJobOwner;
+import uws.job.user.JobOwner;
+import uws.service.UWSUrl;
+import uws.service.UserIdentifier;
 import uws.service.file.LocalUWSFileManager;
 
 public class TestDefaultServiceConnection {
@@ -43,7 +52,7 @@ public class TestDefaultServiceConnection {
 			badSVFormat2Prop, unknownFormatProp, maxAsyncProp,
 			negativeMaxAsyncProp, notIntMaxAsyncProp, defaultOutputLimitProp,
 			maxOutputLimitProp, bothOutputLimitGoodProp,
-			bothOutputLimitBadProp;
+			bothOutputLimitBadProp, userIdentProp, notClassPathUserIdentProp;
 
 	@Before
 	public void setUp() throws Exception{
@@ -111,6 +120,12 @@ public class TestDefaultServiceConnection {
 		bothOutputLimitBadProp = (Properties)validProp.clone();
 		bothOutputLimitBadProp.setProperty(KEY_DEFAULT_OUTPUT_LIMIT, "1000");
 		bothOutputLimitBadProp.setProperty(KEY_MAX_OUTPUT_LIMIT, "100");
+
+		userIdentProp = (Properties)validProp.clone();
+		userIdentProp.setProperty(KEY_USER_IDENTIFIER, "{tap.config.TestDefaultServiceConnection$UserIdentifierTest}");
+
+		notClassPathUserIdentProp = (Properties)validProp.clone();
+		notClassPathUserIdentProp.setProperty(KEY_USER_IDENTIFIER, "foo");
 	}
 
 	/**
@@ -151,6 +166,7 @@ public class TestDefaultServiceConnection {
 			assertEquals(DEFAULT_MAX_ASYNC_JOBS, connection.getNbMaxAsyncJobs());
 			assertTrue(connection.getRetentionPeriod()[0] <= connection.getRetentionPeriod()[1]);
 			assertTrue(connection.getExecutionDuration()[0] <= connection.getExecutionDuration()[1]);
+			assertNull(connection.getUserIdentifier());
 
 			// finally, save metadata in an XML file for the other tests:
 			writer = new PrintWriter(new File(XML_FILE));
@@ -178,6 +194,7 @@ public class TestDefaultServiceConnection {
 			assertEquals(DEFAULT_MAX_ASYNC_JOBS, connection.getNbMaxAsyncJobs());
 			assertTrue(connection.getRetentionPeriod()[0] <= connection.getRetentionPeriod()[1]);
 			assertTrue(connection.getExecutionDuration()[0] <= connection.getExecutionDuration()[1]);
+			assertNull(connection.getUserIdentifier());
 		}catch(Exception e){
 			e.printStackTrace();
 			fail("This MUST have succeeded because the property file is valid! \nCaught exception: " + getPertinentMessage(e));
@@ -318,10 +335,11 @@ public class TestDefaultServiceConnection {
 
 		// A not integer value for max_async_jobs:
 		try{
-			ServiceConnection connection = new DefaultServiceConnection(notIntMaxAsyncProp);
-			assertEquals(DEFAULT_MAX_ASYNC_JOBS, connection.getNbMaxAsyncJobs());
+			new DefaultServiceConnection(notIntMaxAsyncProp);
+			fail("This MUST have failed because a not integer value has been provided for \"" + KEY_MAX_ASYNC_JOBS + "\"!");
 		}catch(Exception e){
-			fail("This MUST have succeeded because a not integer value for max_async_jobs is considered as 'no restriction'! \nCaught exception: " + getPertinentMessage(e));
+			assertEquals(e.getClass(), TAPException.class);
+			assertEquals(e.getMessage(), "Integer expected for the property \"" + KEY_MAX_ASYNC_JOBS + "\", instead of: \"foo\"!");
 		}
 
 		// Test with no output limit specified:
@@ -376,6 +394,25 @@ public class TestDefaultServiceConnection {
 			assertEquals(e.getClass(), TAPException.class);
 			assertEquals(e.getMessage(), "The default output limit (here: 1000) MUST be less or equal to the maximum output limit (here: 100)!");
 		}
+
+		// Valid user identifier:
+		try{
+			ServiceConnection connection = new DefaultServiceConnection(userIdentProp);
+			assertNotNull(connection.getUserIdentifier());
+			assertNotNull(connection.getUserIdentifier().extractUserId(null, null));
+			assertEquals("everybody", connection.getUserIdentifier().extractUserId(null, null).getID());
+		}catch(Exception e){
+			fail("This MUST have succeeded because the class path toward the fake UserIdentifier is correct! \nCaught exception: " + getPertinentMessage(e));
+		}
+
+		// Not a class path for user_identifier:
+		try{
+			new DefaultServiceConnection(notClassPathUserIdentProp);
+			fail("This MUST have failed because the user_identifier value is not a class path!");
+		}catch(Exception e){
+			assertEquals(e.getClass(), TAPException.class);
+			assertEquals(e.getMessage(), "Class path expected for the property \"" + KEY_USER_IDENTIFIER + "\", instead of: \"foo\"!");
+		}
 	}
 
 	public static final String getPertinentMessage(final Exception ex){
@@ -395,4 +432,28 @@ public class TestDefaultServiceConnection {
 		}
 	}
 
+	/**
+	 * A UserIdentifier which always return the same user...that's to say, all users are in a way still anonymous :-)
+	 * This class is only for test purpose.
+	 * 
+	 * @author Gr&eacute;gory Mantelet (ARI)
+	 * @version 02/2015
+	 */
+	public static class UserIdentifierTest implements UserIdentifier {
+		private static final long serialVersionUID = 1L;
+
+		private final JobOwner everybody = new DefaultJobOwner("everybody");
+
+		@Override
+		public JobOwner extractUserId(UWSUrl urlInterpreter, HttpServletRequest request) throws UWSException{
+			return everybody;
+		}
+
+		@Override
+		public JobOwner restoreUser(String id, String pseudo, Map<String,Object> otherData) throws UWSException{
+			return everybody;
+		}
+
+	}
+
 }
-- 
GitLab