Merge pull request #180 from pdowler/main

youcat dependency update

cadc-tap-schema/build.gradle

@@ -16,7 +16,7 @@ sourceCompatibility = 1.8
 
 group = 'org.opencadc'
 
-version = '1.2.2'
+version = '1.2.3'
 
 description = 'OpenCADC TAP-1.1 tap schema server library'
 def git_url = 'https://github.com/opencadc/tap'

cadc-tap-schema/src/main/java/ca/nrc/cadc/tap/db/TableIngester.java

@@ -76,6 +76,7 @@
 import ca.nrc.cadc.tap.schema.TableDesc;
 import ca.nrc.cadc.tap.schema.TapDataType;
 import ca.nrc.cadc.tap.schema.TapPermissions;
+import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -137,47 +138,59 @@ private TableDesc createTableDesc(String schemaName, String tableName)
         final String internalTableName = databaseDataType.toInternalDatabaseObjectName(s);
         final String internalSchemaName = databaseDataType.toInternalDatabaseObjectName(schemaName);
         log.debug(String.format("creating TableDesc for %s %s aka %s", internalSchemaName, internalTableName, tableName));
-        DatabaseMetaData databaseMetaData = dataSource.getConnection().getMetaData();
-        ResultSet rs = databaseMetaData.getTables(null, internalSchemaName, internalTableName, null);
-        if (rs != null && !rs.next()) {
-            log.debug("table does not exist: " + tableName);
-            throw new ResourceNotFoundException("database table not found: " + tableName);
-        }
-
-        log.debug(String.format("querying DatabaseMetadata for schema=%s table=%s", internalSchemaName, internalTableName));
-        //TODO too pg specific? table names are stored lower case in the system tables queried for the metadata
-        ResultSet indexInfo = databaseMetaData.getIndexInfo(null, internalSchemaName, internalTableName, false, false);
-        // get column names for indexed columns
-        List<String> indexedColumns = new ArrayList<String>();
-        while (indexInfo.next()) {
-            String indexedColumn = indexInfo.getString("COLUMN_NAME");
-            indexedColumns.add(indexedColumn);
-            log.debug("indexed column: " + indexedColumn);
-        }
+        Connection conn = dataSource.getConnection();
+        try {
+            DatabaseMetaData databaseMetaData = conn.getMetaData();
+            ResultSet rs = databaseMetaData.getTables(null, internalSchemaName, internalTableName, null);
+            if (rs != null && !rs.next()) {
+                log.debug("table does not exist: " + tableName);
+                throw new ResourceNotFoundException("database table not found: " + tableName);
+            }
+
+            log.debug(String.format("querying DatabaseMetadata for schema=%s table=%s", internalSchemaName, internalTableName));
+            //TODO too pg specific? table names are stored lower case in the system tables queried for the metadata
+            ResultSet indexInfo = databaseMetaData.getIndexInfo(null, internalSchemaName, internalTableName, false, false);
+            List<String> indexedColumns = new ArrayList<String>();
+            try {
+                while (indexInfo.next()) {
+                    String indexedColumn = indexInfo.getString("COLUMN_NAME");
+                    indexedColumns.add(indexedColumn);
+                    log.debug("indexed column: " + indexedColumn);
+                }
+            } finally {
+                indexInfo.close();
+            }
 
-        // build TableDesc
-        TableDesc tableDesc = new TableDesc(schemaName, tableName); // as specified by caller
-        tableDesc.tableType = TableDesc.TableType.TABLE;
-        log.debug(String.format("creating TableDesc %s %s aka %s", internalSchemaName, internalTableName, tableName));
-        //TODO too pg specific? table names are stored lower case in the system tables queried for the metadata
-        ResultSet columnInfo = databaseMetaData.getColumns(null, internalSchemaName, internalTableName, null);
-        while (columnInfo.next()) {
-            String columnName = columnInfo.getString("COLUMN_NAME");
-            String columnType = columnInfo.getString("TYPE_NAME");
-            TapDataType tapDataType = databaseDataType.toTapDataType(columnType, null);
-            if (TapDataType.CHAR.getDatatype().equals(tapDataType.getDatatype()) && tapDataType.xtype == null) {
-                Integer colSize = columnInfo.getInt("COLUMN_SIZE"); // int
-                if (colSize == 1) {
-                    colSize = null; // length 1 means scalar in TAP
+            // build TableDesc
+            TableDesc tableDesc = new TableDesc(schemaName, tableName); // as specified by caller
+            tableDesc.tableType = TableDesc.TableType.TABLE;
+            log.debug(String.format("creating TableDesc %s %s aka %s", internalSchemaName, internalTableName, tableName));
+            //TODO too pg specific? table names are stored lower case in the system tables queried for the metadata
+            ResultSet columnInfo = databaseMetaData.getColumns(null, internalSchemaName, internalTableName, null);
+            try {
+                while (columnInfo.next()) {
+                    String columnName = columnInfo.getString("COLUMN_NAME");
+                    String columnType = columnInfo.getString("TYPE_NAME");
+                    TapDataType tapDataType = databaseDataType.toTapDataType(columnType, null);
+                    if (TapDataType.CHAR.getDatatype().equals(tapDataType.getDatatype()) && tapDataType.xtype == null) {
+                        Integer colSize = columnInfo.getInt("COLUMN_SIZE"); // int
+                        if (colSize == 1) {
+                            colSize = null; // length 1 means scalar in TAP
+                        }
+                        tapDataType = databaseDataType.toTapDataType(columnType, colSize);
+                    }
+                    log.debug(String.format("creating ColumnDesc %s %s %s", tableName, columnName, tapDataType));
+                    ColumnDesc columnDesc = new ColumnDesc(tableName, columnName, tapDataType);
+                    columnDesc.indexed = indexedColumns.contains(columnName);
+                    tableDesc.getColumnDescs().add(columnDesc);
                 }
-                tapDataType = databaseDataType.toTapDataType(columnType, colSize);
+            } finally {
+                columnInfo.close();
             }
-            log.debug(String.format("creating ColumnDesc %s %s %s", tableName, columnName, tapDataType));
-            ColumnDesc columnDesc = new ColumnDesc(tableName, columnName, tapDataType);
-            columnDesc.indexed = indexedColumns.contains(columnName);
-            tableDesc.getColumnDescs().add(columnDesc);
+            return tableDesc;
+        } finally {
+            conn.close();
         }
-        return tableDesc;
     }
 
     String getUnqualifiedTableNameFromTable(String tableName) {

cadc-tap-schema/src/main/java/ca/nrc/cadc/vosi/actions/TableContentHandler.java

@@ -114,6 +114,9 @@ public Content accept(String name, String contentType, InputStream inputStream)
         if (tableName == null) {
             throw new IllegalArgumentException("Missing table name in path");
         }
+        if (contentType == null) {
+            throw new IllegalArgumentException("Table Content-Type is required.");
+        }
 
         TapSchemaDAO ts = parent.getTapSchemaDAO();
         parent.checkTableWritePermissions(ts, tableName);
@@ -125,9 +128,6 @@ public Content accept(String name, String contentType, InputStream inputStream)
 
         log.debug("Content-Type: " + contentType);
         TableDataInputStream tableData = null;
-        if (contentType == null) {
-            throw new IllegalArgumentException("Table Content-Type is requried.");
-        }
         if (contentType.equals(CONTENT_TYPE_CSV) || contentType.equals(CONTENT_TYPE_TSV)) {
             tableData = new AsciiTableData(inputStream, contentType);
         }

cadc-tap-schema/src/main/java/ca/nrc/cadc/vosi/actions/TableUpdateRunner.java

@@ -379,50 +379,46 @@ protected void ingestTable(Map<String, List<String>> params)
             throw new IllegalArgumentException("ingest schema not found in tap_schema: " + schemaName);
         }
 
-        // check if table already exists in tap_schema
+        log.debug("check if table already exists in tap_schema");
         TableDesc tableDesc = tapSchemaDAO.getTable(tableName);
         if (tableDesc != null) {
             throw new IllegalArgumentException("ingest table already exists in tap_schema: " + tableName);
         }
 
-        // add the table to the tap_schema
+        // note: this is outside the transaction because it uses low-level db to get
+        // database metadata
         TableIngester tableIngester = new TableIngester(ds);
+        log.debug("read table from database");
+        TableDesc ingestable = tableIngester.getTableDesc(schemaName, tableName);
+        // check the table is valid ADQL name
+        try {
+            TapSchemaUtil.checkValidTableName(ingestable.getTableName());
+        } catch (ADQLIdentifierException ex) {
+            throw new IllegalArgumentException("invalid table name: " + ingestable.getTableName(), ex);
+        }
+        try {
+            for (ColumnDesc cd : ingestable.getColumnDescs()) {
+                TapSchemaUtil.checkValidIdentifier(cd.getColumnName());
+            }
+        } catch (ADQLIdentifierException ex) {
+            throw new IllegalArgumentException(ex.getMessage());
+        }
+
         DatabaseTransactionManager tm = new DatabaseTransactionManager(ds);
         try {
+            log.debug("start transaction");
             tm.startTransaction();
 
-            TableDesc ingestable = tableIngester.getTableDesc(schemaName, tableName);
-            // check the table is valid ADQL name
-            try {
-                TapSchemaUtil.checkValidTableName(ingestable.getTableName());
-            } catch (ADQLIdentifierException ex) {
-                throw new IllegalArgumentException("invalid table name: " + ingestable.getTableName(), ex);
-            }
-            try {
-                for (ColumnDesc cd : ingestable.getColumnDescs()) {
-                    TapSchemaUtil.checkValidIdentifier(cd.getColumnName());
-                }
-            } catch (ADQLIdentifierException ex) {
-                throw new IllegalArgumentException(ex.getMessage());
-            }
-
             // assign owner
             ingestable.tapPermissions.owner = AuthenticationUtil.getCurrentSubject();
             ingestable.apiCreated = false; // pre-existing table
 
+            log.debug("put table to tap_schema");
             tapSchemaDAO.put(ingestable);
             log.debug(String.format("added table '%s' to tap_schema", tableName));
 
+            log.debug("commit transaction");
             tm.commitTransaction();
-        } catch (IllegalArgumentException | ResourceNotFoundException | UnsupportedOperationException ex) {
-            try {
-                log.debug("ingest table and update tap_schema failed - rollback", ex);
-                tm.rollbackTransaction();
-                log.debug("ingest table and update tap_schema failed - rollback OK");
-            } catch (Exception oops) {
-                log.error("ingest table and update tap_schema failed - rollback: FAIL", ex);
-            }
-            throw ex;
         } catch (Exception ex) {
 
             try {

youcat/VERSION

@@ -1,6 +1,6 @@
 ## deployable containers have a semantic and build tag
 # semantic version tag: major.minor
 # build version tag: timestamp
-VER=0.7.1
+VER=0.7.2
 TAGS="${VER} ${VER}-$(date --utc +"%Y%m%dT%H%M%S")"
 unset VER

youcat/build.gradle

@@ -35,8 +35,8 @@ dependencies {
     compile 'org.opencadc:cadc-uws:[1.0.2,)'
     compile 'org.opencadc:cadc-uws-server:[1.2.22,)'
     compile 'org.opencadc:cadc-tap:[1.1.17,)'
-    compile 'org.opencadc:cadc-tap-schema:[1.2.2,)'
-    compile 'org.opencadc:cadc-tap-server:[1.1.25,)'
+    compile 'org.opencadc:cadc-tap-schema:[1.2.3,)'
+    compile 'org.opencadc:cadc-tap-server:[1.1.26,)'
     compile 'org.opencadc:cadc-tap-server-pg:[1.1.1,)'
     compile 'org.opencadc:cadc-adql:[1.1.4,)'
     compile 'org.opencadc:cadc-vosi:[1.4.3,2.0)'

youcat/src/intTest/java/org/opencadc/youcat/AbstractTablesTest.java

@@ -132,7 +132,8 @@ abstract class AbstractTablesTest {
     Subject schemaOwner;
     Subject subjectWithGroups;
 
-    protected String testSchemaName = "int_test_schema"; 
+    protected String testSchemaName = "int_test_schema";
+    protected final String testCreateSchema = "test_create_schema";
 
     URL anonQueryURL;
     URL certQueryURL;
@@ -180,9 +181,20 @@ abstract class AbstractTablesTest {
         }
     }
 
-    void doDelete(Subject subject, String testTable, boolean fnf) throws Exception {
+    protected void checkTestSchema(String name) {
+        if (name.equals(testCreateSchema) || name.equals(testSchemaName) || name.startsWith(testSchemaName + ".")) {
+            return; // ok
+        }
+        throw new RuntimeException("TEST BUG: attempt to use schema|table name " + name
+                + " not in test schema " + testSchemaName);
+    }
+
+    void doDelete(Subject subject, String tableName, boolean fnf) throws Exception {
+        checkTestSchema(tableName);
+
+        URL tableURL = new URL(certTablesURL.toExternalForm() + "/" + tableName);
+
         // delete
-        URL tableURL = new URL(certTablesURL.toExternalForm() + "/" + testTable);
         HttpDelete del = new HttpDelete(tableURL, false);
         log.info("doDelete: " + tableURL);
         Subject.doAs(subject, new RunnableAction(del));
@@ -196,13 +208,14 @@ void doDelete(Subject subject, String testTable, boolean fnf) throws Exception {
             throw (Exception) del.getThrowable();
         }
 
-        URL getTableURL = new URL(certTablesURL.toExternalForm() + "/" + testTable);
-        HttpGet check = new HttpGet(getTableURL, new ByteArrayOutputStream());
+        HttpGet check = new HttpGet(tableURL, new ByteArrayOutputStream());
         Subject.doAs(subject, new RunnableAction(check));
         Assert.assertEquals("table deleted", 404, check.getResponseCode());
     }
 
     TableDesc doCreateTable(Subject subject, String tableName) throws Exception {
+        checkTestSchema(tableName);
+
         // cleanup just in case
         doDelete(subject, tableName, true);
 
@@ -255,6 +268,8 @@ public void write(OutputStream out) throws IOException {
     }
 
     void doCreateIndex(Subject subject, String tableName, String indexCol, boolean unique, ExecutionPhase expected, String emsg) throws Exception {
+        checkTestSchema(tableName);
+
         Assert.assertNotNull("found async table-update URL", certUpdateURL);
 
         // create job
@@ -311,6 +326,7 @@ protected void clearSchemaPerms() throws MalformedURLException {
     }
 
     protected void setPerms(Subject subject, String name, TapPermissions tp, int expectedCode) throws MalformedURLException {
+        checkTestSchema(name);
 
         StringBuilder perms = new StringBuilder();
         if (tp.isPublic != null) {

youcat/src/intTest/java/org/opencadc/youcat/CreateTableTest.java

@@ -401,17 +401,16 @@ public void write(OutputStream out) throws IOException {
 
     @Test
     public void testCreateUpdateDropSchema() {
-        String schemaName = "test_create_schema";
 
         // TODO: use schemaOwner subject to determine the user name here
         final String owner = "cadcauthtest1";
 
         try {
-            final URL schemaURL = new URL(certTablesURL.toExternalForm() + "/" + schemaName);
+            final URL schemaURL = new URL(certTablesURL.toExternalForm() + "/" + testCreateSchema);
 
-            doDelete(admin, schemaName, true);
+            doDelete(admin, testCreateSchema, true);
 
-            SchemaDesc orig = new SchemaDesc(schemaName);
+            SchemaDesc orig = new SchemaDesc(testCreateSchema);
             orig.description = "original description";
             TableSetWriter w = new TableSetWriter();
             StringWriter sw = new StringWriter();
@@ -427,7 +426,7 @@ public void testCreateUpdateDropSchema() {
             log.info("update: " + create.getResponseCode() + " " + create.getThrowable());
             Assert.assertEquals(200, create.getResponseCode());
 
-            SchemaDesc sd = doVosiSchemaCheck(schemaOwner, schemaName);
+            SchemaDesc sd = doVosiSchemaCheck(schemaOwner, testCreateSchema);
             Assert.assertNotNull(sd);
             Assert.assertEquals(orig.getSchemaName(), sd.getSchemaName());
             Assert.assertEquals(orig.description, sd.description);
@@ -450,12 +449,12 @@ public void testCreateUpdateDropSchema() {
             log.info("update: " + update.getResponseCode() + " " + update.getThrowable());
             Assert.assertEquals(204, update.getResponseCode());
 
-            SchemaDesc sd2 = doVosiSchemaCheck(schemaOwner, schemaName);
+            SchemaDesc sd2 = doVosiSchemaCheck(schemaOwner, testCreateSchema);
             Assert.assertEquals(sd.description, sd2.description);
             Assert.assertEquals(sd.utype, sd2.utype);
 
             // cleanup on success
-            doDelete(admin, schemaName, false);
+            doDelete(admin, testCreateSchema, false);
         } catch (Exception unexpected) {
             log.error("unexpected exception", unexpected);
             Assert.fail("unexpected exception: " + unexpected);