Skip to content

Commit

Permalink
Add excluded_tables configuration to bypass schema validation for
Browse files Browse the repository at this point in the history
specific tables in declarative schema mode.
This is useful when backwards incompatible changes are needed.

GitOrigin-RevId: 7fda8576ffd786cd249f331fd1305fcc28755d0b
  • Loading branch information
damar-block authored and svc-squareup-copybara committed Nov 26, 2024
1 parent cf0ef48 commit 335658e
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 7 deletions.
22 changes: 19 additions & 3 deletions misk-jdbc/api/misk-jdbc.api
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ public final class misk/jdbc/DataSourceConfig {
public fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;)V
public fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;Z)V
public fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;)V
public synthetic fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;Lmisk/jdbc/DeclarativeSchemaConfig;)V
public synthetic fun <init> (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;Lmisk/jdbc/DeclarativeSchemaConfig;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun asReplica ()Lmisk/jdbc/DataSourceConfig;
public final fun buildJdbcUrl (Lwisp/deployment/Deployment;)Ljava/lang/String;
public final fun canRecoverOnReplica ()Z
Expand Down Expand Up @@ -350,14 +351,15 @@ public final class misk/jdbc/DataSourceConfig {
public final fun component32 ()Ljava/util/Map;
public final fun component33 ()Z
public final fun component34 ()Lmisk/jdbc/MigrationsFormat;
public final fun component35 ()Lmisk/jdbc/DeclarativeSchemaConfig;
public final fun component4 ()Ljava/lang/String;
public final fun component5 ()Ljava/lang/String;
public final fun component6 ()Ljava/lang/String;
public final fun component7 ()I
public final fun component8 ()Ljava/time/Duration;
public final fun component9 ()Ljava/time/Duration;
public final fun copy (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;)Lmisk/jdbc/DataSourceConfig;
public static synthetic fun copy$default (Lmisk/jdbc/DataSourceConfig;Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;IILjava/lang/Object;)Lmisk/jdbc/DataSourceConfig;
public final fun copy (Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;Lmisk/jdbc/DeclarativeSchemaConfig;)Lmisk/jdbc/DataSourceConfig;
public static synthetic fun copy$default (Lmisk/jdbc/DataSourceConfig;Lmisk/jdbc/DataSourceType;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/time/Duration;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;ZZLjava/util/Map;ZLmisk/jdbc/MigrationsFormat;Lmisk/jdbc/DeclarativeSchemaConfig;IILjava/lang/Object;)Lmisk/jdbc/DataSourceConfig;
public fun equals (Ljava/lang/Object;)Z
public final fun getAllow_public_key_retrieval ()Z
public final fun getClient_certificate_key_store_password ()Ljava/lang/String;
Expand All @@ -367,6 +369,7 @@ public final class misk/jdbc/DataSourceConfig {
public final fun getConnection_max_lifetime ()Ljava/time/Duration;
public final fun getConnection_timeout ()Ljava/time/Duration;
public final fun getDatabase ()Ljava/lang/String;
public final fun getDeclarative_schema_config ()Lmisk/jdbc/DeclarativeSchemaConfig;
public final fun getEnabledTlsProtocols ()Ljava/util/List;
public final fun getFixed_pool_size ()I
public final fun getGenerate_hibernate_stats ()Ljava/lang/String;
Expand Down Expand Up @@ -444,6 +447,19 @@ public abstract interface class misk/jdbc/DatabasePool {
public abstract interface class misk/jdbc/DatabaseReadyService : com/google/common/util/concurrent/Service {
}

public final class misk/jdbc/DeclarativeSchemaConfig {
public fun <init> ()V
public fun <init> (Ljava/util/List;)V
public synthetic fun <init> (Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Ljava/util/List;
public final fun copy (Ljava/util/List;)Lmisk/jdbc/DeclarativeSchemaConfig;
public static synthetic fun copy$default (Lmisk/jdbc/DeclarativeSchemaConfig;Ljava/util/List;ILjava/lang/Object;)Lmisk/jdbc/DeclarativeSchemaConfig;
public fun equals (Ljava/lang/Object;)Z
public final fun getExcluded_tables ()Ljava/util/List;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public class misk/jdbc/ExtendedQueryExecutionListener : net/ttddyy/dsproxy/listener/MethodExecutionListener, net/ttddyy/dsproxy/listener/QueryExecutionListener {
public static final field Companion Lmisk/jdbc/ExtendedQueryExecutionListener$Companion;
public fun <init> ()V
Expand Down
12 changes: 12 additions & 0 deletions misk-jdbc/src/main/kotlin/misk/jdbc/DataSourceConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,17 @@ data class DataSourceConfig @JvmOverloads constructor(
*/
val mysql_enforce_writable_connections: Boolean = false,
val migrations_format: MigrationsFormat = MigrationsFormat.TRADITIONAL,
val declarative_schema_config: DeclarativeSchemaConfig? = null,
) {
init {
if (migrations_format == MigrationsFormat.DECLARATIVE) {
require(type == DataSourceType.MYSQL) {
"Declarative migrations are only supported for MySQL"
}
} else if (migrations_format == MigrationsFormat.TRADITIONAL) {
require(declarative_schema_config == null) {
"declarative_schmea_config.excluded_tables is only supported for declarative migrations"
}
}
}
fun withDefaults(): DataSourceConfig {
Expand Down Expand Up @@ -370,3 +375,10 @@ class DataSourceClustersConfig : LinkedHashMap<String, DataSourceClusterConfig>,
constructor() : super()
constructor(m: Map<String, DataSourceClusterConfig>) : super(m)
}

data class DeclarativeSchemaConfig @JvmOverloads constructor(
/**
* List of tables to exclude from schema validation
*/
val excluded_tables: List<String> = listOf(),
)
18 changes: 14 additions & 4 deletions misk-jdbc/src/main/kotlin/misk/jdbc/DeclarativeSchemaMigrator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,29 @@ internal class DeclarativeSchemaMigrator(
for (shard in shards.get()) {
val expectedTables = availableMigrations(shard)
val actualTables = appliedMigrations(shard)
compareMigrations(expectedTables, actualTables)
val excludedTables = excludedTables()
compareMigrations(expectedTables, actualTables, excludedTables)
}

return MigrationStatus.Success
}

private fun excludedTables(): Set<String> {
return connector.config().declarative_schema_config?.excluded_tables?.toSet() ?: emptySet()
}

/**
* Compare expected tables from migration files to actual database tables
*/
private fun compareMigrations(
expectedTables: Map<String, Map<String, String>>,
actualTables: Map<String, Map<String, String>>
actualTables: Map<String, Map<String, String>>,
excludedTables: Set<String>
) {
for ((expectedTable, expectedColumns) in expectedTables) {
if (excludedTables.contains(expectedTable)) {
continue
}
// Check if table exists in the database
val actualColumns = actualTables[expectedTable]
?: throw IllegalStateException("Error: Table $expectedTable missing in the database.")
Expand Down Expand Up @@ -118,13 +127,14 @@ internal class DeclarativeSchemaMigrator(
private fun appliedMigrations(shard: Shard): Map<String, Map<String, String>> {
// Store actual database tables and their columns
val actualTables = mutableMapOf<String, MutableMap<String, String>>()
val dbName = connector.config().database

dataSourceService.dataSource.connection.use {
it.target(shard) { conn ->
// Use DatabaseMetaData to get all table names
val metaData = conn.metaData
val tablesResultSet: ResultSet =
metaData.getTables(null, null, "%", arrayOf("TABLE"))
metaData.getTables(dbName, null, "%", arrayOf("TABLE"))

// Iterate through all tables in the database
while (tablesResultSet.next()) {
Expand All @@ -133,7 +143,7 @@ internal class DeclarativeSchemaMigrator(
// For each table, get column names and types
val actualColumns = mutableMapOf<String, String>()
val columnsResultSet: ResultSet =
metaData.getColumns(null, null, tableName, "%")
metaData.getColumns(dbName, null, tableName, "%")
while (columnsResultSet.next()) {
val columnName = columnsResultSet.getString("COLUMN_NAME").lowercase()
val columnType = columnsResultSet.getString("TYPE_NAME").lowercase()
Expand Down
10 changes: 10 additions & 0 deletions misk-jdbc/src/test/kotlin/misk/jdbc/DataSourceConfigTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,14 @@ class DataSourceConfigTest {
}
}
}

@Test
fun errorWhenTraditionalUsesDeclarativeSchemaConfig() {
assertFailsWith<IllegalArgumentException> {
DataSourceConfig(DataSourceType.MYSQL,
migrations_format = MigrationsFormat.TRADITIONAL,
declarative_schema_config = DeclarativeSchemaConfig(listOf("table")),
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -446,4 +446,17 @@ internal class DeclarativeSchemaMigratorTest {
declarativeSchemaMigrator.requireAll()
}
}

@Test
fun skipsValidationForExcludedTables() {
val mainSource = config.migrations_resources!![0]

resourceLoader.put(
"$mainSource/t1.sql", """
CREATE TABLE excluded_table (id bigint, PRIMARY KEY (id))
""".trimMargin()
)

assertThat(declarativeSchemaMigrator.requireAll()).isEqualTo(MigrationStatus.Success)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ mysql_data_source:
- memory:/migrations/sql
- memory:/migrations/library
migrations_format: DECLARATIVE
declarative_schema_config:
excluded_tables:
- excluded_table

0 comments on commit 335658e

Please sign in to comment.