diff --git a/src/main/scala/bad/robot/temperature/Barrier.scala b/src/main/scala/bad/robot/temperature/Barrier.scala new file mode 100644 index 0000000..bbfae20 --- /dev/null +++ b/src/main/scala/bad/robot/temperature/Barrier.scala @@ -0,0 +1,10 @@ +package bad.robot.temperature + +import java.lang.Math._ + +case class Barrier(degrees: Degrees) { + + def breached(previous: Temperature, current: Temperature) = { + abs(current.celsius - previous.celsius) > degrees + } +} diff --git a/src/main/scala/bad/robot/temperature/ErrorOnTemperatureSpike.scala b/src/main/scala/bad/robot/temperature/ErrorOnTemperatureSpike.scala index 5394884..02941fa 100644 --- a/src/main/scala/bad/robot/temperature/ErrorOnTemperatureSpike.scala +++ b/src/main/scala/bad/robot/temperature/ErrorOnTemperatureSpike.scala @@ -1,10 +1,7 @@ package bad.robot.temperature -import java.lang.Math._ - -import bad.robot.temperature.ErrorOnTemperatureSpike.DefaultSpikePercentage -import bad.robot.temperature.PercentageDifference.percentageDifference import bad.robot.logging._ +import bad.robot.temperature.ErrorOnTemperatureSpike.DefaultBarrier import scala.collection.concurrent.TrieMap import scala.util.{Failure, Success, Try} @@ -12,27 +9,27 @@ import scalaz.{-\/, \/} object ErrorOnTemperatureSpike { - private val DefaultSpikePercentage = 25 + private val DefaultBarrier = Barrier(5) /** * @param delegate delegate writer - * @return a [[TemperatureWriter]] that will produce an error (left disjunction) when a spike over n percent + * @return a [[TemperatureWriter]] that will produce an error (left disjunction) when a spike over n degrees * is detected or pass through to the delegate if the system property `avoid.spikes` is not set. * - * n is configured via a system property `avoid.spikes` and must be greater than 10 and less than 100 + * n is configured via a system property `avoid.spikes` and must be greater than 1 and less than 100 */ def apply(delegate: TemperatureWriter): TemperatureWriter = { - sys.props.get("avoid.spikes").map(spike => { - val percentage = toInt(spike).getOrElse(DefaultSpikePercentage) - Log.info(s"Temperature spikes greater than +/-$percentage% will not be recorded") - new ErrorOnTemperatureSpike(delegate, percentage) + sys.props.get("avoid.spikes").map(value => { + val barrier = toBarrier(value) + Log.info(s"Temperature spikes greater than +/-${barrier.degrees} °C will not be recorded") + new ErrorOnTemperatureSpike(delegate, barrier) }).getOrElse(delegate) } - private def toInt(string: String) = Try(string.toInt) match { - case Success(value) if value >= 10 && value <= 100 => Some(value) - case Failure(_) => None - case _ => None + private def toBarrier(string: String) = Try(string.toInt) match { + case Success(value) if value >= 1 && value <= 100 => Barrier(value) + case Failure(_) => DefaultBarrier + case _ => DefaultBarrier } } @@ -46,7 +43,7 @@ object ErrorOnTemperatureSpike { * However, as we know that for every host, at most one call will be made every 30 seconds, there is no risk on * concurrent access for a particular sensor (the key to the cache). */ -class ErrorOnTemperatureSpike(delegate: TemperatureWriter, percentageSpike: Int = DefaultSpikePercentage) extends TemperatureWriter { +class ErrorOnTemperatureSpike(delegate: TemperatureWriter, barrier: Barrier = DefaultBarrier) extends TemperatureWriter { private val temperatures: TrieMap[String, Temperature] = TrieMap() @@ -54,8 +51,8 @@ class ErrorOnTemperatureSpike(delegate: TemperatureWriter, percentageSpike: Int val spiked = measurement.temperatures.flatMap(current => { temperatures.get(current.name) match { - case Some(previous) if spikeBetween(current, previous) => Some(Spike(current.name, previous, current.temperature)) - case _ => None + case Some(previous) if breached(current, previous) => Some(Spike(current.name, previous, current.temperature)) + case _ => None } }) @@ -67,9 +64,8 @@ class ErrorOnTemperatureSpike(delegate: TemperatureWriter, percentageSpike: Int } } - private def spikeBetween(reading: SensorReading, previous: Temperature) = { - val difference = percentageDifference(previous.celsius, reading.temperature.celsius) - abs(difference) >= percentageSpike || difference.isNaN + private def breached(reading: SensorReading, previous: Temperature) = { + barrier.breached(previous, reading.temperature) } } diff --git a/src/main/scala/bad/robot/temperature/PercentageDifference.scala b/src/main/scala/bad/robot/temperature/PercentageDifference.scala index a652fd2..f51f00e 100644 --- a/src/main/scala/bad/robot/temperature/PercentageDifference.scala +++ b/src/main/scala/bad/robot/temperature/PercentageDifference.scala @@ -19,4 +19,4 @@ object PercentageDifference { case _ => BigDecimal(value).setScale(2, RoundingMode.HALF_UP).toDouble } -} +} \ No newline at end of file diff --git a/src/main/scala/bad/robot/temperature/package.scala b/src/main/scala/bad/robot/temperature/package.scala index acc7697..558e457 100644 --- a/src/main/scala/bad/robot/temperature/package.scala +++ b/src/main/scala/bad/robot/temperature/package.scala @@ -14,6 +14,8 @@ import scalaz.syntax.std.either._ package object temperature { + type Degrees = Int + // NB not implicit as this seems to clash with http4s implicits that are kicking around def jsonDecoder[A: Decoder]: EntityDecoder[IO, A] = jsonOf[IO, A] def jsonEncoder[A: Encoder]: EntityEncoder[IO, A] = jsonEncoderWithPrinterOf(spaces2PlatformSpecific) diff --git a/src/test/scala/bad/robot/temperature/BarrierTest.scala b/src/test/scala/bad/robot/temperature/BarrierTest.scala new file mode 100644 index 0000000..244a8f5 --- /dev/null +++ b/src/test/scala/bad/robot/temperature/BarrierTest.scala @@ -0,0 +1,37 @@ +package bad.robot.temperature + +import org.specs2.mutable.Specification + +import scala.Double._ + +class BarrierTest extends Specification { + + "No breach" >> { + val barrier = Barrier(5) + barrier.breached(Temperature(10.23), Temperature(11.22)) must_== false + barrier.breached(Temperature(-10.23), Temperature(-10.23)) must_== false + barrier.breached(Temperature(0), Temperature(0)) must_== false + barrier.breached(Temperature(23.12), Temperature(23.12)) must_== false + barrier.breached(Temperature(23.12), Temperature(24.11)) must_== false + } + + "Positive breach" >> { + val barrier = Barrier(1) + barrier.breached(Temperature(10.23), Temperature(11.24)) must_== true + barrier.breached(Temperature(10.24), Temperature(11.23)) must_== false + } + + "Negative breach" >> { + val barrier = Barrier(1) + barrier.breached(Temperature(-10.23), Temperature(-11.24)) must_== true + barrier.breached(Temperature(-10.24), Temperature(-11.23)) must_== false + } + + "NaN" >> { + val barrier = Barrier(1) + barrier.breached(Temperature(-10.23), Temperature(NaN)) must_== false + barrier.breached(Temperature(NaN), Temperature(10.23)) must_== false + barrier.breached(Temperature(NaN), Temperature(NaN)) must_== false + } + +} diff --git a/src/test/scala/bad/robot/temperature/ErrorOnTemperatureSpikeTest.scala b/src/test/scala/bad/robot/temperature/ErrorOnTemperatureSpikeTest.scala index f5e635e..a54f21c 100644 --- a/src/test/scala/bad/robot/temperature/ErrorOnTemperatureSpikeTest.scala +++ b/src/test/scala/bad/robot/temperature/ErrorOnTemperatureSpikeTest.scala @@ -88,6 +88,22 @@ class ErrorOnTemperatureSpikeTest extends Specification { Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(21.6)))) ) } + + "Negative spikes values (single sensor), example from production" >> { + val delegate = new StubWriter + val writer = new ErrorOnTemperatureSpike(delegate, Barrier(1)) + writer.write(Measurement(Host("example", None), Seconds(1), List(SensorReading("A", Temperature(0.5))))) + writer.write(Measurement(Host("example", None), Seconds(2), List(SensorReading("A", Temperature(0.5))))) + writer.write(Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(0.4))))) + writer.write(Measurement(Host("example", None), Seconds(4), List(SensorReading("A", Temperature(-0.7))))) must be_-\/.like(SensorError) + writer.write(Measurement(Host("example", None), Seconds(5), List(SensorReading("A", Temperature(0.4))))) + delegate.temperatures must_== List( + Measurement(Host("example", None), Seconds(1), List(SensorReading("A", Temperature(0.5)))), + Measurement(Host("example", None), Seconds(2), List(SensorReading("A", Temperature(0.5)))), + Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(0.4)))), + Measurement(Host("example", None), Seconds(5), List(SensorReading("A", Temperature(0.4)))) + ) + } "NaN (32.625 - 0.0 / 0.0 * 100 is NaN)" >> { val delegate = new StubWriter @@ -139,30 +155,24 @@ class ErrorOnTemperatureSpikeTest extends Specification { } } - "What happens with NaN" >> { + "What happens with NaN (let it through)" >> { val delegate = new StubWriter val writer = new ErrorOnTemperatureSpike(delegate) writer.write(Measurement(Host("example", None), Seconds(1), List(SensorReading("A", Temperature(21.1))))) writer.write(Measurement(Host("example", None), Seconds(2), List(SensorReading("A", Temperature(21.6))))) - writer.write(Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(NaN))))) must be_-\/.like { - case e: SensorSpikeError => e.message must_== - """An unexpected spike was encountered on: - | sensor(s) : A - | previous temperatures : 21.6 °C - | spiked temperatures : NaN °C - |""".stripMargin - } + writer.write(Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(NaN))))) must be_\/- writer.write(Measurement(Host("example", None), Seconds(4), List(SensorReading("A", Temperature(21.8))))) delegate.temperatures must containAllOf(List( Measurement(Host("example", None), Seconds(1), List(SensorReading("A", Temperature(21.1)))), Measurement(Host("example", None), Seconds(2), List(SensorReading("A", Temperature(21.6)))), +// Measurement(Host("example", None), Seconds(3), List(SensorReading("A", Temperature(NaN)))), // can't do equality check on NaN Measurement(Host("example", None), Seconds(4), List(SensorReading("A", Temperature(21.8)))) )) } "Toggle the use based on system property" >> { ErrorOnTemperatureSpike(new StubWriter()) must haveClass[StubWriter] - sys.props += ("avoid.spikes" -> "30") + sys.props += ("avoid.spikes" -> "6") ErrorOnTemperatureSpike(new StubWriter()) must haveClass[ErrorOnTemperatureSpike] }