From 3de184d7c79f5cc25200d0c529578045f9bde751 Mon Sep 17 00:00:00 2001 From: Shane Emmons Date: Thu, 20 Dec 2012 13:01:56 -0500 Subject: [PATCH 1/2] use try when destroying device incase it already has been --- lib/gcm_on_rails/app/models/gcm/notification.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/gcm_on_rails/app/models/gcm/notification.rb b/lib/gcm_on_rails/app/models/gcm/notification.rb index 8b144ac..70ba45b 100644 --- a/lib/gcm_on_rails/app/models/gcm/notification.rb +++ b/lib/gcm_on_rails/app/models/gcm/notification.rb @@ -65,18 +65,18 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se when "MissingRegistration" ex = Gcm::Errors::MissingRegistration.new(response[:message]) logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.destroy + notification.device.try(:destroy) when "InvalidRegistration" ex = Gcm::Errors::InvalidRegistration.new(response[:message]) logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.destroy + notification.device.try(:destroy) when "MismatchedSenderId" ex = Gcm::Errors::MismatchSenderId.new(response[:message]) logger.warn(ex.message) when "NotRegistered" ex = Gcm::Errors::NotRegistered.new(response[:message]) logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.destroy + notification.device.try(:destroy) when "MessageTooBig" ex = Gcm::Errors::MessageTooBig.new(response[:message]) logger.warn(ex.message) @@ -97,4 +97,4 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se end end end -end \ No newline at end of file +end From 4efd7eff95f63ef1f3496d77c90a653b006370a6 Mon Sep 17 00:00:00 2001 From: Shane Emmons Date: Tue, 13 May 2014 01:53:30 +0000 Subject: [PATCH 2/2] guard against memory bloat instead of processing all notifications, we fetch all ids and then process them in batches of 1000 --- .../app/models/gcm/notification.rb | 110 ++++++++++-------- 1 file changed, 60 insertions(+), 50 deletions(-) diff --git a/lib/gcm_on_rails/app/models/gcm/notification.rb b/lib/gcm_on_rails/app/models/gcm/notification.rb index 70ba45b..a049735 100644 --- a/lib/gcm_on_rails/app/models/gcm/notification.rb +++ b/lib/gcm_on_rails/app/models/gcm/notification.rb @@ -28,7 +28,15 @@ class << self # {:message=>"{\"multicast_id\":6085691036338669615,\"success\":1,\"failure\":0,\"canonical_ids\":0,\"results\":[{\"message_id\":\"0:1349723376618187%d702725e98d39af3\"}]}", :code=>200} # # - def send_notifications(notifications = Gcm::Notification.all(:conditions => {:sent_at => nil}, :joins => :device, :readonly => false)) + def send_notifications(notifications = :none) + notification_ids = if notifications == :none + Gcm::Notification.all( + :conditions => {:sent_at => nil}, + :select => :id + ).map(&:id) + else + notifications.map(&:id) + end if configatron.gcm_on_rails.delivery_format and configatron.gcm_on_rails.delivery_format == 'plain_text' format = "plain_text" @@ -36,62 +44,64 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se format = "json" end - unless notifications.nil? || notifications.empty? + unless notification_ids.nil? || notification_ids.empty? api_key = Gcm::Connection.open if api_key - notifications.each do |notification| + notification_ids.each_slice(1000) do |ids| + notifications = Gcm::Notification.all(:conditions => {:id => ids}, :joins => :device, :readonly => false) + notifications.each do |notification| - logger.info "notification = #{notification.inspect}" - response = Gcm::Connection.send_notification(notification, api_key, format) - logger.info "response = #{response.inspect}" - - if response[:code] == 200 - if response[:message].nil? - # TODO - Making this assumption might not be right. HTTP status code 200 does not really signify success - # if Gcm servers returned nil for the message - error = "success" - elsif format == "json" - error = "" - message_data = JSON.parse response[:message] - success = message_data['success'] - error = message_data['results'][0]['error'] if success == 0 - elsif format == "plain_text" #format is plain text - message_data = response[:message] - error = response[:message].split('=')[1] - end + logger.info "notification = #{notification.inspect}" + response = Gcm::Connection.send_notification(notification, api_key, format) + logger.info "response = #{response.inspect}" + if response[:code] == 200 + if response[:message].nil? + # TODO - Making this assumption might not be right. HTTP status code 200 does not really signify success + # if Gcm servers returned nil for the message + error = "success" + elsif format == "json" + error = "" + message_data = JSON.parse response[:message] + success = message_data['success'] + error = message_data['results'][0]['error'] if success == 0 + elsif format == "plain_text" #format is plain text + message_data = response[:message] + error = response[:message].split('=')[1] + end - case error - when "MissingRegistration" - ex = Gcm::Errors::MissingRegistration.new(response[:message]) - logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.try(:destroy) - when "InvalidRegistration" - ex = Gcm::Errors::InvalidRegistration.new(response[:message]) - logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.try(:destroy) - when "MismatchedSenderId" - ex = Gcm::Errors::MismatchSenderId.new(response[:message]) - logger.warn(ex.message) - when "NotRegistered" - ex = Gcm::Errors::NotRegistered.new(response[:message]) - logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") - notification.device.try(:destroy) - when "MessageTooBig" - ex = Gcm::Errors::MessageTooBig.new(response[:message]) - logger.warn(ex.message) - else - notification.sent_at = Time.now - notification.save! + case error + when "MissingRegistration" + ex = Gcm::Errors::MissingRegistration.new(response[:message]) + logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") + notification.device.try(:destroy) + when "InvalidRegistration" + ex = Gcm::Errors::InvalidRegistration.new(response[:message]) + logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") + notification.device.try(:destroy) + when "MismatchedSenderId" + ex = Gcm::Errors::MismatchSenderId.new(response[:message]) + logger.warn(ex.message) + when "NotRegistered" + ex = Gcm::Errors::NotRegistered.new(response[:message]) + logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") + notification.device.try(:destroy) + when "MessageTooBig" + ex = Gcm::Errors::MessageTooBig.new(response[:message]) + logger.warn(ex.message) + else + notification.sent_at = Time.now + notification.save! + end + elsif response[:code] == 401 + raise Gcm::Errors::InvalidAuthToken.new(message_data) + elsif response[:code] == 503 + raise Gcm::Errors::ServiceUnavailable.new(message_data) + elsif response[:code] == 500 + raise Gcm::Errors::InternalServerError.new(message_data) end - elsif response[:code] == 401 - raise Gcm::Errors::InvalidAuthToken.new(message_data) - elsif response[:code] == 503 - raise Gcm::Errors::ServiceUnavailable.new(message_data) - elsif response[:code] == 500 - raise Gcm::Errors::InternalServerError.new(message_data) - end + end end end end