Introduce Adapter::Base

Breaking change: - Adapters now inherit Adapter::Base - 'Adapter' is now a module, no longer a class Why? - using a class as a namespace that you also inherit from is complicated and circular at time i.e. buggy (see rails-api#1177) - The class methods on Adapter aren't necessarily related to the instance methods, they're more Adapter functions - named `Base` because it's a Rails-ism - It helps to isolate and highlight what the Adapter interface actually is
bf4 · Sep 20, 2015 · 19de5f7 · 19de5f7
1 parent 7cf0e93
commit 19de5f7
Show file tree

Hide file tree

Showing 30 changed files with 125 additions and 99 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 ### 0.10.0
 
+Breaking changes:
+  * Adapters now inherit Adapter::Base. 'Adapter' is now a module, no longer a class. [@bf4], #1138
+    * using a class as a namespace that you also inherit from is complicated and circular at time i.e.
+      buggy (see https://github.com/rails-api/active_model_serializers/pull/1177)
+    * The class methods on Adapter aren't necessarily related to the instance methods, they're more
+        Adapter functions
+    * named `Base` because it's a Rails-ism
+    * It helps to isolate and highlight what the Adapter interface actually is
+
+Features:
   * adds adapters pattern
   * adds support for `meta` and `meta_key` [@kurko]
   * adds method to override association [@kurko]
@@ -12,3 +22,7 @@
   * adds FlattenJSON as default adapter [@joaomdmoura]
   * adds support for `pagination links` at top level of JsonApi adapter [@bacarini]
   * adds extended format for `include` option to JsonApi adapter [@beauby]
+
+Fixes:
+
+Misc:
diff --git a/lib/active_model/serializer/adapter.rb b/lib/active_model/serializer/adapter.rb
@@ -1,26 +1,30 @@
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       UnknownAdapterError = Class.new(ArgumentError)
       ADAPTER_MAP = {}
       private_constant :ADAPTER_MAP if defined?(private_constant)
       require 'active_model/serializer/adapter/fragment_cache'
       require 'active_model/serializer/adapter/cached_serializer'
 
-      def self.create(resource, options = {})
-        override = options.delete(:adapter)
-        klass = override ? adapter_class(override) : ActiveModel::Serializer.adapter
-        klass.new(resource, options)
-      end
+      class << self # All methods are class functions
+        def new(*args)
+          fail ArgumentError, 'Adapters inherit from Adapter::Base.' \
+            "Adapter.new called with args: '#{args.inspect}', from" \
+            "'caller[0]'."
+        end
 
-      # @see ActiveModel::Serializer::Adapter.lookup
-      def self.adapter_class(adapter)
-        ActiveModel::Serializer::Adapter.lookup(adapter)
-      end
+        def create(resource, options = {})
+          override = options.delete(:adapter)
+          klass = override ? adapter_class(override) : ActiveModel::Serializer.adapter
+          klass.new(resource, options)
+        end
+
+        # @see ActiveModel::Serializer::Adapter.lookup
+        def adapter_class(adapter)
+          ActiveModel::Serializer::Adapter.lookup(adapter)
+        end
 
-      # Only the Adapter class has these methods.
-      # None of the sublasses have them.
-      class << ActiveModel::Serializer::Adapter
         # @return Hash<adapter_name, adapter_class>
         def adapter_map
           ADAPTER_MAP
@@ -76,58 +80,8 @@ def find_by_name(adapter_name)
         private :find_by_name
       end
 
-      # Automatically register adapters when subclassing
-      def self.inherited(subclass)
-        ActiveModel::Serializer::Adapter.register(subclass)
-      end
-
-      attr_reader :serializer, :instance_options
-
-      def initialize(serializer, options = {})
-        @serializer = serializer
-        @instance_options = options
-      end
-
-      def serializable_hash(options = nil)
-        raise NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
-      end
-
-      def as_json(options = nil)
-        hash = serializable_hash(options)
-        include_meta(hash)
-        hash
-      end
-
-      def fragment_cache(*args)
-        raise NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
-      end
-
-      def cache_check(serializer)
-        CachedSerializer.new(serializer).cache_check(self) do
-          yield
-        end
-      end
-
-      private
-
-      def meta
-        serializer.meta if serializer.respond_to?(:meta)
-      end
-
-      def meta_key
-        serializer.meta_key || 'meta'.freeze
-      end
-
-      def root
-        serializer.json_key.to_sym if serializer.json_key
-      end
-
-      def include_meta(json)
-        json[meta_key] = meta if meta
-        json
-      end
-
       # Gotta be at the bottom to use the code above it :(
+      require 'active_model/serializer/adapter/base'
       require 'active_model/serializer/adapter/null'
       require 'active_model/serializer/adapter/attributes'
       require 'active_model/serializer/adapter/json'

diff --git a/lib/active_model/serializer/adapter/attributes.rb b/lib/active_model/serializer/adapter/attributes.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
-    class Adapter
-      class Attributes < Adapter
+    module Adapter
+      class Attributes < Base
         def serializable_hash(options = nil)
           options ||= {}
           if serializer.respond_to?(:each)

diff --git a/lib/active_model/serializer/adapter/base.rb b/lib/active_model/serializer/adapter/base.rb
@@ -0,0 +1,58 @@
+module ActiveModel
+  class Serializer
+    module Adapter
+      class Base
+        # Automatically register adapters when subclassing
+        def self.inherited(subclass)
+          ActiveModel::Serializer::Adapter.register(subclass)
+        end
+
+        attr_reader :serializer, :instance_options
+
+        def initialize(serializer, options = {})
+          @serializer = serializer
+          @instance_options = options
+        end
+
+        def serializable_hash(_options = nil)
+          fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
+        end
+
+        def as_json(options = nil)
+          hash = serializable_hash(options)
+          include_meta(hash)
+          hash
+        end
+
+        def fragment_cache(*_args)
+          fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
+        end
+
+        def cache_check(serializer)
+          CachedSerializer.new(serializer).cache_check(self) do
+            yield
+          end
+        end
+
+        private
+
+        def meta
+          serializer.meta if serializer.respond_to?(:meta)
+        end
+
+        def meta_key
+          serializer.meta_key || 'meta'.freeze
+        end
+
+        def root
+          serializer.json_key.to_sym if serializer.json_key
+        end
+
+        def include_meta(json)
+          json[meta_key] = meta if meta
+          json
+        end
+      end
+    end
+  end
+end
diff --git a/lib/active_model/serializer/adapter/cached_serializer.rb b/lib/active_model/serializer/adapter/cached_serializer.rb
@@ -1,6 +1,6 @@
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class CachedSerializer
         def initialize(serializer)
           @cached_serializer = serializer

diff --git a/lib/active_model/serializer/adapter/fragment_cache.rb b/lib/active_model/serializer/adapter/fragment_cache.rb
@@ -1,6 +1,6 @@
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class FragmentCache
         attr_reader :serializer
 

diff --git a/lib/active_model/serializer/adapter/json.rb b/lib/active_model/serializer/adapter/json.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
-    class Adapter
-      class Json < Adapter
+    module Adapter
+      class Json < Base
         extend ActiveSupport::Autoload
         autoload :FragmentCache
 

diff --git a/lib/active_model/serializer/adapter/json/fragment_cache.rb b/lib/active_model/serializer/adapter/json/fragment_cache.rb
@@ -1,6 +1,6 @@
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class Json
         class FragmentCache
           def fragment_cache(cached_hash, non_cached_hash)

diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
-    class Adapter
-      class JsonApi < Adapter
+    module Adapter
+      class JsonApi < Base
         extend ActiveSupport::Autoload
         autoload :PaginationLinks
         autoload :FragmentCache

diff --git a/lib/active_model/serializer/adapter/json_api/fragment_cache.rb b/lib/active_model/serializer/adapter/json_api/fragment_cache.rb
@@ -1,6 +1,6 @@
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class FragmentCache
           def fragment_cache(root, cached_hash, non_cached_hash)

diff --git a/lib/active_model/serializer/adapter/json_api/pagination_links.rb b/lib/active_model/serializer/adapter/json_api/pagination_links.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
-    class Adapter
-      class JsonApi < Adapter
+    module Adapter
+      class JsonApi < Base
         class PaginationLinks
           FIRST_PAGE = 1
 

diff --git a/lib/active_model/serializer/adapter/null.rb b/lib/active_model/serializer/adapter/null.rb
@@ -1,7 +1,7 @@
 module ActiveModel
   class Serializer
-    class Adapter
-      class Null < Adapter
+    module Adapter
+      class Null < Base
         def serializable_hash(options = nil)
           {}
         end

diff --git a/test/adapter/fragment_cache_test.rb b/test/adapter/fragment_cache_test.rb
@@ -1,7 +1,7 @@
 require 'test_helper'
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class FragmentCacheTest < Minitest::Test
         def setup
           @spam            = Spam::UnrelatedLink.new(id: 'spam-id-1')

diff --git a/test/adapter/json/belongs_to_test.rb b/test/adapter/json/belongs_to_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class Json
         class BelongsToTest < Minitest::Test
           def setup

diff --git a/test/adapter/json/collection_test.rb b/test/adapter/json/collection_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class Json
         class Collection < Minitest::Test
           def setup

diff --git a/test/adapter/json/has_many_test.rb b/test/adapter/json/has_many_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class Json
         class HasManyTestTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/belongs_to_test.rb b/test/adapter/json_api/belongs_to_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class BelongsToTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/collection_test.rb b/test/adapter/json_api/collection_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class CollectionTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/has_many_embed_ids_test.rb b/test/adapter/json_api/has_many_embed_ids_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class HasManyEmbedIdsTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/has_many_explicit_serializer_test.rb b/test/adapter/json_api/has_many_explicit_serializer_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         # Test 'has_many :assocs, serializer: AssocXSerializer'
         class HasManyExplicitSerializerTest < Minitest::Test

diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class HasManyTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/has_one_test.rb b/test/adapter/json_api/has_one_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class HasOneTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/json_api_test.rb b/test/adapter/json_api/json_api_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApiTest < Minitest::Test
         def setup
           ActionController::Base.cache_store.clear

diff --git a/test/adapter/json_api/linked_test.rb b/test/adapter/json_api/linked_test.rb
@@ -1,7 +1,7 @@
 require 'test_helper'
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class LinkedTest < Minitest::Test
           def setup

diff --git a/test/adapter/json_api/pagination_links_test.rb b/test/adapter/json_api/pagination_links_test.rb
@@ -6,7 +6,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class PaginationLinksTest < Minitest::Test
           URI = 'http://example.com'

diff --git a/test/adapter/json_api/resource_type_config_test.rb b/test/adapter/json_api/resource_type_config_test.rb
@@ -2,7 +2,7 @@
 
 module ActiveModel
   class Serializer
-    class Adapter
+    module Adapter
       class JsonApi
         class ResourceTypeConfigTest < Minitest::Test
           def setup