Fixed up suppression some more, so it works.

4 files changed, 160 insertions, 33 deletions

diff --git a/lib/mauve/configuration_builders/person.rb b/lib/mauve/configuration_builders/person.rb
index 014987f..1c012f2 100644
--- a/lib/mauve/configuration_builders/person.rb
+++ b/lib/mauve/configuration_builders/person.rb
@@ -39,10 +39,13 @@ module Mauve
       end
    
       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)
+        raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless h.kind_of?(Hash)
 
-        @result.notification_thresholds[h.values[0]] = Array.new(h.keys[0])
+        h.each do |k,v|
+          raise ArgumentError.new("notification_threshold must be specified as e.g. (10 => 1.minute)") unless k.is_a?(Integer) and v.is_a?(Integer)
+
+          @result.notification_thresholds[v] = Array.new(k)
+        end
       end
     end
   end
diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb
index a656bf3..c066a6d 100644
--- a/lib/mauve/person.rb
+++ b/lib/mauve/person.rb
@@ -5,7 +5,7 @@ require 'log4r'
 module Mauve
   class Person < Struct.new(:username, :password, :holiday_url, :urgent, :normal, :low, :email, :xmpp, :sms)
   
-    attr_reader :notification_thresholds, :last_pop3_login
+    attr_reader :notification_thresholds, :last_pop3_login, :suppressed
   
     def initialize(*args)
       @notification_thresholds = nil
@@ -18,9 +18,22 @@ module Mauve
     end
    
     def logger ; @logger ||= Log4r::Logger.new self.class.to_s ; end
+
+    def suppressed? ; @suppressed ; end
  
-    def suppressed?
-      @suppressed
+    def should_suppress?
+      now = Time.now
+
+      return self.notification_thresholds.any? do |period, previous_alert_times|
+        #
+        # Choose the second one as the first, apart from if the array is only one in length.
+        #
+        first = previous_alert_times.first
+        last  = previous_alert_times.last
+   
+        (first.is_a?(Time) and (now - first) < period) or
+          (last.is_a?(Time) and @suppressed and (now - last) < period) 
+      end
     end
    
     def notification_thresholds
@@ -96,22 +109,9 @@ module Mauve
     def send_alert(level, alert)
       now = Time.now
 
-      was_suppressed = self.suppressed?
+      was_suppressed = @suppressed
+      @suppressed    = self.should_suppress?
 
-      @suppressed = self.notification_thresholds.any? do |period, previous_alert_times|
-          #
-          # Choose the second one as the first.
-          #
-          first = previous_alert_times[1]
-          last  = previous_alert_times[-1]
-
-          first.is_a?(Time) and (
-           (now - first) < period or
-           (was_suppressed and (now - last) < period)
-          )
-      end
-
-        
       logger.info "Starting to send notifications again for #{username}." if was_suppressed and not self.suppressed?
       
       #
@@ -148,12 +148,17 @@ module Mauve
           self.notification_thresholds[period].shift
         end
 
+        #
+        # Re-run the suppression check, to see if we should be suppressed now.
+        #
+        @suppressed    = self.should_suppress?
+
         return true
       end
 
       return false
     end
-    
+   
     # 
     # Returns the subset of current alerts that are relevant to this Person.
     #
diff --git a/test/tc_mauve_alert_changed.rb b/test/tc_mauve_alert_changed.rb
index 9cae742..ba31dd9 100644
--- a/test/tc_mauve_alert_changed.rb
+++ b/test/tc_mauve_alert_changed.rb
@@ -1,11 +1,11 @@
 $:.unshift "../lib"
 
+require 'th_mauve'
 require 'mauve/alert'
 require 'mauve/alert_changed'
 require 'mauve/configuration'
 require 'mauve/configuration_builder'
 require 'mauve/configuration_builders'
-require 'th_mauve'
 
 class TcMauveAlertChanged < Mauve::UnitTest
   include Mauve
@@ -23,10 +23,6 @@ class TcMauveAlertChanged < Mauve::UnitTest
   def test_reminder
 
     config=<<EOF
-server {
-  database "sqlite::memory:"
-}
-
 person("test_person") {
   all { true }
 }
@@ -40,11 +36,11 @@ alert_group("test_group") {
 }
 EOF
 
-    Mauve::Configuration.current = Mauve::ConfigurationBuilder.parse(config)
+    Configuration.current = ConfigurationBuilder.parse(config)
 
     Server.instance.setup
 
-    alert = Mauve::Alert.new(:source => "test", :alert_id => "test_alert", :summary => "test alert")
+    alert = Alert.new(:source => "test", :alert_id => "test_alert", :summary => "test alert")
     alert.raise!
 
     reminders     = 1
@@ -84,6 +80,52 @@ EOF
 
   end
 
+  def test_only_send_one_alert_on_unacknowledge
+    config=<<EOF
+person("test_person") {
+  all { true }
+}
+
+alert_group("test_group") {
+
+  notify("test_person") {
+    every 5.minutes
+  }
+
+}
+EOF
+
+    Configuration.current = ConfigurationBuilder.parse(config)
+
+    Server.instance.setup
+
+    alert = Alert.new(:source => "test", :alert_id => "test_alert", :summary => "test alert")
+    alert.raise!
+    assert_equal(1,Server.instance.notification_buffer.length, "Wrong no of notifications sent after raise.")
+    assert_equal(1,AlertChanged.count, "Wrong no of AlertChangeds created after raise.")
+
+    alert.acknowledge!(Configuration.current.people["test_person"], Time.now + 10.minutes)
+    assert_equal(2,Server.instance.notification_buffer.length, "Wrong no of notifications sent after acknowledge.")
+    assert_equal(2,AlertChanged.count, "Wrong no of AlertChangeds created after acknowledge.")
+
+    Timecop.freeze(Time.now + 10.minutes)
+    AlertChanged.all.each{|ac| ac.poll}
+    assert_equal(2,Server.instance.notification_buffer.length, "Extra notifications sent when alertchangeds are polled.")
+  
+    #
+    # OK if we poll the alert now it should be re-raised.
+    #
+    alert.poll
+    assert(!alert.acknowledged?,"Alert not unacknowledged")
+    assert(alert.raised?,"Alert not raised following unacknowledgment")
+    assert_equal(3,Server.instance.notification_buffer.length, "No re-raise notification sent.")
+    #
+    # If we poll the AlertChangeds again, no further notification should be sent.
+    #
+    AlertChanged.all.each{|ac| ac.poll}
+    assert_equal(3,Server.instance.notification_buffer.length, "Extra notifications sent when alertchangeds are polled.")
+   
+  end
 
 end
 
diff --git a/test/tc_mauve_person.rb b/test/tc_mauve_person.rb
index c659f00..30051b0 100644
--- a/test/tc_mauve_person.rb
+++ b/test/tc_mauve_person.rb
@@ -83,17 +83,94 @@ EOF
 
       person.send_alert(alert.level, alert)
 
-      assert_equal(suppressed,    person.suppressed?)
+      assert_equal(suppressed,    person.suppressed?, "Suppressed (or not) when it should (or shouldn't) be at #{Time.now}.")
 
       if notification_sent 
-        assert_equal(1, $sent_notifications.length, "Notification not sent when it should have been.")
+        assert_equal(1, $sent_notifications.length, "Notification not sent when it should have been at #{Time.now}.")
         #
         # 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")
+        assert_equal(Time.now, person.notification_thresholds[60][-1], "Notification thresholds not updated at #{Time.now}.")
       else
-        assert_equal(0, $sent_notifications.length, "Notification sent when it should not have been.")
+        assert_equal(0, $sent_notifications.length, "Notification sent when it should not have been at #{Time.now}.")
+      end
+
+      logger_pop
+    end
+
+  end
+
+  def test_send_alert_when_only_one_blargh
+    #
+    # Allows us to pick up notifications sent.
+    #
+    $sent_notifications = []
+
+    #
+    # This configuration is a bit different.  We only want one alert per
+    # minute.
+    #
+    config =<<EOF
+person ("test") {
+  all { $sent_notifications << Time.now ; true }
+  suppress_notifications_after( 1 => 1.minute )
+}
+
+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
+
+    #
+    # 1 alerts every 60 seconds.
+    #
+    [ [0, true,   true],
+      [5, false,  true],
+      [15, false, true],
+      [30, false, true],
+      [60, true,  true], # One minute after starting -- should send an alert, but still be suppressed.
+      [90, false, true],
+      [120, true, true] # Two minutes after starting -- should send an alert, but still be suppressed.
+    ].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.should_suppress?, "Suppressed (or not) when it should (or shouldn't) be at #{Time.now}.")
+
+      if notification_sent 
+        assert_equal(1, $sent_notifications.length, "Notification not sent when it should have been at #{Time.now}.")
+        #
+        # 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 at #{Time.now}.")
+      else
+        assert_equal(0, $sent_notifications.length, "Notification sent when it should not have been at #{Time.now}.")
       end
 
       logger_pop