Skip to content

Fix Handle the bytea data received to be stored correctly in the database. #35

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package net.wiringbits.spra.admin.models

trait FieldValue[T] extends Serializable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to extend Serializable? a common pattern is sealed trait FieldValue[T] extends Product with Serializable.

Also, do we expect anyone to extend this class outside of this file? otherwise, this must be a sealed trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The data is received as a string. When dealing with a column of type bytea, storing the value as a string will result in incorrect information. Therefore, it is necessary to convert the string to an Array[Byte] (hence the Serializable) to store it correctly.

In the other PR, I was advised that it would be better to create a custom trait that extends Scala's Serializable to be more descriptive and specific with typing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anything requiring us to make this Serializable? this is usually a bad idea and 99% of the times we shall prefer custom codecs.

val value: T
}

case class StringValue(value: String) extends FieldValue[String]
case class ByteArrayValue(value: Array[Byte]) extends FieldValue[Array[Byte]]
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import net.wiringbits.spra.admin.repositories.daos.DatabaseTablesDAO
import net.wiringbits.spra.admin.repositories.models.{DatabaseTable, ForeignKey, TableColumn, TableData}
import net.wiringbits.spra.admin.utils.models.QueryParameters
import play.api.db.Database
import net.wiringbits.spra.admin.models.{ByteArrayValue, StringValue}
import net.wiringbits.spra.admin.utils.StringParse

import javax.inject.Inject
import scala.concurrent.Future
Expand Down Expand Up @@ -75,7 +77,10 @@ class DatabaseTablesRepository @Inject() (database: Database)(implicit
val fieldsAndValues = body.map { case (key, value) =>
val field =
columns.find(_.name == key).getOrElse(throw new RuntimeException(s"Invalid property in body request: $key"))
(field, value)
if (field.`type` == "bytea")
val byteaValue = StringParse.stringToByteArray(value)
(field, ByteArrayValue(byteaValue))
else (field, StringValue(value))
}
DatabaseTablesDAO.create(
tableName = tableName,
Expand All @@ -100,7 +105,10 @@ class DatabaseTablesRepository @Inject() (database: Database)(implicit
val fieldsAndValues = bodyWithoutNonEditableColumns.map { case (key, value) =>
val field =
columns.find(_.name == key).getOrElse(throw new RuntimeException(s"Invalid property in body request: $key"))
(field, value)
if (field.`type` == "bytea")
val byteaValue = StringParse.stringToByteArray(value)
(field, ByteArrayValue(byteaValue))
else (field, StringValue(value))
}
val primaryKeyType = settings.primaryKeyDataType
DatabaseTablesDAO.update(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import net.wiringbits.spra.admin.config.{CustomDataType, PrimaryKeyDataType, Tab
import net.wiringbits.spra.admin.repositories.models.*
import net.wiringbits.spra.admin.utils.models.{FilterParameter, QueryParameters}
import net.wiringbits.spra.admin.utils.{QueryBuilder, StringRegex}

import net.wiringbits.spra.admin.models.{ByteArrayValue, FieldValue, StringValue}
import java.sql.{Connection, Date, PreparedStatement, ResultSet}
import java.time.LocalDate
import java.util.UUID
Expand Down Expand Up @@ -230,7 +230,7 @@ object DatabaseTablesDAO {
}
def create(
tableName: String,
fieldsAndValues: Map[TableColumn, String],
fieldsAndValues: Map[TableColumn, FieldValue[_]],
primaryKeyField: String,
primaryKeyType: PrimaryKeyDataType = PrimaryKeyDataType.UUID
)(implicit
Expand All @@ -250,7 +250,7 @@ object DatabaseTablesDAO {
// Postgres: INSERT INTO test_serial (id) VALUES(DEFAULT); MySQL: INSERT INTO table (id) VALUES(NULL)

for (j <- i + 1 to fieldsAndValues.size + i) {
val value = fieldsAndValues(fieldsAndValues.keys.toList(j - i - 1))
val value = fieldsAndValues(fieldsAndValues.keys.toList(j - i - 1)).value
preparedStatement.setObject(j, value)
}
val result = preparedStatement.executeQuery()
Expand All @@ -260,17 +260,17 @@ object DatabaseTablesDAO {

def update(
tableName: String,
fieldsAndValues: Map[TableColumn, String],
fieldsAndValues: Map[TableColumn, FieldValue[_]],
primaryKeyField: String,
primaryKeyValue: String,
primaryKeyType: PrimaryKeyDataType = PrimaryKeyDataType.UUID
)(implicit conn: Connection): Unit = {
val sql = QueryBuilder.update(tableName, fieldsAndValues, primaryKeyField)
val preparedStatement = conn.prepareStatement(sql)

val notNullData = fieldsAndValues.filterNot { case (_, value) => value == "null" }
val notNullData = fieldsAndValues.filterNot { case (_, value) => value.value == "null" }
notNullData.zipWithIndex.foreach { case ((_, value), i) =>
preparedStatement.setObject(i + 1, value)
preparedStatement.setObject(i + 1, value.value)
}
// where ... = ?
setPreparedStatementKey(preparedStatement, primaryKeyValue, primaryKeyType, notNullData.size + 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package net.wiringbits.spra.admin.utils

import net.wiringbits.spra.admin.config.PrimaryKeyDataType
import net.wiringbits.spra.admin.repositories.models.TableColumn

import net.wiringbits.spra.admin.models.FieldValue
import scala.collection.mutable

object QueryBuilder {
def create(
tableName: String,
fieldsAndValues: Map[TableColumn, String],
fieldsAndValues: Map[TableColumn, FieldValue[_]],
primaryKeyField: String,
primaryKeyType: PrimaryKeyDataType = PrimaryKeyDataType.UUID
): String = {
Expand All @@ -33,10 +33,10 @@ object QueryBuilder {
|""".stripMargin
}

def update(tableName: String, body: Map[TableColumn, String], primaryKeyField: String): String = {
def update(tableName: String, body: Map[TableColumn, FieldValue[_]], primaryKeyField: String): String = {
val updateStatement = new mutable.StringBuilder("SET")
for ((tableField, value) <- body) {
val resultStatement = if (value == "null") "NULL" else s"?::${tableField.`type`}"
val resultStatement = if (value.value == "null") "NULL" else s"?::${tableField.`type`}"
val statement = s" ${tableField.name} = $resultStatement,"
updateStatement.append(statement)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package net.wiringbits.spra.admin.utils

import scala.util.{Failure, Success, Try}

object StringParse {
def stringToByteArray(value: String): Array[Byte] = {
// Removes whitespace characters (\\s) and brackets ([, ]) to prepare the string for byte array conversion
Try(value.replaceAll("[\\[\\]\\s]", "").split(",").map(_.toByte)) match
case Success(value) => value
case Failure(_) => Array.emptyByteArray
}
}