Skip to content
Snippets Groups Projects
Commit 77132014 authored by gmantele's avatar gmantele
Browse files

[TAP] Fix handling of bad uploaded files:

- empty file
- not a valid VOTable document

In such cases, the following error message is returned:
  "The input file is not a valid VOTable document!"
A cause with more detais (especially the line and column numbers)
may be appended.

Cases handled with no error:

  - If the VOTable document has no rows, an empty table
    is uploaded. No error has to be returned.

  - If a row has a different number of columns than the number
    of declared FIELDs, additional values are ignored and missing
    values are replaced by NULL. This is actually nicely handled by
    STIL.
parent db34b35d
No related branches found
No related tags found
No related merge requests found
...@@ -16,13 +16,16 @@ package tap.data; ...@@ -16,13 +16,16 @@ package tap.data;
* You should have received a copy of the GNU Lesser General Public License * You should have received a copy of the GNU Lesser General Public License
* along with TAPLibrary. If not, see <http://www.gnu.org/licenses/>. * along with TAPLibrary. If not, see <http://www.gnu.org/licenses/>.
* *
* Copyright 2015 - Astronomisches Rechen Institut (ARI) * Copyright 2015-2017 - Astronomisches Rechen Institut (ARI)
*/ */
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.util.NoSuchElementException; import java.util.NoSuchElementException;
import org.xml.sax.SAXParseException;
import adql.db.DBType;
import tap.TAPException; import tap.TAPException;
import tap.metadata.TAPColumn; import tap.metadata.TAPColumn;
import tap.metadata.VotType; import tap.metadata.VotType;
...@@ -34,7 +37,6 @@ import uk.ac.starlink.table.StarTableFactory; ...@@ -34,7 +37,6 @@ import uk.ac.starlink.table.StarTableFactory;
import uk.ac.starlink.table.TableBuilder; import uk.ac.starlink.table.TableBuilder;
import uk.ac.starlink.table.TableFormatException; import uk.ac.starlink.table.TableFormatException;
import uk.ac.starlink.table.TableSink; import uk.ac.starlink.table.TableSink;
import adql.db.DBType;
/** /**
* <p>{@link TableIterator} which lets iterate over a VOTable input stream using STIL.</p> * <p>{@link TableIterator} which lets iterate over a VOTable input stream using STIL.</p>
...@@ -42,7 +44,7 @@ import adql.db.DBType; ...@@ -42,7 +44,7 @@ import adql.db.DBType;
* <p>{@link #getColType()} will return TAP type based on the type declared in the VOTable metadata part.</p> * <p>{@link #getColType()} will return TAP type based on the type declared in the VOTable metadata part.</p>
* *
* @author Gr&eacute;gory Mantelet (ARI) * @author Gr&eacute;gory Mantelet (ARI)
* @version 2.1 (07/2015) * @version 2.1 (03/2017)
* @since 2.0 * @since 2.0
*/ */
public class VOTableIterator implements TableIterator { public class VOTableIterator implements TableIterator {
...@@ -61,10 +63,10 @@ public class VOTableIterator implements TableIterator { ...@@ -61,10 +63,10 @@ public class VOTableIterator implements TableIterator {
* Besides, the metadata returned by StarTable are immediately converted into TAP metadata. If this conversion fails, the error is kept * Besides, the metadata returned by StarTable are immediately converted into TAP metadata. If this conversion fails, the error is kept
* in metaError, so that the VOTable reading can continue if the fact that metadata are missing is not a problem for the class using the * in metaError, so that the VOTable reading can continue if the fact that metadata are missing is not a problem for the class using the
* {@link VOTableIterator}. * {@link VOTableIterator}.
* </p> * </p>
* *
* @author Gr&eacute;gory Mantelet (ARI) * @author Gr&eacute;gory Mantelet (ARI)
* @version 2.0 (04/2015) * @version 2.1 (03/2017)
* @since 2.0 * @since 2.0
*/ */
protected static class StreamVOTableSink implements TableSink { protected static class StreamVOTableSink implements TableSink {
...@@ -92,8 +94,52 @@ public class VOTableIterator implements TableIterator { ...@@ -92,8 +94,52 @@ public class VOTableIterator implements TableIterator {
* but no exception should be thrown to VOTableIterator. * but no exception should be thrown to VOTableIterator.
* </p> * </p>
*/ */
public synchronized void stop(){ public void stop(){
stop(null);
}
/**
* <p>Stop nicely reading the VOTable.</p>
*
* <p>
* An exception will be thrown to the STILTS class using this TableSink,
* but no exception should be thrown to VOTableIterator.
* </p>
*
* @param reason Reason why this Sink should be stop.
* <i>This should be used in case of external grave error that
* should be raised when trying to access data through
* VOTableIterator.
* Example: a wrong VOTable format.</i>
*/
public synchronized void stop(final Throwable reason){
// Prevent further attempt to read the input stream:
endReached = true; endReached = true;
// Set the stop reason (if any):
if (reason != null && metaError == null){
// Case: Wrong VOTable format:
if (reason instanceof TableFormatException){
// build the most precise error message as possible:
String msg = "The input file is not a valid VOTable document!";
if (reason.getCause() != null){
if (reason.getCause() instanceof SAXParseException){
SAXParseException spe = (SAXParseException)reason.getCause();
msg += " Cause: [l." + spe.getLineNumber() + ", c." + spe.getColumnNumber() + "] " + spe.getMessage();
}else if (reason.getCause().getMessage() != null)
msg += " Cause: " + reason.getCause().getMessage();
else
msg += " Cause: {" + reason.getCause().getClass().getName() + "}";
}
// create the exception:
metaError = new DataReadException(msg, reason);
}
// Case: Unknown reason!
else if (reason.getMessage() != null && !reason.getMessage().equals(STREAM_ABORTED_MESSAGE))
metaError = new DataReadException("Unexpected error while reading the uploaded VOTable!", reason);
}
// Stop waiting (=> the reading is aborted):
notifyAll(); notifyAll();
} }
...@@ -116,7 +162,7 @@ public class VOTableIterator implements TableIterator { ...@@ -116,7 +162,7 @@ public class VOTableIterator implements TableIterator {
@Override @Override
public synchronized void acceptRow(final Object[] row) throws IOException{ public synchronized void acceptRow(final Object[] row) throws IOException{
try{ try{
// Wait until the last accepted row has been consumed: // Wait until the last accepted row has been consumed:
while(!endReached && pendingRow != null) while(!endReached && pendingRow != null)
wait(); wait();
...@@ -188,6 +234,9 @@ public class VOTableIterator implements TableIterator { ...@@ -188,6 +234,9 @@ public class VOTableIterator implements TableIterator {
// If there was an error while interpreting the accepted metadata, throw it: // If there was an error while interpreting the accepted metadata, throw it:
if (metaError != null) if (metaError != null)
throw metaError; throw metaError;
// Or if no metadata can be fetched:
else if (meta == null || meta.length == 0)
throw (metaError = new DataReadException("Unexpected VOTable document: no FIELD can be found!"));
// Otherwise, just return the metadata: // Otherwise, just return the metadata:
return meta; return meta;
...@@ -248,7 +297,7 @@ public class VOTableIterator implements TableIterator { ...@@ -248,7 +297,7 @@ public class VOTableIterator implements TableIterator {
/** /**
* Extract an array of {@link TAPColumn} objects. Each corresponds to one of the columns listed in the given table, * Extract an array of {@link TAPColumn} objects. Each corresponds to one of the columns listed in the given table,
* and so corresponds to the metadata of a column. * and so corresponds to the metadata of a column.
* *
* @param table {@link StarTable} which contains only the columns' information. * @param table {@link StarTable} which contains only the columns' information.
* *
...@@ -359,8 +408,9 @@ public class VOTableIterator implements TableIterator { ...@@ -359,8 +408,9 @@ public class VOTableIterator implements TableIterator {
try{ try{
tb.streamStarTable(input, sink, null); tb.streamStarTable(input, sink, null);
}catch(IOException e){ }catch(IOException e){
if (e.getMessage() != null && !e.getMessage().equals(STREAM_ABORTED_MESSAGE)) /* Stop the VOTable sink
e.printStackTrace(); *(otherwise it may still waiting for a Thread notification to wake it up): */
sink.stop(e);
} }
} }
}; };
......
package tap.data; package tap.data;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
...@@ -12,6 +13,8 @@ import java.io.InputStream; ...@@ -12,6 +13,8 @@ import java.io.InputStream;
import org.junit.Test; import org.junit.Test;
import uk.ac.starlink.table.TableFormatException;
public class TestVOTableIterator { public class TestVOTableIterator {
public final static String directory = "./test/tap/data/"; public final static String directory = "./test/tap/data/";
...@@ -193,4 +196,134 @@ public class TestVOTableIterator { ...@@ -193,4 +196,134 @@ public class TestVOTableIterator {
} }
} }
} }
@Test
public void testWithNotAVotable(){
// CASE: Empty file!
File emptyFile = new File("test/tap/data/emptyFile");
InputStream input = null;
VOTableIterator it = null;
try{
// Create an empty file:
emptyFile.createNewFile();
// Prepare reading it:
input = new FileInputStream(emptyFile);
// Start the iteration:
it = new VOTableIterator(input);
// Try to read the data:
it.getMetadata();
fail("An exception was expected because the file is empty!");
}catch(IOException ioe){
ioe.printStackTrace();
fail("Another exception was expected here! (see the console for more details)");
}catch(DataReadException dre){
assertNotNull(dre.getCause());
assertEquals(TableFormatException.class, dre.getCause().getClass());
assertEquals("The input file is not a valid VOTable document! Cause: [l.1, c.1] Premature end of file.", dre.getMessage());
}finally{
// Close the input stream (if open):
if (input != null){
try{
input.close();
}catch(IOException ioe){
ioe.printStackTrace();
}
}
// Close the iterator (if any):
if (it != null){
try{
it.close();
}catch(DataReadException dre){
dre.printStackTrace();
}
}
// Delete the temporary empty file (if exists):
if (emptyFile.exists())
emptyFile.delete();
}
// CASE: Neither empty nor a VOTable file!
File notAVotFile = new File("test/tap/db_testtools/db-test/create-db.sql");
input = null;
it = null;
try{
// Prepare reading it:
input = new FileInputStream(notAVotFile);
// Start the iteration:
it = new VOTableIterator(input);
// Try to read the data:
it.getMetadata();
fail("An exception was expected because the file is not a VOTable!");
}catch(IOException ioe){
ioe.printStackTrace();
fail("Another exception was expected here! (see the console for more details)");
}catch(DataReadException dre){
assertNotNull(dre.getCause());
assertEquals(TableFormatException.class, dre.getCause().getClass());
assertEquals("The input file is not a valid VOTable document! Cause: [l.2, c.1] Content is not allowed in prolog.", dre.getMessage());
}finally{
// Close the input stream (if open):
if (input != null){
try{
input.close();
}catch(IOException ioe){
ioe.printStackTrace();
}
}
// Close the iterator (if any):
if (it != null){
try{
it.close();
}catch(DataReadException dre){
dre.printStackTrace();
}
}
}
// CASE: A VOTable with no field declared!
File notAValidVotableFile = new File("test/tap/data/testdata_no-field.vot");
input = null;
it = null;
try{
// Prepare reading it:
input = new FileInputStream(notAValidVotableFile);
// Start the iteration:
it = new VOTableIterator(input);
// Try to read the data:
it.getMetadata();
fail("An exception was expected because no FIELD is declared in this VOTable!");
}catch(IOException ioe){
ioe.printStackTrace();
fail("Another exception was expected here! (see the console for more details)");
}catch(DataReadException dre){
assertEquals("Unexpected VOTable document: no FIELD can be found!", dre.getMessage());
}finally{
// Close the input stream (if open):
if (input != null){
try{
input.close();
}catch(IOException ioe){
ioe.printStackTrace();
}
}
// Close the iterator (if any):
if (it != null){
try{
it.close();
}catch(DataReadException dre){
dre.printStackTrace();
}
}
}
}
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment