From cf98d1527df74409570125c37dc25ac8afaf4b40 Mon Sep 17 00:00:00 2001 From: "Malik, Junaid" Date: Tue, 30 Apr 2024 13:50:51 +0800 Subject: [PATCH] #1296 use parameterized SQL query builder - this would prevent SQL injection while adding better support for different types i.e. for instance before we had to manually convert values to strings (with or without quotes depending on the type) before using them in SQL query. Now, ignite would do that for us. --- .../vuu/example/ignite/IgniteOrderStore.scala | 33 +++-- .../provider/IgniteOrderDataProvider.scala | 4 +- .../provider/IgniteOrderDataQuery.scala | 4 +- .../example/ignite/IgniteOrderStoreTest.scala | 26 ++-- .../provider/IgniteOrderDataQueryTest.scala | 4 +- .../ignite/FilterAndSortSpecToSql.scala | 12 +- .../vuu/feature/ignite/IgniteSqlQuery.scala | 43 +++++++ .../ignite/filter/IgniteSqlFilterClause.scala | 113 ++++++++---------- .../ignite/sort/IgniteSqlSortBuilder.scala | 10 +- .../feature/ignite/IgniteSqlQueryTest.scala | 94 +++++++++++++++ .../vuu/feature/ignite/IgniteTestStore.scala | 36 +++--- .../filter/IgniteSqlFilteringTest.scala | 12 +- .../sort/IgniteSqlSortBuilderTest.scala | 9 +- 13 files changed, 264 insertions(+), 136 deletions(-) create mode 100644 plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/IgniteSqlQuery.scala create mode 100644 plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteSqlQueryTest.scala diff --git a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/IgniteOrderStore.scala b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/IgniteOrderStore.scala index fca0985a7..6243366a7 100644 --- a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/IgniteOrderStore.scala +++ b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/IgniteOrderStore.scala @@ -7,6 +7,8 @@ import org.apache.ignite.cluster.ClusterState import org.apache.ignite.{IgniteCache, Ignition} import org.finos.vuu.core.module.simul.model.{ChildOrder, OrderStore, ParentOrder} import org.finos.vuu.example.ignite.utils.getListToObjectConverter +import org.finos.vuu.feature.ignite.IgniteSqlQuery +import org.finos.vuu.feature.ignite.IgniteSqlQuery.QuerySeparator import java.util import scala.collection.mutable @@ -87,27 +89,32 @@ class IgniteOrderStore(private val parentOrderCache: IgniteCache[Int, ParentOrde } - def getCount(sqlFilterQueries: String): Long = { + def getCount(filterSql: IgniteSqlQuery): Long = { //todo should this be COUNT_BIG? - val whereClause = if(sqlFilterQueries == null || sqlFilterQueries.isEmpty) "" else s" where $sqlFilterQueries" - val query = new SqlFieldsQuery(s"select COUNT(1) from ChildOrder$whereClause") - val cursor = childOrderCache.query(query) + val whereClause = if (filterSql.isEmpty) filterSql else filterSql.prependSql("WHERE", QuerySeparator.SPACE) + val query = IgniteSqlQuery("SELECT COUNT(1) FROM ChildOrder").appendQuery(whereClause, QuerySeparator.SPACE) - val countValue = cursor.getAll().get(0).get(0) + val cursor = childOrderCache.query(query.buildFieldsQuery()) + val countValue = cursor.getAll.get(0).get(0) val totalCount = countValue.asInstanceOf[Long] - logger.info(s"Ignite returned total count of $totalCount for ChildOrder with filter $sqlFilterQueries") + logger.info(s"Ignite returned total count of `$totalCount` for ChildOrder with filter `$filterSql`") totalCount } - def findChildOrder(sqlFilterQueries: String, sqlSortQueries: String, rowCount: Int, startIndex: Long): Iterator[ChildOrder] = { - val whereClause = if(sqlFilterQueries == null || sqlFilterQueries.isEmpty) "" else s" where $sqlFilterQueries" - val orderByClause = if(sqlSortQueries == null || sqlSortQueries.isEmpty) " order by id" else s" order by $sqlSortQueries" - val query = new SqlFieldsQuery(s"select * from ChildOrder$whereClause$orderByClause limit ? offset ?") - query.setArgs(rowCount, startIndex) + def findChildOrder(filterSql: IgniteSqlQuery, sortSql: IgniteSqlQuery, rowCount: Int, startIndex: Long): Iterator[ChildOrder] = { + val whereClause = if (filterSql.isEmpty) filterSql else filterSql.prependSql("WHERE", QuerySeparator.SPACE) + val orderByClause = if (sortSql.isEmpty) IgniteSqlQuery("ORDER BY id") else sortSql.prependSql("ORDER BY", QuerySeparator.SPACE) + val limitAndOffsetClause = IgniteSqlQuery("limit ? offset ?", List(rowCount, startIndex)) + + val query = IgniteSqlQuery("SELECT * FROM ChildOrder") + .appendQuery(whereClause, QuerySeparator.SPACE) + .appendQuery(orderByClause, QuerySeparator.SPACE) + .appendQuery(limitAndOffsetClause, QuerySeparator.SPACE) - val results = childOrderCache.query(query).asScala.iterator.map(i => toChildOrder(i.asScala.toList)) - logger.info(s"Loaded Ignite ChildOrder for $rowCount rows, from index : $startIndex where $whereClause order by $sqlSortQueries") + val results = childOrderCache.query(query.buildFieldsQuery()).asScala.iterator.map(i => toChildOrder(i.asScala.toList)) + logger.info(s"Loaded Ignite ChildOrder for $rowCount rows, from index : $startIndex with " + + s"WHERE CLAUSE: `$whereClause` | ORDER BY CLAUSE: `$orderByClause`") results } diff --git a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataProvider.scala b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataProvider.scala index ad153a78a..71e4e6679 100644 --- a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataProvider.scala +++ b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataProvider.scala @@ -8,6 +8,7 @@ import org.finos.vuu.example.ignite.module.IgniteOrderDataModule import org.finos.vuu.example.ignite.provider.IgniteOrderDataProvider.columnNameByExternalField import org.finos.vuu.example.ignite.query.IndexCalculator import org.finos.vuu.example.ignite.schema.ChildOrderSchema +import org.finos.vuu.feature.ignite.IgniteSqlQuery import org.finos.vuu.plugin.virtualized.table.{VirtualizedRange, VirtualizedSessionTable, VirtualizedViewPortKeys} import org.finos.vuu.provider.VirtualizedProvider import org.finos.vuu.util.schema.SchemaMapperBuilder @@ -52,8 +53,7 @@ class IgniteOrderDataProvider(final val igniteStore: IgniteOrderStore) viewPort.setKeys(new VirtualizedViewPortKeys(internalTable.primaryKeys)) } - private def getTotalSize(filter: String): Long = - igniteStore.getCount(filter) + private def getTotalSize(filterSql: IgniteSqlQuery): Long = igniteStore.getCount(filterSql) private def tableUpdater(table: VirtualizedSessionTable): (Int, Map[String, Any]) => Unit = { val keyField = table.tableDef.keyField diff --git a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQuery.scala b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQuery.scala index 2291ae448..621b8be03 100644 --- a/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQuery.scala +++ b/example/apache-ignite/src/main/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQuery.scala @@ -3,7 +3,7 @@ package org.finos.vuu.example.ignite.provider import org.finos.vuu.core.module.simul.model.ChildOrder import org.finos.vuu.core.sort.ModelType.SortSpecInternal import org.finos.vuu.example.ignite.IgniteOrderStore -import org.finos.vuu.feature.ignite.FilterAndSortSpecToSql +import org.finos.vuu.feature.ignite.{FilterAndSortSpecToSql, IgniteSqlQuery} import org.finos.vuu.net.FilterSpec import org.finos.vuu.util.schema.SchemaMapper @@ -12,7 +12,7 @@ class IgniteOrderDataQuery private (private val igniteOrderStore: IgniteOrderSto private val filterAndSortSpecToSql = FilterAndSortSpecToSql(schemaMapper) - def getFilterSql(filterSpec: FilterSpec): String = + def getFilterSql(filterSpec: FilterSpec): IgniteSqlQuery = filterAndSortSpecToSql.filterToSql(filterSpec) def fetch(filterSpec: FilterSpec, sortSpec: SortSpecInternal, startIndex: Long, rowCount: Int): Iterator[ChildOrder] = { diff --git a/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/IgniteOrderStoreTest.scala b/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/IgniteOrderStoreTest.scala index 97204a4c3..227b36465 100644 --- a/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/IgniteOrderStoreTest.scala +++ b/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/IgniteOrderStoreTest.scala @@ -3,6 +3,7 @@ package org.finos.vuu.example.ignite import org.apache.ignite.cache.query.IndexQueryCriteriaBuilder import org.apache.ignite.{Ignite, IgniteCache} import org.finos.vuu.core.module.simul.model.{ChildOrder, ParentOrder} +import org.finos.vuu.feature.ignite.IgniteSqlQuery import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} import org.scalatest.funsuite.AnyFunSuiteLike import org.scalatest.matchers.should.Matchers @@ -13,9 +14,6 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B private var childOrderCache: IgniteCache[Int, ChildOrder] = _ private var orderStore: IgniteOrderStore = _ - private val emptySortQueries: String = "" - private val emptyFilterQueries: String = "" - override def beforeAll(): Unit = { ignite = TestUtils.setupIgnite(testName = this.toString) parentOrderCache = ignite.getOrCreateCache("parentOrderCache") @@ -92,8 +90,8 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B parentOrder2 = GivenParentHasChildOrder(parentOrder2, 5) parentOrder2 = GivenParentHasChildOrder(parentOrder2, 6) - val filterQueries = "parentId = 2" - val childOrder = orderStore.findChildOrder(filterQueries, emptySortQueries, 2, 1).toList + val filterQuery = IgniteSqlQuery("parentId = ?", List(2)) + val childOrder = orderStore.findChildOrder(filterQuery, IgniteSqlQuery.empty, 2, 1).toList assert(childOrder != null) assert(childOrder.size == 2) @@ -113,8 +111,8 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B parentOrder2 = GivenParentHasChildOrder(parentOrder2, 5, ric = "VOD.L") parentOrder2 = GivenParentHasChildOrder(parentOrder2, 6, ric = "VOD.L") - val filterQueries = "ric = \'VOD.L\'" - val childOrder = orderStore.findChildOrder(filterQueries, emptySortQueries, 100, 0).toList + val filterQuery = IgniteSqlQuery("ric = ?", List("VOD.L")) + val childOrder = orderStore.findChildOrder(filterQuery, IgniteSqlQuery.empty, 100, 0).toList assert(childOrder != null) assert(childOrder.size == 6) @@ -128,7 +126,7 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B parentOrder2 = GivenParentHasChildOrder(parentOrder2, 4) parentOrder2 = GivenParentHasChildOrder(parentOrder2, 5) - val childOrder = orderStore.findChildOrder(emptyFilterQueries, emptySortQueries, 100, 0).toList + val childOrder = orderStore.findChildOrder(IgniteSqlQuery.empty, IgniteSqlQuery.empty, 100, 0).toList assert(childOrder != null) assert(childOrder.size == 3) @@ -142,8 +140,8 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B var parentOrder2: ParentOrder = GivenParentOrder(2) parentOrder2 = GivenParentHasChildOrder(parentOrder2, 2) - val sortByValues = "id ASC" - val childOrder = orderStore.findChildOrder(emptyFilterQueries, sortByValues, 100, 0).toList + val sortByValues = IgniteSqlQuery("id ASC") + val childOrder = orderStore.findChildOrder(IgniteSqlQuery.empty, sortByValues, 100, 0).toList assert(childOrder != null) assert(childOrder.size == 3) @@ -158,8 +156,8 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B var parentOrder2: ParentOrder = GivenParentOrder(2) parentOrder2 = GivenParentHasChildOrder(parentOrder2, 2) - val sortByValues = "parentId DESC, id ASC" - val childOrder = orderStore.findChildOrder(emptyFilterQueries, sortByValues, 100, 0).toList + val sortByValues = IgniteSqlQuery("parentId DESC, id ASC") + val childOrder = orderStore.findChildOrder(IgniteSqlQuery.empty, sortByValues, 100, 0).toList assert(childOrder != null) assert(childOrder.size == 3) @@ -178,9 +176,9 @@ class IgniteOrderStoreTest extends AnyFunSuiteLike with BeforeAndAfterAll with B parentOrder2 = GivenParentHasChildOrder(parentOrder2, 5, ric = "VOD.L") parentOrder2 = GivenParentHasChildOrder(parentOrder2, 6, ric = "BP.L") - val filterQueries = "ric = \'VOD.L\'" + val filterQuery = IgniteSqlQuery("ric = ?", List("VOD.L")) - val count = orderStore.getCount(filterQueries) + val count = orderStore.getCount(filterQuery) assert(count == 3) } diff --git a/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQueryTest.scala b/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQueryTest.scala index 1830404ef..c4c6974d0 100644 --- a/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQueryTest.scala +++ b/example/apache-ignite/src/test/scala/org/finos/vuu/example/ignite/provider/IgniteOrderDataQueryTest.scala @@ -5,6 +5,7 @@ import org.finos.vuu.core.sort.SortDirection 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.feature.ignite.IgniteSqlQuery import org.finos.vuu.net.FilterSpec import org.finos.vuu.util.schema.{ExternalEntitySchema, ExternalEntitySchemaBuilder, SchemaMapper, SchemaMapperBuilder} import org.scalamock.scalatest.MockFactory @@ -26,7 +27,8 @@ class IgniteOrderDataQueryTest extends AnyFeatureSpec with Matchers with MockFac val filterSpec = FilterSpec("id = 2 and name != \"ABC\"") val sortSpec = Map("value" -> SortDirection.Ascending) - (igniteStore.findChildOrder _).expects("(key = 2 AND name != 'ABC')", "value ASC", *, *).once() + (igniteStore.findChildOrder _) + .expects(IgniteSqlQuery("(key = ? AND name != ?)", List(2, "ABC")), IgniteSqlQuery("value ASC"), *, *).once() igniteDataQuery.fetch(filterSpec, sortSpec, 0, 0) } diff --git a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/FilterAndSortSpecToSql.scala b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/FilterAndSortSpecToSql.scala index 11dcd61b0..24e2ec4ad 100644 --- a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/FilterAndSortSpecToSql.scala +++ b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/FilterAndSortSpecToSql.scala @@ -8,8 +8,8 @@ import org.finos.vuu.net.FilterSpec import org.finos.vuu.util.schema.SchemaMapper trait FilterAndSortSpecToSql { - def filterToSql(filterSpec: FilterSpec): String - def sortToSql(sortSpec: SortSpecInternal): String + def filterToSql(filterSpec: FilterSpec): IgniteSqlQuery + def sortToSql(sortSpec: SortSpecInternal): IgniteSqlQuery } object FilterAndSortSpecToSql { @@ -21,14 +21,16 @@ private class FilterAndSortSpecToSqlImpl(private val schemaMapper: SchemaMapper) private val filterTreeVisitor = new IgniteSqlFilterTreeVisitor private val igniteSqlSortBuilder = new IgniteSqlSortBuilder - override def filterToSql(filterSpec: FilterSpec): String = { + override def filterToSql(filterSpec: FilterSpec): IgniteSqlQuery = { if (filterSpec.filter == null || filterSpec.filter.isEmpty) { - "" + IgniteSqlQuery.empty } else { val clause = FilterSpecParser.parse[IgniteSqlFilterClause](filterSpec.filter, filterTreeVisitor) clause.toSql(schemaMapper) } } - override def sortToSql(sortSpec: SortSpecInternal): String = igniteSqlSortBuilder.toSql(sortSpec, schemaMapper) + override def sortToSql(sortSpec: SortSpecInternal): IgniteSqlQuery = { + if (sortSpec == null) IgniteSqlQuery.empty else igniteSqlSortBuilder.toSql(sortSpec, schemaMapper) + } } diff --git a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/IgniteSqlQuery.scala b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/IgniteSqlQuery.scala new file mode 100644 index 000000000..bfe89296a --- /dev/null +++ b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/IgniteSqlQuery.scala @@ -0,0 +1,43 @@ +package org.finos.vuu.feature.ignite + +import org.apache.ignite.cache.query.SqlFieldsQuery +import org.finos.vuu.feature.ignite.IgniteSqlQuery.QuerySeparator + +object IgniteSqlQuery { + def apply(sqlTemplate: String): IgniteSqlQuery = new IgniteSqlQuery(sqlTemplate, List.empty) + def apply(): IgniteSqlQuery = IgniteSqlQuery("") + def empty: IgniteSqlQuery = IgniteSqlQuery() + + sealed abstract class QuerySeparator(val value: String) + object QuerySeparator { + final case object AND extends QuerySeparator(value = " AND ") + final case object OR extends QuerySeparator(value = " OR ") + final case object SPACE extends QuerySeparator(value = " ") + final case object EMPTY extends QuerySeparator(value = "") + } +} + +case class IgniteSqlQuery(sqlTemplate: String, args: List[Any]) { + + def appendSql(sqlTemplate: String, sep: QuerySeparator = QuerySeparator.EMPTY): IgniteSqlQuery = { + val newTemplate = if (sqlTemplate.isEmpty) this.sqlTemplate else Array(this.sqlTemplate, sqlTemplate).mkString(sep.value) + this.copy(sqlTemplate = newTemplate) + } + + def prependSql(sqlTemplate: String, sep: QuerySeparator = QuerySeparator.EMPTY): IgniteSqlQuery = { + val newTemplate = if (sqlTemplate.isEmpty) this.sqlTemplate else Array(sqlTemplate, this.sqlTemplate).mkString(sep.value) + this.copy(sqlTemplate = newTemplate) + } + + def appendArgs(args: List[Any]): IgniteSqlQuery = { + this.copy(args = this.args ++ args) + } + + def appendQuery(query: IgniteSqlQuery, sep: QuerySeparator = QuerySeparator.EMPTY): IgniteSqlQuery = { + this.appendSql(query.sqlTemplate, sep).appendArgs(query.args) + } + + def isEmpty: Boolean = this.sqlTemplate.isEmpty && this.args.isEmpty + + def buildFieldsQuery(): SqlFieldsQuery = new SqlFieldsQuery(sqlTemplate).setArgs(args.toArray: _*) +} diff --git a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilterClause.scala b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilterClause.scala index 13dc5e56d..982701d4c 100644 --- a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilterClause.scala +++ b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilterClause.scala @@ -1,108 +1,107 @@ package org.finos.vuu.feature.ignite.filter import com.typesafe.scalalogging.StrictLogging -import org.finos.vuu.feature.ignite.filter.IgniteSqlFilterClause.EMPTY_SQL +import org.finos.vuu.feature.ignite.IgniteSqlQuery +import org.finos.vuu.feature.ignite.filter.EqIgniteSqlFilterClause.eqSqlQuery import org.finos.vuu.feature.ignite.filter.FilterColumnValueParser.{ParsedResult, STRING_DATA_TYPE} +import org.finos.vuu.feature.ignite.IgniteSqlQuery.QuerySeparator import org.finos.vuu.util.schema.{SchemaField, SchemaMapper} -private object IgniteSqlFilterClause { - val EMPTY_SQL = "" -} - trait IgniteSqlFilterClause { - def toSql(schemaMapper: SchemaMapper): String + def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery } case class OrIgniteSqlFilterClause(clauses:List[IgniteSqlFilterClause]) extends IgniteSqlFilterClause { - override def toSql(schemaMapper: SchemaMapper): String = { - val sql = clauses.map(c => c.toSql(schemaMapper)).filter(_ != EMPTY_SQL).mkString(" OR ") - if (clauses.length > 1) s"($sql)" else sql + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { + val queries = clauses.map(_.toSql(schemaMapper)) + joinNonEmptyQueries(queries, QuerySeparator.OR) } } case class AndIgniteSqlFilterClause(clauses:List[IgniteSqlFilterClause]) extends IgniteSqlFilterClause { - override def toSql(schemaMapper: SchemaMapper): String = { - val sql = clauses.map(c => c.toSql(schemaMapper)).filter(_ != EMPTY_SQL).mkString(" AND ") - if (clauses.length > 1) s"($sql)" else sql + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { + val queries = clauses.map(_.toSql(schemaMapper)) + joinNonEmptyQueries(queries, QuerySeparator.AND) } } case class EqIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause { - override def toSql(schemaMapper: SchemaMapper): String = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, value) match { - case Right(ParsedResult(f, externalValue)) => eqSql(f.name, convertToString(externalValue, f.dataType)) + case Right(ParsedResult(f, externalValue)) => eqSqlQuery(f.name, externalValue) case Left(errMsg) => logErrorAndReturnEmptySql(errMsg) } } +} - private def eqSql(field: String, processedVal: String): String = { - s"$field = $processedVal" - } +private object EqIgniteSqlFilterClause { + def eqSqlQuery(field: String, value: Any): IgniteSqlQuery = + IgniteSqlQuery(s"$field = ?", List(value)) } case class NeqIgniteSqlFilterClause(columnName: String, value: String) extends IgniteSqlFilterClause { - override def toSql(schemaMapper: SchemaMapper): String = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, value) match { - case Right(ParsedResult(f, externalValue)) => neqSql(f.name, convertToString(externalValue, f.dataType)) + case Right(ParsedResult(f, externalValue)) => neqSql(f.name, externalValue) case Left(errMsg) => logErrorAndReturnEmptySql(errMsg) } } - private def neqSql(field: String, processedVal: String): String = { - s"$field != $processedVal" - } + private def neqSql(field: String, value: Any) = IgniteSqlQuery(s"$field != ?", List(value)) } case class RangeIgniteSqlFilterClause(op: RangeOp)(columnName: String, value: String) extends IgniteSqlFilterClause { - override def toSql(schemaMapper: SchemaMapper): String = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, value) match { - case Right(ParsedResult(f, externalValue)) => rangeSql(f.name, convertToString(externalValue, f.dataType)) + case Right(ParsedResult(f, externalValue)) => rangeSql(f.name, externalValue) case Left(errMsg) => logErrorAndReturnEmptySql(errMsg) } } - private def rangeSql(field: String, processedVal: String): String = s"$field ${op.value} $processedVal" + private def rangeSql(field: String, value: Any) = IgniteSqlQuery(s"$field ${op.value} ?", List(value)) + 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 = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, value) match { - case Right(ParsedResult(f, externalValue)) => startsSql(f, convertToString(externalValue, f.dataType)) + case Right(ParsedResult(f, externalValue)) => startsSql(f, externalValue) 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})") + private def startsSql(f: SchemaField, value: Any): IgniteSqlQuery = f.dataType match { + case STRING_DATA_TYPE => IgniteSqlQuery(s"${f.name} LIKE ?", List(s"$value%")) + 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 = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, value) match { - case Right(ParsedResult(f, externalValue)) => endsSql(f, convertToString(externalValue, f.dataType)) + case Right(ParsedResult(f, externalValue)) => endsSql(f, externalValue) 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})") + private def endsSql(f: SchemaField, value: Any): IgniteSqlQuery = f.dataType match { + case STRING_DATA_TYPE => IgniteSqlQuery(s"${f.name} LIKE ?", List(s"%$value")) + 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 = { + override def toSql(schemaMapper: SchemaMapper): IgniteSqlQuery = { FilterColumnValueParser(schemaMapper).parse(columnName, values) match { - case Right(ParsedResult(f, externalValues)) => inQuery(f.name, externalValues.map(convertToString(_, f.dataType))) + case Right(ParsedResult(f, externalValues)) => inQuery(f.name, externalValues) case Left(errMsg) => logErrorAndReturnEmptySql(errMsg) } } - private def inQuery(field: String, processedValues: List[String]) = { - s"$field IN (${processedValues.mkString(",")})" + private def inQuery(field: String, values: List[Any]): IgniteSqlQuery = { + val eqQueries = values.map(eqSqlQuery(field, _)) + joinNonEmptyQueries(eqQueries, QuerySeparator.OR) } } @@ -114,37 +113,19 @@ object RangeOp { final case object LTE extends RangeOp(value = "<=") } +private object joinNonEmptyQueries { + def apply(queries: List[IgniteSqlQuery], sep: QuerySeparator): IgniteSqlQuery = { + val joinedQuery = queries + .reduceLeftOption((acc, query) => acc.appendQuery(query, sep)) + .getOrElse(IgniteSqlQuery.empty) -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'" -} - -// @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] - ) + if (queries.length > 1) joinedQuery.prependSql("(").appendSql(")") else joinedQuery + } } private object logErrorAndReturnEmptySql extends StrictLogging { - def apply(error: String): String = { + def apply(error: String): IgniteSqlQuery = { logger.error(error) - EMPTY_SQL + IgniteSqlQuery.empty } } diff --git a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilder.scala b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilder.scala index 1e3269424..3fc1a15f7 100644 --- a/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilder.scala +++ b/plugin/ignite-plugin/src/main/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilder.scala @@ -2,6 +2,7 @@ package org.finos.vuu.feature.ignite.sort import org.finos.vuu.core.sort.ModelType.SortSpecInternal import org.finos.vuu.core.sort.SortDirection +import org.finos.vuu.feature.ignite.IgniteSqlQuery import org.finos.vuu.util.schema.SchemaMapper @@ -11,11 +12,14 @@ import org.finos.vuu.util.schema.SchemaMapper class IgniteSqlSortBuilder { private val AscendingSql = "ASC" private val DescendingSql = "DESC" - def toSql(sortColumnsToDirections: SortSpecInternal, schemaMapper: SchemaMapper): String = - sortColumnsToDirections + def toSql(sortColumnsToDirections: SortSpecInternal, schemaMapper: SchemaMapper): IgniteSqlQuery = { + val sql = sortColumnsToDirections .flatMap{case (columnName, direction) => toSortString(columnName, direction, schemaMapper)} .mkString(", ") + IgniteSqlQuery(sqlTemplate = sql) + } + private def toSortString(columnName: String, sortDirection: SortDirection.TYPE, schemaMapper: SchemaMapper): Option[String] = { @@ -26,7 +30,7 @@ class IgniteSqlSortBuilder { } private def toSQL(direction: SortDirection.TYPE) = direction match { - case SortDirection.Ascending => AscendingSql + case SortDirection.Ascending => AscendingSql case SortDirection.Descending => DescendingSql } } diff --git a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteSqlQueryTest.scala b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteSqlQueryTest.scala new file mode 100644 index 000000000..d940c42bb --- /dev/null +++ b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteSqlQueryTest.scala @@ -0,0 +1,94 @@ +package org.finos.vuu.feature.ignite + +import org.finos.vuu.feature.ignite.IgniteSqlQuery.QuerySeparator +import org.scalatest.featurespec.AnyFeatureSpec +import org.scalatest.matchers.should.Matchers + +class IgniteSqlQueryTest extends AnyFeatureSpec with Matchers { + + Feature("appendSql") { + val query = IgniteSqlQuery("SELECT * FROM TABLE") + + Scenario("should return new query with appended sql using the specified separator") { + val newQuery = query.appendSql("ORDER BY id", QuerySeparator.SPACE) + + newQuery.sqlTemplate shouldEqual (query.sqlTemplate + " ORDER BY id") + } + + Scenario("should return sql unchanged if passed sql is empty") { + val newQuery = query.appendSql("", sep = QuerySeparator.OR) + + newQuery.sqlTemplate shouldEqual query.sqlTemplate + } + } + + Feature("prependSql") { + val query = IgniteSqlQuery("ORDER BY id") + + Scenario("should return new query with prepended sql using the specified separator") { + val newQuery = query.prependSql("SELECT * FROM TABLE", QuerySeparator.SPACE) + + newQuery.sqlTemplate shouldEqual ("SELECT * FROM TABLE " + query.sqlTemplate) + } + + Scenario("should return sql unchanged if passed sql is empty") { + val newQuery = query.prependSql("", sep = QuerySeparator.OR) + + newQuery.sqlTemplate shouldEqual query.sqlTemplate + } + } + + Feature("appendQuery") { + val query = IgniteSqlQuery("SELECT * FROM ?", List("TABLE_2")) + + Scenario("should return new joined query with the specified separator") { + val query2 = IgniteSqlQuery("WHERE id = ?", List(23)) + + val joinedQuery = query.appendQuery(query2, QuerySeparator.SPACE) + + joinedQuery.sqlTemplate shouldEqual "SELECT * FROM ? WHERE id = ?" + joinedQuery.args shouldEqual List("TABLE_2", 23) + } + } + + Feature("appendArgs") { + val query = IgniteSqlQuery("SELECT * FROM ?", List("TABLE2")) + + Scenario("should return new query with appended args") { + val args = List(1, "string", 'A') + + val newQuery = query.appendArgs(args) + + newQuery shouldEqual IgniteSqlQuery(query.sqlTemplate, query.args ++ args) + } + } + + Feature("isEmpty") { + Scenario("should return true when both sqlTemplate and args are empty") { + val query = IgniteSqlQuery("", List.empty) + + query.isEmpty shouldBe true + } + + Scenario("should return false when sqlTemplate is not empty") { + val query = IgniteSqlQuery("A", List.empty) + + query.isEmpty shouldBe false + } + + Scenario("should return false when sqlTemplate is empty but args is not") { + val query = IgniteSqlQuery("", List("A")) + + query.isEmpty shouldBe false + } + } + + Feature("toString") { + Scenario("should return expected string representation of the object") { + val query = IgniteSqlQuery("SELECT * FROM ?", List("TABLE2")) + + query.toString shouldEqual "IgniteSqlQuery(SELECT * FROM ?,List(TABLE2))" + } + } + +} diff --git a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteTestStore.scala b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteTestStore.scala index 01dcf1f8b..48407d52b 100644 --- a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteTestStore.scala +++ b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/IgniteTestStore.scala @@ -6,12 +6,12 @@ import org.apache.ignite.cache.{QueryEntity, QueryIndex, QueryIndexType} import org.apache.ignite.{Ignite, IgniteCache, Ignition} import org.apache.ignite.cluster.ClusterState import org.apache.ignite.configuration.{CacheConfiguration, IgniteConfiguration} +import IgniteSqlQuery.QuerySeparator import java.sql.Date import java.math.BigDecimal import java.time.LocalDate import java.util -import scala.collection.mutable import scala.jdk.CollectionConverters._ import scala.reflect.ClassTag @@ -103,31 +103,27 @@ class IgniteTestStore (private val orderCache: IgniteCache[Int, TestOrderEntity] .map(x => x.getValue) } - def getFilteredBy(sqlFilterQuery: String): Iterable[TestOrderEntity] = { - val whereClause = if (sqlFilterQuery == null || sqlFilterQuery.isEmpty) "" else s" where $sqlFilterQuery" - val value = s"select * from TestOrderEntity$whereClause" - getSQLAndMapResult(value) + def getFilteredBy(sqlFilterQuery: IgniteSqlQuery): Iterable[TestOrderEntity] = { + val whereClause = if (sqlFilterQuery.isEmpty) sqlFilterQuery else sqlFilterQuery.prependSql("WHERE", QuerySeparator.SPACE) + + val finalQuery = IgniteSqlQuery("SELECT * FROM TestOrderEntity") + .appendQuery(whereClause, QuerySeparator.SPACE) + .appendQuery(IgniteSqlQuery("ORDER BY id"), QuerySeparator.SPACE) + + getSQLAndMapResult(finalQuery) } - def getSortBy(sqlSortQuery: String): Iterable[TestOrderEntity] = { - val orderByClause = if (sqlSortQuery == null || sqlSortQuery.isEmpty) "" else s" order by $sqlSortQuery" - val value = s"select * from TestOrderEntity$orderByClause" - getSQLAndMapResult(value) + def getSortBy(sortSql: IgniteSqlQuery): Iterable[TestOrderEntity] = { + val orderByClause = if (sortSql.isEmpty) IgniteSqlQuery.empty else sortSql.prependSql("ORDER BY", QuerySeparator.SPACE) + val finalQuery = IgniteSqlQuery("SELECT * FROM TestOrderEntity").appendQuery(orderByClause, QuerySeparator.SPACE) + getSQLAndMapResult(finalQuery) } - private def getSQLAndMapResult(sqlQuery: String): Iterable[TestOrderEntity] = { + private def getSQLAndMapResult(sqlQuery: IgniteSqlQuery): Iterable[TestOrderEntity] = { logger.info("Querying ignite for " + sqlQuery) - val query = new SqlFieldsQuery(sqlQuery) - - val results = orderCache.query(query) - var counter = 0 - val buffer = mutable.ListBuffer[TestOrderEntity]() - results.forEach(item => { - buffer.addOne(TestOrderEntity.createFrom(item)) - counter += 1 - }) + val results = orderCache.query(sqlQuery.buildFieldsQuery()) - buffer + results.getAll.asScala.map(l => TestOrderEntity.createFrom(l)) } } diff --git a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilteringTest.scala b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilteringTest.scala index d533f4927..bc815f5ec 100644 --- a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilteringTest.scala +++ b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/filter/IgniteSqlFilteringTest.scala @@ -212,7 +212,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { Scenario("Support comparison to INT") { givenOrderExistInIgnite(testOrder1, testOrder2, testOrder3) - val filterResult = applyFilter("parentOrderId = 10") + val filterResult = applyFilter("parentOrderId = \"10\"") assertEquavalent(filterResult.toArray, Array(testOrder1, testOrder3)) } @@ -290,7 +290,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { Scenario("Support comparison to INT") { givenOrderExistInIgnite(testOrder1, testOrder2, testOrder3) - val filterResult = applyFilter("parentOrderId != 10") + val filterResult = applyFilter("parentOrderId != \"10\"") assertEquavalent(filterResult.toArray, Array(testOrder2)) } @@ -500,7 +500,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { val filterResult = applyFilter("ric in [\"VAD.DDN\", \"NVD.LDN\"]") - assertEquavalent(filterResult.toArray, Array(testOrder3, testOrder2)) + assertEquavalent(filterResult.toArray, Array(testOrder2, testOrder3)) } Scenario("supports Char column type") { @@ -514,7 +514,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { Scenario("supports Int column type") { givenOrderExistInIgnite(testOrder1, testOrder2, testOrder3) - val filterResult = applyFilter("parentOrderId in [2, 4]") + val filterResult = applyFilter("parentOrderId in [\"2\", \"4\"]") assertEquavalent(filterResult.toArray, Array(testOrder1, testOrder3)) } @@ -579,7 +579,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { ("ric", classOf[String]), ("price", classOf[Double]), ("quantity", classOf[Int]), - ("parentOrderId", classOf[Int]), + ("parentOrderId", classOf[String]), ("rating", classOf[String]), ("isFilled", classOf[Boolean]), ("createdAt", classOf[Long]), @@ -590,7 +590,7 @@ class IgniteSqlFilteringTest extends IgniteTestsBase { private class TestEntitySchema extends ExternalEntitySchema { override val fields: List[SchemaField] = List( SchemaField("id", classOf[Int], 0), - SchemaField("parentId", classOf[String], 1), + SchemaField("parentId", classOf[Int], 1), SchemaField("ric", classOf[String], 2), SchemaField("strategy", classOf[String], 3), SchemaField("quantity", classOf[Int], 4), diff --git a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilderTest.scala b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilderTest.scala index f8e127d59..a6d454bdf 100644 --- a/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilderTest.scala +++ b/plugin/ignite-plugin/src/test/scala/org/finos/vuu/feature/ignite/sort/IgniteSqlSortBuilderTest.scala @@ -4,8 +4,9 @@ import org.finos.vuu.core.sort.SortDirection import org.finos.vuu.util.schema.{SchemaField, SchemaMapper} import org.scalatest.featurespec.AnyFeatureSpec import org.scalamock.scalatest.MockFactory +import org.scalatest.matchers.should.Matchers -class IgniteSqlSortBuilderTest extends AnyFeatureSpec with MockFactory { +class IgniteSqlSortBuilderTest extends AnyFeatureSpec with MockFactory with Matchers { private val sortBuilder = new IgniteSqlSortBuilder() private val schemaMapper = mock[SchemaMapper] @@ -17,7 +18,7 @@ class IgniteSqlSortBuilderTest extends AnyFeatureSpec with MockFactory { val sortSpecInternal = Map("parentOrderId"-> SortDirection.Descending) val sortSql = sortBuilder.toSql(sortSpecInternal, schemaMapper) - assert(sortSql == "orderId DESC") + sortSql.sqlTemplate shouldEqual "orderId DESC" } Scenario("can create sql order by clause for multiple ignite columns") { @@ -30,7 +31,7 @@ class IgniteSqlSortBuilderTest extends AnyFeatureSpec with MockFactory { ) val sortSql = sortBuilder.toSql(sortSpecInternal, schemaMapper) - assert(sortSql == "column1 DESC, column2 ASC") + sortSql.sqlTemplate shouldEqual "column1 DESC, column2 ASC" } Scenario("skip sort if no mapping found to ignite columns") { @@ -39,7 +40,7 @@ class IgniteSqlSortBuilderTest extends AnyFeatureSpec with MockFactory { val sortSpecInternal = Map("someTableColumnNotInMap" -> SortDirection.Descending) val sortSql = sortBuilder.toSql(sortSpecInternal, schemaMapper) - assert(sortSql == "") + sortSql.sqlTemplate shouldEqual "" } }