From 2ec495a44dd28f491f4725df8b3dd6421eff82af Mon Sep 17 00:00:00 2001 From: Antonio171003 Date: Tue, 3 Sep 2024 22:24:08 -0700 Subject: [PATCH 1/2] Fixed an Internal Server Error, Caused when attempting to filter by a column whose data type is not a string. --- .../repositories/daos/DatabaseTablesDAO.scala | 55 ++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala b/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala index 2aed493..e7b2335 100644 --- a/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala +++ b/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala @@ -10,8 +10,7 @@ import java.sql.{Connection, Date, PreparedStatement, ResultSet} import java.time.LocalDate import java.util.UUID import scala.collection.mutable.ListBuffer -import scala.util.Try - +import scala.util.{Failure, Success, Try} object DatabaseTablesDAO { def all(schema: String = "public")(implicit conn: Connection): List[DatabaseTable] = { @@ -73,6 +72,37 @@ object DatabaseTablesDAO { """.as(foreignKeyParser.*) } + private def columnTypeIsDouble(columnType: String): Boolean = { + // 'contains' is used because PostgreSQL types may include additional details like precision or scale + // https://www.postgresql.org/docs/8.1/datatype.html + List("float", "decimal").exists(columnType.contains) + } + + private def columnTypeIsInt(columnType: String): Boolean = { + List("int", "serial").exists(columnType.contains) + } + + private def isUUID(value: String, columnType: String): Boolean = { + Try(UUID.fromString(value)) match { + case Success(_) => columnType == "uuid" + case Failure(_) => false + } + } + + private def isInt(value: String, columnType: String): Boolean = { + value.toIntOption.isDefined && columnTypeIsInt(columnType) + } + + private def isDecimal(value: String, columnType: String): Boolean = { + value.toDoubleOption.isDefined && columnTypeIsDouble(columnType) + } + + private def isNumberOrUUID(value: String, columnType: String): Boolean = { + isInt(value, columnType) || + isDecimal(value, columnType) || + isUUID(value, columnType) + } + def getTableData( settings: TableSettings, columns: List[TableColumn], @@ -88,12 +118,16 @@ object DatabaseTablesDAO { val conditionsSql = queryParameters.filters .map { case FilterParameter(filterField, filterValue) => + val columnType = columns.find(_.name == filterField) match { + case Some(column) => column.`type` + case None => throw Exception(s"Column with name '$filterField' not found.") + } filterValue match { - case dateRegex(_, _, _) => + case dateRegex(_, _, _) if columnType == "date" => s"DATE($filterField) = ?" case _ => - if (filterValue.toIntOption.isDefined || filterValue.toDoubleOption.isDefined) + if (isNumberOrUUID(filterValue, columnType)) s"$filterField = ?" else s"$filterField LIKE ?" @@ -111,20 +145,25 @@ object DatabaseTablesDAO { val preparedStatement = conn.prepareStatement(sql) queryParameters.filters.zipWithIndex - .foreach { case (FilterParameter(_, filterValue), index) => + .foreach { case (FilterParameter(filterField, filterValue), index) => // We have to increment index by 1 because SQL parameterIndex starts in 1 val sqlIndex = index + 1 - + val columnType = columns.find(_.name == filterField) match { + case Some(column) => column.`type` + case None => throw Exception(s"Column with name '$filterField' not found.") + } filterValue match { case dateRegex(year, month, day) => val parsedDate = LocalDate.of(year.toInt, month.toInt, day.toInt) preparedStatement.setDate(sqlIndex, Date.valueOf(parsedDate)) case _ => - if (filterValue.toIntOption.isDefined) + if (isInt(filterValue, columnType)) preparedStatement.setInt(sqlIndex, filterValue.toInt) - else if (filterValue.toDoubleOption.isDefined) + else if (isDecimal(filterValue, columnType)) preparedStatement.setDouble(sqlIndex, filterValue.toDouble) + else if (isUUID(filterValue, columnType)) + preparedStatement.setObject(sqlIndex, UUID.fromString(filterValue)) else preparedStatement.setString(sqlIndex, s"%$filterValue%") } From 6e2c5acdda5bf1e14554051caf2df840342759b5 Mon Sep 17 00:00:00 2001 From: Antonio171003 Date: Wed, 4 Sep 2024 20:40:14 -0700 Subject: [PATCH 2/2] Added tests and corrections --- .../repositories/daos/DatabaseTablesDAO.scala | 9 +- .../controllers/AdminControllerSpec.scala | 102 +++++++++++++++++- 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala b/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala index e7b2335..6f3546d 100644 --- a/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala +++ b/spra-play-server/src/main/scala/net/wiringbits/spra/admin/repositories/daos/DatabaseTablesDAO.scala @@ -82,6 +82,10 @@ object DatabaseTablesDAO { List("int", "serial").exists(columnType.contains) } + private def columnTypeIsDate(columnType: String): Boolean = { + List("date", "time").exists(columnType.contains) + } + private def isUUID(value: String, columnType: String): Boolean = { Try(UUID.fromString(value)) match { case Success(_) => columnType == "uuid" @@ -123,9 +127,8 @@ object DatabaseTablesDAO { case None => throw Exception(s"Column with name '$filterField' not found.") } filterValue match { - case dateRegex(_, _, _) if columnType == "date" => + case dateRegex(_, _, _) if columnTypeIsDate(columnType) => s"DATE($filterField) = ?" - case _ => if (isNumberOrUUID(filterValue, columnType)) s"$filterField = ?" @@ -153,7 +156,7 @@ object DatabaseTablesDAO { case None => throw Exception(s"Column with name '$filterField' not found.") } filterValue match { - case dateRegex(year, month, day) => + case dateRegex(year, month, day) if columnTypeIsDate(columnType) => val parsedDate = LocalDate.of(year.toInt, month.toInt, day.toInt) preparedStatement.setDate(sqlIndex, Date.valueOf(parsedDate)) diff --git a/spra-play-server/src/test/scala/controllers/AdminControllerSpec.scala b/spra-play-server/src/test/scala/controllers/AdminControllerSpec.scala index 7194e23..2d5bcaa 100644 --- a/spra-play-server/src/test/scala/controllers/AdminControllerSpec.scala +++ b/spra-play-server/src/test/scala/controllers/AdminControllerSpec.scala @@ -532,6 +532,106 @@ class AdminControllerSpec extends PlayPostgresSpec with AdminUtils { .futureValue response.headOption.isEmpty must be(true) } + + "don't fail when the column is citext or similar, but the value can be interpreted as Int." in withApiClient { + client => + val name = "wiringbits" + val email = "test17@wiringbits.net" + val request = AdminCreateTable.Request( + Map("name" -> name, "email" -> email, "password" -> "wiringbits") + ) + client.createItem(usersSettings.tableName, request).futureValue + + val response = client + .getTableMetadata( + usersSettings.tableName, + List("email", "ASC"), + List(0, 9), + """{"email":"17"}""" + ) + .futureValue + val head = response.headOption.value + val nameValue = head.find(_._1 == "name").value._2 + val emailValue = head.find(_._1 == "email").value._2 + response.size must be(1) + name must be(nameValue) + email must be(emailValue) + } + + "don't fail when the column is citext or similar, but the value can be interpreted as Decimal." in withApiClient { + client => + val name = "wiringbits" + val email = "test17.10@wiringbits.net" + val request = AdminCreateTable.Request( + Map("name" -> name, "email" -> email, "password" -> "wiringbits") + ) + client.createItem(usersSettings.tableName, request).futureValue + + val response = client + .getTableMetadata( + usersSettings.tableName, + List("email", "ASC"), + List(0, 9), + """{"email":"17.10"}""" + ) + .futureValue + val head = response.headOption.value + val nameValue = head.find(_._1 == "name").value._2 + val emailValue = head.find(_._1 == "email").value._2 + response.size must be(1) + name must be(nameValue) + email must be(emailValue) + } + + "don't fail when the column is citext or similar, but the value can be interpreted as Date." in withApiClient { + client => + val name = "wiringbits" + val email = "test2024-06-06@wiringbits.net" + val request = AdminCreateTable.Request( + Map("name" -> name, "email" -> email, "password" -> "wiringbits") + ) + client.createItem(usersSettings.tableName, request).futureValue + + val response = client + .getTableMetadata( + usersSettings.tableName, + List("email", "ASC"), + List(0, 9), + """{"email":"2024-06"}""" + ) + .futureValue + val head = response.headOption.value + val nameValue = head.find(_._1 == "name").value._2 + val emailValue = head.find(_._1 == "email").value._2 + response.size must be(1) + name must be(nameValue) + email must be(emailValue) + } + + "don't fail when the column is citext or similar, but the value can be interpreted as UUID." in withApiClient { + client => + val name = "wiringbits" + val email = "8c861a28-e384-4a9b-b7c2-a0367aa3f3e8@wiringbits.net" + val request = AdminCreateTable.Request( + Map("name" -> name, "email" -> email, "password" -> "wiringbits") + ) + client.createItem(usersSettings.tableName, request).futureValue + + val response = client + .getTableMetadata( + usersSettings.tableName, + List("name", "ASC"), + List(0, 9), + """{"email":"8c861a28-e384-4a9b-b7c2-a0367aa3f3e8"}""" + ) + .futureValue + val head = response.headOption.value + val nameValue = head.find(_._1 == "name").value._2 + val emailValue = head.find(_._1 == "email").value._2 + response.size must be(1) + name must be(nameValue) + email must be(emailValue) + } } "GET /admin/tables/:tableName/:primaryKey" should { @@ -680,7 +780,7 @@ class AdminControllerSpec extends PlayPostgresSpec with AdminUtils { responseMetadata.head.nonEmpty mustBe true } - + "return new user id" in withApiClient { implicit client => val user = createUser.futureValue val response = client.getTableMetadata(usersSettings.tableName, List("name", "ASC"), List(0, 9), "{}").futureValue