Merge pull request #2014 from huginn/fix_1940

Protect the latest event from automatic deletion when using MySQL

app/models/event.rb

@@ -28,6 +28,15 @@ class Event < ActiveRecord::Base
     where("expires_at IS NOT NULL AND expires_at < ?", Time.now)
   }
 
+  case ActiveRecord::Base.connection.adapter_name
+  when /\Amysql/i
+    # Protect the Event table from InnoDB's AUTO_INCREMENT Counter
+    # Initialization by always keeping the latest event.
+    scope :to_expire, -> { expired.where.not(id: maximum(:id)) }
+  else
+    scope :to_expire, -> { expired }
+  end
+
   scope :with_location, -> {
     where.not(lat: nil).where.not(lng: nil)
   }
@@ -72,9 +81,11 @@ class Event < ActiveRecord::Base
   # Look for Events whose `expires_at` is present and in the past.  Remove those events and then update affected Agents'
   # `events_counts` cache columns.  This method is called by bin/schedule.rb periodically.
   def self.cleanup_expired!
-    affected_agents = Event.expired.group("agent_id").pluck(:agent_id)
-    Event.expired.delete_all
-    Agent.where(:id => affected_agents).update_all "events_count = (select count(*) from events where agent_id = agents.id)"
+    transaction do
+      affected_agents = Event.expired.group("agent_id").pluck(:agent_id)
+      Event.to_expire.delete_all
+      Agent.where(id: affected_agents).update_all "events_count = (select count(*) from events where agent_id = agents.id)"
+    end
   end
 
   protected

spec/models/event_spec.rb

@@ -118,6 +118,20 @@ describe Event do
       Event.cleanup_expired!
       expect(Event.find_by_id(event.id)).not_to be_nil
     end
+
+    it "always keeps the latest Event regardless of its expires_at value only if the database is MySQL" do
+      Event.delete_all
+      event1 = agents(:jane_weather_agent).create_event expires_at: 1.minute.ago
+      event2 = agents(:bob_weather_agent).create_event expires_at: 1.minute.ago
+
+      Event.cleanup_expired!
+      case ActiveRecord::Base.connection.adapter_name
+      when /\Amysql/i
+        expect(Event.all.pluck(:id)).to eq([event2.id])
+      else
+        expect(Event.all.pluck(:id)).to be_empty
+      end
+    end
   end
 
   describe "after destroy" do