Merge pull request rpush#561 from powerhome/apns-notification-payload…

…-size-limit

lib/rpush/client/active_model.rb

@@ -4,10 +4,10 @@
 require 'rpush/client/active_model/payload_data_size_validator'
 require 'rpush/client/active_model/registration_ids_count_validator'
 
-require 'rpush/client/active_model/apns/binary_notification_validator'
 require 'rpush/client/active_model/apns/device_token_format_validator'
 require 'rpush/client/active_model/apns/app'
 require 'rpush/client/active_model/apns/notification'
+require 'rpush/client/active_model/apns/notification_payload_size_validator'
 
 require 'rpush/client/active_model/apns2/app'
 require 'rpush/client/active_model/apns2/notification'

lib/rpush/client/active_model/apns/notification.rb

@@ -7,15 +7,23 @@ module Notification
           APNS_PRIORITY_IMMEDIATE = 10
           APNS_PRIORITY_CONSERVE_POWER = 5
           APNS_PRIORITIES = [APNS_PRIORITY_IMMEDIATE, APNS_PRIORITY_CONSERVE_POWER]
+          MAX_PAYLOAD_BYTESIZE = 2048
+
+          module ClassMethods
+            def max_payload_bytesize
+              MAX_PAYLOAD_BYTESIZE
+            end
+          end
 
           def self.included(base)
+            base.extend ClassMethods
             base.instance_eval do
               validates :device_token, presence: true
               validates :badge, numericality: true, allow_nil: true
               validates :priority, inclusion: { in: APNS_PRIORITIES }, allow_nil: true
 
               validates_with Rpush::Client::ActiveModel::Apns::DeviceTokenFormatValidator
-              validates_with Rpush::Client::ActiveModel::Apns::BinaryNotificationValidator
+              validates_with Rpush::Client::ActiveModel::Apns::NotificationPayloadSizeValidator
 
               base.const_set('APNS_DEFAULT_EXPIRY', APNS_DEFAULT_EXPIRY) unless base.const_defined?('APNS_DEFAULT_EXPIRY')
               base.const_set('APNS_PRIORITY_IMMEDIATE', APNS_PRIORITY_IMMEDIATE) unless base.const_defined?('APNS_PRIORITY_IMMEDIATE')

lib/rpush/client/active_model/apns/notification_payload_size_validator.rb

@@ -0,0 +1,15 @@
+module Rpush
+  module Client
+    module ActiveModel
+      module Apns
+        class NotificationPayloadSizeValidator < ::ActiveModel::Validator
+          def validate(record)
+            limit = record.class.max_payload_bytesize
+            return unless record.payload.bytesize > limit
+            record.errors[:base] << "APN notification cannot be larger than #{limit} bytes. Try condensing your alert and device attributes."
+          end
+        end
+      end
+    end
+  end
+end

lib/rpush/client/active_model/apns2/notification.rb

@@ -3,6 +3,20 @@ module Client
     module ActiveModel
       module Apns2
         include Rpush::Client::ActiveModel::Apns
+
+        module Notification
+          MAX_PAYLOAD_BYTESIZE = 4096
+
+          module ClassMethods
+            def max_payload_bytesize
+              MAX_PAYLOAD_BYTESIZE
+            end
+          end
+
+          def self.included(base)
+            base.extend ClassMethods
+          end
+        end
       end
     end
   end

lib/rpush/client/active_record.rb

@@ -5,6 +5,7 @@
 require 'rpush/client/active_record/notification'
 require 'rpush/client/active_record/app'
 
+require 'rpush/client/active_record/apns/active_record_serializable_notification'
 require 'rpush/client/active_record/apns/notification'
 require 'rpush/client/active_record/apns/feedback'
 require 'rpush/client/active_record/apns/app'

lib/rpush/client/active_record/apns/active_record_serializable_notification.rb

@@ -0,0 +1,65 @@
+module Rpush
+  module Client
+    module ActiveRecord
+      module Apns
+        module ActiveRecordSerializableNotification
+          def alert=(alert)
+            if alert.is_a?(Hash)
+              write_attribute(:alert, multi_json_dump(alert))
+              self.alert_is_json = true if has_attribute?(:alert_is_json)
+            else
+              write_attribute(:alert, alert)
+              self.alert_is_json = false if has_attribute?(:alert_is_json)
+            end
+          end
+
+          def alert
+            string_or_json = read_attribute(:alert)
+
+            if has_attribute?(:alert_is_json)
+              if alert_is_json?
+                multi_json_load(string_or_json)
+              else
+                string_or_json
+              end
+            else
+              begin
+                multi_json_load(string_or_json)
+              rescue StandardError
+                string_or_json
+              end
+            end
+          end
+
+          def sound=(sound)
+            if sound.is_a?(Hash)
+              write_attribute(:sound, multi_json_dump(sound))
+              self.sound_is_json = true if has_attribute?(:sound_is_json)
+            else
+              write_attribute(:sound, sound)
+              self.sound_is_json = false if has_attribute?(:sound_is_json)
+            end
+          end
+
+          def sound
+            string_or_json = read_attribute(:sound)
+
+            if has_attribute?(:sound_is_json)
+              if sound_is_json?
+                multi_json_load(string_or_json)
+              else
+                string_or_json
+              end
+            else
+              begin
+                multi_json_load(string_or_json)
+              rescue StandardError
+                string_or_json
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/rpush/client/active_record/apns/notification.rb

@@ -3,64 +3,8 @@ module Client
     module ActiveRecord
       module Apns
         class Notification < Rpush::Client::ActiveRecord::Notification
-          include Deprecatable
           include Rpush::Client::ActiveModel::Apns::Notification
-
-          def alert=(alert)
-            if alert.is_a?(Hash)
-              write_attribute(:alert, multi_json_dump(alert))
-              self.alert_is_json = true if has_attribute?(:alert_is_json)
-            else
-              write_attribute(:alert, alert)
-              self.alert_is_json = false if has_attribute?(:alert_is_json)
-            end
-          end
-
-          def alert
-            string_or_json = read_attribute(:alert)
-
-            if has_attribute?(:alert_is_json)
-              if alert_is_json?
-                multi_json_load(string_or_json)
-              else
-                string_or_json
-              end
-            else
-              begin
-                multi_json_load(string_or_json)
-              rescue StandardError
-                string_or_json
-              end
-            end
-          end
-
-          def sound=(sound)
-            if sound.is_a?(Hash)
-              write_attribute(:sound, multi_json_dump(sound))
-              self.sound_is_json = true if has_attribute?(:sound_is_json)
-            else
-              write_attribute(:sound, sound)
-              self.sound_is_json = false if has_attribute?(:sound_is_json)
-            end
-          end
-
-          def sound
-            string_or_json = read_attribute(:sound)
-
-            if has_attribute?(:sound_is_json)
-              if sound_is_json?
-                multi_json_load(string_or_json)
-              else
-                string_or_json
-              end
-            else
-              begin
-                multi_json_load(string_or_json)
-              rescue StandardError
-                string_or_json
-              end
-            end
-          end
+          include ActiveRecordSerializableNotification
         end
       end
     end

lib/rpush/client/active_record/apns2/notification.rb

@@ -2,7 +2,10 @@ module Rpush
   module Client
     module ActiveRecord
       module Apns2
-        class Notification < Rpush::Client::ActiveRecord::Apns::Notification
+        class Notification < Rpush::Client::ActiveRecord::Notification
+          include Rpush::Client::ActiveModel::Apns::Notification
+          include Rpush::Client::ActiveModel::Apns2::Notification
+          include Rpush::Client::ActiveRecord::Apns::ActiveRecordSerializableNotification
         end
       end
     end

lib/rpush/client/redis/apns2/notification.rb

@@ -3,6 +3,7 @@ module Client
     module Redis
       module Apns2
         class Notification < Rpush::Client::Redis::Notification
+          include Rpush::Client::ActiveModel::Apns::Notification
           include Rpush::Client::ActiveModel::Apns2::Notification
         end
       end

spec/unit/client/active_record/apns/notification_spec.rb

@@ -6,6 +6,21 @@
   it_behaves_like 'Rpush::Client::Apns::Notification'
   it_behaves_like 'Rpush::Client::ActiveRecord::Notification'
 
+  it "should validate the length of the binary conversion of the notification" do
+    notification = described_class.new
+    notification.app = Rpush::Apns2::App.create(name: 'test', environment: 'development')
+    notification.device_token = "a" * 108
+    notification.alert = ""
+
+    notification.alert << "a" until notification.payload.bytesize == 2048
+    expect(notification.valid?).to be_truthy
+    expect(notification.errors[:base]).to be_empty
+
+    notification.alert << "a"
+    expect(notification.valid?).to be_falsey
+    expect(notification.errors[:base].include?("APN notification cannot be larger than 2048 bytes. Try condensing your alert and device attributes.")).to be_truthy
+  end
+
   describe "multi_json usage" do
     describe "alert" do
       subject(:notification) { described_class.new(alert: { a: 1 }, alert_is_json: true) }

spec/unit/client/active_record/apns2/app_spec.rb

@@ -0,0 +1,4 @@
+require 'unit_spec_helper'
+
+describe Rpush::Client::ActiveRecord::Apns2::App do
+end if active_record?

spec/unit/client/active_record/apns2/notification_spec.rb

@@ -0,0 +1,65 @@
+require "unit_spec_helper"
+
+describe Rpush::Client::ActiveRecord::Apns2::Notification do
+  subject(:notification) { described_class.new }
+
+  it_behaves_like 'Rpush::Client::Apns::Notification'
+  it_behaves_like 'Rpush::Client::ActiveRecord::Notification'
+
+  it "should validate the length of the binary conversion of the notification" do
+    notification = described_class.new
+    notification.app = Rpush::Apns2::App.create(name: 'test', environment: 'development')
+    notification.device_token = "a" * 108
+    notification.alert = ""
+
+    notification.alert << "a" until notification.payload.bytesize == 4096
+    expect(notification.valid?).to be_truthy
+    expect(notification.errors[:base]).to be_empty
+
+    notification.alert << "a"
+    expect(notification.valid?).to be_falsey
+    expect(notification.errors[:base].include?("APN notification cannot be larger than 4096 bytes. Try condensing your alert and device attributes.")).to be_truthy
+  end
+
+  describe "multi_json usage" do
+    describe "alert" do
+      subject(:notification) { described_class.new(alert: { a: 1 }, alert_is_json: true) }
+
+      it "should call MultiJson.load when multi_json version is 1.3.0" do
+        allow(Gem).to receive(:loaded_specs).and_return('multi_json' => Gem::Specification.new('multi_json', '1.3.0'))
+        expect(MultiJson).to receive(:load).with(any_args)
+        notification.alert
+      end
+
+      it "should call MultiJson.decode when multi_json version is 1.2.9" do
+        allow(Gem).to receive(:loaded_specs).and_return('multi_json' => Gem::Specification.new('multi_json', '1.2.9'))
+        expect(MultiJson).to receive(:decode).with(any_args)
+        notification.alert
+      end
+    end
+  end
+
+  it "should default the sound to nil" do
+    expect(notification.sound).to be_nil
+  end
+
+  it 'does not overwrite the mutable-content flag when setting attributes for the device' do
+    notification.mutable_content = true
+    notification.data = { 'hi' => 'mom' }
+    expect(notification.as_json['aps']['mutable-content']).to eq 1
+    expect(notification.as_json['hi']).to eq 'mom'
+  end
+
+  it 'does not overwrite the content-available flag when setting attributes for the device' do
+    notification.content_available = true
+    notification.data = { 'hi' => 'mom' }
+    expect(notification.as_json['aps']['content-available']).to eq 1
+    expect(notification.as_json['hi']).to eq 'mom'
+  end
+
+  it 'does confuse a JSON looking string as JSON if the alert_is_json attribute is not present' do
+    allow(notification).to receive_messages(has_attribute?: false)
+    notification.alert = "{\"one\":2}"
+    expect(notification.alert).to eq('one' => 2)
+  end
+end if active_record?

spec/unit/client/active_record/gcm/notification_spec.rb

@@ -7,18 +7,8 @@
   subject(:notification) { described_class.new }
   let(:app) { Rpush::Gcm::App.create!(name: 'test', auth_key: 'abc') }
 
-  # In Rails 4.2 this value casts to `false` and thus will not be included in
-  # the payload. This changed to match Ruby's semantics, and will casts to
-  # `true` in Rails 5 and above.
-  if ActiveRecord.version <= Gem::Version.new('5')
-    it 'accepts non-booleans as a falsey value' do
-      notification.dry_run = 'Not a boolean'
-      expect(notification.as_json).not_to have_key 'dry_run'
-    end
-  else
-    it 'accepts non-booleans as a truthy value' do
-      notification.dry_run = 'Not a boolean'
-      expect(notification.as_json['dry_run']).to eq true
-    end
+  it 'accepts non-booleans as a truthy value' do
+    notification.dry_run = 'Not a boolean'
+    expect(notification.as_json['dry_run']).to eq true
   end
 end if active_record?

spec/unit/client/redis/apns/notification_spec.rb

@@ -5,6 +5,21 @@
 describe Rpush::Client::Redis::Apns::Notification do
   it_behaves_like 'Rpush::Client::Apns::Notification'
 
+  it "should validate the length of the binary conversion of the notification" do
+    notification = described_class.new
+    notification.app = Rpush::Apns2::App.create(name: 'test', environment: 'development')
+    notification.device_token = "a" * 108
+    notification.alert = ""
+
+    notification.alert << "a" until notification.payload.bytesize == 2048
+    expect(notification.valid?).to be_truthy
+    expect(notification.errors[:base]).to be_empty
+
+    notification.alert << "a"
+    expect(notification.valid?).to be_falsey
+    expect(notification.errors[:base].include?("APN notification cannot be larger than 2048 bytes. Try condensing your alert and device attributes.")).to be_truthy
+  end
+
   it "should default the sound to 'default'" do
     notification = described_class.new
     expect(notification.sound).to eq('default')

spec/unit/client/redis/apns2/app_spec.rb

@@ -0,0 +1,4 @@
+require 'unit_spec_helper'
+
+describe Rpush::Client::Redis::Apns2::App do
+end if redis?