Fix model attributes accessors

lib/active_model_serializers/model.rb

@@ -6,6 +6,17 @@ module ActiveModelSerializers
     include ActiveModel::Serializers::JSON
     include ActiveModel::Model
 
+    # Declare names of attributes to be included in +sttributes+ hash.
+    # Is only available as a class-method since the ActiveModel::Serialization mixin in Rails
+    # uses an +attribute_names+ local variable, which may conflict if we were to add instance methods here.
+    #
+    # @overload attribute_names
+    #   @return [Array<Symbol>]
+    class_attribute :attribute_names, instance_writer: false, instance_reader: false
+    # Initialize +attribute_names+ for all subclasses.  The array is usually
+    # mutated in the +attributes+ method, but can be set directly, as well.
+    self.attribute_names = []
+
     # Easily declare instance attributes with setters and getters for each.
     #
     # All attributes to initialize an instance must have setters.
@@ -25,12 +36,46 @@ module ActiveModelSerializers
     #   @param names [Array<String, Symbol>]
     #   @param name [String, Symbol]
     def self.attributes(*names)
+      self.attribute_names |= names.map(&:to_sym)
       # Silence redefinition of methods warnings
       ActiveModelSerializers.silence_warnings do
         attr_accessor(*names)
       end
     end
 
+    # Opt-in to breaking change
+    def self.derive_attributes_from_names_and_fix_accessors
+      unless included_modules.include?(DeriveAttributesFromNamesAndFixAccessors)
+        prepend(DeriveAttributesFromNamesAndFixAccessors)
+      end
+    end
+
+    module DeriveAttributesFromNamesAndFixAccessors
+      def self.included(base)
+        # NOTE that +id+ will always be in +attributes+.
+        base.attributes :id
+      end
+
+      # Override the initialize method so that attributes aren't processed.
+      #
+      # @param attributes [Hash]
+      def initialize(attributes = {})
+        @errors = ActiveModel::Errors.new(self)
+        super
+      end
+
+      # Override the +attributes+ method so that the hash is derived from +attribute_names+.
+      #
+      # The the fields in +attribute_names+ determines the returned hash.
+      # +attributes+ are returned frozen to prevent any expectations that mutation affects
+      # the actual values in the model.
+      def attributes
+        self.class.attribute_names.each_with_object({}) do |attribute_name, result|
+          result[attribute_name] = public_send(attribute_name).freeze
+        end.with_indifferent_access.freeze
+      end
+    end
+
     # Support for validation and other ActiveModel::Errors
     # @return [ActiveModel::Errors]
     attr_reader :errors

test/action_controller/adapter_selector_test.rb

@@ -3,6 +3,15 @@ require 'test_helper'
 module ActionController
   module Serialization
     class AdapterSelectorTest < ActionController::TestCase
+      class Profile < Model
+        attributes :id, :name, :description
+        associations :comments
+      end
+      class ProfileSerializer < ActiveModel::Serializer
+        type 'profiles'
+        attributes :name, :description
+      end
+
       class AdapterSelectorTestController < ActionController::Base
         def render_using_default_adapter
           @profile = Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1')

test/action_controller/namespace_lookup_test.rb

@@ -4,7 +4,7 @@ module ActionController
   module Serialization
     class NamespaceLookupTest < ActionController::TestCase
       class Book < ::Model
-        attributes :title, :body
+        attributes :id, :title, :body
         associations :writer, :chapters
       end
       class Chapter < ::Model

test/adapter/json/has_many_test.rb

@@ -4,6 +4,10 @@ module ActiveModelSerializers
   module Adapter
     class Json
       class HasManyTestTest < ActiveSupport::TestCase
+        class ModelWithoutSerializer < ::Model
+          attributes :id, :name
+        end
+
         def setup
           ActionController::Base.cache_store.clear
           @author = Author.new(id: 1, name: 'Steve K.')
@@ -16,7 +20,7 @@ module ActiveModelSerializers
           @second_comment.post = @post
           @blog = Blog.new(id: 1, name: 'My Blog!!')
           @post.blog = @blog
-          @tag = Tag.new(id: 1, name: '#hash_tag')
+          @tag = ModelWithoutSerializer.new(id: 1, name: '#hash_tag')
           @post.tags = [@tag]
         end
 
@@ -30,7 +34,11 @@ module ActiveModelSerializers
         end
 
         def test_has_many_with_no_serializer
-          serializer = PostWithTagsSerializer.new(@post)
+          post_serializer_class = Class.new(ActiveModel::Serializer) do
+            attributes :id
+            has_many :tags
+          end
+          serializer = post_serializer_class.new(@post)
           adapter = ActiveModelSerializers::Adapter::Json.new(serializer)
           assert_equal({
             id: 42,

test/adapter/json_api/has_many_test.rb

@@ -4,6 +4,10 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi
       class HasManyTest < ActiveSupport::TestCase
+        class ModelWithoutSerializer < ::Model
+          attributes :id, :name
+        end
+
         def setup
           ActionController::Base.cache_store.clear
           @author = Author.new(id: 1, name: 'Steve K.')
@@ -26,7 +30,7 @@ module ActiveModelSerializers
           @blog.articles = [@post]
           @post.blog = @blog
           @post_without_comments.blog = nil
-          @tag = Tag.new(id: 1, name: '#hash_tag')
+          @tag = ModelWithoutSerializer.new(id: 1, name: '#hash_tag')
           @post.tags = [@tag]
           @serializer = PostSerializer.new(@post)
           @adapter = ActiveModelSerializers::Adapter::JsonApi.new(@serializer)
@@ -129,7 +133,11 @@ module ActiveModelSerializers
         end
 
         def test_has_many_with_no_serializer
-          serializer = PostWithTagsSerializer.new(@post)
+          post_serializer_class = Class.new(ActiveModel::Serializer) do
+            attributes :id
+            has_many :tags
+          end
+          serializer = post_serializer_class.new(@post)
           adapter = ActiveModelSerializers::Adapter::JsonApi.new(serializer)
 
           assert_equal({

test/adapter/json_api/include_data_if_sideloaded_test.rb

@@ -14,11 +14,21 @@ module ActiveModel
               [{ foo: 'bar' }]
             end
           end
+          class Tag < ::Model
+            attributes :id, :name
+          end
 
           class TagSerializer < ActiveModel::Serializer
+            type 'tags'
             attributes :id, :name
           end
 
+          class PostWithTagsSerializer < ActiveModel::Serializer
+            type 'posts'
+            attributes :id
+            has_many :tags
+          end
+
           class IncludeParamAuthorSerializer < ActiveModel::Serializer
             class_attribute :comment_loader
 

test/cache_test.rb

@@ -4,6 +4,20 @@ require 'tempfile'
 
 module ActiveModelSerializers
   class CacheTest < ActiveSupport::TestCase
+    class Article < ::Model
+      attributes :title
+      # To confirm error is raised when cache_key is not set and cache_key option not passed to cache
+      undef_method :cache_key
+    end
+    class ArticleSerializer < ActiveModel::Serializer
+      cache only: [:place], skip_digest: true
+      attributes :title
+    end
+
+    class Author < ::Model
+      attributes :id, :name
+      associations :posts, :bio, :roles
+    end
     # Instead of a primitive cache key (i.e. a string), this class
     # returns a list of objects that require to be expanded themselves.
     class AuthorWithExpandableCacheElements < Author
@@ -27,30 +41,32 @@ module ActiveModelSerializers
         ]
       end
     end
-
     class UncachedAuthor < Author
       # To confirm cache_key is set using updated_at and cache_key option passed to cache
       undef_method :cache_key
     end
+    class AuthorSerializer < ActiveModel::Serializer
+      cache key: 'writer', skip_digest: true
+      attributes :id, :name
 
-    class Article < ::Model
-      attributes :title
-      # To confirm error is raised when cache_key is not set and cache_key option not passed to cache
-      undef_method :cache_key
+      has_many :posts
+      has_many :roles
+      has_one :bio
     end
 
-    class ArticleSerializer < ActiveModel::Serializer
-      cache only: [:place], skip_digest: true
-      attributes :title
+    class Blog < ::Model
+      attributes :name
+      associations :writer
     end
+    class BlogSerializer < ActiveModel::Serializer
+      cache key: 'blog'
+      attributes :id, :name
 
-    class InheritedRoleSerializer < RoleSerializer
-      cache key: 'inherited_role', only: [:name, :special_attribute]
-      attribute :special_attribute
+      belongs_to :writer
     end
 
     class Comment < ::Model
-      attributes :body
+      attributes :id, :body
       associations :post, :author
 
       # Uses a custom non-time-based cache key
@@ -58,14 +74,52 @@ module ActiveModelSerializers
         "comment/#{id}"
       end
     end
+    class CommentSerializer < ActiveModel::Serializer
+      cache expires_in: 1.day, skip_digest: true
+      attributes :id, :body
+      belongs_to :post
+      belongs_to :author
+    end
+
+    class Post < ::Model
+      attributes :id, :title, :body
+      associations :author, :comments, :blog
+    end
+    class PostSerializer < ActiveModel::Serializer
+      cache key: 'post', expires_in: 0.1, skip_digest: true
+      attributes :id, :title, :body
+
+      has_many :comments
+      belongs_to :blog
+      belongs_to :author
+    end
+
+    class Role < ::Model
+      attributes :name, :description, :special_attribute
+      associations :author
+    end
+    class RoleSerializer < ActiveModel::Serializer
+      cache only: [:name, :slug], skip_digest: true
+      attributes :id, :name, :description
+      attribute :friendly_id, key: :slug
+      belongs_to :author
+
+      def friendly_id
+        "#{object.name}-#{object.id}"
+      end
+    end
+    class InheritedRoleSerializer < RoleSerializer
+      cache key: 'inherited_role', only: [:name, :special_attribute]
+      attribute :special_attribute
+    end
 
     setup do
       cache_store.clear
       @comment        = Comment.new(id: 1, body: 'ZOMG A COMMENT')
-      @post           = Post.new(title: 'New Post', body: 'Body')
+      @post           = Post.new(id: 'post', title: 'New Post', body: 'Body')
       @bio            = Bio.new(id: 1, content: 'AMS Contributor')
-      @author         = Author.new(name: 'Joao M. D. Moura')
-      @blog           = Blog.new(id: 999, name: 'Custom blog', writer: @author, articles: [])
+      @author         = Author.new(id: 'author', name: 'Joao M. D. Moura')
+      @blog           = Blog.new(id: 999, name: 'Custom blog', writer: @author)
       @role           = Role.new(name: 'Great Author')
       @location       = Location.new(lat: '-23.550520', lng: '-46.633309')
       @place          = Place.new(name: 'Amazing Place')
@@ -325,12 +379,14 @@ module ActiveModelSerializers
 
     def test_uses_file_digest_in_cache_key
       render_object_with_cache(@blog)
-      key = "#{@blog.cache_key}/#{adapter.cache_key}/#{::Model::FILE_DIGEST}"
+      file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read)
+      key = "#{@blog.cache_key}/#{adapter.cache_key}/#{file_digest}"
       assert_equal(@blog_serializer.attributes, cache_store.fetch(key))
     end
 
     def test_cache_digest_definition
-      assert_equal(::Model::FILE_DIGEST, @post_serializer.class._cache_digest)
+      file_digest = Digest::MD5.hexdigest(File.open(__FILE__).read)
+      assert_equal(file_digest, @post_serializer.class._cache_digest)
     end
 
     def test_object_cache_keys
@@ -532,7 +588,7 @@ module ActiveModelSerializers
       role_hash = role_serializer.fetch_attributes_fragment(adapter_instance)
       assert_equal(role_hash, expected_result)
 
-      role.attributes[:id] = 'this has been updated'
+      role.id = 'this has been updated'
       role.name = 'this was cached'
 
       role_hash = role_serializer.fetch_attributes_fragment(adapter_instance)

test/fixtures/poro.rb

@@ -1,6 +1,4 @@
 class Model < ActiveModelSerializers::Model
-  FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read)
-
   attr_writer :id
 
   # At this time, just for organization of intent
@@ -108,10 +106,6 @@ class PostPreviewSerializer < ActiveModel::Serializer
   has_many :comments, serializer: ::CommentPreviewSerializer
   belongs_to :author, serializer: ::AuthorPreviewSerializer
 end
-class PostWithTagsSerializer < ActiveModel::Serializer
-  attributes :id
-  has_many :tags
-end
 class PostWithCustomKeysSerializer < ActiveModel::Serializer
   attributes :id
   has_many :comments, key: :reviews
@@ -208,10 +202,6 @@ module Spam
   end
 end
 
-class Tag < Model
-  attributes :name
-end
-
 class VirtualValue < Model; end
 class VirtualValueSerializer < ActiveModel::Serializer
   attributes :id

test/serializers/associations_test.rb

@@ -2,13 +2,17 @@ require 'test_helper'
 module ActiveModel
   class Serializer
     class AssociationsTest < ActiveSupport::TestCase
+      class ModelWithoutSerializer < ::Model
+        attributes :id, :name
+      end
+
       def setup
         @author = Author.new(name: 'Steve K.')
         @author.bio = nil
         @author.roles = []
         @blog = Blog.new(name: 'AMS Blog')
         @post = Post.new(title: 'New Post', body: 'Body')
-        @tag = Tag.new(id: 'tagid', name: '#hashtagged')
+        @tag = ModelWithoutSerializer.new(id: 'tagid', name: '#hashtagged')
         @comment = Comment.new(id: 1, body: 'ZOMG A COMMENT')
         @post.comments = [@comment]
         @post.tags = [@tag]
@@ -46,7 +50,11 @@ module ActiveModel
       end
 
       def test_has_many_with_no_serializer
-        PostWithTagsSerializer.new(@post).associations.each do |association|
+        post_serializer_class = Class.new(ActiveModel::Serializer) do
+          attributes :id
+          has_many :tags
+        end
+        post_serializer_class.new(@post).associations.each do |association|
           key = association.key
           serializer = association.serializer
           options = association.options