* Reminder notifications now take the same path to notify as initial alerts

* Threading tidied -- processor will not do anything unless the timer has frozen
* Person#send_alert now tidied and merged with alert_changed
* POP3 server only shows alerts relevant to the user
* Server now defaults to using an in-memory SQLite database (good for testing)
* Server initializes a blank mauve config.
* Tests tidied up

10 files changed, 130 insertions, 219 deletions

diff --git a/lib/mauve/alert.rb b/lib/mauve/alert.rb
index 07421d5..28dfcf1 100644
--- a/lib/mauve/alert.rb
+++ b/lib/mauve/alert.rb
@@ -235,7 +235,11 @@ module Mauve
     public
     
     def notify
-      self.alert_group.notify(self)
+      if self.alert_group.nil?
+        logger.warn "Could not notify for #{self} since there are no matching alert groups"
+      else
+        self.alert_group.notify(self)
+      end
     end
 
     def acknowledge!(person, ack_until = Time.now+3600)
@@ -531,7 +535,8 @@ module Mauve
             alert_db.clear!
           end
         end
-       
+      
+        return nil 
       end
 
       def logger
diff --git a/lib/mauve/alert_changed.rb b/lib/mauve/alert_changed.rb
index 9840b84..9b74396 100644
--- a/lib/mauve/alert_changed.rb
+++ b/lib/mauve/alert_changed.rb
@@ -19,7 +19,6 @@ module Mauve
     property :remind_at, Time
     # property :updated_at, Time, :required => true
 
-    
     def inspect
       "#<AlertChanged #{id}: alert_id #{alert_id}, for #{person}, update_type #{update_type}>"
     end
@@ -44,35 +43,6 @@ module Mauve
      Log4r::Logger.new self.class.to_s
     end
 
-    ## Checks to see if a raise was send to the person.
-    #
-    # @TODO: Recurence is broken in ruby, change this so that it does not 
-    #        use it.
-    #
-    # @author Matthew Bloch
-    # @return [Boolean] true if it was relevant, false otherwise.
-    def was_relevant_when_raised?
-
-      if "acknowledged" == update_type and true == was_relevant
-        return true 
-      end
-
-      return was_relevant if update_type == "raised"
-
-      previous = AlertChanged.first(:id.lt => id, 
-                                    :alert_id => alert_id,
-                                    :person => person)
-      if previous
-        previous.was_relevant_when_raised?
-      else
-        # a bug, but hardly inconceivable :)
-        logger.info("Could not see that #{alert} was raised with #{person} "+
-                     "but further updates exist (e.g. #{self}) "+
-                     "- you may see spurious notifications as a result")
-        true
-      end
-    end
-    
     # Sends a reminder about this alert state change, or forget about it if
     # the alert has been acknowledged
     #
@@ -82,69 +52,31 @@ module Mauve
         destroy!
         return false
       end
-
-
       
-      alert_group = AlertGroup.matches(alert)[0]
-      
-      if !alert_group || alert.acknowledged?
-        logger.info((alert_group ? 
-          "Alert already acknowledged" : 
-          "No alert group matches any more"
-          ) + " => no reminder due for #{self.alert.inspect}"
-        )
+      if !alert_group 
+        logger.info("No alert group matches any more.  Clearing reminder for #{self.alert}.")
         self.remind_at = nil
-        save
-      else
-        logger.info "Sending a reminder for #{self.alert.inspect}"
-
-        saved = false
-        unless alert_group.notifications.nil?
+        return save
+      end
 
-          alert_group.notifications.each do |notification|
 
-            #
-            # Build an array of people that could/should be notified.
-            #
-            notification_people = []
+      if alert.acknowledged?
+        logger.info("Alert already acknowledged. Clearing reminder due for #{self.alert}.")
+        self.remind_at = nil
+        return save
+      end
 
-            notification.people.each do |np|
-              case np
-                when Person
-                  notification_people << np.username
-                when PeopleList
-                  notification_people += np.list
-              end
-            end
+      alert_group.notify(alert)
+      #
+      # Need to make sure this reminder is cleared.
+      #
+      self.remind_at = nil
 
-            #
-            # For each person, send a notification
-            #
-            notification_people.sort.uniq.each do |np|
-              if np == self.person
-                #
-                # Only remind if the time is right. 
-                #
-                if DuringRunner.new(Time.now, alert, &notification.during).now?
-                  Configuration.current.people[np].send_alert(level, alert)
-                end
-                self.remind_at = notification.remind_at_next(alert)
-                save
-                saved = true
-              end
-            end
-          end
-        end
-        
-        if !saved
-          logger.warn("#{self.inspect} did not match any people, maybe configuration has changed but I'm going to delete this and not try to remind anyone again")
-          destroy!
-        end
-      end
+      save
     end
     
     def due_at # mimic interface from Alert
-      remind_at ? remind_at.to_time : nil
+      remind_at ? remind_at : nil
     end
     
     def poll # mimic interface from Alert
diff --git a/lib/mauve/alert_group.rb b/lib/mauve/alert_group.rb
index 6cff33b..114c39b 100644
--- a/lib/mauve/alert_group.rb
+++ b/lib/mauve/alert_group.rb
@@ -26,38 +26,13 @@ module Mauve
         grps
       end
 
-      # If there is any significant change to a set of alerts, the Alert
-      # class sends the list here so that appropriate action can be taken
-      # for each one.  We scan the list of alert groups to find out which
-      # alerts match which groups, then send a notification to each group
-      # object in turn.
-      #
-      def notify(alerts)
-        alerts.each do |alert|
-          groups = matches(alert)
-          
-          # 
-          # Make sure we've got a matching group
-          #
-          if groups.empty?
-            logger.warn "no groups found for #{alert}!" 
-            next
-          end
-
-          #
-          # Notify just the first group 
-          #
-          this_group = groups.first
-          logger.info("notifying group #{this_group} of #{alert}")
-          this_group.notify(alert)
-        end
-      end
- 
       def logger
         Log4r::Logger.new self.to_s
       end
 
       def all
+        return [] if Configuration.current.nil?
+
         Configuration.current.alert_groups
       end
       
@@ -83,7 +58,7 @@ module Mauve
       self.includes = Proc.new { true }
     end
     
-    def to_s
+    def inspect
       "#<AlertGroup:#{name} (level #{level})>"
     end
   
@@ -129,7 +104,7 @@ module Mauve
       # The notifications are specified in the config file.
       #
       notifications.each do |notification|
-        notification.alert_changed(alert)
+        notification.notify(alert)
       end
     end
 
diff --git a/lib/mauve/mauve_thread.rb b/lib/mauve/mauve_thread.rb
index 52c2801..77df95e 100644
--- a/lib/mauve/mauve_thread.rb
+++ b/lib/mauve/mauve_thread.rb
@@ -18,10 +18,18 @@ module Mauve
 
     def poll_every=(i)
       raise ArgumentError.new("poll_every must be numeric") unless i.is_a?(Numeric)
+      # 
+      # Set the minimum poll frequency.
+      #
+      if i.to_f < 0.2
+        logger.debug "Increasing thread polling interval to 0.2s from #{i}"
+        i = 0.2 
+      end
+
       @poll_every = i
     end
 
-    def run_thread(interval = 0.1)
+    def run_thread(interval = 1.0)
       #
       # Good to go.
       #
@@ -70,18 +78,21 @@ module Mauve
 
     def state=(s)
       raise "Bad state for mauve_thread #{s.inspect}" unless [:stopped, :starting, :started, :freezing, :frozen, :stopping, :killing, :killed].include?(s)
+
       unless @state == s
         @state = s
         logger.debug(s.to_s.capitalize) 
       end
+
+      @state
     end
 
     def freeze
       self.state = :freezing
       
-      20.times { Kernel.sleep 0.1 ; break if @thread.stop? }
+      20.times { Kernel.sleep 0.2 ; break if @thread.stop? }
 
-      logger.debug("Thread has not frozen!") unless @thread.stop?
+      logger.warn("Thread has not frozen!") unless @thread.stop?
     end
 
     def frozen?
diff --git a/lib/mauve/notification.rb b/lib/mauve/notification.rb
index dea07a3..8e125be 100644
--- a/lib/mauve/notification.rb
+++ b/lib/mauve/notification.rb
@@ -149,20 +149,6 @@ module Mauve
 
     def logger ;  Log4r::Logger.new self.class.to_s ; end
 
-    # Updated code, now takes account of lists of people.
-    #
-    # @TODO refactor so we can test this more easily.
-    #
-    # @TODO Make sure that if no notifications is send at all, we log this
-    #       as an error so that an email is send to the developers.  Hum, we
-    #       could have person.alert_changed return true if a notification was 
-    #       send (false otherwise) and add it to a queue.  Then, dequeue till 
-    #       we see a "true" and abort.  However, this needs a timeout loop 
-    #       around it and we will slow down the whole notificatin since it
-    #       will have to wait untill such a time as it gets a true or timeout.
-    #       Not ideal.  A quick fix is to make sure that the clause in the 
-    #       configuration has a fall back that will send an alert in all cases.
-    #
     def notify(alert)
 
       if people.nil? or people.empty?
@@ -173,6 +159,8 @@ module Mauve
       # Should we notify at all?
       is_relevant = DuringRunner.new(Time.now, alert, &during).now?
 
+      n_sent = 0
+
       people.collect do |person|
         case person
           when Person
@@ -184,14 +172,13 @@ module Mauve
             []
         end
       end.flatten.uniq.each do |person|
-        person.send_alert(level, alert, is_relevant, remind_at_next(alert))
+        n_sent += 1 if person.send_alert(self.level, alert, is_relevant, remind_at_next(alert))
       end
 
-      return nil
+      return n_sent
     end
     
     def remind_at_next(alert)
-
       return DuringRunner.new(Time.now, alert, &during).find_next(every) if alert.raised?
 
       return nil
diff --git a/lib/mauve/notifier.rb b/lib/mauve/notifier.rb
index 1c3bf9b..d660a54 100644
--- a/lib/mauve/notifier.rb
+++ b/lib/mauve/notifier.rb
@@ -14,9 +14,8 @@ module Mauve
       #
       sz = Server.notification_buffer_size
 
-      return if sz == 0
- 
       my_threads = []
+
       sz.times do
         person, *args = Server.notification_pop
         
diff --git a/lib/mauve/person.rb b/lib/mauve/person.rb
index 76b52e0..3831529 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
+    attr_reader :notification_thresholds, :last_pop3_login
   
     def initialize(*args)
       #
@@ -13,6 +13,10 @@ module Mauve
       #
       @notification_thresholds = { 60 => Array.new(10) }
       @suppressed = false
+      #
+      # TODO fix up web login so pop3 can be used as a proxy.
+      #
+      @last_pop3_login = {:from => nil, :at => nil}
       super(*args)
     end
    
@@ -37,10 +41,11 @@ module Mauve
       def logger ; @logger ||= Log4r::Logger.new self.class.to_s ; end
 
       #
-      # This method makes sure things liek
+      # This method makes sure things like
+      #
+      #   xmpp
       #
-      #   xmpp 
-      #   works
+      #  works
       #
       def method_missing(name, *args)
         #
@@ -79,83 +84,65 @@ module Mauve
       end
 
     end 
-    
-    ## Deals with changes in an alert.
-    # 
-    # == Old comments by Matthew.
-    #
-    # An AlertGroup tells a Person that an alert has changed.  Within
-    # this alert group, the alert may or may not be "relevant" to this
-    # person, but it is ultimately up to the Person to decide whether to
-    # send a notification.  (i.e. notification of acks/clears should
-    # always go out to a Person who was notified of the original alert,
-    # even if the alert is no longer relevant to them).
-    # 
-    # == New comment
-    #
-    # The old code works like this:  An alert arrives, with a relevance.  An
-    # AlertChanged is created and the alert may or may not be send.  The 
-    # problem is that alerts can be relevant AFTER the initial raise and this
-    # code (due to AlertChange.was_relevant_when_raised?()) will ignore it.
-    # This is wrong.
-    # 
+
     #
-    # The Thread.exclusive wrapper around the AlertChanged creation makes 
-    # sure that two AlertChanged are not created at the same time.  This 
-    # caused both instances to set the remind_at time of the other to nil. 
-    # Thus reminders were never seen which is clearly wrong.  This bug was
-    # only showing on jruby due to green threads in MRI. 
+    # Sends the alert, and updates when the AlertChanged database to set the next reminder.
     #
-    # 
-    # @author Matthew Bloch, Yann Golanski
-    # @param [symb] level Level of the alert.
-    # @param [Alert] alert An alert object.
-    # @param [Boolean] Whether the alert is relevant as defined by notification
-    #                  class.
-    # @param [MauveTime] When to send remind.
-    # @return [NULL] nada
-    def alert_changed(level, alert, is_relevant=true, remind_at=nil)
-        # User should get notified but will not since on holiday.
-        str = String.new
-#        if is_on_holiday?
-#          is_relevant = false
-#          str = ' (user on holiday)'
-#        end
-
-        # Deals with AlertChange database entry.
-        last_change = AlertChanged.first(:alert_id => alert.id, :person => username)
-
-        if not last_change.nil?
-          if not last_change.remind_at.nil? and not remind_at.nil?
-            if last_change.remind_at.to_time < remind_at
-              remind_at = last_change.remind_at.to_time
-            end
+    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
+
+
+      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
 
-        new_change = AlertChanged.create(
-            :level => level.to_s,
-            :alert_id => alert.id, 
-            :at => MauveTime.now, 
-            :person => username, 
-            :update_type => alert.update_type,
-            :remind_at => remind_at,
-            :was_relevant => is_relevant)
-
-        # We need to look at the AlertChanged objects to reset them to
-        # the right value.  What is the right value?   Well...
-        if true == is_relevant
-          last_change.was_relevant = true if false == last_change.nil?
-        end
+      this_reminder.save unless this_reminder.nil?
 
-        send_alert(level, alert) if is_relevant # last_change.was_relevant_when_raised? 
-    end
-   
-    #
-    # This just wraps send_alert by sending the job to a queue.
-    #
-    def send_alert(level, alert)
-      Server.notification_push([self, level, alert])
+      if is_relevant
+        Server.notification_push([self, level, alert])
+        return true
+      end
+
+      return false
     end
    
     def do_send_alert(level, alert)
@@ -214,12 +201,11 @@ module Mauve
           @notification_thresholds[period].push Time.now
           @notification_thresholds[period].shift
         end
-        true
-
-      else
-        false
 
+        return true
       end
+
+      return false
     end
     
     # Returns the subset of current alerts that are relevant to this Person.
diff --git a/lib/mauve/pop3_server.rb b/lib/mauve/pop3_server.rb
index ef307d5..f4dbf3a 100644
--- a/lib/mauve/pop3_server.rb
+++ b/lib/mauve/pop3_server.rb
@@ -283,7 +283,7 @@ module Mauve
         #
         # A maximum of the 100 most recent alerts.
         #
-        AlertChanged.first(100, :person => self.user).each do |a|
+        AlertChanged.first(100, :person => self.user, :was_relevant => true).each do |a|
           #
           # Not interested in alerts 
           #
diff --git a/lib/mauve/processor.rb b/lib/mauve/processor.rb
index 86ed5dd..d68c551 100644
--- a/lib/mauve/processor.rb
+++ b/lib/mauve/processor.rb
@@ -34,6 +34,13 @@ module Mauve
       sz = Server.packet_buffer_size
 
       sz.times do
+        Timer.instance.freeze if Timer.instance.alive? and !Timer.instance.frozen?
+
+        #
+        # Hmm.. timer not frozen.
+        #
+        break unless Timer.instance.frozen?
+
         data, client, received_at = Server.packet_pop
 
         #
@@ -41,7 +48,6 @@ module Mauve
         #
         break if data.nil?
         
-        Timer.instance.freeze if Timer.instance.alive? and !Timer.instance.frozen?
 
         # logger.debug("Got #{data.inspect} from #{client.inspect}")
 
diff --git a/lib/mauve/server.rb b/lib/mauve/server.rb
index 5cc8484..27157a5 100644
--- a/lib/mauve/server.rb
+++ b/lib/mauve/server.rb
@@ -32,7 +32,7 @@ module Mauve
     def initialize
       super
       @hostname    = "localhost"
-      @database    = "sqlite3:///./mauvealert.db"
+      @database    = "sqlite3::memory:"
       
       @started_at = Time.now
       @initial_sleep = 300
@@ -65,6 +65,12 @@ module Mauve
     end
 
     def setup
+      #
+      #
+      #
+      @packet_buffer       = []
+      @notification_buffer = []
+
       DataMapper.setup(:default, @database)
       # DataObjects::Sqlite3.logger = Log4r::Logger.new("Mauve::DataMapper") 
 
@@ -75,6 +81,10 @@ module Mauve
       AlertChanged.auto_upgrade!
       History.auto_upgrade!
       Mauve::AlertEarliestDate.create_view!
+
+      Mauve::Configuration.current = Mauve::Configuration.new if Mauve::Configuration.current.nil?
+
+      return nil
     end
 
     def start