From e6e9b8da64bcbba9062901a18cbbb24ff35677b0 Mon Sep 17 00:00:00 2001 From: Andreas Mueller Date: Thu, 27 Jan 2011 09:17:57 +0100 Subject: [PATCH 1/2] modified send_notifications_for_cert routine to decrease db load Rather than making a query for each device, we can get all unsent notifications for a given app with one SQL query. The change also required a slight update of the app_spec. --- lib/apn_on_rails/app/models/apn/app.rb | 17 ++++++++--------- spec/apn_on_rails/app/models/apn/app_spec.rb | 10 +++------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/apn_on_rails/app/models/apn/app.rb b/lib/apn_on_rails/app/models/apn/app.rb index a0927374..81404900 100644 --- a/lib/apn_on_rails/app/models/apn/app.rb +++ b/lib/apn_on_rails/app/models/apn/app.rb @@ -40,18 +40,17 @@ def self.send_notifications def self.send_notifications_for_cert(the_cert, app_id) # unless self.unsent_notifications.nil? || self.unsent_notifications.empty? if (app_id == nil) - conditions = "app_id is null" + conditions = "app_id is null and sent_at is null" else - conditions = ["app_id = ?", app_id] + conditions = ["app_id = ? and sent_at is null", app_id] end begin APN::Connection.open_for_delivery({:cert => the_cert}) do |conn, sock| - APN::Device.find_each(:conditions => conditions) do |dev| - dev.unsent_notifications.each do |noty| - conn.write(noty.message_for_sending) - noty.sent_at = Time.now - noty.save - end + notifications = APN::Notification.find(:all, :conditions => conditions, :joins => " INNER JOIN apn_devices ON apn_devices.id = apn_notifications.device_id") + notifications.each do |noty| + conn.write(noty.message_for_sending) + noty.sent_at = Time.now + noty.save end end rescue Exception => e @@ -148,4 +147,4 @@ def log_connection_exception(ex) puts ex.message end -end \ No newline at end of file +end diff --git a/spec/apn_on_rails/app/models/apn/app_spec.rb b/spec/apn_on_rails/app/models/apn/app_spec.rb index d98707b2..e8e73686 100644 --- a/spec/apn_on_rails/app/models/apn/app_spec.rb +++ b/spec/apn_on_rails/app/models/apn/app_spec.rb @@ -20,10 +20,7 @@ APN::App.should_receive(:all).once.and_return([app]) app.should_receive(:cert).twice.and_return(app.apn_dev_cert) - APN::Device.should_receive(:find_each).twice.and_yield(device) - - device.should_receive(:unsent_notifications).and_return(notifications,[]) - + APN::Notification.should_receive(:find).and_return(notifications, []) ssl_mock = mock('ssl_mock') ssl_mock.should_receive(:write).with('message-0') @@ -52,8 +49,7 @@ notify.should_receive(:save) end - APN::Device.should_receive(:find_each).and_yield(device) - device.should_receive(:unsent_notifications).and_return(notifications) + APN::Notification.should_receive(:find).and_return(notifications) ssl_mock = mock('ssl_mock') ssl_mock.should_receive(:write).with('message-0') @@ -227,4 +223,4 @@ end end -end \ No newline at end of file +end From 9462d19e42050b2d672cc1021bbbc5faf5e5e771 Mon Sep 17 00:00:00 2001 From: Andreas Mueller Date: Thu, 27 Jan 2011 10:40:31 +0100 Subject: [PATCH 2/2] specify :select in find to avoid :read_only The APN::Notification.find call returned read-only ActiveRecords, which can be prevented by specifying :select. -> see http://stackoverflow.com/questions/639171/what-is-causing-this-activerecordreadonlyrecord-error --- lib/apn_on_rails/app/models/apn/app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/apn_on_rails/app/models/apn/app.rb b/lib/apn_on_rails/app/models/apn/app.rb index 81404900..49584be9 100644 --- a/lib/apn_on_rails/app/models/apn/app.rb +++ b/lib/apn_on_rails/app/models/apn/app.rb @@ -46,7 +46,7 @@ def self.send_notifications_for_cert(the_cert, app_id) end begin APN::Connection.open_for_delivery({:cert => the_cert}) do |conn, sock| - notifications = APN::Notification.find(:all, :conditions => conditions, :joins => " INNER JOIN apn_devices ON apn_devices.id = apn_notifications.device_id") + notifications = APN::Notification.find(:all, :select => "apn_notifications.*", :conditions => conditions, :joins => " INNER JOIN apn_devices ON apn_devices.id = apn_notifications.device_id") notifications.each do |noty| conn.write(noty.message_for_sending) noty.sent_at = Time.now