From 904b6a315eb07093f434f04b126a6520b8426ab3 Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Wed, 4 Mar 2015 12:05:59 +0100
Subject: [PATCH] [TAP] Change the default value for output and upload limits ;
 only negative value must mean 'no restriction' ; 0 must be a normal allowed
 value.

---
 .../config/ConfigurableServiceConnection.java |  2 +
 src/tap/config/ConfigurableTAPFactory.java    |  2 +
 src/tap/config/TAPConfiguration.java          |  3 +-
 src/tap/config/tap_configuration_file.html    | 34 +++++++-------
 src/tap/config/tap_full.properties            | 47 +++++++++----------
 src/tap/formatter/TextFormat.java             |  2 +-
 src/tap/parameters/MaxRecController.java      |  6 +--
 src/tap/upload/Uploader.java                  |  2 +-
 test/tap/config/TestTAPConfiguration.java     |  6 ++-
 9 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/src/tap/config/ConfigurableServiceConnection.java b/src/tap/config/ConfigurableServiceConnection.java
index c444954..540f44f 100644
--- a/src/tap/config/ConfigurableServiceConnection.java
+++ b/src/tap/config/ConfigurableServiceConnection.java
@@ -651,6 +651,8 @@ public final class ConfigurableServiceConnection implements ServiceConnection {
 		if (propValue != null){
 			// ...parse the value:
 			Object[] limit = parseLimit(propValue, KEY_UPLOAD_MAX_FILE_SIZE, true);
+			if (((Integer)limit[0]).intValue() <= 0)
+				limit[0] = new Integer(TAPConfiguration.DEFAULT_UPLOAD_MAX_FILE_SIZE);
 			// ...check that the unit is correct (bytes): 
 			if (!LimitUnit.bytes.isCompatibleWith((LimitUnit)limit[1]))
 				throw new TAPException("The maximum upload file size " + KEY_UPLOAD_MAX_FILE_SIZE + " (here: " + propValue + ") can not be expressed in a unit different from bytes (B, kB, MB, GB)!");
diff --git a/src/tap/config/ConfigurableTAPFactory.java b/src/tap/config/ConfigurableTAPFactory.java
index c0faed6..2d4a99b 100644
--- a/src/tap/config/ConfigurableTAPFactory.java
+++ b/src/tap/config/ConfigurableTAPFactory.java
@@ -265,6 +265,8 @@ public final class ConfigurableTAPFactory extends AbstractTAPFactory {
 	@Override
 	public UWSBackupManager createUWSBackupManager(UWSService uws) throws TAPException{
 		try{
+			System.out.println("BACKUP FREQUENCY ? " + backupFrequency);
+			System.out.println("BACKUP BY USER ? " + backupByUser);
 			return (backupFrequency < 0) ? null : new DefaultTAPBackupManager(uws, backupByUser, backupFrequency);
 		}catch(UWSException ex){
 			throw new TAPException("Impossible to create a backup manager, because: " + ex.getMessage(), ex);
diff --git a/src/tap/config/TAPConfiguration.java b/src/tap/config/TAPConfiguration.java
index f49fa7f..330ec82 100644
--- a/src/tap/config/TAPConfiguration.java
+++ b/src/tap/config/TAPConfiguration.java
@@ -292,6 +292,7 @@ public final class TAPConfiguration {
 	 * 	Where unit is optional and should be one of the following values: r or R, B, kB, MB, GB.
 	 * 	If the unit is not specified, it is set by default to ROWS.
 	 * </p>
+	 * <p><i>Note: If the value is strictly less than 0 (whatever is the unit), the returned value will be -1.</i></p>
 	 * 
 	 * @param value				Property value (must follow the limit syntax: num_val[unit] ; ex: 20kB or 2000 (for 2000 rows)).
 	 * @param propertyName		Name of the property which specify the limit.
@@ -375,7 +376,7 @@ public final class TAPConfiguration {
 			}
 		}
 
-		return new Object[]{((numValue <= 0) ? -1 : numValue),unit};
+		return new Object[]{((numValue < 0) ? -1 : numValue),unit};
 	}
 
 }
diff --git a/src/tap/config/tap_configuration_file.html b/src/tap/config/tap_configuration_file.html
index f665039..59a6ee8 100644
--- a/src/tap/config/tap_configuration_file.html
+++ b/src/tap/config/tap_configuration_file.html
@@ -319,7 +319,7 @@
 					if the client does not provide a value for it.</p>
 					<p>The default period MUST be less or equals to the maximum retention period. If this rule is not respected, the default retention period is set immediately
 					 to the maximum retention period.</p>
-					<p>A negative or null value means there is no restriction over the default retention period: job results will be kept forever. Float values are not allowed.</p>
+					<p>A negative or null value means there is no restriction on the default retention period: job results will be kept forever. Float values are not allowed.</p>
 					<p><em>By default query results are kept forever: default_retention_period=0.</em></p></td>
 				<td>86400 <em>(1 day)</em></td>
 			</tr>
@@ -331,7 +331,7 @@
 					<p>The maximum period (in seconds) to keep query results. The prefix "max" means here that the client can not set a retention period greater than this one.</p>
 					<p>The maximum period MUST be greater or equals to the default retention period. If this rule is not respected, the default retention period is set immediately
 					 to the maximum retention period.</p>
-					<p>A negative or null value means there is no restriction over the maximum retention period: the job results will be kept forever. Float values are not allowed.</p>
+					<p>A negative or null value means there is no restriction on the maximum retention period: the job results will be kept forever. Float values are not allowed.</p>
 					<p><em>Default: <code>max_retention_period=0</code> (results kept for ever)</em></p></td>
 				<td>604800 <em>(1 week)</em></td>
 			</tr>
@@ -412,7 +412,7 @@
 				<td>integer</td>
 				<td>
 					<p>Maximum number of asynchronous jobs that can run simultaneously.</p>
-					<p>A negative or null value means there is no restriction over the number of running asynchronous jobs.</p>
+					<p>A negative or null value means there is no restriction on the number of running asynchronous jobs.</p>
 					<p><em>Default: <code>max_async_jobs=0</code> (no restriction)</em></p>
 				</td>
 				<td><ul><li>0 <em>(default)</em></li><li>10</li></ul></td>
@@ -427,7 +427,7 @@
 					<p>Default time (in milliseconds) for query execution. The prefix "default" means here that the execution duration will be this one if the client does not set one.</p>
 					<p>The default duration MUST be less or equals to the maximum execution duration. If this rule is not respected, the default execution duration is set immediately
 					 to the maximum execution duration.</p>
-					<p>A negative or null value means there is no restriction over the default execution duration: the execution could never end. Float values are not allowed.</p>
+					<p>A negative or null value means there is no restriction on the default execution duration: the execution could never end. Float values are not allowed.</p>
 					<p><em>Default: <code>default_execution_duration=0</code> (no restriction)</em></p>
 				</td>
 				<td>600000 <em>(10 minutes)</em></td>
@@ -440,7 +440,7 @@
 					<p>Maximum time (in milliseconds) for query execution. The prefix "max" means here that the client can not set a time greater than this one.</p>
 					<p>The maximum duration MUST be greater or equals to the default execution duration. If this rule is not respected, the default execution duration is set immediately
 					 to the maximum execution duration.</p>
-					<p>A negative or null value means there is no restriction over the maximum execution duration: the execution could never end. Float values are not allowed.</p>
+					<p>A negative or null value means there is no restriction on the maximum execution duration: the execution could never end. Float values are not allowed.</p>
 					<p><em>Default: <code>max_execution_duration=0</code> (no restriction)</em></p>
 				</td>
 				<td>3600000 <em>(1 hour)</em></td>
@@ -472,11 +472,11 @@
 				<td>
 					<p>Default limit for the result output. The prefix "default" means here that this value will be set if the client does not provide one.</p>
 					<p>This limit can be expressed in only one unit: rows.</p>
-					<p>A negative or null value means there is no restriction over this limit. Float values are not allowed.</p>
+					<p>A negative value means there is no restriction on this limit. Float values are not allowed.</p>
 					<p>Obviously this limit MUST be less or equal than output_max_limit.</p>
-					<p><em>Default: <code>output_default_limit=0</code> (no restriction)</em></p>
+					<p><em>Default: <code>output_default_limit=-1</code> (no restriction)</em></p>
 				</td>
-				<td><ul><li>0 <em>(default)</em></li><li>20</li><li>20r</li><li>20R</li></ul></td>
+				<td><ul><li>-1 <em>(default)</em></li><li>20</li><li>20r</li><li>20R</li></ul></td>
 			</tr>
 			<tr class="optional">
 				<td class="done">output_max_limit</td>
@@ -485,11 +485,11 @@
 				<td>
 					<p>Maximum limit for the result output. The prefix "max" means here that the client can not set a limit greater than this one.</p>
 					<p>This limit can be expressed in only one unit: rows.</p>
-					<p>A negative or null value means there is no restriction over this limit. Float values are not allowed.</p>
+					<p>A negative value means there is no restriction on this limit. Float values are not allowed.</p>
 					<p>Obviously this limit MUST be greater or equal than output_default_limit.</p>
-					<p><em>Default: <code>output_max_limit=0</code> (no restriction)</em></p>
+					<p><em>Default: <code>output_max_limit=-1</code> (no restriction)</em></p>
 				</td>
-				<td><ul><li>0 <em>(default)</em></li><li>1000</li><li>10000r</li><li>10000R</li></ul></td>
+				<td><ul><li>-1 <em>(default)</em></li><li>1000</li><li>10000r</li><li>10000R</li></ul></td>
 			</tr>
 			
 			<tr><td colspan="5">Upload</td></tr>
@@ -513,12 +513,12 @@
 					<p>This limit can be expressed with 2 types: rows or bytes. For rows, you just have to suffix the value by a "r" (upper- or lower-case)
 					or by nothing (by default, nothing will mean "rows"). For bytes, you have to suffix the numeric value by "B", "kB", "MB" or "GB".
 					Here, unit is case sensitive. No other storage unit is allowed.</p>
-					<p>A negative or null value means there is no restriction over this limit. Float values are not allowed.</p>
+					<p>A negative value means there is no restriction on this limit. Float values are not allowed.</p>
 					<p><b>Warning!</b> Obviously this limit MUST be less or equal than upload_max_db_limit, and MUST be of the same type as it.
 					If the chosen type is rows, this limit MUST also be strictly less than upload_max_file_size.</p>
-					<p><em>Default: <code>upload_default_db_limit=0</code> (no restriction)</em></p>
+					<p><em>Default: <code>upload_default_db_limit=-1</code> (no restriction)</em></p>
 				</td>
-				<td><ul><li>0 <em>(default)</em></li><li>20</li><li>20r</li><li>20R</li><li>200kB</li></ul></td>
+				<td><ul><li>-1 <em>(default)</em></li><li>20</li><li>20r</li><li>20R</li><li>200kB</li></ul></td>
 			</tr>
 			<tr class="optional">
 				<td class="done">upload_max_db_limit</td>
@@ -529,12 +529,12 @@
 					<p>This limit can be expressed with 2 types: rows or bytes. For rows, you just have to suffix the value by a "r" (upper- or lower-case),
 					with nothing (by default, nothing will mean "rows"). For bytes, you have to suffix the numeric value by "B", "kB", "MB" or "GB".
 					Here, unit is case sensitive. No other storage unit is allowed.</p>
-					<p>A negative or null value means there is no restriction over this limit. Float values are not allowed.</p>
+					<p>A negative value means there is no restriction on this limit. Float values are not allowed.</p>
 					<p><b>Warning!</b> Obviously this limit MUST be greater or equal than upload_default_db_limit, and MUST be of the same type as it.
 					If the chosen type is rows, this limit MUST also be strictly less than upload_max_file_size.</p>
-					<p><em>Default: <code>upload_max_db_limit=0</code> (no restriction)</em></p>
+					<p><em>Default: <code>upload_max_db_limit=-1</code> (no restriction)</em></p>
 				</td>
-				<td><ul><li>0 <em>(default)</em></li><li>10000</li><li>10000r</li><li>10000R</li><li>1MB</li></ul></td>
+				<td><ul><li>-1 <em>(default)</em></li><li>10000</li><li>10000r</li><li>10000R</li><li>1MB</li></ul></td>
 			</tr>
 			<tr class="optional">
 				<td class="done">upload_max_file_size</td>
diff --git a/src/tap/config/tap_full.properties b/src/tap/config/tap_full.properties
index db0930d..52bfd3b 100644
--- a/src/tap/config/tap_full.properties
+++ b/src/tap/config/tap_full.properties
@@ -2,7 +2,7 @@
 #             FULL TAP CONFIGURATION FILE                #
 #                                                        #
 # TAP Version: 2.0                                       #
-# Date: 27 Feb. 2015                                     #
+# Date: 04 Mars 2015                                     #
 # Author: Gregory Mantelet (ARI)                         #
 #                                                        #
 ########################################################## 
@@ -178,7 +178,7 @@ group_user_dir = true
 # The default period MUST be less or equals to the maximum retention period. If this rule is not respected, the default retention period is set
 # immediately to the maximum retention period.
 # 
-# A negative or null value means there is no restriction over the default retention period: job results will be kept forever. Float values are not allowed.
+# A negative or null value means there is no restriction on the default retention period: job results will be kept forever. Float values are not allowed.
 # 
 # Default: 0 (results kept forever).
 default_retention_period = 0
@@ -191,7 +191,7 @@ default_retention_period = 0
 # The maximum period MUST be greater or equals to the default retention period. If this rule is not respected, the default retention period is set
 # immediately to the maximum retention period.
 # 
-# A negative or null value means there is no restriction over the maximum retention period: the job results will be kept forever. Float values are not allowed.
+# A negative or null value means there is no restriction on the maximum retention period: the job results will be kept forever. Float values are not allowed.
 # 
 # Default: 0 (results kept forever).
 max_retention_period = 0
@@ -258,7 +258,7 @@ backup_mode = user
 # [OPTIONAL]
 # Maximum number of asynchronous jobs that can run simultaneously.
 # 
-# A negative or null value means there is no restriction over the number of running asynchronous jobs.
+# A negative or null value means there is no restriction on the number of running asynchronous jobs.
 # 
 # Default: there is no restriction => max_async_jobs=0.
 max_async_jobs = 0
@@ -275,7 +275,7 @@ max_async_jobs = 0
 # The default duration MUST be less or equals to the maximum execution duration. If this rule is not respected, the default execution duration is set
 # immediately to the maximum execution duration.
 # 
-# A negative or null value means there is no restriction over the default execution duration: the execution could never end. Float values are not allowed.
+# A negative or null value means there is no restriction on the default execution duration: the execution could never end. Float values are not allowed.
 # 
 # Default: there is no restriction => default_execution_duration=0.
 default_execution_duration = 0
@@ -288,7 +288,7 @@ default_execution_duration = 0
 # The maximum duration MUST be greater or equals to the default execution duration. If this rule is not respected, the default execution duration is set
 # immediately to the maximum execution duration.
 # 
-# A negative or null value means there is no restriction over the maximum execution duration: the execution could never end. Float values are not allowed.
+# A negative or null value means there is no restriction on the maximum execution duration: the execution could never end. Float values are not allowed.
 # 
 # Default: there is no restriction => max_execution_duration=0.
 max_execution_duration = 0
@@ -325,24 +325,24 @@ output_add_formats = ALL
 # 
 # This limit can be expressed in only one unit: rows.
 # 
-# A negative or null value means there is no restriction over this limit. Float values are not allowed.
+# A negative value means there is no restriction on this limit. Float values are not allowed.
 # 
 # Obviously this limit MUST be less or equal than output_max_limit.
 # 
-# Default: there is no restriction: output_default_limit=0
-output_default_limit = 0
+# Default: there is no restriction: output_default_limit=-1
+output_default_limit = -1
 
 # [OPTIONAL]
 # Maximum limit for the result output. The prefix "max" means here that the client can not set a limit greater than this one.
 # 
 # This limit can be expressed in only one unit: rows.
 # 
-# A negative or null value means there is no restriction over this limit. Float values are not allowed.
+# A negative value means there is no restriction on this limit. Float values are not allowed.
 # 
 # Obviously this limit MUST be greater or equal than output_default_limit.
 # 
-# Default: there is no restriction => output_max_limit=0
-output_max_limit = 0
+# Default: there is no restriction => output_max_limit=-1
+output_max_limit = -1
 
 ##########
 # UPLOAD #
@@ -366,12 +366,12 @@ upload_enabled = false
 # with nothing (by default, nothing will mean "rows"). For bytes, you have to suffix the numeric value by "b", "kb", "Mb" or "Gb". Here,
 # unit is case sensitive (except for the last character: "b"). No other storage unit is allowed.
 # 
-# A negative or null value means there is no restriction over this limit. Float values are not allowed.
+# A negative value means there is no restriction on this limit. Float values are not allowed.
 # 
 # Obviously this limit MUST be less or equal than upload_max_db_limit.
 # 
-# Default: there is no restriction: upload_default_db_limit=0
-upload_default_db_limit = 0
+# Default: there is no restriction: upload_default_db_limit=-1
+upload_default_db_limit = -1
 
 # [OPTIONAL]
 # Maximum limit for the number of uploaded records that can be inserted inside the database.
@@ -382,25 +382,24 @@ upload_default_db_limit = 0
 # with nothing (by default, nothing will mean "rows"). For bytes, you have to suffix the numeric value by "b", "kb", "Mb" or "Gb". Here,
 # unit is case sensitive (except for the last character: "b"). No other storage unit is allowed.
 # 
-# A negative or null value means there is no restriction over this limit. Float values are not allowed.
+# A negative value means there is no restriction on this limit. Float values are not allowed.
 # 
 # Obviously this limit MUST be greater or equal than upload_default_db_limit.
 # 
-# Default: there is no restriction: upload_max_db_limit=0
-upload_max_db_limit = 0
+# Default: there is no restriction: upload_max_db_limit=-1
+upload_max_db_limit = -1
 
 # [OPTIONAL]
 # Maximum allowed size for the uploaded file.
 # 
-# This limit MUST be expressed in bytes. Thus, you have to suffix the numeric value by "b", "kb", "Mb" or "Gb". Here, unit is case sensitive
-# (except for the last character: "b"). No other storage unit is allowed.
+# This limit MUST be expressed in bytes. Thus, you have to suffix the numeric value by "B", "kB", "MB" or "GB". Here, unit is case sensitive. No other storage unit is allowed.
 # 
-# A negative or null value means there is no restriction over this limit. Float values are not allowed.
+# Warning! When the upload is enabled, there must be a maximum file size. Here, no "unlimited" value is possible ; 0 and any negative value are not allowed.
 # 
-# 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.
+# Warning! In function of the chosen upload_max_db_limit type, upload_max_file_size MUST be greater in order to figure out the file metadata part.
 # 
-# Default: there is no restriction => upload_max_file_size=0
-upload_max_file_size = 0
+# Default: upload_max_file_size=2147483647B (~2GB ; maximum possible value)
+upload_max_file_size = 2147483647B
 
 #######################
 # USER IDENTIFICATION #
diff --git a/src/tap/formatter/TextFormat.java b/src/tap/formatter/TextFormat.java
index 5ead35d..11b5939 100644
--- a/src/tap/formatter/TextFormat.java
+++ b/src/tap/formatter/TextFormat.java
@@ -223,6 +223,6 @@ public class TextFormat implements OutputFormat {
 	protected void writeFieldValue(final Object value, final DBColumn tapCol, final StringBuffer line){
 		Object obj = value;
 		if (obj != null)
-			line.append(obj.toString());
+			line.append('"').append(obj.toString()).append('"');
 	}
 }
diff --git a/src/tap/parameters/MaxRecController.java b/src/tap/parameters/MaxRecController.java
index da3b94b..9424a0a 100644
--- a/src/tap/parameters/MaxRecController.java
+++ b/src/tap/parameters/MaxRecController.java
@@ -49,7 +49,7 @@ import uws.job.parameters.InputParamController;
  * </ul>
  * 
  * @author Gr&eacute;gory Mantelet (CDS;ARI)
- * @version 2.0 (11/2014)
+ * @version 2.0 (03/2015)
  */
 public class MaxRecController implements InputParamController {
 
@@ -73,7 +73,7 @@ public class MaxRecController implements InputParamController {
 		// Get the default output limit:
 		int defaultLimit = TAPJob.UNLIMITED_MAX_REC;
 		if (service.getOutputLimit() != null && service.getOutputLimit().length >= 2 && service.getOutputLimitType() != null && service.getOutputLimitType().length == service.getOutputLimit().length){
-			if (service.getOutputLimit()[0] > 0 && service.getOutputLimitType()[0] == LimitUnit.rows)
+			if (service.getOutputLimit()[0] >= 0 && service.getOutputLimitType()[0] == LimitUnit.rows)
 				defaultLimit = service.getOutputLimit()[0];
 		}
 
@@ -92,7 +92,7 @@ public class MaxRecController implements InputParamController {
 	public final int getMaxOutputLimit(){
 		// If a maximum output limit is set by the TAP service connection, return it:
 		if (service.getOutputLimit() != null && service.getOutputLimit().length >= 2 && service.getOutputLimitType() != null && service.getOutputLimitType().length == service.getOutputLimit().length){
-			if (service.getOutputLimit()[1] > 0 && service.getOutputLimitType()[1] == LimitUnit.rows)
+			if (service.getOutputLimit()[1] >= 0 && service.getOutputLimitType()[1] == LimitUnit.rows)
 				return service.getOutputLimit()[1];
 		}
 		// Otherwise, there is no limit:
diff --git a/src/tap/upload/Uploader.java b/src/tap/upload/Uploader.java
index 994552b..b2f7c69 100644
--- a/src/tap/upload/Uploader.java
+++ b/src/tap/upload/Uploader.java
@@ -119,7 +119,7 @@ public class Uploader {
 		// Ensure UPLOAD is allowed by the TAP service specification...
 		if (this.service.uploadEnabled()){
 			// ...and set the rows or bytes limit:
-			if (this.service.getUploadLimitType()[1] != null && this.service.getUploadLimit()[1] > 0){
+			if (this.service.getUploadLimitType()[1] != null && this.service.getUploadLimit()[1] >= 0){
 				limit = (int)(this.service.getUploadLimitType()[1].bytesFactor() * this.service.getUploadLimit()[1]);
 				limitUnit = (this.service.getUploadLimitType()[1] == LimitUnit.rows) ? LimitUnit.rows : LimitUnit.bytes;
 			}else{
diff --git a/test/tap/config/TestTAPConfiguration.java b/test/tap/config/TestTAPConfiguration.java
index c77c320..61a4104 100644
--- a/test/tap/config/TestTAPConfiguration.java
+++ b/test/tap/config/TestTAPConfiguration.java
@@ -272,13 +272,17 @@ public class TestTAPConfiguration {
 		final String propertyName = KEY_DEFAULT_OUTPUT_LIMIT + " or " + KEY_MAX_OUTPUT_LIMIT;
 		// Test empty or negative or null values => OK!
 		try{
-			String[] testValues = new String[]{null,"","  	 ","0","-123"};
+			String[] testValues = new String[]{null,"","  	 ","-123"};
 			Object[] limit;
 			for(String v : testValues){
 				limit = parseLimit(v, propertyName, false);
 				assertEquals(limit[0], -1);
 				assertEquals(limit[1], LimitUnit.rows);
 			}
+			// 0 test:
+			limit = parseLimit("0", propertyName, false);
+			assertEquals(limit[0], 0);
+			assertEquals(limit[1], LimitUnit.rows);
 		}catch(TAPException te){
 			fail("All these empty limit values are valid, so these tests should have succeeded!\nCaught exception: " + getPertinentMessage(te));
 		}
-- 
GitLab