* Race condition fixed (fixes #1861).

 * Reminders get sent at start of during period (fixes #1821)

7 files changed, 57 insertions, 83 deletions

diff --git a/lib/mauve/alert.rb b/lib/mauve/alert.rb
index eb5f11b..c294e8c 100644
--- a/lib/mauve/alert.rb
+++ b/lib/mauve/alert.rb
@@ -392,7 +392,7 @@ module Mauve
       #
       #
       def all_raised
-        all(:raised_at.not => nil, :cleared_at => nil) - all_acknowledged
+        all(:raised_at.not => nil, :acknowledged_at => nil, :cleared_at => nil)
       end
 
       def all_acknowledged
@@ -400,7 +400,7 @@ module Mauve
       end
 
       def all_cleared
-        all(:cleared_at.not => nil) - all_acknowledged
+        all(:cleared_at.not => nil, :acknowledged_at => nil)
       end
 
       # Returns a hash of all the :urgent, :normal and :low alerts.
diff --git a/lib/mauve/alert_changed.rb b/lib/mauve/alert_changed.rb
index 83332ff..ac864a0 100644
--- a/lib/mauve/alert_changed.rb
+++ b/lib/mauve/alert_changed.rb
@@ -19,12 +19,10 @@ module Mauve
     property :remind_at, Time
     # property :updated_at, Time, :required => true
 
-    def inspect
+    def to_s
       "#<AlertChanged #{id}: alert_id #{alert_id}, for #{person}, update_type #{update_type}>"
     end
 
-    alias to_s inspect
-    
     belongs_to :alert
     
     def was_relevant=(value)
diff --git a/lib/mauve/alert_group.rb b/lib/mauve/alert_group.rb
index 114c39b..df76f40 100644
--- a/lib/mauve/alert_group.rb
+++ b/lib/mauve/alert_group.rb
@@ -95,16 +95,45 @@ module Mauve
       #
       # If there are no notifications defined. 
       #
-      if  notifications.nil?
+      if notifications.nil?
         logger.warn("No notifications found for #{self.inspect}")
         return
       end
 
       #
+      # This is where we set the reminder -- i.e. on a per-alert-group basis.
+      #
+      remind_at = notifications.inject(nil) do |reminder_time, notification|
+        this_time = notification.remind_at_next(alert)
+        if reminder_time.nil? or (!this_time.nil? and  reminder_time > this_time)
+          this_time
+        else
+          reminder_time 
+        end
+      end
+
+      #
+      # OK got the next reminder time.
+      #
+      unless remind_at.nil?
+        this_reminder = AlertChanged.new(
+          :level => level.to_s,
+          :alert_id => alert.id,
+          :person => self.name,
+          :at => Time.now,
+          :update_type => alert.update_type,
+          :remind_at => remind_at,
+          :was_relevant => true)
+
+        this_reminder.save
+      end
+
+      #
       # The notifications are specified in the config file.
       #
+      sent_to = []
       notifications.each do |notification|
-        notification.notify(alert)
+        sent_to = notification.notify(alert, sent_to)
       end
     end
 
diff --git a/lib/mauve/notification.rb b/lib/mauve/notification.rb
index c57d2a9..43de1a7 100644
--- a/lib/mauve/notification.rb
+++ b/lib/mauve/notification.rb
@@ -145,7 +145,6 @@ module Mauve
     attr_reader :thread_list
 
     def initialize(people, level)
-
       self.level = level
       self.every = 300
       self.people = people
@@ -153,7 +152,7 @@ module Mauve
 
     def logger ;  Log4r::Logger.new self.class.to_s ; end
 
-    def notify(alert)
+    def notify(alert, already_sent_to = [])
 
       if people.nil? or people.empty?
         logger.warn "No people found in for notification #{list}"
@@ -161,9 +160,7 @@ module Mauve
       end
 
       # Should we notify at all?
-      is_relevant = DuringRunner.new(Time.now, alert, &during).now?
-
-      n_sent = 0
+      return already_sent_to unless DuringRunner.new(Time.now, alert, &during).now?
 
       people.collect do |person|
         case person
@@ -176,16 +173,29 @@ module Mauve
             []
         end
       end.flatten.uniq.each do |person|
-        n_sent += 1 if person.send_alert(self.level, alert, is_relevant, remind_at_next(alert))
+        #
+        # A bit of alert de-bouncing.
+        #
+        if already_sent_to.include?(person.username)
+          logger.info("Already sent notification of #{alert} to #{person.username}")
+        else
+          Server.notification_push([person, level, alert])
+          already_sent_to << person.username
+        end
       end
 
-      return n_sent
+      return already_sent_to
     end
     
     def remind_at_next(alert)
-      return DuringRunner.new(Time.now, alert, &during).find_next(every) if alert.raised?
+      return nil unless alert.raised?
+
+      if DuringRunner.new(Time.now, alert, &during).now?
+        return DuringRunner.new(Time.now, alert, &during).find_next(every)
+      else
+        return DuringRunner.new(Time.now, alert, &during).find_next(0)
+      end
 
-      return nil
     end
 
   end
diff --git a/lib/mauve/notifier.rb b/lib/mauve/notifier.rb
index 4737c37..2853485 100644
--- a/lib/mauve/notifier.rb
+++ b/lib/mauve/notifier.rb
@@ -25,7 +25,7 @@ module Mauve
         #
         next if person.nil?
 
-        person.do_send_alert(*args) 
+        person.send_alert(*args) 
       end
     end
 
diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb
index 26425da..60ad60a 100644
--- a/lib/mauve/person.rb
+++ b/lib/mauve/person.rb
@@ -86,72 +86,9 @@ module Mauve
     end 
 
     #
-    # Sends the alert, and updates when the AlertChanged database to set the next reminder.
+    # Sends the alert
     #
-    def send_alert(level, alert, is_relevant=true, remind_at=nil)
-      #
-      # First check that we've not just sent an notification to this person for
-      # this alert
-      #
-      last_reminder = AlertChanged.first(:alert => alert, :person => username, :update_type => alert.update_type, :at.gte => (Time.now - 1.minute) )
-
-      if last_reminder and last_reminder.at >= (Time.now - 1.minute)
-        #
-        #
-        logger.info("Not sending notification to #{username} for #{alert} because one has just been sent.")
-        return false
-      end
-
-      #
-      # For the love of God, do not remind about ack'd or cleared alerts.
-      #
-      if alert.acknowledged? or alert.cleared?
-        remind_at = nil
-      end
-
-      this_reminder = AlertChanged.new(
-        :level => level.to_s,
-        :alert_id => alert.id, 
-        :person => username, 
-        :at => Time.now,
-        :update_type => alert.update_type,
-        :remind_at => remind_at,
-        :was_relevant => is_relevant)
-
-      #
-      # Check to make sure that we've not got a sooner reminder set.
-      #
-      unless remind_at.nil?
-        next_reminder = AlertChanged.first(:alert => alert, :remind_at.gt => Time.now, :person => username, :update_type => alert.update_type)
-
-        if next_reminder
-          #
-          # If the reminder is further in the future than the one we're about
-          # to put on, then just update it.
-          #
-          # Otherwise if it is sooner, we don't need to create a new one.
-          #
-          if next_reminder.remind_at > remind_at
-            next_reminder.remind_at = remind_at
-            logger.info("Not inserting a new reminder, as there is already one in place sooner")
-            this_reminder = next_reminder
-          else
-            this_reminder = nil
-          end
-        end
-      end
-
-      this_reminder.save unless this_reminder.nil?
-
-      if is_relevant
-        Server.notification_push([self, level, alert])
-        return true
-      end
-
-      return false
-    end
-   
-    def do_send_alert(level, alert)
+    def send_alert(level, alert)
       now = Time.now
 
       was_suppressed = self.suppressed?
diff --git a/lib/mauve/version.rb b/lib/mauve/version.rb
index 1c9b8d7..7e94be0 100644
--- a/lib/mauve/version.rb
+++ b/lib/mauve/version.rb
@@ -1,5 +1,5 @@
 module Mauve
 
-  VERSION="3.4.2"
+  VERSION="3.4.3"
 
 end