Merge pull request #1360 from junaidzm13/improve-inmem-typeahead-perf…

…ormance

#1353 improve in-mem typeahead's performance

docs/rpc/service.md

@@ -26,13 +26,13 @@ object TypeAheadModule extends DefaultModule {
 
   def apply()(implicit clock: Clock, lifecycle: LifecycleContainer): ViewServerModule = {
     ModuleFactory.withNamespace(NAME)
-      .addRpcHandler(server => new TypeAheadRpcHandlerImpl(server.tableContainer))
+      .addRpcHandler(server => new GenericTypeAheadRpcHandler(server.tableContainer))
       .asModule()
   }
 }
 ```
 
-You can see we've defined an RpcHandler called TypeAheadRpcHandlerImpl, which implements the interface:
+You can see we've defined an RpcHandler called GenericTypeAheadRpcHandler, which implements the interface:
 
 ```scala
 trait TypeAheadRpcHandler{

vuu/src/main/scala/org/finos/vuu/core/module/typeahead/TypeAheadRpcHandler.scala

@@ -1,70 +1,8 @@
 package org.finos.vuu.core.module.typeahead
 
-import com.typesafe.scalalogging.StrictLogging
-import org.finos.vuu.core.table.{Column, DataTable, TableContainer}
 import org.finos.vuu.net.RequestContext
-import org.finos.vuu.net.rpc.RpcHandler
 
 trait TypeAheadRpcHandler{
   def getUniqueFieldValues(tableMap: Map[String, String], column: String, ctx: RequestContext): Array[String]
   def getUniqueFieldValuesStartingWith(tableMap: Map[String, String], column: String, starts: String, ctx: RequestContext): Array[String]
 }
-
-
-class TypeAheadRpcHandlerImpl(val tableContainer: TableContainer) extends RpcHandler with StrictLogging with TypeAheadRpcHandler {
-
-  private def addUnique(dt: DataTable, c: Column, set: Set[String], key: String):  Set[String] = {
-    val row = dt.pullRow(key)
-    row.get(c) match {
-      case null =>
-        Set()
-      case x: String =>
-        set.+(x)
-      case x: Long =>
-        set.+(x.toString)
-      case x: Double =>
-        set.+(x.toString)
-      case x: Int =>
-        set.+(x.toString)
-      case x =>
-        set.+(x.toString)
-    }
-  }
-
-
-  override def getUniqueFieldValuesStartingWith(tableMap: Map[String, String], column: String, starts: String, ctx: RequestContext): Array[String] = {
-    val tableName = tableMap("table")
-
-    tableContainer.getTable(tableName) match {
-      case dataTable: DataTable =>
-        dataTable.columnForName(column) match {
-          case c: Column =>
-            dataTable.primaryKeys.foldLeft(Set[String]())(addUnique(dataTable, c, _, _)).filter(_.startsWith(starts)).toArray.sorted.take(10)
-          case null =>
-            logger.error(s"Column ${column} not found in table ${tableName}")
-            Array()
-        }
-      case null =>
-        throw new Exception("Could not find table by name:" + tableName)
-    }
-  }
-
-  def getUniqueFieldValues(tableMap: Map[String, String], column: String, ctx: RequestContext): Array[String] = {
-
-    val tableName = tableMap("table")
-
-    tableContainer.getTable(tableName) match {
-      case dataTable: DataTable =>
-        dataTable.columnForName(column) match {
-          case c: Column =>
-            dataTable.primaryKeys.foldLeft(Set[String]())(addUnique(dataTable, c, _, _)).toArray.sorted.take(10)
-          case null =>
-            logger.error(s"Column ${column} not found in table ${tableName}")
-            Array()
-        }
-      case null =>
-        throw new Exception("Could not find table by name:" + tableName)
-    }
-  }
-
-}

vuu/src/main/scala/org/finos/vuu/core/table/ColumnValueProvider.scala

@@ -3,15 +3,8 @@ package org.finos.vuu.core.table
 import com.typesafe.scalalogging.StrictLogging
 
 trait ColumnValueProvider {
-
-  //todo currently only returns first 10 results.. so can't scrolling through values
-  //could return everything let ui decide how many results to display but there is cost to the json serialisig for large dataset
-  //todo how to handle nulls - for different data types
-  //todo should this be returning null or rely on json deserialiser rules?
-
   def getUniqueValues(columnName:String):Array[String]
   def getUniqueValuesStartingWith(columnName:String, starts: String):Array[String]
-
 }
 
 class EmptyColumnValueProvider extends ColumnValueProvider {
@@ -28,41 +21,38 @@ object InMemColumnValueProvider {
     }
   }
 }
+
 class InMemColumnValueProvider(dataTable: InMemDataTable) extends ColumnValueProvider with StrictLogging {
+  private val get10DistinctValues = DistinctValuesGetter(10)
 
   override def getUniqueValues(columnName: String): Array[String] =
     dataTable.columnForName(columnName) match {
-    case c: Column =>
-      dataTable.primaryKeys.foldLeft(Set[String]())(addUnique(dataTable, c, _, _)).toArray.sorted.take(10)
-    case null =>
-      logger.error(s"Column $columnName not found in table ${dataTable.name}")
-      Array()
+    case c: Column => get10DistinctValues(c)
+    case null      => logger.error(s"Column $columnName not found in table ${dataTable.name}"); Array.empty;
   }
 
   override def getUniqueValuesStartingWith(columnName: String, starts: String): Array[String] =
     dataTable.columnForName(columnName) match {
-      case c: Column =>
-        dataTable.primaryKeys.foldLeft(Set[String]())(addUnique(dataTable, c, _, _)).filter(_.startsWith(starts)).toArray.sorted.take(10)
-      case null =>
-        logger.error(s"Column $columnName not found in table ${dataTable.name}")
-        Array()
+      case c: Column => get10DistinctValues(c, _.startsWith(starts))
+      case null      => logger.error(s"Column $columnName not found in table ${dataTable.name}"); Array.empty;
     }
 
-  private def addUnique(dt: DataTable, c: Column, set: Set[String], key: String): Set[String] = {
-    val row = dt.pullRow(key)
-    row.get(c) match {
-      case null =>
-        Set()
-      case x: String =>
-        set.+(x)
-      case x: Long =>
-        set.+(x.toString)
-      case x: Double =>
-        set.+(x.toString)
-      case x: Int =>
-        set.+(x.toString)
-      case x =>
-        set.+(x.toString)
+
+  private case class DistinctValuesGetter(n: Int) {
+    private type Filter = String => Boolean
+
+    def apply(c: Column, filter: Filter = _ => true): Array[String] = getDistinctValues(c, filter).take(n).toArray
+
+    private def getDistinctValues(c: Column, filter: Filter): Iterator[String] = {
+      dataTable.primaryKeys
+        .iterator
+        .map(dataTable.pullRow(_).get(c))
+        .distinct
+        .flatMap(valueToString)
+        .filter(filter)
     }
+
+    private def valueToString(value: Any): Option[String] = Option(value).map(_.toString)
   }
+
 }

vuu/src/test/scala/org/finos/vuu/core/module/typeahead/TypeAheadModuleTest.scala

@@ -20,7 +20,7 @@ class TypeAheadModuleTest extends AnyFeatureSpec with Matchers with GivenWhenThe
 
   def callGetUniqueFieldValues(tables: TableContainer, column: String): Array[String] = {
 
-    val typeAheadRpc = new TypeAheadRpcHandlerImpl(tables)
+    val typeAheadRpc = new GenericTypeAheadRpcHandler(tables)
 
     val ctx = RequestContext("", ClientSessionId("", ""), null, "")
 
@@ -33,7 +33,7 @@ class TypeAheadModuleTest extends AnyFeatureSpec with Matchers with GivenWhenThe
 
   def callGetUniqueFieldValuesStarting(tables: TableContainer, column: String, starts: String): Array[String] = {
 
-    val typeAheadRpc = new TypeAheadRpcHandlerImpl(tables)
+    val typeAheadRpc = new GenericTypeAheadRpcHandler(tables)
 
     val ctx = new RequestContext("", ClientSessionId("", ""), null, "")
 

vuu/src/test/scala/org/finos/vuu/core/table/InMemColumnValueProviderTest.scala

@@ -5,56 +5,82 @@ import org.finos.toolbox.lifecycle.LifecycleContainer
 import org.finos.toolbox.time.{Clock, TestFriendlyClock}
 import org.finos.vuu.api.TableDef
 import org.finos.vuu.provider.{JoinTableProviderImpl, MockProvider}
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.Assertions
 import org.scalatest.featurespec.AnyFeatureSpec
 import org.scalatest.matchers.should.Matchers
 
-class InMemColumnValueProviderTest extends AnyFeatureSpec with Matchers {
+class InMemColumnValueProviderTest extends AnyFeatureSpec with Matchers with MockFactory  {
 
   implicit val clock: Clock = new TestFriendlyClock(10001L)
   implicit val lifecycle: LifecycleContainer = new LifecycleContainer()
   implicit val metricsProvider: MetricsProvider = new MetricsProviderImpl
-
   private val pricesDef: TableDef = TableDef(
     "prices",
-    "ric",
-    Columns.fromNames("ric:String", "bid:Double", "ask:Double"),
+    "id",
+    Columns.fromNames("id:Long", "ric:String", "bid:Double", "ask:Double"),
   )
 
   Feature("InMemColumnValueProvider") {
 
     Scenario("Get all unique value of a given column") {
+      val table = givenTable(pricesDef)
+      val provider = new MockProvider(table)
+      val columnValueProvider = new InMemColumnValueProvider(table)
 
-      val joinProvider = JoinTableProviderImpl()
-      val table = new InMemDataTable(pricesDef, joinProvider)
+      provider.tick("1", Map("id" -> "1", "ric" -> "VOD.L", "bid" -> 220, "ask" -> 223))
+      provider.tick("2", Map("id" -> "2", "ric" -> "BT.L", "bid" -> 500, "ask" -> 550))
+      provider.tick("3", Map("id" -> "3", "ric" -> "VOD.L", "bid" -> 240, "ask" -> 244))
+
+      val uniqueValues = columnValueProvider.getUniqueValues("ric")
+
+      uniqueValues should contain theSameElementsAs Vector("BT.L", "VOD.L")
+    }
+
+    Scenario("Get all unique value of a given column filtering out null") {
+      val table = givenTable(pricesDef)
       val provider = new MockProvider(table)
       val columnValueProvider = new InMemColumnValueProvider(table)
 
-      provider.tick("VOD.L", Map("ric" -> "VOD.L", "bid" -> 220, "ask" -> 223))
-      provider.tick("BT.L", Map("ric" -> "BT.L", "bid" -> 500, "ask" -> 550))
-      provider.tick("VOD.L", Map("ric" -> "VOD.L", "bid" -> 240, "ask" -> 244))
+      provider.tick("1", Map("id" -> "1", "ric" -> "VOD.L", "bid" -> 220, "ask" -> 223))
+      provider.tick("2", Map("id" -> "2", "ric" -> null, "bid" -> 500, "ask" -> 550))
+      provider.tick("3", Map("id" -> "3", "ric" ->  "VOD.L", "bid" -> 240, "ask" -> 244))
 
       val uniqueValues = columnValueProvider.getUniqueValues("ric")
 
-      uniqueValues shouldBe Array("BT.L", "VOD.L")
+      uniqueValues should contain theSameElementsAs Vector("VOD.L")
+
     }
 
+    Scenario("Get all unique value of a given column returns empty when all values are null") {
+      val table = givenTable(pricesDef)
+      val provider = new MockProvider(table)
+      val columnValueProvider = new InMemColumnValueProvider(table)
 
-    Scenario("Get all unique value of a given column that starts with specified string") {
+      provider.tick("1", Map("id" -> "1", "ric" -> null, "bid" -> 220, "ask" -> 223))
+      provider.tick("2", Map("id" -> "2", "ric" -> null, "bid" -> 500, "ask" -> 550))
+
+      val uniqueValues = columnValueProvider.getUniqueValues("ric")
 
-      val joinProvider = JoinTableProviderImpl()
-      val table = new InMemDataTable(pricesDef, joinProvider)
+      uniqueValues shouldBe empty
+    }
+
+    Scenario("Get all unique value of a given column that starts with specified string") {
+      val table = givenTable(pricesDef)
       val provider = new MockProvider(table)
       val columnValueProvider = new InMemColumnValueProvider(table)
 
-      provider.tick("VOA.L", Map("ric" -> "VOA.L", "bid" -> 220, "ask" -> 223))
-      provider.tick("BT.L", Map("ric" -> "BT.L", "bid" -> 500, "ask" -> 550))
-      provider.tick("VOV.L", Map("ric" -> "VOV.L", "bid" -> 240, "ask" -> 244))
+      provider.tick("1", Map("id" -> "1", "ric" -> "VOA.L", "bid" -> 220, "ask" -> 223))
+      provider.tick("2", Map("id" -> "2", "ric" -> "BT.L", "bid" -> 500, "ask" -> 550))
+      provider.tick("3", Map("id" -> "3", "ric" -> "VOV.L", "bid" -> 240, "ask" -> 244))
+      provider.tick("4", Map("id" -> "4", "ric" -> null, "bid" -> 240, "ask" -> 244))
 
       val uniqueValues = columnValueProvider.getUniqueValuesStartingWith("ric", "VO")
 
-      uniqueValues shouldBe Array("VOA.L", "VOV.L")
+      uniqueValues should contain theSameElementsAs Vector("VOA.L", "VOV.L")
     }
 
-    //todo match for start with string should not be case sensitive
   }
+
+  private def givenTable(tableDef: TableDef): InMemDataTable = new InMemDataTable(tableDef, JoinTableProviderImpl())
 }