Skip to content

Commit c5001ff

Browse files
authored
[Bug] Indexed filters do not take into account previously applied row permission filters. (#1902)
* #1901 Fix (WiP) * #1901 Fix (WiP) * #1901 Fix (WiP)
1 parent ea42442 commit c5001ff

File tree

5 files changed

+93
-30
lines changed

5 files changed

+93
-30
lines changed

plugin/virtualized-table-plugin/src/main/scala/org/finos/vuu/plugin/virtualized/table/VirtualizedTableKeys.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,7 @@ case class VirtualizedTableKeys(window: MovingWindow[String], dataSize: Int) ext
3636
}
3737

3838
override def iterator: Iterator[String] = window.iterator
39+
40+
override def intersect(keys: Iterable[String]): TablePrimaryKeys = ???
41+
3942
}

vuu/src/main/scala/org/finos/vuu/core/filter/FilterClause.scala

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package org.finos.vuu.core.filter
22

33
import org.finos.toolbox.collection.array.ImmutableArray
44
import org.finos.vuu.core.filter.FilterClause.joinResults
5-
import org.finos.vuu.core.index._
5+
import org.finos.vuu.core.index.*
66
import org.finos.vuu.core.table.column.{Error, Result}
77
import org.finos.vuu.core.table.{RowData, TablePrimaryKeys}
88
import org.finos.vuu.feature.inmem.InMemTablePrimaryKeys
@@ -32,9 +32,11 @@ sealed trait RowFilterClause extends FilterClause {
3232
))
3333

3434
override def validate(vpColumns: ViewPortColumns): Result[true] = columnExistsInVpColumns(vpColumns)
35+
3536
private def columnExistsInVpColumns(vpColumns: ViewPortColumns): Result[true] =
3637
if (vpColumns.columnExists(this.columnName)) Result(true)
3738
else Error(s"Column `$columnName` not found.")
39+
3840
}
3941

4042
case class NotClause(decorated: FilterClause) extends FilterClause {
@@ -92,12 +94,12 @@ case class InClause(columnName: String, values: List[String]) extends RowFilterC
9294
override def filterAll(rows: RowSource, rowKeys: TablePrimaryKeys, viewPortColumns: ViewPortColumns): TablePrimaryKeys = {
9395
val column = rows.asTable.columnForName(columnName)
9496
rows.asTable.indexForColumn(column) match {
95-
case Some(ix: StringIndexedField) => InMemTablePrimaryKeys( ix.find(values) )
96-
case Some(ix: IntIndexedField) => InMemTablePrimaryKeys( ix.find(values.map(s => s.toInt)))
97-
case Some(ix: LongIndexedField) => InMemTablePrimaryKeys( ix.find(values.map(s => s.toLong)))
98-
case Some(ix: DoubleIndexedField) => InMemTablePrimaryKeys( ix.find(values.map(s => s.toDouble)))
99-
case Some(ix: BooleanIndexedField) => InMemTablePrimaryKeys( ix.find(values.map(s => s.toBoolean)))
100-
case None => super.filterAll(rows, rowKeys, viewPortColumns)
97+
case Some(ix: StringIndexedField) => rowKeys.intersect(ix.find(values))
98+
case Some(ix: IntIndexedField) => rowKeys.intersect(ix.find(values.map(s => s.toInt)))
99+
case Some(ix: LongIndexedField) => rowKeys.intersect(ix.find(values.map(s => s.toLong)))
100+
case Some(ix: DoubleIndexedField) => rowKeys.intersect(ix.find(values.map(s => s.toDouble)))
101+
case Some(ix: BooleanIndexedField) => rowKeys.intersect(ix.find(values.map(s => s.toBoolean)))
102+
case _ => super.filterAll(rows, rowKeys, viewPortColumns)
101103
}
102104
}
103105
}
@@ -113,10 +115,10 @@ case class GreaterThanClause(columnName: String, value: Double) extends RowFilte
113115
override def filterAll(rows: RowSource, rowKeys: TablePrimaryKeys, viewPortColumns: ViewPortColumns): TablePrimaryKeys = {
114116
val column = rows.asTable.columnForName(columnName)
115117
rows.asTable.indexForColumn(column) match {
116-
case Some(ix: DoubleIndexedField) => InMemTablePrimaryKeys(ix.greaterThan(value))
117-
case Some(ix: IntIndexedField) => InMemTablePrimaryKeys(ix.greaterThan(value.toInt))
118-
case Some(ix: LongIndexedField) => InMemTablePrimaryKeys(ix.greaterThan(value.toLong))
119-
case None => super.filterAll(rows, rowKeys, viewPortColumns)
118+
case Some(ix: DoubleIndexedField) => rowKeys.intersect(ix.greaterThan(value))
119+
case Some(ix: IntIndexedField) => rowKeys.intersect(ix.greaterThan(value.toInt))
120+
case Some(ix: LongIndexedField) => rowKeys.intersect(ix.greaterThan(value.toLong))
121+
case _ => super.filterAll(rows, rowKeys, viewPortColumns)
120122
}
121123
}
122124
}
@@ -132,10 +134,10 @@ case class LessThanClause(columnName: String, value: Double) extends RowFilterCl
132134
override def filterAll(rows: RowSource, rowKeys: TablePrimaryKeys, viewPortColumns: ViewPortColumns): TablePrimaryKeys = {
133135
val column = rows.asTable.columnForName(columnName)
134136
rows.asTable.indexForColumn(column) match {
135-
case Some(ix: DoubleIndexedField) => InMemTablePrimaryKeys(ix.lessThan(value))
136-
case Some(ix: IntIndexedField) => InMemTablePrimaryKeys(ix.lessThan(value.toInt))
137-
case Some(ix: LongIndexedField) => InMemTablePrimaryKeys(ix.lessThan(value.toInt))
138-
case None => super.filterAll(rows, rowKeys, viewPortColumns)
137+
case Some(ix: DoubleIndexedField) => rowKeys.intersect(ix.lessThan(value))
138+
case Some(ix: IntIndexedField) => rowKeys.intersect(ix.lessThan(value.toInt))
139+
case Some(ix: LongIndexedField) => rowKeys.intersect(ix.lessThan(value.toInt))
140+
case _ => super.filterAll(rows, rowKeys, viewPortColumns)
139141
}
140142
}
141143
}
@@ -156,12 +158,14 @@ case class EqualsClause(columnName: String, value: String) extends RowFilterClau
156158
override def filterAll(rows: RowSource, rowKeys: TablePrimaryKeys, viewPortColumns: ViewPortColumns): TablePrimaryKeys = {
157159
val column = rows.asTable.columnForName(columnName)
158160
rows.asTable.indexForColumn(column) match {
159-
case Some(ix: StringIndexedField) => InMemTablePrimaryKeys(ix.find(value))
160-
case Some(ix: IntIndexedField) => InMemTablePrimaryKeys(ix.find(value.toInt))
161-
case Some(ix: LongIndexedField) => InMemTablePrimaryKeys(ix.find(value.toLong))
162-
case Some(ix: DoubleIndexedField) => InMemTablePrimaryKeys(ix.find(value.toDouble))
163-
case Some(ix: BooleanIndexedField) => InMemTablePrimaryKeys(ix.find(value.toBoolean))
164-
case None => super.filterAll(rows, rowKeys, viewPortColumns)
161+
case Some(ix: StringIndexedField) => rowKeys.intersect(ix.find(value))
162+
case Some(ix: IntIndexedField) => rowKeys.intersect(ix.find(value.toInt))
163+
case Some(ix: LongIndexedField) => rowKeys.intersect(ix.find(value.toLong))
164+
case Some(ix: DoubleIndexedField) => rowKeys.intersect(ix.find(value.toDouble))
165+
case Some(ix: BooleanIndexedField) => rowKeys.intersect(ix.find(value.toBoolean))
166+
case _ => super.filterAll(rows, rowKeys, viewPortColumns)
165167
}
166168
}
167169
}
170+
171+

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,26 @@ object EmptyTablePrimaryKeys extends TablePrimaryKeys{
77
override def length: Int = 0
88
override def add(key: String): TablePrimaryKeys = this.+(key)
99
override def +(key: String): TablePrimaryKeys = InMemTablePrimaryKeys(ImmutableArray.from(Array(key)))
10-
override def remove(key: String): TablePrimaryKeys = EmptyTablePrimaryKeys
10+
override def remove(key: String): TablePrimaryKeys = EmptyTablePrimaryKeys
1111
override def -(key: String): TablePrimaryKeys = EmptyTablePrimaryKeys
1212
override def sliceTableKeys(from: Int, until: Int): TablePrimaryKeys = EmptyTablePrimaryKeys
1313
override def iterator: Iterator[String] = Array[String]().iterator
1414

1515
override def get(index: Int): String = null
1616
override def set(index: Int, key: String): TablePrimaryKeys = this
17+
override def intersect(keys: Iterable[String]): TablePrimaryKeys = EmptyTablePrimaryKeys
1718
}
1819

1920
trait TablePrimaryKeys extends Iterable[String] {
2021
def length: Int
2122
def add(key: String): TablePrimaryKeys
2223
def +(key: String): TablePrimaryKeys
23-
def remove(key: String): TablePrimaryKeys
24+
def remove(key: String): TablePrimaryKeys
2425
def -(key: String): TablePrimaryKeys
2526
def sliceTableKeys(from: Int, until: Int): TablePrimaryKeys
2627
def get(index: Int): String
2728
def set(index: Int, key: String): TablePrimaryKeys
29+
def intersect(keys: Iterable[String]): TablePrimaryKeys
2830
}
2931

3032

vuu/src/main/scala/org/finos/vuu/feature/inmem/InMemTablePrimaryKeys.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,9 @@ case class InMemTablePrimaryKeys(keys: ImmutableArray[String]) extends TablePrim
1818
override def -(key: String): TablePrimaryKeys = InMemTablePrimaryKeys(keys - key)
1919
override def get(index: Int): String = keys.getIndex(index)
2020
override def set(index: Int, key: String): TablePrimaryKeys = InMemTablePrimaryKeys(keys.set(index, key))
21+
22+
override def intersect(otherKeys: Iterable[String]): TablePrimaryKeys = {
23+
val intersection = keys.filter(otherKeys.toSet.contains).toArray
24+
InMemTablePrimaryKeys(ImmutableArray.from(intersection))
25+
}
2126
}

vuu/src/test/scala/org/finos/vuu/viewport/FilterAndSortTest.scala

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,23 @@ package org.finos.vuu.viewport
33
import org.finos.toolbox.jmx.{MetricsProvider, MetricsProviderImpl}
44
import org.finos.toolbox.lifecycle.LifecycleContainer
55
import org.finos.toolbox.time.{Clock, DefaultClock}
6+
import org.finos.vuu.api.{Index, Indices, TableDef}
67
import org.finos.vuu.client.messages.RequestId
8+
import org.finos.vuu.core.auths.VuuUser
79
import org.finos.vuu.core.filter.{EqualsClause, LessThanClause, NoFilter}
810
import org.finos.vuu.core.sort.{AlphaSort, AntlrBasedFilter, SortDirection, UserDefinedFilterAndSort}
9-
import org.finos.vuu.core.table.ViewPortColumnCreator
10-
import org.finos.vuu.net.ClientSessionId
11-
import org.finos.vuu.provider.MockProvider
11+
import org.finos.vuu.core.table.{Columns, RowData, TableContainer, ViewPortColumnCreator}
12+
import org.finos.vuu.feature.inmem.VuuInMemPlugin
13+
import org.finos.vuu.net.{ClientSessionId, FilterSpec}
14+
import org.finos.vuu.plugin.DefaultPluginRegistry
15+
import org.finos.vuu.provider.{JoinTableProviderImpl, MockProvider, ProviderContainer}
1216
import org.finos.vuu.util.OutboundRowPublishQueue
1317
import org.finos.vuu.util.table.TableAsserts
18+
import org.finos.vuu.util.table.TableAsserts.assertVpEq
1419
import org.scalatest.GivenWhenThen
1520
import org.scalatest.featurespec.AnyFeatureSpec
1621
import org.scalatest.matchers.should.Matchers
1722
import org.scalatest.prop.Tables.Table
18-
import org.finos.vuu.core.auths.VuuUser
1923

2024
import java.time.{LocalDateTime, ZoneId}
2125

@@ -242,9 +246,8 @@ class FilterAndSortTest extends AnyFeatureSpec with Matchers with ViewPortSetup
242246
val updates3 = combineQs(viewport).filter( vp => vp.vpUpdate == RowUpdateType)
243247

244248
updates3.size should be (1)
245-
updates3(0).vp.size should equal(1)
246-
//make sure we clean up the mappings
247-
updates3(0).vp.getRowKeyMappingSize_ForTest should equal(1)
249+
updates3.head.vp.size should equal(1)
250+
updates3.head.vp.getRowKeyMappingSize_ForTest should equal(1)
248251

249252
assertVpEq(updates3){
250253
Table(
@@ -474,6 +477,52 @@ class FilterAndSortTest extends AnyFeatureSpec with Matchers with ViewPortSetup
474477

475478
}
476479

480+
Scenario("test index hit does not override row permission filter") {
481+
482+
given lifecycle: LifecycleContainer = new LifecycleContainer
483+
484+
val pricesDef = TableDef(name = "prices",
485+
keyField = "ric",
486+
columns = Columns.fromNames("ric:String", "bid:Double", "ask:Double"),
487+
indices = Indices.apply(Index.apply("ric")),
488+
joinFields = "ric")
489+
.withPermissions((vp, tc) => (row: RowData) => {
490+
val ric = row.get("ric").asInstanceOf[String]
491+
ric != "VOD.L"
492+
})
493+
494+
val joinProvider = JoinTableProviderImpl()
495+
val tableContainer = new TableContainer(joinProvider)
496+
val prices = tableContainer.createTable(pricesDef)
497+
val pricesProvider = new MockProvider(prices)
498+
val providerContainer = new ProviderContainer(joinProvider)
499+
val pluginRegistry = new DefaultPluginRegistry()
500+
pluginRegistry.registerPlugin(new VuuInMemPlugin)
501+
val viewPortContainer = new ViewPortContainer(tableContainer, providerContainer, pluginRegistry)
502+
503+
pricesProvider.tick("VOD.L", Map("ric" -> "VOD.L", "bid" -> 220.0, "ask" -> 222.0))
504+
pricesProvider.tick("BT.L", Map("ric" -> "BT.L", "bid" -> 500.0, "ask" -> 501.0))
505+
506+
val queue = new OutboundRowPublishQueue()
507+
508+
val columns = ViewPortColumnCreator.create(prices, prices.getTableDef.columns.map(_.name).toList)
509+
510+
val viewport = viewPortContainer.create(RequestId.oneNew(), VuuUser("B"),
511+
ClientSessionId("A", "C"), queue, prices, ViewPortRange(0, 20), columns,
512+
filterSpec = FilterSpec("ric = \"VOD.L\" or ric = \"BT.L\""))
513+
514+
viewPortContainer.runOnce()
515+
516+
val updates = combineQs(viewport)
517+
518+
assertVpEq(updates) {
519+
Table(
520+
("ric", "bid", "ask"),
521+
("BT.L", 500.0, 501.0)
522+
)
523+
}
524+
525+
}
477526

478527
}
479528

0 commit comments

Comments
 (0)