From d1c44c719d08df24c2d8d26bda7bfd4201804972 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 10 Nov 2015 03:21:40 -0600 Subject: [PATCH] Update for review per maurogeorge feedback --- README.md | 4 + docs/general/instrumentation.md | 22 ++- lib/action_controller/serialization.rb | 3 + lib/active_model_serializers.rb | 2 - lib/active_model_serializers/logging.rb | 176 ++++++++++++------------ 5 files changed, 118 insertions(+), 89 deletions(-) diff --git a/README.md b/README.md index e2bab278..684b39c1 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,10 @@ All serializable resources must pass the ActiveModel::Serializer::Lint::Tests. See the ActiveModelSerializers::Model for a base class that implements the full API for a plain-old Ruby object (PORO). +## Hooks + +To run a hook when ActiveModelSerializers is loaded, use `ActiveSupport.on_load(:active_model_serializers) do end` + ## Getting Help If you find a bug, please report an [Issue](https://github.com/rails-api/active_model_serializers/issues/new). diff --git a/docs/general/instrumentation.md b/docs/general/instrumentation.md index 3adaefbd..160a9e76 100644 --- a/docs/general/instrumentation.md +++ b/docs/general/instrumentation.md @@ -1,7 +1,8 @@ # Instrumentation -ActiveModelSerializers uses the ActiveSupport::Notification API, which -allows for subscribing to events, such as for logging. +ActiveModelSerializers uses the +[ActiveSupport::Notification API](http://guides.rubyonrails.org/active_support_instrumentation.html#subscribing-to-an-event), +which allows for subscribing to events, such as for logging. ## Events @@ -17,3 +18,20 @@ Payload (example): adapter: ActiveModel::Serializer::Adapter::Attributes } ``` + +Subscribing: + +```ruby +ActiveSupport::Notifications.subscribe 'render.active_model_serializers' do |name, started, finished, unique_id, data| + # whatever +end +ActiveSupport::Notifications.subscribe 'render.active_model_serializers' do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + # event.payload + # whatever +end + +## [LogSubscriber](http://api.rubyonrails.org/classes/ActiveSupport/LogSubscriber.html) + +ActiveModelSerializers includes an `ActiveModelSerializers::LogSubscriber` that attaches to +`render.active_model_serializers`. diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index 901653af..5fefaedf 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -31,6 +31,9 @@ module ActionController serializable_resource.serialization_scope ||= serialization_scope serializable_resource.serialization_scope_name = _serialization_scope begin + # Necessary to ensure we have an adapter for the serializable resource + # after it has been figured. + # TODO: This logic should be less opaque and probably moved into the SerializableResource. serializable_resource.tap(&:adapter) rescue ActiveModel::Serializer::CollectionSerializer::NoSerializerError resource diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index ce09b08a..c7a4d1d4 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -1,7 +1,5 @@ require 'active_model' require 'active_support' -require 'active_support/tagged_logging' -require 'active_support/logger' require 'action_controller' require 'action_controller/railtie' module ActiveModelSerializers diff --git a/lib/active_model_serializers/logging.rb b/lib/active_model_serializers/logging.rb index 5b30a1a1..cc1cdc34 100644 --- a/lib/active_model_serializers/logging.rb +++ b/lib/active_model_serializers/logging.rb @@ -3,108 +3,114 @@ # # https://github.com/rails/rails/blob/280654ef88/activejob/lib/active_job/logging.rb # -module ActiveModelSerializers::Logging - extend ActiveSupport::Concern +module ActiveModelSerializers + module Logging + extend ActiveSupport::Concern - included do - include ActiveModelSerializers::Callbacks - extend NotificationMacro - instrument_rendering - end + included do + include ActiveModelSerializers::Callbacks + extend Macros + instrument_rendering + end - module ClassMethods - def instrument_rendering - around_render do |args, block| - tag_logger do - notify_render do - block.call(args) + module ClassMethods + def instrument_rendering + around_render do |args, block| + tag_logger do + notify_render do + block.call(args) + end end end end end - end - # Adapted from: - # https://github.com/rubygems/rubygems/blob/cb28f5e991/lib/rubygems/deprecate.rb - # Provides a single method +notify+ to be used to declare when - # something a method notifies, with the argument +callback_name+ of the notification callback. - # - # class Adapter - # def self.klass_method - # # ... - # end - # - # def instance_method - # # ... - # end - # - # include ActiveModelSerializers::Logging - # notify :instance_method, :render - # - # class << self - # extend ActiveModelSerializers::Logging::NotificationMacro - # notify :klass_method, :render - # end - # end - module NotificationMacro - ## - # Simple notify method that wraps up +name+ - # in a dummy method. It notifies on with the +callback_name+ notifier on - # each call to the dummy method, telling what the current serializer and adapter - # are being rendered. + # Macros that can be used to customize the logging of class or instance methods, + # by extending the class or its singleton. + # # Adapted from: # https://github.com/rubygems/rubygems/blob/cb28f5e991/lib/rubygems/deprecate.rb - def notify(name, callback_name) - class_eval do - old = "_notifying_#{callback_name}_#{name}" - alias_method old, name - define_method name do |*args, &block| - run_callbacks callback_name do - send old, *args, &block + # + # Provides a single method +notify+ to be used to declare when + # something a method notifies, with the argument +callback_name+ of the notification callback. + # + # class Adapter + # def self.klass_method + # # ... + # end + # + # def instance_method + # # ... + # end + # + # include ActiveModelSerializers::Logging::Macros + # notify :instance_method, :render + # + # class << self + # extend ActiveModelSerializers::Logging::Macros + # notify :klass_method, :render + # end + # end + module Macros + ## + # Simple notify method that wraps up +name+ + # in a dummy method. It notifies on with the +callback_name+ notifier on + # each call to the dummy method, telling what the current serializer and adapter + # are being rendered. + # Adapted from: + # https://github.com/rubygems/rubygems/blob/cb28f5e991/lib/rubygems/deprecate.rb + def notify(name, callback_name) + class_eval do + old = "_notifying_#{callback_name}_#{name}" + alias_method old, name + define_method name do |*args, &block| + run_callbacks callback_name do + send old, *args, &block + end end end end end - end - def notify_render(*) - event_name = 'render.active_model_serializers'.freeze - ActiveSupport::Notifications.instrument(event_name, notify_render_payload) do - yield - end - end - - def notify_render_payload - { serializer: serializer, adapter: adapter } - end - - private - - def tag_logger(*tags) - if ActiveModelSerializers.logger.respond_to?(:tagged) - tags.unshift 'AMS'.freeze unless logger_tagged_by_active_model_serializers? - ActiveModelSerializers.logger.tagged(*tags) { yield } - else - yield - end - end - - def logger_tagged_by_active_model_serializers? - ActiveModelSerializers.logger.formatter.current_tags.include?('AMS'.freeze) - end - - class LogSubscriber < ActiveSupport::LogSubscriber - def render(event) - info do - serializer = event.payload[:serializer] - adapter = event.payload[:adapter] - duration = event.duration.round(2) - "Rendered #{serializer.name} with #{adapter.class} (#{duration}ms)" + def notify_render(*) + event_name = 'render.active_model_serializers'.freeze + ActiveSupport::Notifications.instrument(event_name, notify_render_payload) do + yield end end - def logger - ActiveModelSerializers.logger + def notify_render_payload + { serializer: serializer, adapter: adapter } + end + + private + + def tag_logger(*tags) + if ActiveModelSerializers.logger.respond_to?(:tagged) + tags.unshift 'AMS'.freeze unless logger_tagged_by_active_model_serializers? + ActiveModelSerializers.logger.tagged(*tags) { yield } + else + yield + end + end + + def logger_tagged_by_active_model_serializers? + ActiveModelSerializers.logger.formatter.current_tags.include?('AMS'.freeze) + end + + class LogSubscriber < ActiveSupport::LogSubscriber + def render(event) + info do + serializer = event.payload[:serializer] + adapter = event.payload[:adapter] + duration = event.duration.round(2) + "Rendered #{serializer.name} with #{adapter.class} (#{duration}ms)" + end + end + + def logger + ActiveModelSerializers.logger + end end end end