From 876d6ef47519c5d317e9a0497430c97f94ef3131 Mon Sep 17 00:00:00 2001 From: Vladimir Evdokimov Date: Thu, 28 Nov 2024 16:49:41 +0400 Subject: [PATCH] fix: correctly report support for catalog and schema (#372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix null shanpshotId when run Liquibase harness tests * Add recursive check for snapshotId presence * fix: register correct schema and catalog support Spanner supports both Catalog and Schema (although each database contains at most one catalog). Our Liquibase implementation should indicate this correctly, and also correctly indicate that we do not want to include the default schema in queries and other object IDs. This would otherwise generate invalid object identifiers in the form `.my_table` (as the default schema name is an empty string). --------- Co-authored-by: Knut Olav Løite --- .../liquibase/ext/spanner/CloudSpanner.java | 25 +- .../ext/spanner/AbstractMockServerTest.java | 262 +++++++++--------- .../ext/spanner/GenerateSnapshotTest.java | 72 ++++- .../ext/spanner/JdbcMetadataQueries.java | 29 ++ 4 files changed, 256 insertions(+), 132 deletions(-) diff --git a/src/main/java/liquibase/ext/spanner/CloudSpanner.java b/src/main/java/liquibase/ext/spanner/CloudSpanner.java index 60a8114c..22a127b3 100644 --- a/src/main/java/liquibase/ext/spanner/CloudSpanner.java +++ b/src/main/java/liquibase/ext/spanner/CloudSpanner.java @@ -17,8 +17,10 @@ import com.google.cloud.spanner.jdbc.CloudSpannerJdbcConnection; import java.sql.DriverManager; import java.sql.SQLException; +import liquibase.Scope; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; +import liquibase.database.OfflineConnection; import liquibase.database.jvm.JdbcConnection; public class CloudSpanner extends AbstractJdbcDatabase implements ICloudSpanner { @@ -89,7 +91,18 @@ protected String getConnectionSchemaName() { if (getConnection() == null) { return null; } - return defaultSchemaName; + if (getConnection() instanceof OfflineConnection) { + return ((OfflineConnection) getConnection()).getSchema(); + } + if (!(getConnection() instanceof JdbcConnection)) { + return defaultSchemaName; + } + try { + return ((JdbcConnection) getConnection()).getUnderlyingConnection().getSchema(); + } catch (SQLException e) { + Scope.getCurrentScope().getLog(getClass()).info("Error getting default schema", e); + } + return null; } @Override @@ -166,6 +179,11 @@ public boolean supportsSequences() { @Override public boolean supportsCatalogs() { + return true; + } + + @Override + public boolean getOutputDefaultCatalog() { return false; } @@ -176,6 +194,11 @@ public boolean isCaseSensitive() { @Override public boolean supportsSchemas() { + return true; + } + + @Override + public boolean getOutputDefaultSchema() { return false; } diff --git a/src/test/java/liquibase/ext/spanner/AbstractMockServerTest.java b/src/test/java/liquibase/ext/spanner/AbstractMockServerTest.java index c2fb0916..9dde3149 100644 --- a/src/test/java/liquibase/ext/spanner/AbstractMockServerTest.java +++ b/src/test/java/liquibase/ext/spanner/AbstractMockServerTest.java @@ -156,136 +156,138 @@ public ServerCall.Listener interceptCall(ServerCall schemaSet = snapshot.get(Schema.class); + assertThat(schemaSet).hasSize(1); + Schema schemaCatalog = schemaSet.iterator().next(); + // Recursively traverse all types in the snapshot + verifySnapshotIdsInDatabaseObjects(schemaCatalog, new HashSet<>()); Set tables = snapshot.get(Table.class); assertThat(tables).hasSize(1); Table singers = tables.iterator().next(); @@ -165,4 +182,57 @@ void testGenerateSnapshot() throws Exception { assertEquals("Idx_Singers_FirstName", index.getName()); } } -} + + private void verifySnapshotIdsInDatabaseObjects(Object object, Set visited) throws NoSuchFieldException { + if (object == null) { + return; // Nothing to traverse + } + // Check if the object was already visited + if (visited.contains(object)) { + return; + } + // Mark the current object as visited + visited.add(object); + + if (object instanceof DatabaseObject) { + DatabaseObject dbObject = (DatabaseObject) object; + //assertThat(dbObject.getSnapshotId()).isNotNull(); + assertWithMessage("Object: " + dbObject.getClass() + " Should have snapshotId") + .that(dbObject.getSnapshotId()).isNotNull(); + Set attributesObj = dbObject.getAttributes(); + for (String attribute : attributesObj) { + Object attributesField = dbObject.getAttribute(attribute, new Object()); + if (attributesField instanceof DatabaseObject) { + verifySnapshotIdsInDatabaseObjects(attributesField, visited); + } else if (attributesField instanceof Map) { + if (!((Map) attributesField).isEmpty()) { + Map attributes = (Map) attributesField; + attributes.forEach((key, value) -> { + if (value instanceof Set) { + ((Set) value).forEach(setValue -> { + if (setValue instanceof DatabaseObject) { + try { + verifySnapshotIdsInDatabaseObjects(setValue, visited); + } catch (NoSuchFieldException ignored) { + } + } + }); + } + }); + } + } else if (attributesField instanceof List) { + List objectList = (List) attributesField; + objectList.forEach((value) -> { + if (value instanceof DatabaseObject) { + try { + verifySnapshotIdsInDatabaseObjects(value, visited); + } catch (NoSuchFieldException ignored) { + + } + } + }); + } + } + } + } +} \ No newline at end of file diff --git a/src/test/java/liquibase/ext/spanner/JdbcMetadataQueries.java b/src/test/java/liquibase/ext/spanner/JdbcMetadataQueries.java index a47bc212..ad0b750e 100644 --- a/src/test/java/liquibase/ext/spanner/JdbcMetadataQueries.java +++ b/src/test/java/liquibase/ext/spanner/JdbcMetadataQueries.java @@ -21,6 +21,10 @@ class JdbcMetadataQueries { private static final AbstractStatementParser PARSER = AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL); + static final String GET_SCHEMAS = + convertPositionalParametersToNamedParameters( + PARSER.removeCommentsAndTrim( + readMetaDataSqlFromFile("DatabaseMetaData_GetSchemas.sql"))); static final String GET_TABLES = convertPositionalParametersToNamedParameters( PARSER.removeCommentsAndTrim( @@ -42,6 +46,20 @@ class JdbcMetadataQueries { PARSER.removeCommentsAndTrim( readMetaDataSqlFromFile("DatabaseMetaData_GetImportedKeys.sql"))); + static final ResultSetMetadata GET_SCHEMAS_METADATA = + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("TABLE_SCHEM") + .setType(Type.newBuilder().setCode(TypeCode.STRING))) + .addFields( + Field.newBuilder() + .setName("CATALOG_NAME") + .setType(Type.newBuilder().setCode(TypeCode.STRING)))) + .build(); + static final ResultSetMetadata GET_TABLES_METADATA = ResultSetMetadata.newBuilder() .setRowType( @@ -88,6 +106,17 @@ class JdbcMetadataQueries { .setType(Type.newBuilder().setCode(TypeCode.STRING)))) .build(); + static ResultSet createGetSchemasResultSet() { + ResultSet.Builder builder = ResultSet.newBuilder().setMetadata(GET_SCHEMAS_METADATA); + for (String name : new String[]{"", "INFORMATION_SCHEMA", "SPANNER_SYS"}) { + builder.addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue(name)) + .addValues(Value.newBuilder().setStringValue(""))); + } + return builder.build(); + } + static ResultSet createGetTablesResultSet(Iterable names) { ResultSet.Builder builder = ResultSet.newBuilder().setMetadata(GET_TABLES_METADATA); for (String name : names) {