Skip to content

Commit

Permalink
1296 schema mapper type conversions (#1311)
Browse files Browse the repository at this point in the history
* #1296 add TypeConverter class with simple tests and some default converters

#1296 support for conversion from primitive to wrapper types for consistency

#1296 add TypeConverterContainer and related tests

Also some improvements and refactoring on previous changes

#1296 improvements and refactoring on previous changes / TypeConverterContainer enhancements / TypeUtils introduced

#1296 add 3 more default type converters + tests for DefaultTypeConverters

* #1296 add type conversions capability to SchemaMapper

* #1296 introduce SchemaMapperBuilder for easier instantiation

- this commit also adds more DefaultTypeConverters and some missing tests
  for DataType.parseToDataType.

* #1296 add todos, stuff that needs to be changed before final PR

* #1296 add type conversion related SQL filter tests

* #1296 add small improvements to TypeConverterContainer class

* #1296 complete IgniteSqlFilterClause changes and add tests

- also removes `convertExternalValueToString` from `SchemaMapper`
  and moves it to Filtering use-case as it has nothing to do with
  schema mapping logic.

* #1296 move util.schema.typeConversion folder to util.types

* #1296 improve and add pending cleanups to ExternalEntitySchema

* #1296 fix failing tests in IgniteSqlFilteringTest

* #1296 improve SqlFilterColumnValueParser to use opt-in SqlStringConverterContainer

- this allows users to specify custom external-value -> sql-string converters.

* #1296 add tests in IgniteSqlFilteringTest for missing Boolean data type

* #1296 make FilterColumnValueParser independent of SQL use-case

- it is now just a utility to convert column value (String) from ANTLR
  to external value (original external date type) that will then be used
  with external data source for filtering.
- In the process makes it responsible only for one thing i.e. parse column
  value (String) to external value's data type. Should not care about how
  we convert the resulting external value to a String understood by SQL.
  That logic's been moved back to IgniteSqlFilterClause.
- also renames SqlStringConverterContainer to less verbose ToSqlStringContainer.

* #1296 make `parseToDataType` to return Option for easier usage with java

* #1296 remove ToSqlStringContainer.scala
  • Loading branch information
junaidzm13 authored Apr 30, 2024
1 parent dba74d9 commit d618f9c
Show file tree
Hide file tree
Showing 32 changed files with 1,156 additions and 264 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import org.finos.toolbox.time.Clock
import org.finos.vuu.api.ViewPortDef
import org.finos.vuu.core.module.{DefaultModule, ModuleFactory, TableDefContainer, ViewServerModule}
import org.finos.vuu.core.table.{Column, Columns}
import org.finos.vuu.example.ignite.{IgniteColumnValueProvider, IgniteOrderStore}
import org.finos.vuu.example.ignite.IgniteOrderStore
import org.finos.vuu.example.ignite.provider.IgniteOrderDataProvider
import org.finos.vuu.net.rpc.RpcHandler
import org.finos.vuu.plugin.virtualized.api.VirtualizedSessionTableDef
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ import org.finos.vuu.example.ignite.query.IndexCalculator
import org.finos.vuu.example.ignite.schema.ChildOrderSchema
import org.finos.vuu.plugin.virtualized.table.{VirtualizedRange, VirtualizedSessionTable, VirtualizedViewPortKeys}
import org.finos.vuu.provider.VirtualizedProvider
import org.finos.vuu.util.schema.SchemaMapper
import org.finos.vuu.util.schema.SchemaMapperBuilder
import org.finos.vuu.viewport.ViewPort

import java.util.concurrent.atomic.AtomicInteger

class IgniteOrderDataProvider(final val igniteStore: IgniteOrderStore)
(implicit clock: Clock) extends VirtualizedProvider with StrictLogging {

private val schemaMapper = SchemaMapper(ChildOrderSchema.schema, IgniteOrderDataModule.columns, columnNameByExternalField)
private val schemaMapper = SchemaMapperBuilder(ChildOrderSchema.schema, IgniteOrderDataModule.columns)
.withFieldsMap(columnNameByExternalField)
.build()
private val dataQuery = IgniteOrderDataQuery(igniteStore, schemaMapper)
private val indexCalculator = IndexCalculator(extraRowsCount = 5000)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuil

object ChildOrderSchema {
val schema: ExternalEntitySchema = ExternalEntitySchemaBuilder()
.withCaseClass[ChildOrder]
.withEntity(classOf[ChildOrder])
.withIndex("PARENTID_IDX", List("parentId"))
.withIndex("CHILDID_IDX", List("id"))
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import org.finos.vuu.example.ignite.module.IgniteOrderDataModule
import org.finos.vuu.example.ignite.schema.ChildOrderSchema
import org.finos.vuu.example.ignite.{IgniteOrderStore, TestUtils}
import org.finos.vuu.net.FilterSpec
import org.finos.vuu.util.schema.SchemaMapper
import org.finos.vuu.util.schema.SchemaMapperBuilder
import org.scalatest.funsuite.AnyFunSuiteLike
import org.scalatest.matchers.should.Matchers
import org.scalatest.BeforeAndAfterAll

class IgniteOrderDataQueryFunctionalTest extends AnyFunSuiteLike with BeforeAndAfterAll with Matchers {
private val schemaMapper = SchemaMapper(ChildOrderSchema.schema, IgniteOrderDataModule.columns, IgniteOrderDataProvider.columnNameByExternalField)
private val schemaMapper = SchemaMapperBuilder(ChildOrderSchema.schema, IgniteOrderDataModule.columns)
.withFieldsMap(IgniteOrderDataProvider.columnNameByExternalField)
.build()
private var ignite: Ignite = _
private var orderStore: IgniteOrderStore = _
private var dataQuery: IgniteOrderDataQuery = _
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ import org.finos.vuu.core.table.{Column, Columns}
import org.finos.vuu.example.ignite.IgniteOrderStore
import org.finos.vuu.example.ignite.provider.IgniteOrderDataQueryTest.{entitySchema, internalColumns, internalColumnsByExternalFields}
import org.finos.vuu.net.FilterSpec
import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuilder, SchemaMapper}
import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuilder, SchemaMapper, SchemaMapperBuilder}
import org.scalamock.scalatest.MockFactory
import org.scalatest.featurespec.AnyFeatureSpec
import org.scalatest.matchers.should.Matchers

class IgniteOrderDataQueryTest extends AnyFeatureSpec with Matchers with MockFactory {


private val schemaMapper = SchemaMapperBuilder(entitySchema, internalColumns)
.withFieldsMap(internalColumnsByExternalFields)
.build()
private val igniteStore: IgniteOrderStore = mock[IgniteOrderStore]
private val schemaMapper = SchemaMapper(entitySchema, internalColumns, internalColumnsByExternalFields)
private val igniteDataQuery = IgniteOrderDataQuery(igniteStore, schemaMapper)

Feature("fetch") {
Expand All @@ -41,5 +44,5 @@ private object IgniteOrderDataQueryTest {
"value" -> "value",
)

val entitySchema: ExternalEntitySchema = ExternalEntitySchemaBuilder().withCaseClass[TestDto].build()
val entitySchema: ExternalEntitySchema = ExternalEntitySchemaBuilder().withEntity(classOf[TestDto]).build()
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ import org.finos.vuu.example.rest.client.InstrumentServiceClient
import org.finos.vuu.example.rest.model.Instrument
import org.finos.vuu.example.rest.provider.InstrumentsProvider.{INSTRUMENTS_COUNT, columnNameByExternalField, externalSchema}
import org.finos.vuu.provider.DefaultProvider
import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuilder, SchemaMapper}
import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuilder, SchemaMapperBuilder}

import scala.util.{Failure, Success}

class InstrumentsProvider(table: DataTable, client: InstrumentServiceClient)
(implicit clock: Clock) extends DefaultProvider with StrictLogging {


private val schemaMapper = SchemaMapper(externalSchema, table.getTableDef.columns, columnNameByExternalField)
private val schemaMapper = SchemaMapperBuilder(externalSchema, table.getTableDef.columns)
.withFieldsMap(columnNameByExternalField)
.build()
private val keyField = table.getTableDef.keyField

override def doStart(): Unit = {
Expand All @@ -35,7 +37,7 @@ class InstrumentsProvider(table: DataTable, client: InstrumentServiceClient)
}

object InstrumentsProvider {
val externalSchema: ExternalEntitySchema = ExternalEntitySchemaBuilder().withCaseClass[Instrument].build()
val externalSchema: ExternalEntitySchema = ExternalEntitySchemaBuilder().withEntity(classOf[Instrument]).build()
val columnNameByExternalField: Map[String, String] = Map(
"id" -> "id",
"ric" -> "ric",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ trait FilterAndSortSpecToSql {
}

object FilterAndSortSpecToSql {
def apply(schemaMapper: SchemaMapper): FilterAndSortSpecToSql = {
new FilterAndSortSpecToSqlImpl(schemaMapper)
}
def apply(schemaMapper: SchemaMapper): FilterAndSortSpecToSql = new FilterAndSortSpecToSqlImpl(schemaMapper)
}

private class FilterAndSortSpecToSqlImpl(private val schemaMapper: SchemaMapper) extends FilterAndSortSpecToSql {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package org.finos.vuu.feature.ignite.filter

import com.typesafe.scalalogging.StrictLogging
import org.finos.vuu.core.table.{Column, DataType}
import org.finos.vuu.feature.ignite.filter.FilterColumnValueParser.{ErrorMessage, ParsedResult}
import org.finos.vuu.util.schema.{SchemaField, SchemaMapper}

protected trait FilterColumnValueParser {
def parse(columnName: String, columnValue: String): Either[ErrorMessage, ParsedResult[Any]]
def parse(columnName: String, columnValues: List[String]): Either[ErrorMessage, ParsedResult[List[Any]]]
}

protected object FilterColumnValueParser {
def apply(schemaMapper: SchemaMapper): FilterColumnValueParser = {
new ColumnValueParser(schemaMapper)
}

case class ParsedResult[T](externalField: SchemaField, externalData: T)

type ErrorMessage = String

val STRING_DATA_TYPE: Class[String] = classOf[String]
}

private class ColumnValueParser(private val mapper: SchemaMapper) extends FilterColumnValueParser with StrictLogging {

override def parse(columnName: String, columnValue: String): Either[ErrorMessage, ParsedResult[Any]] = {
mapper.externalSchemaField(columnName) match {
case Some(f) => RawColumnValueParser(f).parse(columnValue).map(ParsedResult(f, _))
case None => Left(externalFieldNotFoundError(columnName))
}
}

override def parse(columnName: String, columnValues: List[String]): Either[ErrorMessage, ParsedResult[List[Any]]] = {
mapper.externalSchemaField(columnName) match {
case Some(f) => parseValues(RawColumnValueParser(f), columnValues)
case None => Left(externalFieldNotFoundError(columnName))
}
}

private def parseValues(parser: RawColumnValueParser,
columnValues: List[String]): Either[ErrorMessage, ParsedResult[List[Any]]] = {
val (errors, parsedValues) = columnValues.partitionMap(parser.parse)
val combinedError = errors.mkString("\n")

if (parsedValues.isEmpty) {
Left(combinedError)
} else {
if (errors.nonEmpty) logger.error(
s"Failed to parse some of the column values corresponding to the column ${parser.column.name}: \n $combinedError"
)
Right(ParsedResult(parser.field, parsedValues))
}
}

private def externalFieldNotFoundError(columnName: String): String =
s"Failed to find mapped external field for column `$columnName`"

private case class RawColumnValueParser(field: SchemaField) {
val column: Column = mapper.tableColumn(field.name).get

def parse(columnValue: String): Either[ErrorMessage, Any] = {
parseStringToColumnDataType(columnValue).flatMap(convertColumnValueToExternalFieldType)
}

private def parseStringToColumnDataType(value: String): Either[ErrorMessage, Any] =
DataType.parseToDataType(value, column.dataType) match {
case Some(v) => Right(v)
case None => Left(s"Failed to parse column value '$value' to data type `${column.dataType}`.")
}

private def convertColumnValueToExternalFieldType(columnValue: Any): Either[ErrorMessage, Any] =
mapper.toMappedExternalFieldType(column.name, columnValue)
.toRight(s"Failed to convert column value `$columnValue` from `${column.dataType}` to external type `${field.dataType}`")
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.finos.vuu.feature.ignite.filter

import com.typesafe.scalalogging.StrictLogging
import org.finos.vuu.core.table.DataType.{CharDataType, StringDataType}
import org.finos.vuu.feature.ignite.filter.IgniteSqlFilterClause.EMPTY_SQL
import org.finos.vuu.util.schema.SchemaMapper
import org.finos.vuu.feature.ignite.filter.FilterColumnValueParser.{ParsedResult, STRING_DATA_TYPE}
import org.finos.vuu.util.schema.{SchemaField, SchemaMapper}

private object IgniteSqlFilterClause {
val EMPTY_SQL = ""
Expand All @@ -28,76 +28,77 @@ case class AndIgniteSqlFilterClause(clauses:List[IgniteSqlFilterClause]) extends
}

case class EqIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause {
override def toSql(schemaMapper: SchemaMapper): String =
schemaMapper.externalSchemaField(columnName) match {
case Some(f) => f.dataType match {
case CharDataType | StringDataType => eqSql(f.name, quotedString(value))
case _ => eqSql(f.name, value)
}
case None => logMappingErrorAndReturnEmptySql(columnName)
override def toSql(schemaMapper: SchemaMapper): String = {
FilterColumnValueParser(schemaMapper).parse(columnName, value) match {
case Right(ParsedResult(f, externalValue)) => eqSql(f.name, convertToString(externalValue, f.dataType))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def eqSql(field: String, processedVal: String): String = {
s"$field = $processedVal"
}
}

case class NeqIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause {
override def toSql(schemaMapper: SchemaMapper): String =
schemaMapper.externalSchemaField(columnName) match {
case Some(field) => field.dataType match {
case CharDataType | StringDataType => neqSql(field.name, quotedString(value))
case _ => neqSql(field.name, value)
}
case None => logMappingErrorAndReturnEmptySql(columnName)
override def toSql(schemaMapper: SchemaMapper): String = {
FilterColumnValueParser(schemaMapper).parse(columnName, value) match {
case Right(ParsedResult(f, externalValue)) => neqSql(f.name, convertToString(externalValue, f.dataType))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def neqSql(field: String, processedVal: String): String = {
s"$field != $processedVal"
}
}

//todo why is number cast double? need to cast back to original type?
case class RangeIgniteSqlFilterClause(op: RangeOp)(columnName: String, value: Double) extends IgniteSqlFilterClause {
override def toSql(schemaMapper: SchemaMapper): String =
schemaMapper.externalSchemaField(columnName) match {
case Some(f) => s"${f.name} ${op.value} $value"
case None => logMappingErrorAndReturnEmptySql(columnName)
case class RangeIgniteSqlFilterClause(op: RangeOp)(columnName: String, value: String) extends IgniteSqlFilterClause {
override def toSql(schemaMapper: SchemaMapper): String = {
FilterColumnValueParser(schemaMapper).parse(columnName, value) match {
case Right(ParsedResult(f, externalValue)) => rangeSql(f.name, convertToString(externalValue, f.dataType))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def rangeSql(field: String, processedVal: String): String = s"$field ${op.value} $processedVal"
override def toString = s"RangeIgniteSqlFilterClause[$op]($columnName, $value)"
}

case class StartsIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause with StrictLogging {
override def toSql(schemaMapper: SchemaMapper): String = {
schemaMapper.externalSchemaField(columnName) match {
case Some(f) => f.dataType match {
case StringDataType => s"${f.name} LIKE '$value%'"
case _ => logErrorAndReturnEmptySql(s"`Starts` clause unsupported for non string column: `${f.name}` (${f.dataType})")
}
case None => logMappingErrorAndReturnEmptySql(columnName)
FilterColumnValueParser(schemaMapper).parse(columnName, value) match {
case Right(ParsedResult(f, externalValue)) => startsSql(f, convertToString(externalValue, f.dataType))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def startsSql(f: SchemaField, value: String): String = f.dataType match {
case STRING_DATA_TYPE => s"${f.name} LIKE ${value.stripSuffix("'")}%'"
case _ => logErrorAndReturnEmptySql(s"`Starts` clause unsupported for non string column: `${f.name}` (${f.dataType})")
}
}

case class EndsIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause with StrictLogging {
override def toSql(schemaMapper: SchemaMapper): String =
schemaMapper.externalSchemaField(columnName) match {
case Some(f) => f.dataType match {
case StringDataType => s"${f.name} LIKE '%$value'"
case _ => logErrorAndReturnEmptySql(s"`Ends` clause unsupported for non string column: `${f.name}` (${f.dataType})")
}
case None => logMappingErrorAndReturnEmptySql(columnName)
override def toSql(schemaMapper: SchemaMapper): String = {
FilterColumnValueParser(schemaMapper).parse(columnName, value) match {
case Right(ParsedResult(f, externalValue)) => endsSql(f, convertToString(externalValue, f.dataType))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def endsSql(f: SchemaField, value: String): String = f.dataType match {
case STRING_DATA_TYPE => s"${f.name} LIKE '%${value.stripPrefix("'")}"
case _ => logErrorAndReturnEmptySql(s"`Ends` clause unsupported for non string column: `${f.name}` (${f.dataType})")
}
}

case class InIgniteSqlFilterClause(columnName: String, values: List[String]) extends IgniteSqlFilterClause with StrictLogging {
override def toSql(schemaMapper: SchemaMapper): String =
schemaMapper.externalSchemaField(columnName) match {
case Some(f) => f.dataType match {
case CharDataType | StringDataType => inQuery(f.name, values.map(quotedString(_)))
case _ => inQuery(f.name, values)
}
case None => logMappingErrorAndReturnEmptySql(columnName)
override def toSql(schemaMapper: SchemaMapper): String = {
FilterColumnValueParser(schemaMapper).parse(columnName, values) match {
case Right(ParsedResult(f, externalValues)) => inQuery(f.name, externalValues.map(convertToString(_, f.dataType)))
case Left(errMsg) => logErrorAndReturnEmptySql(errMsg)
}
}

private def inQuery(field: String, processedValues: List[String]) = {
Expand All @@ -113,13 +114,32 @@ object RangeOp {
final case object LTE extends RangeOp(value = "<=")
}


private object convertToString {
def apply(value: Any, dataType: Class[_]): String = addQuotesIfRequired(defaultToString(value), dataType)
private def defaultToString(value: Any): String = Option(value).map(_.toString).orNull
}

private object quotedString {
def apply(s: String) = s"'$s'"
}

private object logMappingErrorAndReturnEmptySql {
def apply(columnName: String): String =
logErrorAndReturnEmptySql(s"Failed to find mapped external field for column `$columnName`")
// @todo move from building SQL query as string to move away from adding quotes manually by types
// using a parametrized query builder should help with this and with minimizing threat of SQL injection
private object addQuotesIfRequired {
def apply(v: String, dataType: Class[_]): String = if (requireQuotes(dataType)) quotedString(v) else v

private def requireQuotes(dt: Class[_]): Boolean = dataTypesRequiringQuotes.contains(dt)

private val dataTypesRequiringQuotes: Set[Class[_]] = Set(
classOf[String],
classOf[Char],
classOf[java.lang.Character],
classOf[java.sql.Date],
classOf[java.sql.Time],
classOf[java.sql.Timestamp],
classOf[java.time.LocalDate]
)
}

private object logErrorAndReturnEmptySql extends StrictLogging {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ class IgniteSqlFilterTreeVisitor extends FilterBaseVisitor[IgniteSqlFilterClause
NeqIgniteSqlFilterClause(ctx.ID().getText, ctx.scalar().getText)

override def visitOperationGt(ctx: OperationGtContext): IgniteSqlFilterClause =
RangeIgniteSqlFilterClause(RangeOp.GT)(ctx.ID().getText, ctx.NUMBER().getText.toDouble)
RangeIgniteSqlFilterClause(RangeOp.GT)(ctx.ID().getText, ctx.NUMBER().getText)

override def visitOperationGte(ctx: OperationGteContext): IgniteSqlFilterClause =
RangeIgniteSqlFilterClause(RangeOp.GTE)(ctx.ID().getText, ctx.NUMBER().getText.toDouble)
RangeIgniteSqlFilterClause(RangeOp.GTE)(ctx.ID().getText, ctx.NUMBER().getText)

override def visitOperationLt(ctx: OperationLtContext): IgniteSqlFilterClause =
RangeIgniteSqlFilterClause(RangeOp.LT)(ctx.ID().getText, ctx.NUMBER().getText.toDouble)
RangeIgniteSqlFilterClause(RangeOp.LT)(ctx.ID().getText, ctx.NUMBER().getText)

override def visitOperationLte(ctx: OperationLteContext): IgniteSqlFilterClause =
RangeIgniteSqlFilterClause(RangeOp.LTE)(ctx.ID().getText, ctx.NUMBER().getText.toDouble)
RangeIgniteSqlFilterClause(RangeOp.LTE)(ctx.ID().getText, ctx.NUMBER().getText)

override def visitOperationStarts(ctx: OperationStartsContext): IgniteSqlFilterClause =
StartsIgniteSqlFilterClause(ctx.ID().getText, ctx.STRING().getText)
Expand Down
Loading

0 comments on commit d618f9c

Please sign in to comment.