Skip to content
This repository

suggested chang to fix issue #20 (https://github.com/PRX/apn_on_rails/issues#issue/20) #25

Open
wants to merge 2 commits into from

3 participants

Andreas Müller stuartkhall jeff day
Andreas Müller

As suggested, the code now uses only one query (rather than a query per device) to get all notifications. I have tested the code only locally, so please review carefully ..

Andreas Müller andreasmueller 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.
e6e9b8d
Andreas Müller andreasmueller 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
9462d19
stuartkhall

Please merge this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Jan 27, 2011
Andreas Müller andreasmueller 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.
e6e9b8d
Andreas Müller andreasmueller 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
9462d19
This page is out of date. Refresh to see the latest.
17 lib/apn_on_rails/app/models/apn/app.rb
@@ -40,18 +40,17 @@ def self.send_notifications
40 40 def self.send_notifications_for_cert(the_cert, app_id)
41 41 # unless self.unsent_notifications.nil? || self.unsent_notifications.empty?
42 42 if (app_id == nil)
43   - conditions = "app_id is null"
  43 + conditions = "app_id is null and sent_at is null"
44 44 else
45   - conditions = ["app_id = ?", app_id]
  45 + conditions = ["app_id = ? and sent_at is null", app_id]
46 46 end
47 47 begin
48 48 APN::Connection.open_for_delivery({:cert => the_cert}) do |conn, sock|
49   - APN::Device.find_each(:conditions => conditions) do |dev|
50   - dev.unsent_notifications.each do |noty|
51   - conn.write(noty.message_for_sending)
52   - noty.sent_at = Time.now
53   - noty.save
54   - end
  49 + notifications = APN::Notification.find(:all, :select => "apn_notifications.*", :conditions => conditions, :joins => " INNER JOIN apn_devices ON apn_devices.id = apn_notifications.device_id")
  50 + notifications.each do |noty|
  51 + conn.write(noty.message_for_sending)
  52 + noty.sent_at = Time.now
  53 + noty.save
55 54 end
56 55 end
57 56 rescue Exception => e
@@ -148,4 +147,4 @@ def log_connection_exception(ex)
148 147 puts ex.message
149 148 end
150 149
151   -end
  150 +end
10 spec/apn_on_rails/app/models/apn/app_spec.rb
@@ -20,10 +20,7 @@
20 20 APN::App.should_receive(:all).once.and_return([app])
21 21 app.should_receive(:cert).twice.and_return(app.apn_dev_cert)
22 22
23   - APN::Device.should_receive(:find_each).twice.and_yield(device)
24   -
25   - device.should_receive(:unsent_notifications).and_return(notifications,[])
26   -
  23 + APN::Notification.should_receive(:find).and_return(notifications, [])
27 24
28 25 ssl_mock = mock('ssl_mock')
29 26 ssl_mock.should_receive(:write).with('message-0')
@@ -52,8 +49,7 @@
52 49 notify.should_receive(:save)
53 50 end
54 51
55   - APN::Device.should_receive(:find_each).and_yield(device)
56   - device.should_receive(:unsent_notifications).and_return(notifications)
  52 + APN::Notification.should_receive(:find).and_return(notifications)
57 53
58 54 ssl_mock = mock('ssl_mock')
59 55 ssl_mock.should_receive(:write).with('message-0')
@@ -227,4 +223,4 @@
227 223 end
228 224 end
229 225
230   -end
  226 +end

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.