From e14fd23eadb5765d3694593d9d2e2dff0f67b018 Mon Sep 17 00:00:00 2001 From: "Daniel J. Summers" Date: Fri, 28 Feb 2025 18:10:47 -0500 Subject: [PATCH] Fix count tests; use types for varying comparison values --- src/integration-test/kotlin/common/Count.kt | 6 +- src/main/kotlin/Comparison.kt | 60 ++++---- src/main/kotlin/Field.kt | 30 ++-- src/main/kotlin/Parameter.kt | 37 +++++ src/main/kotlin/Parameters.kt | 35 +---- src/test/kotlin/ComparisonTest.kt | 143 +++++++++++++++----- 6 files changed, 204 insertions(+), 107 deletions(-) diff --git a/src/integration-test/kotlin/common/Count.kt b/src/integration-test/kotlin/common/Count.kt index adffb9f..878d079 100644 --- a/src/integration-test/kotlin/common/Count.kt +++ b/src/integration-test/kotlin/common/Count.kt @@ -17,7 +17,7 @@ object Count { JsonDocument.load(db) assertEquals( 3L, - db.conn.countByFields(TEST_TABLE, listOf(Field.between("num_value", 10, 20))), + db.conn.countByFields(TEST_TABLE, listOf(Field.between("numValue", 10, 20))), "There should have been 3 matching documents" ) } @@ -53,7 +53,7 @@ object Count { JsonDocument.load(db) assertEquals( 2L, - db.conn.countByJsonPath(TEST_TABLE, "$.num_value ? (@ < 5)"), + db.conn.countByJsonPath(TEST_TABLE, "$.numValue ? (@ < 5)"), "There should have been 2 matching documents" ) } @@ -62,7 +62,7 @@ object Count { JsonDocument.load(db) assertEquals( 0L, - db.conn.countByJsonPath(TEST_TABLE, "$.num_value ? (@ > 100)"), + db.conn.countByJsonPath(TEST_TABLE, "$.numValue ? (@ > 100)"), "There should have been no matching documents" ) } diff --git a/src/main/kotlin/Comparison.kt b/src/main/kotlin/Comparison.kt index ac0c0e0..f187882 100644 --- a/src/main/kotlin/Comparison.kt +++ b/src/main/kotlin/Comparison.kt @@ -1,25 +1,43 @@ package solutions.bitbadger.documents +/** + * Information required to generate a JSON field comparison + */ interface Comparison { + /** The operation for the field comparison */ val op: Op - val isNumeric: Boolean - + /** The value against which the comparison will be made */ val value: T + + /** Whether the value should be considered numeric */ + val isNumeric: Boolean } /** - * A single-value comparison against a field in a JSON document + * Function to determine if a value is numeric * - * @property op The operation for the field comparison - * @property value The value against which the comparison will be made + * @param it The value in question + * @return True if it is a numeric type, false if not */ -class SingleComparison(override val op: Op, override val value: T) : Comparison { +private fun isNumeric(it: T) = + it is Byte || it is Short || it is Int || it is Long - /** Is the value for this comparison a numeric value? */ - override val isNumeric: Boolean - get() = value.let { it is Byte || it is Short || it is Int || it is Long } +/** + * A single-value comparison against a field in a JSON document + */ +class ComparisonSingle(override val op: Op, override val value: T) : Comparison { + + init { + when (op) { + Op.BETWEEN, Op.IN, Op.IN_ARRAY -> + throw DocumentException("Cannot use single comparison for multiple values") + else -> { } + } + } + + override val isNumeric = isNumeric(value) override fun toString() = "$op $value" @@ -28,27 +46,23 @@ class SingleComparison(override val op: Op, override val value: T) : Comparis /** * A range comparison against a field in a JSON document */ -class BetweenComparison(override val op: Op = Op.BETWEEN, override val value: Pair) : Comparison> { - - override val isNumeric: Boolean - get() = value.first.let { it is Byte || it is Short || it is Int || it is Long } +class ComparisonBetween(override val value: Pair) : Comparison> { + override val op = Op.BETWEEN + override val isNumeric = isNumeric(value.first) } /** * A check within a collection of values */ -class InComparison(override val op: Op = Op.IN, override val value: Collection) : Comparison> { - - override val isNumeric: Boolean - get() = !value.isEmpty() && value.elementAt(0).let { it is Byte || it is Short || it is Int || it is Long } +class ComparisonIn(override val value: Collection) : Comparison> { + override val op = Op.IN + override val isNumeric = !value.isEmpty() && isNumeric(value.elementAt(0)) } /** - * A check within a collection of values + * A check within a collection of values against an array in a document */ -class InArrayComparison(override val op: Op = Op.IN_ARRAY, override val value: Pair>) : Comparison>> { - - override val isNumeric: Boolean - get() = !value.second.isEmpty() && value.second.elementAt(0) - .let { it is Byte || it is Short || it is Int || it is Long } +class ComparisonInArray(override val value: Pair>) : Comparison>> { + override val op = Op.IN_ARRAY + override val isNumeric = false } diff --git a/src/main/kotlin/Field.kt b/src/main/kotlin/Field.kt index 680609a..4a3d8e8 100644 --- a/src/main/kotlin/Field.kt +++ b/src/main/kotlin/Field.kt @@ -101,18 +101,18 @@ class Field private constructor( fun appendParameter(existing: MutableCollection>): MutableCollection> { val typ = if (comparison.isNumeric) ParameterType.NUMBER else ParameterType.STRING when (comparison) { - is BetweenComparison<*> -> { + is ComparisonBetween<*> -> { existing.add(Parameter("${parameterName}min", typ, comparison.value.first)) existing.add(Parameter("${parameterName}max", typ, comparison.value.second)) } - is InComparison<*> -> { + is ComparisonIn<*> -> { comparison.value.forEachIndexed { index, item -> existing.add(Parameter("${parameterName}_$index", typ, item)) } } - is InArrayComparison<*> -> { + is ComparisonInArray<*> -> { val mkString = Configuration.dialect("append parameters for InArray") == Dialect.POSTGRESQL // TODO: I think this is actually Pair> comparison.value.second.forEachIndexed { index, item -> @@ -147,7 +147,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun equal(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.EQUAL, value), paramName) + Field(name, ComparisonSingle(Op.EQUAL, value), paramName) /** * Create a field greater-than comparison @@ -158,7 +158,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun greater(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.GREATER, value), paramName) + Field(name, ComparisonSingle(Op.GREATER, value), paramName) /** * Create a field greater-than-or-equal-to comparison @@ -169,7 +169,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun greaterOrEqual(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.GREATER_OR_EQUAL, value), paramName) + Field(name, ComparisonSingle(Op.GREATER_OR_EQUAL, value), paramName) /** * Create a field less-than comparison @@ -180,7 +180,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun less(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.LESS, value), paramName) + Field(name, ComparisonSingle(Op.LESS, value), paramName) /** * Create a field less-than-or-equal-to comparison @@ -191,7 +191,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun lessOrEqual(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.LESS_OR_EQUAL, value), paramName) + Field(name, ComparisonSingle(Op.LESS_OR_EQUAL, value), paramName) /** * Create a field inequality comparison @@ -202,7 +202,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun notEqual(name: String, value: T, paramName: String? = null) = - Field(name, SingleComparison(Op.NOT_EQUAL, value), paramName) + Field(name, ComparisonSingle(Op.NOT_EQUAL, value), paramName) /** * Create a field range comparison @@ -214,7 +214,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun between(name: String, minValue: T, maxValue: T, paramName: String? = null) = - Field(name, BetweenComparison(value = Pair(minValue, maxValue)), paramName) + Field(name, ComparisonBetween(Pair(minValue, maxValue)), paramName) /** * Create a field where any values match (SQL `IN`) @@ -225,7 +225,7 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun any(name: String, values: Collection, paramName: String? = null) = - Field(name, InComparison(value = values), paramName) + Field(name, ComparisonIn(values), paramName) /** * Create a field where values should exist in a document's array @@ -237,16 +237,16 @@ class Field private constructor( * @return A `Field` with the given comparison */ fun inArray(name: String, tableName: String, values: Collection, paramName: String? = null) = - Field(name, InArrayComparison(value = Pair(tableName, values)), paramName) + Field(name, ComparisonInArray(Pair(tableName, values)), paramName) fun exists(name: String) = - Field(name, SingleComparison(Op.EXISTS, "")) + Field(name, ComparisonSingle(Op.EXISTS, "")) fun notExists(name: String) = - Field(name, SingleComparison(Op.NOT_EXISTS, "")) + Field(name, ComparisonSingle(Op.NOT_EXISTS, "")) fun named(name: String) = - Field(name, SingleComparison(Op.EQUAL, "")) + Field(name, ComparisonSingle(Op.EQUAL, "")) fun nameToPath(name: String, dialect: Dialect, format: FieldFormat): String { val path = StringBuilder("data") diff --git a/src/main/kotlin/Parameter.kt b/src/main/kotlin/Parameter.kt index 0503812..1bd707b 100644 --- a/src/main/kotlin/Parameter.kt +++ b/src/main/kotlin/Parameter.kt @@ -1,5 +1,8 @@ package solutions.bitbadger.documents +import java.sql.PreparedStatement +import java.sql.Types + /** * A parameter to use for a query * @@ -8,11 +11,45 @@ package solutions.bitbadger.documents * @property value The value of the parameter */ class Parameter(val name: String, val type: ParameterType, val value: T) { + init { if (!name.startsWith(':') && !name.startsWith('@')) throw DocumentException("Name must start with : or @ ($name)") } + /** + * Bind this parameter to a prepared statement at the given index + * + * @param stmt The prepared statement to which this parameter should be bound + * @param index The index (1-based) to which the parameter should be bound + */ + fun bind(stmt: PreparedStatement, index: Int) { + when (type) { + ParameterType.NUMBER -> { + when (value) { + null -> stmt.setNull(index, Types.NULL) + is Byte -> stmt.setByte(index, value) + is Short -> stmt.setShort(index, value) + is Int -> stmt.setInt(index, value) + is Long -> stmt.setLong(index, value) + else -> throw DocumentException( + "Number parameter must be Byte, Short, Int, or Long (${value!!::class.simpleName})" + ) + } + } + + ParameterType.STRING -> { + when (value) { + null -> stmt.setNull(index, Types.NULL) + is String -> stmt.setString(index, value) + else -> stmt.setString(index, value.toString()) + } + } + + ParameterType.JSON -> stmt.setObject(index, value as String, Types.OTHER) + } + } + override fun toString() = "$type[$name] = $value" } diff --git a/src/main/kotlin/Parameters.kt b/src/main/kotlin/Parameters.kt index c428795..11ff18b 100644 --- a/src/main/kotlin/Parameters.kt +++ b/src/main/kotlin/Parameters.kt @@ -3,7 +3,6 @@ package solutions.bitbadger.documents import java.sql.Connection import java.sql.PreparedStatement import java.sql.SQLException -import java.sql.Types /** * Functions to assist with the creation and implementation of parameters for SQL queries @@ -58,7 +57,7 @@ object Parameters { */ fun replaceNamesInQuery(query: String, parameters: Collection>) = parameters.sortedByDescending { it.name.length }.fold(query) { acc, param -> acc.replace(param.name, "?") } - .also(::println) + /** * Apply the given parameters to the given query, returning a prepared statement * @@ -69,6 +68,7 @@ object Parameters { * @throws DocumentException If parameter names are invalid or number value types are invalid */ fun apply(conn: Connection, query: String, parameters: Collection>): PreparedStatement { + if (parameters.isEmpty()) return try { conn.prepareStatement(query) } catch (ex: SQLException) { @@ -88,34 +88,9 @@ object Parameters { replaceNamesInQuery(query, parameters) .let { conn.prepareStatement(it) } .also { stmt -> - replacements.sortedBy { it.first }.map { it.second }.forEachIndexed { index, param -> - val idx = index + 1 - when (param.type) { - ParameterType.NUMBER -> { - when (param.value) { - null -> stmt.setNull(idx, Types.NULL) - is Byte -> stmt.setByte(idx, param.value) - is Short -> stmt.setShort(idx, param.value) - is Int -> stmt.setInt(idx, param.value) - is Long -> stmt.setLong(idx, param.value) - else -> throw DocumentException( - "Number parameter must be Byte, Short, Int, or Long " + - "(${param.value::class.simpleName})" - ) - } - } - - ParameterType.STRING -> { - when (param.value) { - null -> stmt.setNull(idx, Types.NULL) - is String -> stmt.setString(idx, param.value) - else -> stmt.setString(idx, param.value.toString()) - } - } - - ParameterType.JSON -> stmt.setObject(idx, param.value as String, Types.OTHER) - } - } + replacements.sortedBy { it.first } + .map { it.second } + .forEachIndexed { index, param -> param.bind(stmt, index + 1) } } } catch (ex: SQLException) { throw DocumentException("Error creating query / binding parameters: ${ex.message}", ex) diff --git a/src/test/kotlin/ComparisonTest.kt b/src/test/kotlin/ComparisonTest.kt index fb558df..f92fb68 100644 --- a/src/test/kotlin/ComparisonTest.kt +++ b/src/test/kotlin/ComparisonTest.kt @@ -2,94 +2,165 @@ package solutions.bitbadger.documents import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue /** - * Unit tests for the `Comparison` class + * Unit tests for the `ComparisonBetween` class */ -@DisplayName("Comparison") -class ComparisonTest { +@DisplayName("ComparisonBetween") +class ComparisonBetweenTest { + + @Test + @DisplayName("op is set to BETWEEN") + fun op() = + assertEquals(Op.BETWEEN, ComparisonBetween(Pair(0, 0)).op, "Between comparison should have BETWEEN op") + + @Test + @DisplayName("isNumeric is false with strings") + fun isNumericFalseForStringsAndBetween() = + assertFalse(ComparisonBetween(Pair("eh", "zed")).isNumeric, + "A BETWEEN with strings should not be numeric") + + @Test + @DisplayName("isNumeric is true with bytes") + fun isNumericTrueForByteAndBetween() = + assertTrue(ComparisonBetween(Pair(7, 11)).isNumeric, "A BETWEEN with bytes should be numeric") + + @Test + @DisplayName("isNumeric is true with shorts") + fun isNumericTrueForShortAndBetween() = + assertTrue(ComparisonBetween(Pair(0, 9)).isNumeric, + "A BETWEEN with shorts should be numeric") + + @Test + @DisplayName("isNumeric is true with ints") + fun isNumericTrueForIntAndBetween() = + assertTrue(ComparisonBetween(Pair(15, 44)).isNumeric, "A BETWEEN with ints should be numeric") + + @Test + @DisplayName("isNumeric is true with longs") + fun isNumericTrueForLongAndBetween() = + assertTrue(ComparisonBetween(Pair(9L, 12L)).isNumeric, "A BETWEEN with longs should be numeric") +} + +/** + * Unit tests for the `ComparisonIn` class + */ +@DisplayName("ComparisonIn") +class ComparisonInTest { + + @Test + @DisplayName("op is set to IN") + fun op() = + assertEquals(Op.IN, ComparisonIn(listOf()).op, "In comparison should have IN op") @Test @DisplayName("isNumeric is false for empty list of values") fun isNumericFalseForEmptyList() = - assertFalse(InComparison(Op.IN, listOf()).isNumeric, "An IN with empty list should not be numeric") + assertFalse(ComparisonIn(listOf()).isNumeric, "An IN with empty list should not be numeric") @Test - @DisplayName("isNumeric is false for IN with strings") + @DisplayName("isNumeric is false with strings") fun isNumericFalseForStringsAndIn() = - assertFalse(InComparison(Op.IN, listOf("a", "b", "c")).isNumeric, "An IN with strings should not be numeric") + assertFalse(ComparisonIn(listOf("a", "b", "c")).isNumeric, "An IN with strings should not be numeric") @Test - @DisplayName("isNumeric is true for IN with bytes") + @DisplayName("isNumeric is true with bytes") fun isNumericTrueForByteAndIn() = - assertTrue(InComparison(Op.IN, listOf(4, 8)).isNumeric, "An IN with bytes should be numeric") + assertTrue(ComparisonIn(listOf(4, 8)).isNumeric, "An IN with bytes should be numeric") @Test - @DisplayName("isNumeric is true for IN with shorts") + @DisplayName("isNumeric is true with shorts") fun isNumericTrueForShortAndIn() = - assertTrue(InComparison(Op.IN, listOf(18, 22)).isNumeric, "An IN with shorts should be numeric") + assertTrue(ComparisonIn(listOf(18, 22)).isNumeric, "An IN with shorts should be numeric") @Test - @DisplayName("isNumeric is true for IN with ints") + @DisplayName("isNumeric is true with ints") fun isNumericTrueForIntAndIn() = - assertTrue(InComparison(Op.IN, listOf(7, 8, 9)).isNumeric, "An IN with ints should be numeric") + assertTrue(ComparisonIn(listOf(7, 8, 9)).isNumeric, "An IN with ints should be numeric") @Test - @DisplayName("isNumeric is true for IN with longs") + @DisplayName("isNumeric is true with longs") fun isNumericTrueForLongAndIn() = - assertTrue(InComparison(Op.IN, listOf(3L)).isNumeric, "An IN with longs should be numeric") + assertTrue(ComparisonIn(listOf(3L)).isNumeric, "An IN with longs should be numeric") +} + +/** + * Unit tests for the `ComparisonInArray` class + */ +@DisplayName("ComparisonInArray") +class ComparisonInArrayTest { @Test - @DisplayName("isNumeric is false for BETWEEN with strings") - fun isNumericFalseForStringsAndBetween() = - assertFalse(BetweenComparison(Op.BETWEEN, Pair("eh", "zed")).isNumeric, - "A BETWEEN with strings should not be numeric") + @DisplayName("op is set to IN_ARRAY") + fun op() = + assertEquals( + Op.IN_ARRAY, + ComparisonInArray(Pair(TEST_TABLE, listOf())).op, + "InArray comparison should have IN_ARRAY op" + ) @Test - @DisplayName("isNumeric is true for BETWEEN with bytes") - fun isNumericTrueForByteAndBetween() = - assertTrue(BetweenComparison(Op.BETWEEN, Pair(7, 11)).isNumeric, "A BETWEEN with bytes should be numeric") + @DisplayName("isNumeric is false for empty list of values") + fun isNumericFalseForEmptyList() = + assertFalse(ComparisonIn(listOf()).isNumeric, "An IN_ARRAY with empty list should not be numeric") @Test - @DisplayName("isNumeric is true for BETWEEN with shorts") - fun isNumericTrueForShortAndBetween() = - assertTrue(BetweenComparison(Op.BETWEEN, Pair(0, 9)).isNumeric, - "A BETWEEN with shorts should be numeric") + @DisplayName("isNumeric is false with strings") + fun isNumericFalseForStringsAndIn() = + assertFalse(ComparisonIn(listOf("a", "b", "c")).isNumeric, "An IN_ARRAY with strings should not be numeric") @Test - @DisplayName("isNumeric is true for BETWEEN with ints") - fun isNumericTrueForIntAndBetween() = - assertTrue(BetweenComparison(Op.BETWEEN, Pair(15, 44)).isNumeric, "A BETWEEN with ints should be numeric") + @DisplayName("isNumeric is false with bytes") + fun isNumericTrueForByteAndIn() = + assertTrue(ComparisonIn(listOf(4, 8)).isNumeric, "An IN_ARRAY with bytes should not be numeric") @Test - @DisplayName("isNumeric is true for BETWEEN with longs") - fun isNumericTrueForLongAndBetween() = - assertTrue(BetweenComparison(Op.BETWEEN, Pair(9L, 12L)).isNumeric, "A BETWEEN with longs should be numeric") + @DisplayName("isNumeric is false with shorts") + fun isNumericTrueForShortAndIn() = + assertTrue(ComparisonIn(listOf(18, 22)).isNumeric, "An IN_ARRAY with shorts should not be numeric") + + @Test + @DisplayName("isNumeric is false with ints") + fun isNumericTrueForIntAndIn() = + assertTrue(ComparisonIn(listOf(7, 8, 9)).isNumeric, "An IN_ARRAY with ints should not be numeric") + + @Test + @DisplayName("isNumeric is false with longs") + fun isNumericTrueForLongAndIn() = + assertTrue(ComparisonIn(listOf(3L)).isNumeric, "An IN_ARRAY with longs should not be numeric") +} + +/** + * Unit tests for the `ComparisonSingle` class + */ +@DisplayName("ComparisonSingle") +class ComparisonSingleTest { @Test @DisplayName("isNumeric is false for string value") fun isNumericFalseForString() = - assertFalse(SingleComparison(Op.EQUAL, "80").isNumeric, "A string should not be numeric") + assertFalse(ComparisonSingle(Op.EQUAL, "80").isNumeric, "A string should not be numeric") @Test @DisplayName("isNumeric is true for byte value") fun isNumericTrueForByte() = - assertTrue(SingleComparison(Op.EQUAL, 47.toByte()).isNumeric, "A byte should be numeric") + assertTrue(ComparisonSingle(Op.EQUAL, 47.toByte()).isNumeric, "A byte should be numeric") @Test @DisplayName("isNumeric is true for short value") fun isNumericTrueForShort() = - assertTrue(SingleComparison(Op.EQUAL, 2.toShort()).isNumeric, "A short should be numeric") + assertTrue(ComparisonSingle(Op.EQUAL, 2.toShort()).isNumeric, "A short should be numeric") @Test @DisplayName("isNumeric is true for int value") fun isNumericTrueForInt() = - assertTrue(SingleComparison(Op.EQUAL, 555).isNumeric, "An int should be numeric") + assertTrue(ComparisonSingle(Op.EQUAL, 555).isNumeric, "An int should be numeric") @Test @DisplayName("isNumeric is true for long value") fun isNumericTrueForLong() = - assertTrue(SingleComparison(Op.EQUAL, 82L).isNumeric, "A long should be numeric") + assertTrue(ComparisonSingle(Op.EQUAL, 82L).isNumeric, "A long should be numeric") }