Skip to content

cadc-tap-server: update BasicUploadManager to use table validation from cadc-tap #253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pdowler merged 2 commits into from
May 4, 2026
Merged

cadc-tap-server: update BasicUploadManager to use table validation from cadc-tap #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c71ac3
cadc-tap-server: update BasicUploadManager to use table validation fr…
pdowler May 1, 2026
85d1f1b
Merge branch 'table-validation' of https://github.com/opencadc/tap in…
pdowler May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions cadc-tap-schema/src/intTest/java/ca/nrc/cadc/tap/db/TableCreatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,45 +361,6 @@ public void testCreateUniqueIndex() {
}
}


@Test
public void testInvalidTableName() {
try {
String testTable = testSchemaName + ".testInvalidTableName;drop table tap_schema.tables";
TableDesc orig = new TableDesc(testSchemaName, testTable);
orig.tableType = TableDesc.TableType.TABLE;
orig.getColumnDescs().add(new ColumnDesc(testTable, "c0", TapDataType.STRING));

TableCreator tc = new TableCreator(dataSource);
tc.createTable(orig);
Assert.fail("expected IllegalArgumentException - createTable returned");
} catch (IllegalArgumentException expected) {
log.info("caught expected exception: " + expected);
} catch (Exception unexpected) {
log.error("unexpected exception", unexpected);
Assert.fail("unexpected exception: " + unexpected);
}
}

@Test
public void testInvalidColumnName() {
try {
String testTable = testSchemaName + ".testInvalidColumnName;drop table tap_schema.tables";
TableDesc orig = new TableDesc(testSchemaName, testTable);
orig.tableType = TableDesc.TableType.TABLE;
orig.getColumnDescs().add(new ColumnDesc(testTable, "c0", TapDataType.STRING));

TableCreator tc = new TableCreator(dataSource);
tc.createTable(orig);
Assert.fail("expected IllegalArgumentException - createTable returned");
} catch (IllegalArgumentException expected) {
log.info("caught expected exception: " + expected);
} catch (Exception unexpected) {
log.error("unexpected exception", unexpected);
Assert.fail("unexpected exception: " + unexpected);
}
}

public static class SimpleRowMapper implements RowMapper {

@Override
Expand Down
2 changes: 1 addition & 1 deletion cadc-tap-server/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ sourceCompatibility = 11

group = 'org.opencadc'

version = '1.1.38'
version = '1.1.39'

description = 'OpenCADC TAP-1.1 tap server library'
def git_url = 'https://github.com/opencadc/tap'
Expand Down
100 changes: 39 additions & 61 deletions cadc-tap-server/src/intTest/java/ca/nrc/cadc/tap/BasicUploadManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("empty table name didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

// table names with a spaces
paramList.clear();
Expand All @@ -363,7 +365,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with a space didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "table name,http://localhost/foo"));
Expand All @@ -372,7 +376,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with a space didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "tablename ,http://localhost/foo"));
Expand All @@ -381,7 +387,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with a space didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

// Invalid table names
paramList.clear();
Expand All @@ -391,7 +399,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name starting with a digit didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "_tablename,http://localhost/foo"));
Expand All @@ -400,7 +410,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name starting with an underscore didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "tablename!,http://localhost/foo"));
Expand All @@ -409,7 +421,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with a exclaimation didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "?tablename,http://localhost/foo"));
Expand All @@ -418,7 +432,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with a question mark didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

paramList.clear();
paramList.add(new Parameter("UPLOAD", "&tablename,http://localhost/foo"));
Expand All @@ -427,7 +443,9 @@ public void testInvalidTableNames()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("table name with an ampersand didn't cause an Exception");
}
catch (RuntimeException ignore) { }
catch (IllegalArgumentException expected) {
log.info("caught expected: " + expected);
}

log.debug("testInvalidTableNames passed.");
}
Expand Down Expand Up @@ -461,15 +479,8 @@ public void testColumnNameStartsWithDigit()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name starting with a digit didn't cause an Exception");
}
catch (RuntimeException e)
{
Throwable cause = e.getCause();
while(cause.getCause() != null)
cause = cause.getCause();
if (cause instanceof ADQLIdentifierException)
log.debug("caught expected: " + cause);
else
Assert.fail(e.getCause().getMessage());
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameStartsWithDigit passed.");
Expand Down Expand Up @@ -504,15 +515,8 @@ public void testColumnNameStartsWithUnderscore()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name starting with an underscore didn't cause an Exception");
}
catch (RuntimeException e)
{
Throwable cause = e.getCause();
while(cause.getCause() != null)
cause = cause.getCause();
if (cause instanceof ADQLIdentifierException)
log.debug("caught expected: " + cause);
else
Assert.fail(e.getCause().getMessage());
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameStartsWithUnderscore passed.");
Expand Down Expand Up @@ -594,15 +598,8 @@ public void testColumnNameContainsSpace()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name containing a space didn't cause an Exception");
}
catch (RuntimeException e)
{
Throwable cause = e.getCause();
while(cause.getCause() != null)
cause = cause.getCause();
if (cause instanceof ADQLIdentifierException)
log.debug("caught expected: " + cause);
else
Assert.fail(e.getCause().getMessage());
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameContainsSpace passed.");
Expand Down Expand Up @@ -681,18 +678,8 @@ public void testColumnNameStartsWithInvalidLetter()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name starting with an invalid letter didn't cause an Exception");
}
catch (RuntimeException e)
{
Throwable cause = e.getCause();
while(cause.getCause() != null)
cause = cause.getCause();
if (cause instanceof ADQLIdentifierException)
log.debug("caught expected: " + cause);
else
{
log.error("unexpected exception", e);
Assert.fail("unexpected exception: " + e);
}
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameStartsWithInvalidLetter passed.");
Expand Down Expand Up @@ -727,15 +714,8 @@ public void testColumnNameContainsInvalidLetter()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name containing an invalid letter didn't cause an Exception");
}
catch (RuntimeException e)
{
Throwable cause = e.getCause();
while(cause.getCause() != null)
cause = cause.getCause();
if (cause instanceof ADQLIdentifierException)
log.debug("caught expected: " + cause);
else
Assert.fail(e.getCause().getMessage());
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameContainsInvalidLetter passed.");
Expand Down Expand Up @@ -770,10 +750,8 @@ public void testColumnNameEndsWithInvalidLetter()
Map<String, TableDesc> tableDescs = uploadManager.upload(paramList, jobID);
Assert.fail("column name ending with an invalid letter didn't cause an Exception");
}
catch (RuntimeException e)
{
if (e.getCause() instanceof MissingResourceException)
Assert.fail(e.getCause().getMessage());
catch (IllegalArgumentException expected) {
log.info("expected exception: " + expected);
}

log.debug("testColumnNameEndsWithInvalidLetter passed.");
Expand Down
25 changes: 24 additions & 1 deletion cadc-tap-server/src/main/java/ca/nrc/cadc/tap/BasicUploadManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@

package ca.nrc.cadc.tap;

import ca.nrc.cadc.dali.tables.ListTableData;
import ca.nrc.cadc.dali.tables.TableData;
import ca.nrc.cadc.dali.tables.votable.VOTableTable;
import ca.nrc.cadc.date.DateUtil;
import ca.nrc.cadc.io.ResourceIterator;
Expand All @@ -78,6 +80,8 @@
import ca.nrc.cadc.tap.db.TapConstants;
import ca.nrc.cadc.tap.schema.ColumnDesc;
import ca.nrc.cadc.tap.schema.TableDesc;
import ca.nrc.cadc.tap.schema.TapSchemaUtil;
import ca.nrc.cadc.tap.schema.validator.ValidatorConfig;
import ca.nrc.cadc.tap.upload.JDOMVOTableParser;
import ca.nrc.cadc.tap.upload.UploadLimits;
import ca.nrc.cadc.tap.upload.UploadParameters;
Expand Down Expand Up @@ -225,6 +229,7 @@ public Map<String, TableDesc> upload(List<Parameter> paramList, String jobID) {
final String tableName = tableDesc.getTableName();
try {
tableDesc.setTableName(databaseTableName);
validateTable(tableDesc);
storeTable(tableDesc, parser.getVOTable());
} finally {
tableDesc.setTableName(tableName);
Expand All @@ -241,6 +246,20 @@ public Map<String, TableDesc> upload(List<Parameter> paramList, String jobID) {
return metadata;
}

/**
* Use TapSchemaUtil to validate the table structure, including names of schema, table,
* and all columns.
* @param td input table descriptor
*/
protected void validateTable(TableDesc td) {
// none() includes identifier validation, just not UCDs, units, etc
ValidatorConfig vc = ValidatorConfig.none();
String report = TapSchemaUtil.validateTableDesc(td, vc);
if (report != null) {
throw new IllegalArgumentException("invalid input table: " + report);
}
}

/**
* Use TableCreator and TableLoader to create and load the table using the dataSource.
*
Expand All @@ -261,7 +280,11 @@ public void close() {

@Override
public ResourceIterator<List<Object>> iterator() throws IOException {
return vot.getTableData().iterator();
TableData tdata = vot.getTableData();
if (tdata == null) {
tdata = new ListTableData();
}
return tdata.iterator();
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions cadc-tap-server/src/main/java/ca/nrc/cadc/tap/upload/UploadParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,32 +165,32 @@ protected String validateTableName(Parameter parameter, String[] tableNameUri)
try {
tableName = tableNameUri[0];
} catch (IndexOutOfBoundsException e) {
throw new UnsupportedOperationException("UPLOAD table name is missing: " + parameter);
throw new IllegalArgumentException("UPLOAD table name is missing: " + parameter);
}

// Does table name start with a valid schema.
int index = tableName.indexOf('.');
if (index >= 0) {
String schema = tableName.substring(0, index);
if (!schema.equalsIgnoreCase(UploadManager.SCHEMA)) {
throw new UnsupportedOperationException("UPLOAD table schema name is invalid: " + tableName);
throw new IllegalArgumentException("UPLOAD table schema name is invalid: " + tableName);
}
if (tableName.length() == index + 1) {
throw new UnsupportedOperationException("UPLOAD table name is missing: " + tableName);
throw new IllegalArgumentException("UPLOAD table name is missing: " + tableName);
}
tableName = tableName.substring(index + 1);
}

try {
TapSchemaUtil.checkValidIdentifier(tableName);
} catch (ADQLIdentifierException e) {
throw new UnsupportedOperationException("UPLOAD table name not a valid ADQL identifier: " + tableName, e);
throw new IllegalArgumentException("UPLOAD table name not a valid ADQL identifier: " + tableName, e);
}

// duplicate table names are not allowed.
for (UploadTable uploadTable : uploadTables) {
if (uploadTable.tableName.equals(tableName)) {
throw new UnsupportedOperationException("UPLOAD table name is a duplicate: " + tableName);
throw new IllegalArgumentException("UPLOAD table name is a duplicate: " + tableName);
}
}
return tableName;
Expand Down
Loading