From 52a901dc1499c96e8c941461b34f74ef90eb78fc Mon Sep 17 00:00:00 2001 From: "Kenneth J. Shackleton" Date: Mon, 3 May 2021 18:41:33 +0100 Subject: [PATCH] Patch 0.12.6. Signed-off-by: Kenneth J. Shackleton --- .../com/bloomberg/selekt/android/Cursors.kt | 18 ++---- .../android/support/SupportSQLiteDatabase.kt | 11 ++-- CHANGELOG.md | 9 +++ .../kotlin/com/bloomberg/selekt/Queries.kt | 2 +- .../com/bloomberg/selekt/commons/Database.kt | 10 +++- .../selekt/pools/CommonObjectPool.kt | 11 ++-- .../selekt/pools/SingleObjectPool.kt | 9 +-- .../com/bloomberg/selekt/SQLConnectionTest.kt | 8 +++ .../selekt/commons/DatabaseKtTest.kt | 56 ++++++++++++++++++- .../selekt/pools/CommonObjectPoolTest.kt | 15 +++++ .../selekt/pools/SingleObjectPoolTest.kt | 15 +++++ gradle.properties | 2 +- 12 files changed, 133 insertions(+), 33 deletions(-) diff --git a/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/Cursors.kt b/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/Cursors.kt index 7731755620..a84eaab325 100644 --- a/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/Cursors.kt +++ b/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/Cursors.kt @@ -20,20 +20,14 @@ import android.database.AbstractCursor import android.database.Cursor import com.bloomberg.selekt.ColumnType import com.bloomberg.selekt.ICursor -import com.bloomberg.selekt.SQL_BLOB -import com.bloomberg.selekt.SQL_FLOAT -import com.bloomberg.selekt.SQL_INTEGER -import com.bloomberg.selekt.SQL_NULL -import com.bloomberg.selekt.SQL_TEXT import javax.annotation.concurrent.NotThreadSafe -private fun ColumnType.asAndroidCursorFieldType() = when (sqlDataType) { - SQL_TEXT -> Cursor.FIELD_TYPE_STRING - SQL_INTEGER -> Cursor.FIELD_TYPE_INTEGER - SQL_NULL -> Cursor.FIELD_TYPE_NULL - SQL_FLOAT -> Cursor.FIELD_TYPE_FLOAT - SQL_BLOB -> Cursor.FIELD_TYPE_BLOB - else -> error("Unrecognised column type: $sqlDataType") +private fun ColumnType.asAndroidCursorFieldType() = when (this) { + ColumnType.STRING -> Cursor.FIELD_TYPE_STRING + ColumnType.INTEGER -> Cursor.FIELD_TYPE_INTEGER + ColumnType.NULL -> Cursor.FIELD_TYPE_NULL + ColumnType.FLOAT -> Cursor.FIELD_TYPE_FLOAT + ColumnType.BLOB -> Cursor.FIELD_TYPE_BLOB } internal fun ICursor.asAndroidCursor(): Cursor = CursorWrapper(this) diff --git a/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/support/SupportSQLiteDatabase.kt b/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/support/SupportSQLiteDatabase.kt index d814ea6a80..289232c3b0 100644 --- a/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/support/SupportSQLiteDatabase.kt +++ b/AndroidLib/src/main/kotlin/com/bloomberg/selekt/android/support/SupportSQLiteDatabase.kt @@ -88,13 +88,12 @@ private class SupportSQLiteDatabase constructor( override fun execSQL(@Language("RoomSql") sql: String, bindArgs: Array) = database.exec(sql, bindArgs) - override fun getAttachedDbs() = sequence> { - database.query("PRAGMA database_list", null).use { - while (it.moveToNext()) { - yield(Pair(it.getString(1), it.getString(2))) - } + override fun getAttachedDbs() = database.query("PRAGMA database_list", null).use { + List>(it.count) { _ -> + it.moveToNext() + Pair(it.getString(1), it.getString(2)) } - }.toList() + } override fun getMaximumSize() = database.maximumSize diff --git a/CHANGELOG.md b/CHANGELOG.md index 323d6d059a..fe47a08fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ Change Log ========== +## Version 0.12.6 + +### Fixes + +* Fix convenience method for deleting database files, and extend its test coverage. +* Use the `ColumnType`-enum directly when converting to an Android Cursor column type. This increases code coverage by removing a branch we cannot test. +* Appeal instead to a Java-cast in Queries when casting an array, Kotlin's `as` results in a generated null-check branch which won't be encountered here and can't be tested. This increases code coverage. +* Reduce the number of logical branches in the pool implementations, simplifying these implementations and increasing their test coverage. + ## Version 0.12.5 ### Fixes diff --git a/Lib/src/main/kotlin/com/bloomberg/selekt/Queries.kt b/Lib/src/main/kotlin/com/bloomberg/selekt/Queries.kt index 3533727206..b3a5896938 100644 --- a/Lib/src/main/kotlin/com/bloomberg/selekt/Queries.kt +++ b/Lib/src/main/kotlin/com/bloomberg/selekt/Queries.kt @@ -46,7 +46,7 @@ internal class SQLQuery internal constructor( @Language("RoomSql") sql: String, statementType: SQLStatementType, args: Array - ) = SQLQuery(session, sql, statementType, args.copyOf() as Array) + ) = SQLQuery(session, sql, statementType, Array::class.java.cast(args.copyOf())) } override fun bindBlob(index: Int, value: ByteArray) = bind(index, value) diff --git a/Lib/src/main/kotlin/com/bloomberg/selekt/commons/Database.kt b/Lib/src/main/kotlin/com/bloomberg/selekt/commons/Database.kt index abeabf4928..9598dd6ef9 100644 --- a/Lib/src/main/kotlin/com/bloomberg/selekt/commons/Database.kt +++ b/Lib/src/main/kotlin/com/bloomberg/selekt/commons/Database.kt @@ -25,8 +25,12 @@ fun deleteDatabase(file: File) = file.run { File("$absolutePath-shm").delete() or File("$absolutePath-wal").delete() parentFile?.let { - val prefix = "${it.name}-mj" - it.listFiles { f -> f.name.startsWith(prefix) } - }?.forEach { deleted = deleted or it.delete() } + val prefix = "$name-mj" + it.listFiles { + f -> f.name.startsWith(prefix) + }?.forEach { + deleted = deleted or it.delete() + } + } deleted } diff --git a/Lib/src/main/kotlin/com/bloomberg/selekt/pools/CommonObjectPool.kt b/Lib/src/main/kotlin/com/bloomberg/selekt/pools/CommonObjectPool.kt index 7d14ef6e7a..1409a2ef5f 100644 --- a/Lib/src/main/kotlin/com/bloomberg/selekt/pools/CommonObjectPool.kt +++ b/Lib/src/main/kotlin/com/bloomberg/selekt/pools/CommonObjectPool.kt @@ -186,11 +186,12 @@ class CommonObjectPool>( } } - private infix fun T.shouldBeRemovedAt(priority: Priority?): Boolean { - fun T.isIdle() = this@isIdle.tag != this@CommonObjectPool.tag - return priority == null && isIdle() && future?.isCancelled == false || - !priority.isHigh() && isIdle() || + private infix fun T.shouldBeRemovedAt( + priority: Priority? + ) = (this@shouldBeRemovedAt.tag != this@CommonObjectPool.tag).let { + priority == null && it && future?.isCancelled == false || isClosed.get() || - priority.isHigh() + priority.isHigh() || + it } } diff --git a/Lib/src/main/kotlin/com/bloomberg/selekt/pools/SingleObjectPool.kt b/Lib/src/main/kotlin/com/bloomberg/selekt/pools/SingleObjectPool.kt index e37d0bce4d..8d71efc478 100644 --- a/Lib/src/main/kotlin/com/bloomberg/selekt/pools/SingleObjectPool.kt +++ b/Lib/src/main/kotlin/com/bloomberg/selekt/pools/SingleObjectPool.kt @@ -149,9 +149,10 @@ class SingleObjectPool>( } } - private infix fun T.shouldBeRemovedAt(priority: Priority?) = - priority == null && canEvict && future?.isCancelled == false || - !priority.isHigh() && canEvict || + private infix fun T.shouldBeRemovedAt(priority: Priority?) = canEvict.let { + priority == null && it && future?.isCancelled == false || isClosed || - priority.isHigh() + priority.isHigh() || + it + } } diff --git a/Lib/src/test/kotlin/com/bloomberg/selekt/SQLConnectionTest.kt b/Lib/src/test/kotlin/com/bloomberg/selekt/SQLConnectionTest.kt index 5f49c1ce25..d029f5534d 100644 --- a/Lib/src/test/kotlin/com/bloomberg/selekt/SQLConnectionTest.kt +++ b/Lib/src/test/kotlin/com/bloomberg/selekt/SQLConnectionTest.kt @@ -281,6 +281,14 @@ internal class SQLConnectionTest { } } + @Test + fun releaseMemory() { + SQLConnection("file::memory:", sqlite, databaseConfiguration, 0, CommonThreadLocalRandom, null).use { + it.releaseMemory() + verify(sqlite, times(1)).databaseReleaseMemory(any()) + } + } + private companion object { const val DB = 1L } diff --git a/Lib/src/test/kotlin/com/bloomberg/selekt/commons/DatabaseKtTest.kt b/Lib/src/test/kotlin/com/bloomberg/selekt/commons/DatabaseKtTest.kt index 4564884c5b..99563e2d30 100644 --- a/Lib/src/test/kotlin/com/bloomberg/selekt/commons/DatabaseKtTest.kt +++ b/Lib/src/test/kotlin/com/bloomberg/selekt/commons/DatabaseKtTest.kt @@ -16,15 +16,69 @@ package com.bloomberg.selekt.commons +import com.nhaarman.mockitokotlin2.doReturn +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever import org.assertj.core.api.Assertions.assertThatExceptionOfType +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertDoesNotThrow +import java.io.File import java.nio.file.Files +import kotlin.test.assertEquals +import kotlin.test.assertTrue internal class DatabaseKtTest { + private val parent = Files.createTempDirectory("foo").toFile().also { it.deleteOnExit() } + + @AfterEach + fun tearDown() { + assertTrue(parent.deleteRecursively()) + } + + @Test + fun deleteDatabase() { + arrayOf( + "db", + "db-journal", + "db-shm", + "db-wal" + ).forEach { + Files.createFile(File("${parent.absolutePath}/$it").toPath()) + } + Files.createFile(File("${parent.absolutePath}/db-mj-bar").toPath()) + assertEquals(false, parent.listFiles { f -> f.name.startsWith("db") }?.isEmpty()) + deleteDatabase(File("${parent.absolutePath}/db")) + assertEquals(true, parent.listFiles { f -> f.name.startsWith("db") }?.isEmpty()) + } + @Test fun deleteDatabaseDirectory() { assertThatExceptionOfType(IllegalArgumentException::class.java).isThrownBy { - deleteDatabase(Files.createTempDirectory("foo").toFile().also { it.deleteOnExit() }) + deleteDatabase(parent) + } + } + + @Test + fun deleteDatabaseWithoutParent() { + mock().apply { + whenever(isFile) doReturn true + }.let { + assertDoesNotThrow { + deleteDatabase(it) + } + } + } + + @Test + fun deleteDatabaseWithoutParentFile() { + mock().apply { + whenever(isFile) doReturn true + whenever(parentFile) doReturn File("foo") + }.let { + assertDoesNotThrow { + deleteDatabase(it) + } } } } diff --git a/Lib/src/test/kotlin/com/bloomberg/selekt/pools/CommonObjectPoolTest.kt b/Lib/src/test/kotlin/com/bloomberg/selekt/pools/CommonObjectPoolTest.kt index 562e620a0b..b0d89e5f0f 100644 --- a/Lib/src/test/kotlin/com/bloomberg/selekt/pools/CommonObjectPoolTest.kt +++ b/Lib/src/test/kotlin/com/bloomberg/selekt/pools/CommonObjectPoolTest.kt @@ -260,6 +260,21 @@ internal class CommonObjectPoolTest { assertSame("Object was evicted.", obj, it.borrowObject()) } + @Test + fun evictionNegativeIntervalFails() = CommonObjectPool(object : IObjectFactory { + override fun close() = Unit + + override fun destroyObject(obj: PooledObject) = Unit + + override fun makeObject() = makePrimaryObject() + + override fun makePrimaryObject() = PooledObject() + }, executor, configuration.copy(evictionIntervalMillis = -1L, evictionDelayMillis = 100L), other).use { + val obj = it.borrowObject().apply { it.returnObject(this) } + Thread.sleep(500L) + assertSame(obj, it.borrowObject().apply { it.returnObject(this) }, "Pool must return the same object.") + } + @Test fun throwsOnBorrowAfterClose(): Unit = pool.run { close() diff --git a/Lib/src/test/kotlin/com/bloomberg/selekt/pools/SingleObjectPoolTest.kt b/Lib/src/test/kotlin/com/bloomberg/selekt/pools/SingleObjectPoolTest.kt index 006811b183..1ed7e17ce5 100644 --- a/Lib/src/test/kotlin/com/bloomberg/selekt/pools/SingleObjectPoolTest.kt +++ b/Lib/src/test/kotlin/com/bloomberg/selekt/pools/SingleObjectPoolTest.kt @@ -132,6 +132,21 @@ internal class SingleObjectPoolTest { assertSame(two, borrowObject().also { returnObject(it) }, "Pool must return the same object.") } + @Test + fun evictionNegativeIntervalFails() = SingleObjectPool(object : IObjectFactory { + override fun close() = Unit + + override fun destroyObject(obj: PooledObject) = Unit + + override fun makeObject() = makePrimaryObject() + + override fun makePrimaryObject() = PooledObject() + }, executor, 100L, -1L).use { + val obj = it.borrowObject().apply { it.returnObject(this) } + Thread.sleep(500L) + assertSame(obj, it.borrowObject().apply { it.returnObject(this) }, "Pool must return the same object.") + } + @Test fun newObjectAfterSuccessfulEviction() = pool.run { val obj = borrowObject().also { returnObject(it) } diff --git a/gradle.properties b/gradle.properties index ce524a96a2..0609ab5e97 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -selekt.versionName=0.12.5 +selekt.versionName=0.12.6 openssl.version=1.1.1 openssl.version.suffix=d