From abc1c18b49102b6e653f8fa8c899f5bb9f0918df Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Thu, 25 Sep 2014 14:52:22 +0200
Subject: [PATCH] [TAP] Fix bug in upload (uploaded tables were given twice to
the db checker) & Allow customization of the DB upload schema in the tap
factory
---
src/tap/AbstractTAPFactory.java | 31 +++--------------
src/tap/metadata/TAPMetadata.java | 58 -------------------------------
src/tap/upload/Uploader.java | 44 ++++++++++++++++-------
3 files changed, 37 insertions(+), 96 deletions(-)
diff --git a/src/tap/AbstractTAPFactory.java b/src/tap/AbstractTAPFactory.java
index 678d6fd..642aa10 100644
--- a/src/tap/AbstractTAPFactory.java
+++ b/src/tap/AbstractTAPFactory.java
@@ -56,6 +56,7 @@ import adql.query.ADQLQuery;
*/
public abstract class AbstractTAPFactory extends TAPFactory {
+ /** The error writer to use when any error occurs while executing a resource or to format an error occurring while executing an asynchronous job. */
protected final ServiceErrorWriter errorWriter;
/**
@@ -104,8 +105,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* Unless the standard implementation - {@link ADQLExecutor} - does not fit exactly your needs,
* it should not be necessary to extend this class and to extend this function (implemented here by default).
* </i></p>
- *
- * @see tap.TAPFactory#createADQLExecutor()
*/
@Override
public ADQLExecutor createADQLExecutor() throws TAPException{
@@ -118,8 +117,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* {@link ADQLQuery} part ; it could be the addition of one or several user defined function
* or the modification of any ADQL function or clause specific to your implementation.
* </i></p>
- *
- * @see tap.TAPFactory#createQueryFactory()
*/
@Override
public ADQLQueryFactory createQueryFactory() throws TAPException{
@@ -135,8 +132,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* <p><i>Note:
* This function can not be overrided, but {@link #createQueryChecker(Collection)} can be.
* </i></p>
- *
- * @see tap.TAPFactory#createQueryChecker(tap.metadata.TAPSchema)
*/
@Override
public final QueryChecker createQueryChecker(final TAPSchema uploadSchema) throws TAPException{
@@ -175,8 +170,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* @return A new ADQL query checker.
*
* @throws TAPException If any error occurs while creating the query checker.
- *
- * @see DBChecker
*/
protected QueryChecker createQueryChecker(final Collection<TAPTable> tables) throws TAPException{
return new DBChecker(tables);
@@ -190,13 +183,11 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* <p>This implementation just create an {@link Uploader} instance with the given database connection.</p>
*
* <p><i>Note:
- * This function should be overrided if the way user tables are interpreted and then created into the database
- * does not fit yours needs.
+ * This function should be overrided if you need to change the DB name of the TAP_UPLOAD schema.
+ * Indeed, by overriding this function you can specify a given TAPSchema to use as TAP_UPLOAD schema
+ * in the constructor of {@link Uploader}. But do not forget that this {@link TAPSchema} instance MUST have
+ * an ADQL name equals to "TAP_UPLOAD", otherwise, a TAPException will be thrown.
* </i></p>
- *
- * @see tap.TAPFactory#createUploader(tap.db.DBConnection)
- *
- * @see Uploader
*/
@Override
public Uploader createUploader(final DBConnection dbConn) throws TAPException{
@@ -214,8 +205,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* This implementation is largely enough for a TAP service. It is not recommended to override
* this function.
* </i></p>
- *
- * @see tap.TAPFactory#createUWS()
*/
@Override
public UWSService createUWS() throws TAPException{
@@ -229,8 +218,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* It means that no asynchronous job will be restored and backuped.</p>
*
* <p>You must override this function if you want enable the backup feature.</p>
- *
- * @see tap.TAPFactory#createUWSBackupManager(uws.service.UWSService)
*/
@Override
public UWSBackupManager createUWSBackupManager(final UWSService uws) throws TAPException{
@@ -244,8 +231,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* If you need to add or modify the behavior of some functions of a {@link TAPJob},
* you must override this function and return your own extension of {@link TAPJob}.
* </p>
- *
- * @see tap.TAPFactory#createTAPJob(javax.servlet.http.HttpServletRequest, uws.job.user.JobOwner)
*/
@Override
protected TAPJob createTAPJob(final HttpServletRequest request, final JobOwner owner) throws UWSException{
@@ -264,8 +249,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* If you need to add or modify the behavior of some functions of a {@link TAPJob},
* you must override this function and return your own extension of {@link TAPJob}.
* </p>
- *
- * @see tap.TAPFactory#createTAPJob(javax.servlet.http.HttpServletRequest, uws.job.user.JobOwner)
*/
@Override
protected TAPJob createTAPJob(final String jobId, final JobOwner owner, final TAPParameters params, final long quote, final long startTime, final long endTime, final List<Result> results, final ErrorSummary error) throws UWSException{
@@ -285,8 +268,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* However, if you want to manage them in another way, you must extend {@link TAPParameters} and override
* this function in order to return an instance of your extension.
* </p>
- *
- * @see tap.TAPFactory#createTAPParameters(javax.servlet.http.HttpServletRequest)
*/
@Override
public TAPParameters createTAPParameters(final HttpServletRequest request) throws TAPException{
@@ -306,8 +287,6 @@ public abstract class AbstractTAPFactory extends TAPFactory {
* However, if you want to manage them in another way, you must extend {@link TAPParameters} and override
* this function in order to return an instance of your extension.
* </p>
- *
- * @see tap.TAPFactory#createTAPParameters(javax.servlet.http.HttpServletRequest)
*/
@Override
public TAPParameters createTAPParameters(final Map<String,Object> params) throws TAPException{
diff --git a/src/tap/metadata/TAPMetadata.java b/src/tap/metadata/TAPMetadata.java
index a000ea1..0d156d9 100644
--- a/src/tap/metadata/TAPMetadata.java
+++ b/src/tap/metadata/TAPMetadata.java
@@ -204,64 +204,6 @@ public class TAPMetadata implements Iterable<TAPSchema>, VOSIResource, TAPResour
return schemas.get(schemaName);
}
- /**
- * <p>Get the TAPSchema describing the TAP_UPLOAD schema.</p>
- *
- * <p><i>Note:
- * This function is equivalent to {@link #getSchema(String)} with {@link STDSchema#UPLOADSCHEMA} in parameter.
- * It should be reminded that the research done with {@link #getSchema(String)} is case sensitive.
- * </i></p>
- *
- * @return The description of the TAP_UPLOAD schema,
- * or NULL if none is defined here.
- *
- * @since 2.0
- */
- public final TAPSchema getUploadSchema(){
- return getSchema(STDSchema.UPLOADSCHEMA.label);
- }
-
- /**
- * <p>Add/Update the description of the TAP_UPLOAD schema.</p>
- *
- * <p><i><b>Important note:<b>
- * The addition/update won't be effective if the ADQL name of the given {@link TAPSchema} is not exactly equals to {@link STDSchema#UPLOADSCHEMA}.
- * </i></p>
- *
- * <p><i>Note:
- * This function is equivalent to {@link #addSchema(TAPSchema)} with the given {@link TAPSchema} object in parameter.
- * </i></p>
- *
- * @param uploadSchema {@link TAPSchema} object describing the TAP_UPLOAD schema.
- *
- * @return <i>true</i> if the TAP_UPLOAD schema has been successfully added/updated,
- * <i>false</i> otherwise (and particularly if the given {@link TAPSchema} is NULL).
- *
- * @since 2.0
- */
- public final boolean setUploadSchema(final TAPSchema uploadSchema){
- if (uploadSchema != null && uploadSchema.getADQLName().equals(STDSchema.UPLOADSCHEMA.label)){
- addSchema(uploadSchema);
- return true;
- }else
- return false;
- }
-
- /**
- * <p>Add a default definition of the TAP_UPLOAD schema, and return it.</p>
- *
- * <p>The ADQL name and the DB name will be the same, that's to say: {@link STDSchema#UPLOADSCHEMA} (TAP_UPLOAD).</p>
- *
- * @return The default upload schema added to this {@link TAPMetadata} object.
- *
- * @since 2.0
- */
- public TAPSchema setDefaultUploadSchema(){
- TAPSchema uploadSchema = new TAPSchema(STDSchema.UPLOADSCHEMA.label, "Schema for tables uploaded by users.");
- addSchema(uploadSchema);
- return uploadSchema;
- }
-
/**
* Get the number of schemas contained in this TAP metadata set.
*
diff --git a/src/tap/upload/Uploader.java b/src/tap/upload/Uploader.java
index 4e610df..36e7b9e 100644
--- a/src/tap/upload/Uploader.java
+++ b/src/tap/upload/Uploader.java
@@ -32,6 +32,7 @@ import tap.data.TableIterator;
import tap.data.VOTableIterator;
import tap.db.DBConnection;
import tap.metadata.TAPColumn;
+import tap.metadata.TAPMetadata;
import tap.metadata.TAPMetadata.STDSchema;
import tap.metadata.TAPSchema;
import tap.metadata.TAPTable;
@@ -58,6 +59,9 @@ public class Uploader {
protected final ServiceConnection service;
/** Connection to the "database" (which lets upload the content of any given VOTable). */
protected final DBConnection dbConn;
+ /** Description of the TAP_UPLOAD schema to use.
+ * @since 2.0 */
+ protected final TAPSchema uploadSchema;
/** Type of limit to set: ROWS or BYTES. <i>MAY be NULL ; if NULL, no limit will be set.</i> */
protected final LimitUnit limitUnit;
/** Limit on the number of rows or bytes (depending of {@link #limitUnit}) allowed to be uploaded in once (whatever is the number of tables). */
@@ -75,6 +79,20 @@ public class Uploader {
* @throws TAPException If any error occurs while building this {@link Uploader}.
*/
public Uploader(final ServiceConnection service, final DBConnection dbConn) throws TAPException{
+ this(service, dbConn, null);
+ }
+
+ /**
+ * Build an {@link Uploader} object.
+ *
+ * @param service Specification of the TAP service using this uploader.
+ * @param dbConn A valid (open) connection to the "database".
+ *
+ * @throws TAPException If any error occurs while building this {@link Uploader}.
+ *
+ * @since 2.0
+ */
+ public Uploader(final ServiceConnection service, final DBConnection dbConn, final TAPSchema uplSchema) throws TAPException{
// NULL tests:
if (service == null)
throw new NullPointerException("The given ServiceConnection is NULL !");
@@ -85,6 +103,17 @@ public class Uploader {
this.service = service;
this.dbConn = dbConn;
+ // Set the given upload schema:
+ if (uplSchema != null){
+ if (!uplSchema.getADQLName().equalsIgnoreCase(TAPMetadata.STDSchema.UPLOADSCHEMA.label))
+ throw new TAPException("Incorrect upload schema! Its ADQL name MUST be \"" + TAPMetadata.STDSchema.UPLOADSCHEMA.label + "\" ; here is is \"" + uplSchema.getADQLName() + "\".", UWSException.INTERNAL_SERVER_ERROR);
+ else
+ this.uploadSchema = uplSchema;
+ }
+ // ...or the default one:
+ else
+ this.uploadSchema = new TAPSchema(TAPMetadata.STDSchema.UPLOADSCHEMA.label, "Schema for tables uploaded by users.");
+
// Ensure UPLOAD is allowed by the TAP service specification...
if (this.service.uploadEnabled()){
// ...and set the rows or bytes limit:
@@ -103,9 +132,9 @@ public class Uploader {
* <p>Upload all the given VOTable inputs.</p>
*
* <p><i>Note:
- * The {@link TAPTable} objects representing the uploaded tables will be associated with the TAP_UPLOAD schema defined in the TAP metadata.
- * If no such schema is defined, a default one (whose DB name will be equals to the ADQL name, that's to say {@link STDSchema#UPLOADSCHEMA})
- * is created and added into the TAP metadata. The corresponding schema should be then created in the database automatically by the {@link DBConnection}.
+ * The {@link TAPTable} objects representing the uploaded tables will be associated with the TAP_UPLOAD schema specified at the creation of this {@link Uploader}.
+ * If no such schema was specified, a default one (whose DB name will be equals to the ADQL name, that's to say {@link STDSchema#UPLOADSCHEMA})
+ * is created, will be associated with the uploaded tables and will be returned by this function.
* </i></p>
*
* @param loaders Array of tables to upload.
@@ -117,15 +146,6 @@ public class Uploader {
* @see DBConnection#addUploadedTable(TAPTable, tap.data.TableIterator)
*/
public TAPSchema upload(final TableLoader[] loaders) throws TAPException{
- // Get the TAP_UPLOAD schema as defined in the TAP metadata:
- TAPSchema uploadSchema = service.getTAPMetadata().getUploadSchema();
-
- // If no TAP_UPLOAD schema is defined, create a default one and update the TAP metadata with it:
- if (uploadSchema == null){
- uploadSchema = new TAPSchema(STDSchema.UPLOADSCHEMA.label);
- service.getTAPMetadata().setUploadSchema(uploadSchema);
- }
-
InputStream votable = null;
String tableName = null;
try{
--
GitLab