From f12819a2cd2824a6b17d15d5e1cadf0bfb4c85ac Mon Sep 17 00:00:00 2001 From: Greg Barnett Date: Wed, 24 Oct 2012 15:54:10 -0700 Subject: [PATCH] fix bug with invalid json being generated --- .../app/models/gcm/notification.rb | 57 ++++++++++--------- lib/gcm_on_rails/gcm_on_rails.rb | 9 ++- lib/gcm_on_rails/libs/connection.rb | 12 ++-- 3 files changed, 46 insertions(+), 32 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..4ee116b 100644 --- a/lib/gcm_on_rails/app/models/gcm/notification.rb +++ b/lib/gcm_on_rails/app/models/gcm/notification.rb @@ -45,7 +45,8 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se response = Gcm::Connection.send_notification(notification, api_key, format) logger.info "response = #{response.inspect}" - if response[:code] == 200 + case response[:code] + when 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 @@ -62,34 +63,36 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se 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.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 - 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 - when "MessageTooBig" - ex = Gcm::Errors::MessageTooBig.new(response[:message]) - logger.warn(ex.message) - else - notification.sent_at = Time.now - notification.save! + when "MissingRegistration" + ex = Gcm::Errors::MissingRegistration.new(response[:message]) + logger.warn("#{ex.message}, destroying gcm_device with id #{notification.device.id}") + notification.device.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 + 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 + when "MessageTooBig" + ex = Gcm::Errors::MessageTooBig.new(response[:message]) + logger.warn(ex.message) + else + notification.sent_at = Time.now end - elsif response[:code] == 401 + notification.save! + when 400 + raise Gcm::Errors::InvalidJSON.new(message_data) + when 401 raise Gcm::Errors::InvalidAuthToken.new(message_data) - elsif response[:code] == 503 - raise Gcm::Errors::ServiceUnavailable.new(message_data) - elsif response[:code] == 500 + when 500 raise Gcm::Errors::InternalServerError.new(message_data) + when 503 + raise Gcm::Errors::ServiceUnavailable.new(message_data) end end @@ -97,4 +100,4 @@ def send_notifications(notifications = Gcm::Notification.all(:conditions => {:se end end end -end \ No newline at end of file +end diff --git a/lib/gcm_on_rails/gcm_on_rails.rb b/lib/gcm_on_rails/gcm_on_rails.rb index 9db5d55..4555dab 100644 --- a/lib/gcm_on_rails/gcm_on_rails.rb +++ b/lib/gcm_on_rails/gcm_on_rails.rb @@ -4,6 +4,13 @@ module Gcm module Errors + # Invalid JSON + class InvalidJSON < StandardError + def initialize(message) # :nodoc: + super("Invalid JSON: '#{message}'") + end + end + # Missing registration_id. class MissingRegistration < StandardError def initialize(message) # :nodoc: @@ -85,4 +92,4 @@ def initialize(message) ActiveSupport::Dependencies.autoload_paths << path ActiveSupport::Dependencies.autoload_once_paths.delete(path) end -end \ No newline at end of file +end diff --git a/lib/gcm_on_rails/libs/connection.rb b/lib/gcm_on_rails/libs/connection.rb index c240329..3b3b4e9 100644 --- a/lib/gcm_on_rails/libs/connection.rb +++ b/lib/gcm_on_rails/libs/connection.rb @@ -9,9 +9,13 @@ def send_notification(notification, api_key, format) headers = {"Content-Type" => "application/json", "Authorization" => "key=#{api_key}"} - data = notification.data.merge({:collapse_key => notification.collapse_key}) unless notification.collapse_key.nil? - data = data.merge({:delay_while_idle => notification.delay_while_idle}) unless notification.delay_while_idle.nil? - data = data.merge({:time_to_live => notification.time_to_live}) unless notification.time_to_live.nil? + data = { + data: notification.data, + registration_ids: [notification.device.registration_id] + } + data = data.merge({collapse_key: notification.collapse_key}) unless notification.collapse_key.nil? + data = data.merge({delay_while_idle: notification.delay_while_idle}) unless notification.delay_while_idle.nil? + data = data.merge({time_to_live: notification.time_to_live}) unless notification.time_to_live.nil? data = data.to_json else #plain text format headers = {"Content-Type" => "application/x-www-form-urlencoded;charset=UTF-8", @@ -40,4 +44,4 @@ def open end end end -end \ No newline at end of file +end