Skip to content
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.

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

feat: EXPOSED-255 Generate database migration script that can be used with any migration tool #1968

Merged
merged 7 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions docs/BREAKING_CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Breaking Changes

## 0.47.0

The function `SchemaUtils.checkExcessiveIndices` used to check both excessive indices and excessive foreign key
constraints. It now has a different behaviour and deals with excessive indices only. Also, its return type is now
`List<Index>` instead of `Unit`. A new function, `SchemaUtils.checkExcessiveForeignKeyConstraints`, deals with excessive
foreign key constraints and has a return type `List<ForeignKeyConstraint>`.

## 0.46.0

* When an Exposed table object is created with a keyword identifier (a table or column name) it now retains the exact case used before being automatically quoted in generated SQL.
Expand Down
10 changes: 9 additions & 1 deletion exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,9 @@ public final class org/jetbrains/exposed/sql/Exists : org/jetbrains/exposed/sql/
public fun toQueryBuilder (Lorg/jetbrains/exposed/sql/QueryBuilder;)V
}

public abstract interface annotation class org/jetbrains/exposed/sql/ExperimentalDatabaseMigrationApi : java/lang/annotation/Annotation {
}

public abstract interface annotation class org/jetbrains/exposed/sql/ExperimentalKeywordApi : java/lang/annotation/Annotation {
}

Expand Down Expand Up @@ -1843,7 +1846,8 @@ public final class org/jetbrains/exposed/sql/SchemaUtils {
public final fun addMissingColumnsStatements ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public static synthetic fun addMissingColumnsStatements$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Table;ZILjava/lang/Object;)Ljava/util/List;
public final fun checkCycle ([Lorg/jetbrains/exposed/sql/Table;)Z
public final fun checkExcessiveIndices ([Lorg/jetbrains/exposed/sql/Table;)V
public final fun checkExcessiveForeignKeyConstraints ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public final fun checkExcessiveIndices ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public final fun checkMappingConsistence ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public static synthetic fun checkMappingConsistence$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Table;ZILjava/lang/Object;)Ljava/util/List;
public final fun create ([Lorg/jetbrains/exposed/sql/Table;Z)V
Expand All @@ -1867,11 +1871,15 @@ public final class org/jetbrains/exposed/sql/SchemaUtils {
public static synthetic fun dropSchema$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Schema;ZZILjava/lang/Object;)V
public final fun dropSequence ([Lorg/jetbrains/exposed/sql/Sequence;Z)V
public static synthetic fun dropSequence$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Sequence;ZILjava/lang/Object;)V
public final fun generateMigrationScript ([Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;Ljava/lang/String;Z)Ljava/io/File;
public static synthetic fun generateMigrationScript$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;Ljava/lang/String;ZILjava/lang/Object;)Ljava/io/File;
public final fun listDatabases ()Ljava/util/List;
public final fun listTables ()Ljava/util/List;
public final fun setSchema (Lorg/jetbrains/exposed/sql/Schema;Z)V
public static synthetic fun setSchema$default (Lorg/jetbrains/exposed/sql/SchemaUtils;Lorg/jetbrains/exposed/sql/Schema;ZILjava/lang/Object;)V
public final fun sortTablesByReferences (Ljava/lang/Iterable;)Ljava/util/List;
public final fun statementsRequiredForDatabaseMigration ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public static synthetic fun statementsRequiredForDatabaseMigration$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Table;ZILjava/lang/Object;)Ljava/util/List;
public final fun statementsRequiredToActualizeScheme ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List;
public static synthetic fun statementsRequiredToActualizeScheme$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Table;ZILjava/lang/Object;)Ljava/util/List;
public final fun withDataBaseLock (Lorg/jetbrains/exposed/sql/Transaction;Lkotlin/jvm/functions/Function0;)V
Expand Down
193 changes: 160 additions & 33 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.jetbrains.exposed.sql
import org.jetbrains.exposed.exceptions.ExposedSQLException
import org.jetbrains.exposed.sql.transactions.TransactionManager
import org.jetbrains.exposed.sql.vendors.*
import java.io.File
import java.math.BigDecimal

/** Utility functions that assist with creating, altering, and dropping database schema objects. */
Expand Down Expand Up @@ -513,55 +514,104 @@ object SchemaUtils {
*/
fun checkMappingConsistence(vararg tables: Table, withLogs: Boolean = true): List<String> {
if (withLogs) {
checkExcessiveIndices(tables = tables)
checkExcessiveForeignKeyConstraints(tables = tables, withLogs = true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would drop withLogs flag and rely on the log level

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For computations, we can make our own inline function with message: () -> String argument or check if the logging library has one.

@bog-walk, what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that I would drop withLogs, at least for the new generate function.

We could check for the enabled level, but, for example, the results in logTimeSpent() are meant for the INFO level, which I believe a lot of users set as the default. So relying on a guard like exposedLogger.isInfoEnabled, or the implicit level check in info(), might evaluate to true for many and result in users getting logs they don't want. Maybe if we were logging everything to DEBUG it would be easier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For computations, we can make our own inline function with message: () -> String argument or check if the logging library has one.

@e5l What do you mean by "computations"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you put an expression in a string template, it will be computed regardless of the log level:

logger.info("foo: ${heavyFunctionCall()}")

Otherwise, we could have an extension function with block:

logger.info { "foo: ${heavyFunctionCall()}" }

In this case, the string template is computed only when we call lambda (and we can condition it to the log level)

I think it's not critical for this PR, feel free to implement it later or ignore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to leave this as is right now just to keep this PR small, but I made a note of it for the next PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@e5l @joc-a If the end goal is to avoid these computations, we could also switch to parameterized logging, which is supported by slf4j. This would mean using the existing function variants that take a second argument:

exposedLogger.info("foo: {}", heavyFunctionCall())

According to the docs, if I'm understanding correctly:

This form avoids superfluous object creation when the logger is disabled for the INFO level.

checkExcessiveIndices(tables = tables, withLogs = true)
}
return checkMissingIndices(tables = tables, withLogs).flatMap { it.createStatement() }
}

/**
* Checks all [tables] for any that have more than one defined foreign key constraint or index and
* logs the findings.
* Log Exposed table mappings <-> real database mapping problems and returns DDL Statements to fix them, including
* DROP/DELETE statements (unlike [checkMappingConsistence])
*/
private fun mappingConsistenceRequiredStatements(vararg tables: Table, withLogs: Boolean = true): List<String> {
return checkMissingIndices(tables = tables, withLogs).flatMap { it.createStatement() } +
checkExcessiveForeignKeyConstraints(tables = tables, withLogs).flatMap { it.dropStatement() } +
checkExcessiveIndices(tables = tables, withLogs).flatMap { it.dropStatement() }
}

/**
* Checks all [tables] for any that have more than one defined index and logs the findings. If found, this function
* also logs the SQL statements that can be used to drop these indices.
*
* If found, this function also logs the SQL statements that can be used to drop these constraints.
* @return List of indices that are excessive and can be dropped.
*/
fun checkExcessiveIndices(vararg tables: Table) {
val excessiveConstraints = currentDialect.columnConstraints(*tables).filter { it.value.size > 1 }

if (excessiveConstraints.isNotEmpty()) {
exposedLogger.warn("List of excessive foreign key constraints:")
excessiveConstraints.forEach { (pair, fk) ->
val constraint = fk.first()
val fkPartToLog = fk.joinToString(", ") { it.fkName }
exposedLogger.warn(
"\t\t\t'${pair.first}'.'${pair.second}' -> '${constraint.fromTableName}':\t$fkPartToLog"
)
@Suppress("NestedBlockDepth")
fun checkExcessiveIndices(vararg tables: Table, withLogs: Boolean): List<Index> {
val excessiveIndices =
currentDialect.existingIndices(*tables).flatMap { (_, indices) ->
indices
}.groupBy { index -> Triple(index.table, index.unique, index.columns.joinToString { column -> column.name }) }
.filter { (_, indices) -> indices.size > 1 }

return if (excessiveIndices.isEmpty()) {
emptyList()
} else {
val toDrop = HashSet<Index>()

if (withLogs) {
exposedLogger.warn("List of excessive indices:")
excessiveIndices.forEach { (triple, indices) ->
val indexNames = indices.joinToString(", ") { index -> index.indexName }
exposedLogger.warn("\t\t\t'${triple.first.tableName}'.'${triple.third}' -> $indexNames")
}

exposedLogger.info("SQL Queries to remove excessive indices:")
}

exposedLogger.info("SQL Queries to remove excessive keys:")
excessiveConstraints.forEach { (_, value) ->
value.take(value.size - 1).forEach {
exposedLogger.info("\t\t\t${it.dropStatement()};")
excessiveIndices.forEach { (_, indices) ->
indices.take(indices.size - 1).forEach { index ->
toDrop.add(index)

if (withLogs) {
exposedLogger.info("\t\t\t${index.dropStatement()};")
}
}
}

toDrop.toList()
}
}

val excessiveIndices =
currentDialect.existingIndices(*tables).flatMap {
it.value
}.groupBy { Triple(it.table, it.unique, it.columns.joinToString { it.name }) }
.filter { it.value.size > 1 }
if (excessiveIndices.isNotEmpty()) {
exposedLogger.warn("List of excessive indices:")
excessiveIndices.forEach { (triple, indices) ->
val indexNames = indices.joinToString(", ") { it.indexName }
exposedLogger.warn("\t\t\t'${triple.first.tableName}'.'${triple.third}' -> $indexNames")
/**
* Checks all [tables] for any that have more than one defined foreign key constraint and logs the findings. If
* found, this function also logs the SQL statements that can be used to drop these foreign key constraints.
*
* @return List of foreign key constraints that are excessive and can be dropped.
*/
@Suppress("NestedBlockDepth")
fun checkExcessiveForeignKeyConstraints(vararg tables: Table, withLogs: Boolean): List<ForeignKeyConstraint> {
val excessiveConstraints = currentDialect.columnConstraints(*tables).filter { (_, fkConstraints) -> fkConstraints.size > 1 }

return if (excessiveConstraints.isEmpty()) {
emptyList()
} else {
val toDrop = HashSet<ForeignKeyConstraint>()

if (withLogs) {
exposedLogger.warn("List of excessive foreign key constraints:")
excessiveConstraints.forEach { (table, columns), fkConstraints ->
val constraint = fkConstraints.first()
val fkPartToLog = fkConstraints.joinToString(", ") { fkConstraint -> fkConstraint.fkName }
exposedLogger.warn(
"\t\t\t'$table'.'$columns' -> '${constraint.fromTableName}':\t$fkPartToLog"
)
}

exposedLogger.info("SQL Queries to remove excessive keys:")
}
exposedLogger.info("SQL Queries to remove excessive indices:")
excessiveIndices.forEach {
it.value.take(it.value.size - 1).forEach {
exposedLogger.info("\t\t\t${it.dropStatement()};")

excessiveConstraints.forEach { (_, fkConstraints) ->
fkConstraints.take(fkConstraints.size - 1).forEach { fkConstraint ->
toDrop.add(fkConstraint)

if (withLogs) {
exposedLogger.info("\t\t\t${fkConstraint.dropStatement()};")
}
}
}

toDrop.toList()
}
}

Expand Down Expand Up @@ -624,6 +674,75 @@ object SchemaUtils {
return toCreate.toList()
}

/**
* @param tables The tables whose changes will be used to generate the migration script.
* @param scriptName The name to be used for the generated migration script.
* @param scriptDirectory The directory (path from repository root) in which to create the migration script.
* @param withLogs By default, a description for each intermediate step, as well as its execution time, is logged at
* the INFO level. This can be disabled by setting [withLogs] to `false`.
*
* @return The generated migration script.
*
* @throws IllegalArgumentException if no argument is passed for the [tables] parameter.
*
* This function simply generates the migration script without applying the migration. Its purpose is to show what
* the migration script will look like before applying the migration.
* If a migration script with the same name already exists, its content will be overwritten.
*/
@ExperimentalDatabaseMigrationApi
fun generateMigrationScript(vararg tables: Table, scriptDirectory: String, scriptName: String, withLogs: Boolean = true): File {
require(tables.isNotEmpty()) { "Tables argument must not be empty" }

val allStatements = statementsRequiredForDatabaseMigration(*tables, withLogs = withLogs)

val migrationScript = File("$scriptDirectory/$scriptName.sql")
migrationScript.createNewFile()

// Clear existing content
migrationScript.writeText("")

// Append statements
allStatements.forEach { statement ->
// Add semicolon only if it's not already there
val conditionalSemicolon = if (statement.last() == ';') "" else ";"

migrationScript.appendText("$statement$conditionalSemicolon\n")
}

return migrationScript
}

/**
* Returns the SQL statements that need to be executed to make the existing database schema compatible with
* the table objects defined using Exposed. Unlike [statementsRequiredToActualizeScheme], DROP/DELETE statements are
* included.
*
* **Note:** Some dialects, like SQLite, do not support `ALTER TABLE ADD COLUMN` syntax completely,
* which restricts the behavior when adding some missing columns. Please check the documentation.
*
* By default, a description for each intermediate step, as well as its execution time, is logged at the INFO level.
* This can be disabled by setting [withLogs] to `false`.
*/
fun statementsRequiredForDatabaseMigration(vararg tables: Table, withLogs: Boolean = true): List<String> {
joc-a marked this conversation as resolved.
Show resolved Hide resolved
val (tablesToCreate, tablesToAlter) = tables.partition { !it.exists() }
val createStatements = logTimeSpent("Preparing create tables statements", withLogs) {
createStatements(tables = tablesToCreate.toTypedArray())
}
val alterStatements = logTimeSpent("Preparing alter table statements", withLogs) {
addMissingColumnsStatements(tables = tablesToAlter.toTypedArray(), withLogs)
}

val modifyTablesStatements = logTimeSpent("Checking mapping consistence", withLogs) {
mappingConsistenceRequiredStatements(
tables = tables,
withLogs
).filter { it !in (createStatements + alterStatements) }
}

val allStatements = createStatements + alterStatements + modifyTablesStatements
return allStatements
}

/**
* Creates table with name "busy" (if not present) and single column to be used as "synchronization" point. Table wont be dropped after execution.
*
Expand Down Expand Up @@ -745,3 +864,11 @@ object SchemaUtils {
}
}
}

@RequiresOptIn(
message = "This database migration API is experimental. " +
"Its usage must be marked with '@OptIn(org.jetbrains.exposed.sql.ExperimentalDatabaseMigrationApi::class)' " +
"or '@org.jetbrains.exposed.sql.ExperimentalDatabaseMigrationApi'."
)
@Target(AnnotationTarget.FUNCTION)
annotation class ExperimentalDatabaseMigrationApi
Loading
Loading