From af410c54e6979fc01d0f808cfe05889cedbf207c Mon Sep 17 00:00:00 2001 From: Yosi Attias Date: Fri, 17 Mar 2017 13:54:01 +0200 Subject: [PATCH 1/4] Removing `instrumentation_keys` in order to fix the payload See - https://github.com/rails-api/active_model_serializers/issues/2067 --- lib/active_model/array_serializer.rb | 5 ----- lib/active_model/default_serializer.rb | 6 +----- lib/active_model/serializable.rb | 9 +-------- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/active_model/array_serializer.rb b/lib/active_model/array_serializer.rb index 78455537..2c6a4738 100644 --- a/lib/active_model/array_serializer.rb +++ b/lib/active_model/array_serializer.rb @@ -64,10 +64,5 @@ module ActiveModel end end - private - - def instrumentation_keys - [:object, :scope, :root, :meta_key, :meta, :each_serializer, :resource_name, :key_format, :context] - end end end diff --git a/lib/active_model/default_serializer.rb b/lib/active_model/default_serializer.rb index c2db6b1d..8c755cc2 100644 --- a/lib/active_model/default_serializer.rb +++ b/lib/active_model/default_serializer.rb @@ -21,12 +21,8 @@ module ActiveModel @wrap_in_array ? [hash] : hash end end + alias serializable_hash as_json alias serializable_object as_json - - private - def instrumentation_keys - [:object, :wrap_in_array] - end end end diff --git a/lib/active_model/serializable.rb b/lib/active_model/serializable.rb index d23aae7d..3b412d17 100644 --- a/lib/active_model/serializable.rb +++ b/lib/active_model/serializable.rb @@ -52,15 +52,8 @@ module ActiveModel end def instrument(action, &block) - payload = instrumentation_keys.inject({ serializer: self.class.name }) do |payload, key| - payload[:payload] = self.instance_variable_get(:"@#{key}") - payload - end + payload = { serializer: self.class.name } ActiveSupport::Notifications.instrument("#{action}.active_model_serializers", payload, &block) end - - def instrumentation_keys - [:object, :scope, :root, :meta_key, :meta, :wrap_in_array, :only, :except, :key_format] - end end end From 08fd8705f336269d662daed14e124cc760a95888 Mon Sep 17 00:00:00 2001 From: Yosi Attias Date: Fri, 24 Mar 2017 15:16:01 +0300 Subject: [PATCH 2/4] extract the instrumentation key to be const and frozen string --- lib/active_model/default_serializer.rb | 2 +- lib/active_model/serializable.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/active_model/default_serializer.rb b/lib/active_model/default_serializer.rb index 8c755cc2..6c809787 100644 --- a/lib/active_model/default_serializer.rb +++ b/lib/active_model/default_serializer.rb @@ -15,7 +15,7 @@ module ActiveModel end def as_json(options={}) - instrument('!serialize') do + instrument do return [] if @object.nil? && @wrap_in_array hash = @object.as_json @wrap_in_array ? [hash] : hash diff --git a/lib/active_model/serializable.rb b/lib/active_model/serializable.rb index 3b412d17..300869f5 100644 --- a/lib/active_model/serializable.rb +++ b/lib/active_model/serializable.rb @@ -2,12 +2,14 @@ require 'active_model/serializable/utils' module ActiveModel module Serializable + INSTRUMENTATION_KEY = "!serialize.active_model_serializers".freeze + def self.included(base) base.extend Utils end def as_json(options={}) - instrument('!serialize') do + instrument do if root = options.fetch(:root, json_key) hash = { root => serializable_object(options) } hash.merge!(serializable_data) @@ -19,9 +21,7 @@ module ActiveModel end def serializable_object_with_notification(options={}) - instrument('!serialize') do - serializable_object(options) - end + instrument { serializable_object(options) } end def serializable_data @@ -51,9 +51,9 @@ module ActiveModel modules[0..-2].join('::') if modules.size > 1 end - def instrument(action, &block) + def instrument(&block) payload = { serializer: self.class.name } - ActiveSupport::Notifications.instrument("#{action}.active_model_serializers", payload, &block) + ActiveSupport::Notifications.instrument(INSTRUMENTATION_KEY, payload, &block) end end end From 182758889785d1635f03dc1c5719ba30541baa18 Mon Sep 17 00:00:00 2001 From: Yosi Attias Date: Fri, 21 Apr 2017 18:25:27 +0300 Subject: [PATCH 3/4] Adding tests. --- lib/active_model/serializable.rb | 2 +- test/unit/active_model/serilizable_test.rb | 50 ++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/unit/active_model/serilizable_test.rb diff --git a/lib/active_model/serializable.rb b/lib/active_model/serializable.rb index 300869f5..46584039 100644 --- a/lib/active_model/serializable.rb +++ b/lib/active_model/serializable.rb @@ -2,7 +2,7 @@ require 'active_model/serializable/utils' module ActiveModel module Serializable - INSTRUMENTATION_KEY = "!serialize.active_model_serializers".freeze + INSTRUMENTATION_KEY = '!serialize.active_model_serializers'.freeze def self.included(base) base.extend Utils diff --git a/test/unit/active_model/serilizable_test.rb b/test/unit/active_model/serilizable_test.rb new file mode 100644 index 00000000..99060689 --- /dev/null +++ b/test/unit/active_model/serilizable_test.rb @@ -0,0 +1,50 @@ +require 'test_helper' + +module ActiveModel + class SerializableTest + class InstrumentationTest < Minitest::Test + def setup + @events = [] + + @subscriber = ActiveSupport::Notifications.subscribe('!serialize.active_model_serializers') do |name, start, finish, id, payload| + @events << { name: name, serializer: payload[:serializer] } + end + end + + def teardown + ActiveSupport::Notifications.unsubscribe(@subscriber) if defined?(@subscriber) + end + + def test_instruments_default_serializer + DefaultSerializer.new(1).as_json + + assert_equal [{ name: '!serialize.active_model_serializers', serializer: 'ActiveModel::DefaultSerializer' }], @events + end + + def test_instruments_serializer + profile = Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1') + serializer = ProfileSerializer.new(profile) + + serializer.as_json + + assert_equal [{ name: '!serialize.active_model_serializers', serializer: 'ProfileSerializer' }], @events + end + + def test_instruments_array_serializer + profiles = [ + Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1'), + Profile.new(name: 'Name 2', description: 'Description 2', comments: 'Comments 2') + ] + serializer = ArraySerializer.new(profiles) + + serializer.as_json + + assert_equal [ + { name: '!serialize.active_model_serializers', serializer: 'ProfileSerializer' }, + { name: '!serialize.active_model_serializers', serializer: 'ProfileSerializer' }, + { name: '!serialize.active_model_serializers', serializer: 'ActiveModel::ArraySerializer' } + ], @events + end + end + end +end From 9cc4b0531ccd9af25bd20ebaf7903b1ceb96b130 Mon Sep 17 00:00:00 2001 From: Yosi Attias Date: Fri, 21 Apr 2017 19:11:30 +0300 Subject: [PATCH 4/4] Adding changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index acb151f7..94cece74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ### [0-9-stable](https://github.com/rails-api/active_model_serializers/compare/v0.9.5...0-9-stable) +- [#2080](https://github.com/rails-api/active_model_serializers/pull/2080) remove `{ payload: nil }` from `!serialize.active_model_serializers` ActiveSupport::Notification. `payload` never had a value. Changes, for example `{ serializer: 'ActiveModel::DefaultSerializer', payload: nil }` to be `{ serializer: 'ActiveModel::DefaultSerializer' }` (@yosiat) + ### [v0.9.6 (2017-01-10)](https://github.com/rails-api/active_model_serializers/compare/v0.9.5...v0.9.6) - [#2008](https://github.com/rails-api/active_model_serializers/pull/2008) Fix warning on Thor. (@kirs)