Change lookup of spike filter settings to use method signature (#110)

* Change lookup of spike filter settings to use method signature

* Update method signature lookup to avoid db query

Co-authored-by: tomas <tomas.mccandless@gmail.com>

* WIP spike filter setting refactor

* wip spike filter refactoring

* spike filter refactoring

* minor cleanup

---------

Co-authored-by: Jonathan Mao <jonathan.mao@workday.com>
Co-authored-by: tomas <tomas.mccandless@gmail.com>
Co-authored-by: tomas mccandless <tomas.mccandless@workday.com>

warp-core/src/main/scala/com/workday/warp/arbiters/ArbiterLike.scala

@@ -1,11 +1,12 @@
 package com.workday.warp.arbiters
 
 import com.workday.warp.TestId
-import com.workday.warp.config.CoreWarpProperty.{WARP_ARBITER_SPIKE_FILTER_ENABLED, WARP_ARBITER_SPIKE_FILTER_ALERT_ON_NTH}
+import com.workday.warp.config.CoreWarpProperty.{WARP_ARBITER_SPIKE_FILTER_ALERT_ON_NTH, WARP_ARBITER_SPIKE_FILTER_ENABLED}
 import com.workday.warp.persistence.PersistenceAware
 import com.workday.warp.persistence.TablesLike._
 import com.workday.warp.persistence.Tables._
 
+
 /**
   * Represents a requirement imposed on a measured test.
   *
@@ -79,25 +80,41 @@ trait ArbiterLike extends PersistenceAware with CanReadHistory {
     * @param testExecution [[TestExecutionRowLikeType]] we are voting on.
     * @return a wrapped error with a useful message, or None if the measured test passed its requirement.
     */
-  final def voteWithSpikeFilter[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): Option[Throwable] = {
-    val (spikeFilterEnabled, alertOnNth) = spikeFilterSettings(testExecution.idTestDefinition)
+  def voteWithSpikeFilter[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): Option[Throwable] = {
+    // TODO maybe could be made nicer with just an idTestExecution, we have to hit the db anyway
+    val (spikeFilterEnabled, alertOnNth) = spikeFilterSettings(ballot, testExecution)
     voteWithSpikeFilter(ballot, testExecution, spikeFilterEnabled, alertOnNth)
   }
 
 
+  /**
+    * Reads spike filter settings, possibly from sources other than the database.
+    * Should be overridden in implementing classes.
+    *
+    * @param ballot
+    * @param testExecution
+    * @tparam T
+    * @return
+    */
+  def readSpikeFilterSettings[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): Option[(Boolean, Int)] = {
+    this.persistenceUtils.getSpikeFilterSettings(ballot.testId.id)
+      .map(settings => (settings.spikeFilterEnabled, settings.alertOnNth))
+  }
+
   /**
     * Whether spike filtering is enabled (notification settings).
     *
+    * Can be overridden by WarpProperties.
+    *
     * Order of precedence:
     * - WarpProperties
     * - DB
     * - defaults to off
     *
     * @return spike filtering settings.
     */
-  def spikeFilterSettings(idTestDefinition: Int): (Boolean, Int) = {
-    var settings: (Boolean, Int) = this.persistenceUtils.getSpikeFilterSettings(idTestDefinition)
-      .map(setting => (setting.spikeFilterEnabled, setting.alertOnNth))
+  def spikeFilterSettings[T: TestExecutionRowLikeType](ballot: Ballot, testExecution: T): (Boolean, Int) = {
+    var settings: (Boolean, Int) = this.readSpikeFilterSettings(ballot, testExecution)
       .getOrElse((false, 1))
     logger.trace(s"base spike filter settings: $settings")
     // allow individual overrides from properties if they are present

warp-core/src/main/scala/com/workday/warp/persistence/AbstractQueries.scala

@@ -357,10 +357,10 @@ trait AbstractQueries {
   /**
     * Creates a [[DBIO]] for reading spike filter settings.
     *
-    * @param idTestDefinition test definition to read spike filter settings for.
+    * @param methodSignature method signature to read spike filter settings for.
     * @return a [[DBIO]] (not yet executed) for reading spike filter settings for the given test execution.
     */
-  def getSpikeFilterSettingsQuery(idTestDefinition: Int): DBIO[Option[SpikeFilterSettingsRowLike]]
+  def getSpikeFilterSettingsQuery(methodSignature: String): DBIO[Option[SpikeFilterSettingsRowLike]]
 
 
   /**

warp-core/src/main/scala/com/workday/warp/persistence/CorePersistenceAware.scala

@@ -399,11 +399,11 @@ trait CorePersistenceAware extends PersistenceAware with WarpLogging {
     /**
       * Reads spike filter settings for the given test definition.
       *
-      * @param idTEstDefinition test definition to look up spike filter settings for.
+      * @param methodSignature method signature to look up spike filter settings for.
       * @return spike filter settings for the given test execution.
       */
-    override def getSpikeFilterSettings(idTestDefinition: Int): Option[SpikeFilterSettingsRowLike] =
-      this.synchronously(getSpikeFilterSettingsQuery(idTestDefinition))
+    override def getSpikeFilterSettings(methodSignature: String): Option[SpikeFilterSettingsRowLike] =
+      this.synchronously(getSpikeFilterSettingsQuery(methodSignature))
 
 
     /**

warp-core/src/main/scala/com/workday/warp/persistence/CoreQueries.scala

@@ -585,15 +585,17 @@ trait CoreQueries extends AbstractQueries {
   /**
     * Creates a [[DBIO]] for reading spike filter settings.
     *
-    * @param idTestDefinition test definition to read spike filter settings for.
+    * @param methodSignature method signature to read spike filter settings for.
     * @return a [[DBIO]] (not yet executed) for reading spike filter settings for the given test execution.
     */
-  override def getSpikeFilterSettingsQuery(idTestDefinition: Int):
+  override def getSpikeFilterSettingsQuery(methodSignature: String):
   DBIO[Option[SpikeFilterSettingsRowLike]] = {
-    SpikeFilterSettings
-      .filter(_.idTestDefinition === idTestDefinition)
-      .result
-      .headOption
+    val query: Query[SpikeFilterSettings, SpikeFilterSettingsRow, Seq] = for {
+      testDefinition <- TestDefinition if testDefinition.methodSignature === methodSignature
+      spikeFilterSetting <- SpikeFilterSettings if spikeFilterSetting.idTestDefinition === testDefinition.idTestDefinition
+    } yield spikeFilterSetting
+
+    query.result.map(_.headOption)
   }
 
 

warp-core/src/main/scala/com/workday/warp/persistence/PersistenceAware.scala

@@ -399,10 +399,10 @@ trait PersistenceAware extends WarpLogging {
     /**
       * Reads spike filter settings for the given test execution.
       *
-      * @param idTestDefinition test definition to look up spike filter settings for.
+      * @param methodSignature method signature to look up spike filter settings for.
       * @return spike filter settings for the given test execution.
       */
-    def getSpikeFilterSettings(idTestDefinition: Int): Option[SpikeFilterSettingsRowLike]
+    def getSpikeFilterSettings(methodSignature: String): Option[SpikeFilterSettingsRowLike]
 
 
     /**

warp-core/src/test/scala/com/workday/warp/arbiters/ArbiterLikeSpec.scala

@@ -5,6 +5,7 @@ import com.workday.warp.junit.{UnitTest, WarpJUnitSpec}
 import com.workday.warp.persistence.Tables._
 import com.workday.warp.persistence.Tables.RowTypeClasses._
 import com.workday.warp.persistence.TablesLike._
+import com.workday.warp.persistence.TablesLike.RowTypeClasses._
 
 import java.time.Instant
 import java.util.UUID
@@ -13,14 +14,15 @@ class ArbiterLikeSpec extends WarpJUnitSpec with ArbiterLike {
 
   @UnitTest
   def readSpikeFilterSettings(): Unit = {
-    val methodSignature = s"com.workday.warp.arbiters.${UUID.randomUUID.toString}"
-    val testId = TestId.fromString(methodSignature)
+    val methodSignature: String = s"com.workday.warp.arbiters.${UUID.randomUUID.toString}"
+    val testId: TestId = TestId.fromString(methodSignature)
+    val ballot: Ballot = new Ballot(testId)
     val testExec: TestExecutionRowLike = this.persistenceUtils.createTestExecution(testId, Instant.now(), 5.0, 6.0)
 
-    val settingsRow = SpikeFilterSettingsRow(testExec.idTestDefinition, false, 10, 10)
+    val settingsRow: SpikeFilterSettingsRow = SpikeFilterSettingsRow(testExec.idTestDefinition, false, 10, 10)
     this.persistenceUtils.writeSpikeFilterSettings(Seq(settingsRow))
 
-    this.spikeFilterSettings(testExec.idTestDefinition) should be (settingsRow.spikeFilterEnabled, settingsRow.alertOnNth)
+    this.spikeFilterSettings(ballot, testExec) should be (settingsRow.spikeFilterEnabled, settingsRow.alertOnNth)
   }
 
   /**