Fixed up alert suppression to be less jumpy

12 files changed, 149 insertions, 43 deletions

diff --git a/lib/mauve/alert.rb b/lib/mauve/alert.rb
index 3810d25..ee5c1ba 100644
--- a/lib/mauve/alert.rb
+++ b/lib/mauve/alert.rb
@@ -172,15 +172,16 @@ module Mauve
       html_permitted_in = [:detail]
 
       attributes.each do |key, val|
-        next unless val.is_a?(String)
         next if html_permitted_in.include?(key)
+        next unless val.is_a?(String)
 
         attribute_set(key, Alert.remove_html(val))
       end
 
-      html_permitted_in.each do |key|
+      attributes.each do |key, val|
+        next unless html_permitted_in.include?(key)
         next unless val.is_a?(String)
-        val = attribute_get(key)
+
         attribute_set(key, Alert.clean_html(val))
       end
     end
diff --git a/lib/mauve/alert_group.rb b/lib/mauve/alert_group.rb
index df76f40..b320f18 100644
--- a/lib/mauve/alert_group.rb
+++ b/lib/mauve/alert_group.rb
@@ -133,8 +133,9 @@ module Mauve
       #
       sent_to = []
       notifications.each do |notification|
-        sent_to = notification.notify(alert, sent_to)
+        sent_to << notification.notify(alert, sent_to)
       end
+
     end
 
     #
diff --git a/lib/mauve/configuration_builders/person.rb b/lib/mauve/configuration_builders/person.rb
index 1f2af71..014987f 100644
--- a/lib/mauve/configuration_builders/person.rb
+++ b/lib/mauve/configuration_builders/person.rb
@@ -41,6 +41,7 @@ module Mauve
       def suppress_notifications_after(h)
         raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless
           h.kind_of?(Hash) && h.keys[0].kind_of?(Integer) && h.values[0].kind_of?(Integer)
+
         @result.notification_thresholds[h.values[0]] = Array.new(h.keys[0])
       end
     end
@@ -53,6 +54,10 @@ module Mauve
     def created_person(person)
       name = person.username
       raise BuildException.new("Duplicate person '#{name}'") if @result.people[name]
+      #
+      # Add a default notification threshold
+      #
+      person.notification_thresholds[60] = Array.new(10) if person.notification_thresholds.empty?
       @result.people[person.username] = person
     end
 
diff --git a/lib/mauve/notification.rb b/lib/mauve/notification.rb
index 2007b90..00ce989 100644
--- a/lib/mauve/notification.rb
+++ b/lib/mauve/notification.rb
@@ -99,6 +99,7 @@ module Mauve
     ## Return true if the alert has not been acknowledged within a certain time.
     # 
     def unacknowledged(seconds)
+      @test_time = @time if @test_time.nil?
       @alert &&
         @alert.raised? &&
         !@alert.acknowledged? &&
diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb
index 60ad60a..c26c456 100644
--- a/lib/mauve/person.rb
+++ b/lib/mauve/person.rb
@@ -8,10 +8,7 @@ module Mauve
     attr_reader :notification_thresholds, :last_pop3_login
   
     def initialize(*args)
-      #
-      # By default send 10 thresholds in a minute maximum
-      #
-      @notification_thresholds = { 60 => Array.new(10) }
+      @notification_thresholds = nil
       @suppressed = false
       #
       # TODO fix up web login so pop3 can be used as a proxy.
@@ -25,7 +22,14 @@ module Mauve
     def suppressed?
       @suppressed
     end
-    
+   
+    def notification_thresholds
+      #
+      # By default send 10 thresholds in a minute maximum
+      #
+      @notification_thresholds ||= { } 
+    end
+ 
     # This class implements an instance_eval context to execute the blocks
     # for running a notification block for each person.
     # 
@@ -86,6 +90,7 @@ module Mauve
     end 
 
     #
+    #
     # Sends the alert
     #
     def send_alert(level, alert)
@@ -93,20 +98,21 @@ module Mauve
 
       was_suppressed = self.suppressed?
 
-      @suppressed = @notification_thresholds.any? do |period, previous_alert_times|
+      @suppressed = self.notification_thresholds.any? do |period, previous_alert_times|
           #
           # Choose the second one as the first.
           #
           first = previous_alert_times[1]
-          first.is_a?(Time) and (now - first) < period
+          last  = previous_alert_times[-1]
+
+          first.is_a?(Time) and (
+           (now - first) < period or
+           (was_suppressed and (now - last) < period)
+          )
       end
 
-      if self.suppressed?
-        logger.info("Suspending further notifications to #{username} until further notice.") unless was_suppressed
         
-      else
-        logger.info "Starting to send notifications again for #{username}." if was_suppressed
-      end
+      logger.info "Starting to send notifications again for #{username}." if was_suppressed and not self.suppressed?
       
       #
       # We only suppress notifications if we were suppressed before we started,
@@ -135,12 +141,12 @@ module Mauve
         # 
         # Remember that we've sent an alert
         #
-        @notification_thresholds.each do |period, previous_alert_times|
+        self.notification_thresholds.each do |period, previous_alert_times|
           #
           # Hmm.. not sure how to make this thread-safe.
           #
-          @notification_thresholds[period].push Time.now
-          @notification_thresholds[period].shift
+          self.notification_thresholds[period].push Time.now
+          self.notification_thresholds[period].shift
         end
 
         return true
diff --git a/lib/mauve/server.rb b/lib/mauve/server.rb
index 0d56f7f..a217aab 100644
--- a/lib/mauve/server.rb
+++ b/lib/mauve/server.rb
@@ -121,11 +121,11 @@ module Mauve
       #
       # Check buffer sizes
       #
-      if self.class.notification_buffer_size > 10
+      if self.class.notification_buffer_size >= 10
         logger.warn "Notification buffer has #{self.class.notification_buffer_size} messages in it"
       end
       
-      if self.class.packet_buffer_size > 50
+      if self.class.packet_buffer_size >= 100
         logger.warn "Packet buffer has #{self.class.packet_buffer_size} updates in it"
       end
 
diff --git a/test/tc_mauve_alert.rb b/test/tc_mauve_alert.rb
index f85236e..a9aef5f 100644
--- a/test/tc_mauve_alert.rb
+++ b/test/tc_mauve_alert.rb
@@ -109,12 +109,16 @@ EOF
     )
 
     alert.raise!
+    logger_pop
+
     assert(alert.raised?)
 
     #
     # This acknowledges an alert for 3 mins.
     #
     alert.acknowledge!(person, Time.now + 3.minutes)
+    logger_pop
+
     assert_equal(person.username, alert.acknowledged_by)
     assert_equal(Time.now, alert.acknowledged_at)
     assert_equal(Time.now + 3.minutes, alert.will_unacknowledge_at)
@@ -132,6 +136,8 @@ EOF
     # The alert should unacknowledge itself.
     #
     alert.poll
+    logger_pop
+
     assert(!alert.acknowledged?)
   end
 
diff --git a/test/tc_mauve_alert_changed.rb b/test/tc_mauve_alert_changed.rb
index 3c43a2a..9cae742 100644
--- a/test/tc_mauve_alert_changed.rb
+++ b/test/tc_mauve_alert_changed.rb
@@ -64,7 +64,7 @@ EOF
         reminders     += 1
       end
 
-      AlertChanged.all.each{|ac| ac.poll}
+      AlertChanged.all.each{|ac| ac.poll; logger_pop}
     end
 
     # OK now clear the alert, send one notification and but not an alert_changed.
diff --git a/test/tc_mauve_notification.rb b/test/tc_mauve_notification.rb
index 018c31a..4243874 100644
--- a/test/tc_mauve_notification.rb
+++ b/test/tc_mauve_notification.rb
@@ -100,7 +100,7 @@ class TcMauveDuringRunner < Mauve::UnitTest
 
 
   def test_x_in_list_of_y
-    mdr = Mauve::DuringRunner.new(Time.now)
+    dr = DuringRunner.new(Time.now)
     [
       [[0,1,3,4], 2, false],
       [[0,2,4,6], 2, true],
@@ -108,14 +108,14 @@ class TcMauveDuringRunner < Mauve::UnitTest
       [[0..2, 4,5],2, true],
       [[0,1..3], 2, true],
     ].each do |y,x,result|
-      assert_equal(result, mdr.send(:x_in_list_of_y, x,y))
+      assert_equal(result, dr.send(:x_in_list_of_y, x,y))
     end
   end
 
   def test_hours_in_day
     t = Time.gm(2010,1,2,3,4,5)
     # => Sat Jan 02 03:04:05 UTC 2010
-    mdr = Mauve::DuringRunner.new(t)
+    dr = DuringRunner.new(t)
     [
       [[0,1,3,4], true],
       [[0,2,4,6], false],
@@ -126,14 +126,14 @@ class TcMauveDuringRunner < Mauve::UnitTest
       [[0,1..3], true],
       [[4..12], false]
     ].each do |hours, result|
-      assert_equal(result, mdr.send(:hours_in_day, hours))
+      assert_equal(result, dr.send(:hours_in_day, hours))
     end
   end
 
   def test_days_in_week
     t = Time.gm(2010,1,2,3,4,5)
     # => Sat Jan 02 03:04:05 UTC 2010
-    mdr = Mauve::DuringRunner.new(t)
+    dr = DuringRunner.new(t)
     [
       [[0,1,3,4], false],
       [[0,2,4,6], true],
@@ -144,11 +144,25 @@ class TcMauveDuringRunner < Mauve::UnitTest
       [[0,1..3], false],
       [[4..6], true]
     ].each do |days, result|
-      assert_equal(result, mdr.send(:days_in_week, days), "#{t.wday} in #{days.join(", ")}")
+      assert_equal(result, dr.send(:days_in_week, days), "#{t.wday} in #{days.join(", ")}")
     end
   end
 
   def test_unacknowledged
+    Server.instance.setup
+    alert = Alert.new(
+      :alert_id  => "test", 
+      :source    => "test",
+      :subject   => "test"
+    )
+    alert.raise!
+
+    Timecop.freeze(Time.now+1.hour)
+
+    dr = DuringRunner.new(Time.now, alert)
+
+    assert(!dr.send(:unacknowledged, 2.hours))
+    assert(dr.send(:unacknowledged, 1.hour))
   end
 
 end
@@ -300,4 +314,5 @@ EOF
 
   end
 
+
 end
diff --git a/test/tc_mauve_person.rb b/test/tc_mauve_person.rb
index 0026db6..c659f00 100644
--- a/test/tc_mauve_person.rb
+++ b/test/tc_mauve_person.rb
@@ -9,6 +9,8 @@ require 'pp'
 
 class TcMauvePerson < Mauve::UnitTest 
 
+  include Mauve
+
   def setup
     super
     setup_database
@@ -19,15 +21,83 @@ class TcMauvePerson < Mauve::UnitTest
     super
   end
 
-  def test_suppressed?
-
-  end
-
   def test_send_alert
-
-  end
-
-  def test_do_send_alert
+    #
+    # Allows us to pick up notifications sent.
+    #
+    $sent_notifications = []
+
+    config =<<EOF
+person ("test") {
+  all { $sent_notifications << Time.now ; true }
+  suppress_notifications_after( 6 => 60.seconds )
+}
+
+alert_group("default") {
+  level URGENT
+
+  notify("test") {
+    every 10.seconds
+  } 
+}
+EOF
+  
+    Configuration.current = ConfigurationBuilder.parse(config)
+    Server.instance.setup
+    person = Configuration.current.people["test"]
+
+    alert = Alert.new(
+      :alert_id  => "test",
+      :source    => "test",
+      :subject   => "test"
+    )
+    alert.raise!
+    assert_equal(false,    person.suppressed?, "Person suppressed before we even begin!")
+
+    start_time = Time.now
+
+    #
+    # 6 alerts every 60 seconds.
+    #
+    [ [0, true, false],
+      [5, true, false],
+      [10, true, false],
+      [15, true, false],
+      [20, true, false],
+      [25, true, true], # 6th alert -- suppress from now on
+      [30, false, true], 
+      [35, false, true],
+      [40, false, true],
+      [60, false, true], # One minute after starting -- should still be suppressed
+      [65, false, true],
+      [70, false, true],
+      [75, false, true],
+      [80, false, true],
+      [85, true, false], # One minute after the last alert was sent, start sending again.
+      [90, true, false]
+    ].each do |offset, notification_sent, suppressed|
+      # 
+      # Advance in to the future!
+      #
+      Timecop.freeze(start_time + offset)
+
+      person.send_alert(alert.level, alert)
+
+      assert_equal(suppressed,    person.suppressed?)
+
+      if notification_sent 
+        assert_equal(1, $sent_notifications.length, "Notification not sent when it should have been.")
+        #
+        # Pop the notification off the buffer.
+        #
+        last_notification_sent_at = $sent_notifications.pop
+        assert_equal(Time.now, person.notification_thresholds[60][-1], "Notification thresholds not updated")
+      else
+        assert_equal(0, $sent_notifications.length, "Notification sent when it should not have been.")
+      end
+
+      logger_pop
+    end
 
   end
 
diff --git a/test/test_mauve.rb b/test/test_mauve.rb
index fba47eb..bdce3aa 100644
--- a/test/test_mauve.rb
+++ b/test/test_mauve.rb
@@ -6,22 +6,23 @@ end
 
 require 'pp'
 require 'test/unit'
+require 'th_mauve'
 
 %w(
+tc_mauve_alert_changed.rb
+tc_mauve_alert_group.rb
+tc_mauve_alert.rb
 tc_mauve_configuration_builder.rb
 tc_mauve_configuration_builders_alert_group.rb
 tc_mauve_configuration_builders_logger.rb
 tc_mauve_configuration_builders_notification_method.rb
 tc_mauve_configuration_builders_person.rb
 tc_mauve_configuration_builders_server.rb
-tc_mauve_source_list.rb
-tc_mauve_people_list.rb
-tc_mauve_person.rb
-tc_mauve_alert.rb
 tc_mauve_history.rb
-tc_mauve_alert_group.rb
-tc_mauve_alert_changed.rb
 tc_mauve_notification.rb
+tc_mauve_people_list.rb
+tc_mauve_person.rb
+tc_mauve_source_list.rb
 tc_mauve_time.rb
 ).each do |s|
   require s
diff --git a/test/th_mauve.rb b/test/th_mauve.rb
index 76a4933..7972b9d 100644
--- a/test/th_mauve.rb
+++ b/test/th_mauve.rb
@@ -69,7 +69,7 @@ module Mauve
       @logger      = Log4r::Logger.new 'Mauve'
       @outputter   = Mauve::TestOutputter.new("test")
       @outputter.formatter = Log4r::PatternFormatter.new( :pattern => "%d %l %m" )
-      @outputter.level     = Log4r::DEBUG
+      @outputter.level     = Log4r::WARN
       @logger.outputters   << @outputter
       return @logger
     end