jdbc: fix primary keys meta retrieval

ztarvos · ztarvos · commit 06b7ffbc340b · 2018-10-10T16:07:40.000+03:00
Fixed several mistakes in get primary keys metadata API. Corrected type mismatch when parsing server response. Added sorting of result rows by column name. Fixed order of columns in result set. Wrapped errors into SQLException. Improved test coverage. Closes tarantool#41
diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
@@ -10,6 +10,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 
@@ -765,29 +766,53 @@ public ResultSet getVersionColumns(String catalog, String schema, String table)
 
     @Override
     public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException {
-        List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 2, Arrays.asList(table), 0, 1, 0);
+        final List<String> colNames = Arrays.asList(
+                "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME");
 
-        List<List<Object>> rows = new ArrayList<List<Object>>();
+        if (table == null || table.isEmpty())
+            return emptyResultSet(colNames);
+
+        try {
+            List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0);
+
+            if (spaces == null || spaces.size() == 0)
+                return emptyResultSet(colNames);
 
-        if (spaces != null && spaces.size() > 0) {
-            List<Object> space = spaces.get(0);
-            List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
-            List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
-            List<Object> primaryKey = indexes.get(0);
-            List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
+            List space = ensureType(List.class, spaces.get(0));
+            List fields = ensureType(List.class, space.get(FORMAT_IDX));
+            int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue();
+            List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0);
+            List primaryKey = ensureType(List.class, indexes.get(0));
+            List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX));
+
+            List<List<Object>> rows = new ArrayList<List<Object>>();
             for (int i = 0; i < parts.size(); i++) {
-                List<Object> part = parts.get(i);
-                int ordinal = ((Number) part.get(0)).intValue();
-                String column = (String) fields.get(ordinal).get("name");
-                rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
+                // For native spaces, the 'parts' is 'List of Lists'.
+                // We only accept SQL spaces, for which the parts is 'List of Maps'.
+                Map part = checkType(Map.class, parts.get(i));
+                if (part == null)
+                    return emptyResultSet(colNames);
+
+                int ordinal = ensureType(Number.class, part.get("field")).intValue();
+                Map field = ensureType(Map.class, fields.get(ordinal));
+                // The 'name' field is optional in the format structure. But it is present for SQL space.
+                String column = ensureType(String.class, field.get("name"));
+                rows.add(Arrays.asList(null, null, table, column, i + 1, primaryKey.get(NAME_IDX)));
             }
+            // Sort results by column name.
+            Collections.sort(rows, new Comparator<List<Object>>() {
+                @Override
+                public int compare(List<Object> row0, List<Object> row1) {
+                    String col0 = (String) row0.get(3);
+                    String col1 = (String) row1.get(3);
+                    return col0.compareTo(col1);
+                }
+            });
+            return new SQLNullResultSet((JDBCBridge.mock(colNames, rows)));
+        }
+        catch (Throwable t) {
+            throw new SQLException("Error processing metadata for table \"" + table + "\".", t);
         }
-        return new SQLNullResultSet((JDBCBridge.mock(
-                Arrays.asList("TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME",
-                        //nulls
-                        "TABLE_CAT", "TABLE_SCHEM"
-                ),
-                rows)));
     }
 
     @Override
@@ -1026,4 +1051,21 @@ public <T> T unwrap(Class<T> iface) throws SQLException {
     public boolean isWrapperFor(Class<?> iface) throws SQLException {
         throw new SQLFeatureNotSupportedException();
     }
+
+    private static <T> T ensureType(Class<T> cls, Object v) throws Exception {
+        if (v == null || !cls.isAssignableFrom(v.getClass())) {
+            throw new Exception(String.format("Wrong value type '%s', expected '%s'.",
+                    v == null ? "null" : v.getClass().getName(), cls.getName()));
+        }
+        return cls.cast(v);
+    }
+
+    private static <T> T checkType(Class<T> cls, Object v) {
+        return (v != null && cls.isAssignableFrom(v.getClass())) ? cls.cast(v) : null;
+    }
+
+    private ResultSet emptyResultSet(List<String> colNames) {
+        return new SQLNullResultSet((JDBCBridge.mock(colNames, Collections.<List<Object>>emptyList())));
+    }
+
 }
diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
@@ -32,9 +32,6 @@ public abstract class AbstractJdbcIT {
     private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass);
 
     private static String[] initSql = new String[] {
-            "DROP TABLE IF EXISTS test",
-            "DROP TABLE IF EXISTS test_types",
-
             "CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))",
             "INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')",
 
@@ -78,12 +75,15 @@ public abstract class AbstractJdbcIT {
                     "X'010203040506'," + //LONGVARBINARY
                     "'1983-03-14'," + //DATE
                     "'12:01:06'," + //TIME
-                    "129479994)" //TIMESTAMP
+                    "129479994)", //TIMESTAMP
+
+            "CREATE TABLE test_compound(id1 INT, id2 INT, val VARCHAR(100), PRIMARY KEY (id2, id1))"
     };
 
     private static String[] cleanSql = new String[] {
             "DROP TABLE IF EXISTS test",
-            "DROP TABLE IF EXISTS test_types"
+            "DROP TABLE IF EXISTS test_types",
+            "DROP TABLE IF EXISTS test_compound"
     };
 
     static Object[] testRow = new Object[] {
@@ -112,6 +112,7 @@ public abstract class AbstractJdbcIT {
 
     @BeforeAll
     public static void setupEnv() throws Exception {
+        sqlExec(cleanSql);
         sqlExec(initSql);
     }
 
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -1,11 +1,11 @@
 package org.tarantool.jdbc;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.BeforeEach;
 
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -18,7 +18,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
     private DatabaseMetaData meta;
 
     @BeforeEach
-    public void setUp() throws Exception {
+    public void setUp() throws SQLException {
         meta = conn.getMetaData();
     }
 
@@ -45,6 +45,9 @@ public void testGetAllTables() throws SQLException {
         assertTrue(rs.next());
         assertEquals("TEST_TYPES", rs.getString("TABLE_NAME"));
 
+        assertTrue(rs.next());
+        assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+
         assertFalse(rs.next());
 
         rs.close();
@@ -84,23 +87,100 @@ public void testGetColumns() throws SQLException {
         rs.close();
     }
 
-    @Disabled(value="Test ignored, issue#41")
     @Test
     public void testGetPrimaryKeys() throws SQLException {
         ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
 
         assertNotNull(rs);
         assertTrue(rs.next());
 
-        assertNull(rs.getString("TABLE_CAT"));
-        assertNull(rs.getString("TABLE_SCHEM"));
-        assertEquals("TEST", rs.getString("TABLE_NAME"));
-        assertEquals("ID", rs.getString("COLUMN_NAME"));
-        assertEquals(1, rs.getInt("KEY_SEQ"));
-        assertEquals("pk_unnamed_TEST_1", rs.getString("PK_NAME"));
+        checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+
+        assertFalse(rs.next());
+
+        rs.close();
+    }
+
+    @Test
+    public void testGetPrimaryKeysCompound() throws SQLException {
+        ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
+
+        assertNotNull(rs);
+        assertTrue(rs.next());
+        checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID1", "pk_unnamed_TEST_COMPOUND_1", 2);
+
+        assertTrue(rs.next());
+        checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID2", "pk_unnamed_TEST_COMPOUND_1", 1);
+
+        assertFalse(rs.next());
 
+        rs.close();
+    }
+
+    @Test
+    public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
+        String[] vals = new String[] {null, "", "IGNORE"};
+        for (String cat : vals) {
+            for (String schema : vals) {
+                ResultSet rs = meta.getPrimaryKeys(cat, schema, "TEST");
+
+                assertNotNull(rs);
+                assertTrue(rs.next());
+                checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+                assertFalse(rs.next());
+                rs.close();
+            }
+        }
+    }
+
+    @Test
+    public void testGetPrimaryKeysNotFound() throws SQLException {
+        String[] tables = new String[] {null, "", "NOSUCHTABLE"};
+        for (String t : tables) {
+            ResultSet rs = meta.getPrimaryKeys(null, null, t);
+            assertNotNull(rs);
+            assertFalse(rs.next());
+            rs.close();
+        }
+    }
+
+    @Test
+    public void testGetPrimaryKeyNonSQLSpace() throws SQLException {
+        ResultSet rs = meta.getPrimaryKeys(null, null, "_vspace");
+        assertNotNull(rs);
         assertFalse(rs.next());
+        rs.close();
+    }
 
+    @Test
+    public void testGetPrimaryKeysOrderOfColumns() throws SQLException {
+        ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
+        assertNotNull(rs);
+        ResultSetMetaData rsMeta = rs.getMetaData();
+        assertEquals(6, rsMeta.getColumnCount());
+        assertEquals("TABLE_CAT", rsMeta.getColumnName(1));
+        assertEquals("TABLE_SCHEM", rsMeta.getColumnName(2));
+        assertEquals("TABLE_NAME", rsMeta.getColumnName(3));
+        assertEquals("COLUMN_NAME", rsMeta.getColumnName(4));
+        assertEquals("KEY_SEQ", rsMeta.getColumnName(5));
+        assertEquals("PK_NAME", rsMeta.getColumnName(6));
         rs.close();
     }
+
+    private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
+        throws SQLException {
+        assertNull(rs.getString("TABLE_CAT"));
+        assertNull(rs.getString("TABLE_SCHEM"));
+        assertEquals(table, rs.getString("TABLE_NAME"));
+        assertEquals(colName, rs.getString("COLUMN_NAME"));
+        assertEquals(seq, rs.getInt("KEY_SEQ"));
+        assertEquals(pkName, rs.getString("PK_NAME"));
+
+        assertNull(rs.getString(1));
+        assertNull(rs.getString(2));
+        assertEquals(table, rs.getString(3));
+        assertEquals(colName, rs.getString(4));
+        assertEquals(seq, rs.getInt(5));
+        assertEquals(pkName, rs.getString(6));
+    }
 }
diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
@@ -0,0 +1,60 @@
+package org.tarantool.jdbc;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+import org.tarantool.TarantoolConnection;
+
+import java.sql.DatabaseMetaData;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE;
+
+public class JdbcExceptionHandlingTest {
+    /**
+     * Simulates meta parsing error: missing "name" field in a space format for the primary key.
+     *
+     * @throws SQLException on failure.
+     */
+    @Test
+    public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException {
+        TarantoolConnection tntCon = mock(TarantoolConnection.class);
+        SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
+
+        Object[] spc = new Object[7];
+        spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>());
+        spc[SPACE_ID_IDX] = 1000;
+
+        doReturn(Collections.singletonList(Arrays.asList(spc))).when(tntCon)
+                .select(_VSPACE, 2, Collections.singletonList("TEST"), 0, 1, 0);
+
+        Object[] idx = new Object[6];
+        idx[INDEX_FORMAT_IDX] = Collections.singletonList(
+                new HashMap<String, Object>() {{ put("field", 0);}});
+
+        doReturn(Collections.singletonList(Arrays.asList(idx))).when(tntCon)
+                .select(_VINDEX, 0, Arrays.asList(1000, 0), 0, 1, 0);
+
+        final DatabaseMetaData meta = conn.getMetaData();
+
+        Throwable t = assertThrows(SQLException.class, new Executable() {
+            @Override
+            public void execute() throws Throwable {
+                meta.getPrimaryKeys(null, null, "TEST");
+            }
+        }, "Error processing metadata for table \"TEST\".");
+
+        assertTrue(t.getCause().getMessage().contains("Wrong value type"));
+    }
+}
