Skip to content

Commit

Permalink
fix: EXPOSED-662 SchemaUtils.listTables() returns empty list & closes…
Browse files Browse the repository at this point in the history
… db connection (#2331)

* fix: EXPOSED-662 SchemaUtils.listTables() closes connection to db

For some unclear reason, the changes made [here](008faf3) were causing the database connection to close. This fixes that by adding a new function `tableNamesForAllSchemas()` and invoking that in `allTablesNames()`.

* fix: EXPOSED-662 SchemaUtils.listTables() returns empty list & closes db connection

- Revert change to VendorDialect.allTablesNames() so that both it and SchemaUtils.listTables()
 has original behavior (only returns lists in current schema)
- Add new SchemaUtils method to alternatively return list of all table names in all
schemas.
- Adjust tests to invoke listTables() without prior call to exists().

* fix: EXPOSED-662 SchemaUtils.listTables() returns empty list & closes db connection

- Fix KDocs for new SchemaUtils method
- Override CachableMapWithDefault properties to throw unsupported exception &
prevent future similar issues

* fix: EXPOSED-662 SchemaUtils.listTables() returns empty list & closes db connection

- Undo changes to CachableMapWithDefault

* Disallow `entries` and `keys` fields of CachableMapWithDefault

---------

Co-authored-by: Chantal Loncle <82039410+bog-walk@users.noreply.github.com>
Co-authored-by: Oleg Babichev <oleg.babichev@jetbrains.com>
  • Loading branch information
3 people authored Jan 10, 2025
1 parent b5c934d commit 9105cc1
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 17 deletions.
3 changes: 3 additions & 0 deletions exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -2202,6 +2202,7 @@ public final class org/jetbrains/exposed/sql/SchemaUtils {
public static synthetic fun dropSequence$default (Lorg/jetbrains/exposed/sql/SchemaUtils;[Lorg/jetbrains/exposed/sql/Sequence;ZILjava/lang/Object;)V
public final fun listDatabases ()Ljava/util/List;
public final fun listTables ()Ljava/util/List;
public final fun listTablesInAllSchemas ()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;
Expand Down Expand Up @@ -3824,6 +3825,7 @@ public abstract interface class org/jetbrains/exposed/sql/vendors/DatabaseDialec
public static final field Companion Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect$Companion;
public abstract fun addPrimaryKey (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;[Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/String;
public abstract fun allTablesNames ()Ljava/util/List;
public abstract fun allTablesNamesInAllSchemas ()Ljava/util/List;
public abstract fun catalog (Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/String;
public abstract fun checkTableMapping (Lorg/jetbrains/exposed/sql/Table;)Z
public abstract fun columnConstraints ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
Expand Down Expand Up @@ -4320,6 +4322,7 @@ public abstract class org/jetbrains/exposed/sql/vendors/VendorDialect : org/jetb
public fun <init> (Ljava/lang/String;Lorg/jetbrains/exposed/sql/vendors/DataTypeProvider;Lorg/jetbrains/exposed/sql/vendors/FunctionProvider;)V
public fun addPrimaryKey (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;[Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/String;
public fun allTablesNames ()Ljava/util/List;
public fun allTablesNamesInAllSchemas ()Ljava/util/List;
public fun catalog (Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/String;
public fun checkTableMapping (Lorg/jetbrains/exposed/sql/Table;)Z
public fun columnConstraints ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,12 +856,22 @@ object SchemaUtils {
}

/**
* Retrieves a list of all table names in the current database.
* Retrieves a list of all table names in the current database schema.
* The names will be returned with schema prefixes if the database supports it.
*
* @return A list of table names as strings.
*/
fun listTables(): List<String> = currentDialect.allTablesNames()

/**
* Returns a list with the names of all the tables in all database schemas.
* The names will be returned with schema prefixes, if the database supports it, and non-user defined tables,
* like system information table names, will be included.
*
* @return A list of table names as strings.
*/
fun listTablesInAllSchemas(): List<String> = currentDialect.allTablesNamesInAllSchemas()

/** Drops all [tables], using a batch execution if [inBatch] is set to `true`. */
fun drop(vararg tables: Table, inBatch: Boolean = false) {
if (tables.isEmpty()) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,19 @@ interface DatabaseDialect {
/** Returns the name of the current database. */
fun getDatabase(): String

/** Returns a list with the names of all the defined tables. */
/**
* Returns a list with the names of all the defined tables in the current database schema.
* The names will be returned with schema prefixes if the database supports it.
*/
fun allTablesNames(): List<String>

/**
* Returns a list with the names of all the tables in all database schemas.
* The names will be returned with schema prefixes, if the database supports it, and non-user defined tables,
* like system information table names, will be included.
*/
fun allTablesNamesInAllSchemas(): List<String>

/** Checks if the specified table exists in the database. */
fun tableExists(table: Table): Boolean

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,23 @@ abstract class VendorDialect(
private var _allTableNames: Map<String, List<String>>? = null
private var _allSchemaNames: List<String>? = null

/** Returns a list with the names of all the defined tables within default scheme. */
/** Returns a list with the names of all the defined tables in the current schema. */
val allTablesNames: List<String>
get() {
val connection = TransactionManager.current().connection
return connection.metadata { tableNamesByCurrentSchema(getAllTableNamesCache()).tableNames }
}

protected fun getAllTableNamesCache(): Map<String, List<String>> {
val connection = TransactionManager.current().connection
if (_allTableNames == null) {
_allTableNames = connection.metadata { tableNames }
_allTableNames = TransactionManager.current().connection.metadata { tableNames }
}
return _allTableNames!!
}

private fun getAllSchemaNamesCache(): List<String> {
val connection = TransactionManager.current().connection
if (_allSchemaNames == null) {
_allSchemaNames = connection.metadata { schemaNames }
_allSchemaNames = TransactionManager.current().connection.metadata { schemaNames }
}
return _allSchemaNames!!
}
Expand All @@ -51,9 +49,24 @@ abstract class VendorDialect(

override fun getDatabase(): String = catalog(TransactionManager.current())

/** Returns a list with the names of all the defined tables with schema prefixes if the database supports it. */
/**
* Returns a list with the names of all the defined tables in the current database schema.
* The names will be returned with schema prefixes if the database supports it.
*
* **Note:** This method always re-reads data from the database. Using `allTablesNames` field is
* the preferred way to avoid unnecessary metadata queries.
*/
override fun allTablesNames(): List<String> = TransactionManager.current().connection.metadata {
getAllTableNamesCache().flatMap { it.value }
tableNamesByCurrentSchema(null).tableNames
}

/**
* Returns a list with the names of all the tables in all database schemas.
* The names will be returned with schema prefixes, if the database supports it, and non-user defined tables,
* like system information table names, will be included.
*/
override fun allTablesNamesInAllSchemas(): List<String> = getAllSchemaNamesCache().flatMap { schema ->
getAllTableNamesCache().getValue(schema)
}

override fun tableExists(table: Table): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData)
override fun get(key: K): V? = map.getOrPut(key) { default(key) }
override fun containsKey(key: K): Boolean = true
override fun isEmpty(): Boolean = false

override val entries: Set<Map.Entry<K, V>>
get() = throw UnsupportedOperationException(
"The entries field should not be used on CachableMapWithDefault because the lazy population of the collection for missing keys " +
"and entries may lead to inconsistencies between calls."
)

override val keys: Set<K>
get() = throw UnsupportedOperationException(
"The keys field should not be used on CachableMapWithDefault because the lazy population of the collection for missing keys " +
"and keys may lead to inconsistencies between calls."
)
}

override val tableNames: Map<String, List<String>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import org.jetbrains.exposed.sql.tests.shared.Category
import org.jetbrains.exposed.sql.tests.shared.Item
import org.jetbrains.exposed.sql.tests.shared.assertEqualCollections
import org.jetbrains.exposed.sql.tests.shared.assertEquals
import org.jetbrains.exposed.sql.tests.shared.assertFalse
import org.jetbrains.exposed.sql.tests.shared.assertTrue
import org.jetbrains.exposed.sql.transactions.TransactionManager
import org.jetbrains.exposed.sql.vendors.MysqlDialect
import org.jetbrains.exposed.sql.vendors.OracleDialect
import org.jetbrains.exposed.sql.vendors.SQLServerDialect
import org.jetbrains.exposed.sql.vendors.SQLiteDialect
import org.junit.Test
import java.util.*
import kotlin.test.assertFails
Expand Down Expand Up @@ -595,20 +597,15 @@ class CreateTableTests : DatabaseTestsBase() {
assertEquals(true, OneTable.exists())
assertEquals(false, OneOneTable.exists())

val defaultSchemaName = when (currentDialectTest) {
is SQLServerDialect -> "dbo"
is OracleDialect -> testDb.user
is MysqlDialect -> testDb.db!!.name
else -> "public"
}
assertTrue(SchemaUtils.listTables().any { it.equals("$defaultSchemaName.${OneTable.tableName}", ignoreCase = true) })
val schemaPrefixedName = testDb.getDefaultSchemaPrefixedTableName(OneTable.tableName)
assertTrue(SchemaUtils.listTables().any { it.equals(schemaPrefixedName, ignoreCase = true) })

SchemaUtils.createSchema(one)
SchemaUtils.create(OneOneTable)
assertEquals(true, OneTable.exists())
assertEquals(true, OneOneTable.exists())

assertTrue(SchemaUtils.listTables().any { it.equals(OneOneTable.tableName, ignoreCase = true) })
assertTrue(SchemaUtils.listTablesInAllSchemas().any { it.equals(OneOneTable.tableName, ignoreCase = true) })
} finally {
SchemaUtils.drop(OneTable, OneOneTable)
val cascade = testDb != TestDB.SQLSERVER
Expand All @@ -617,6 +614,57 @@ class CreateTableTests : DatabaseTestsBase() {
}
}

@Test
fun testListTablesInCurrentSchema() {
withDb { testDb ->
SchemaUtils.create(OneTable)

val schemaPrefixedName = testDb.getDefaultSchemaPrefixedTableName(OneTable.tableName)
assertTrue(SchemaUtils.listTables().any { it.equals(schemaPrefixedName, ignoreCase = true) })
}

withDb { testDb ->
// ensures that db connection has not been lost by calling listTables()
assertEquals(testDb != TestDB.SQLITE, OneTable.exists())

SchemaUtils.drop(OneTable)
}
}

private fun TestDB.getDefaultSchemaPrefixedTableName(tableName: String): String = when (currentDialectTest) {
is SQLServerDialect -> "dbo.$tableName"
is OracleDialect -> "${this.user}.$tableName"
is MysqlDialect -> "${this.db!!.name}.$tableName"
is SQLiteDialect -> tableName
else -> "public.$tableName"
}

@Test
fun testListTablesInAllSchemas() {
withDb { testDb ->
if (currentDialectTest.supportsCreateSchema) {
val one = prepareSchemaForTest("one")

try {
SchemaUtils.createSchema(one)
// table "one.one" is created in new schema by db because of name
// even though current schema has not been set to the new one above
SchemaUtils.create(OneOneTable)

// so new table will not appear in list of tables in current schema
assertFalse(SchemaUtils.listTables().any { it.equals(OneOneTable.tableName, ignoreCase = true) })
// but new table appears in list of tables from all schema
assertTrue(SchemaUtils.listTablesInAllSchemas().any { it.equals(OneOneTable.tableName, ignoreCase = true) })
assertTrue(OneOneTable.exists())
} finally {
SchemaUtils.drop(OneOneTable)
val cascade = testDb != TestDB.SQLSERVER
SchemaUtils.dropSchema(one, cascade = cascade)
}
}
}
}

@Test
fun `create table with quoted name with camel case`() {
val testTable = object : IntIdTable("quotedTable") {
Expand Down

0 comments on commit 9105cc1

Please sign in to comment.