Make test attributes explicit

lib/active_model_serializers/model.rb

@@ -3,42 +3,50 @@
 # serializable non-activerecord objects.
 module ActiveModelSerializers
   class Model
-    include ActiveModel::Model
     include ActiveModel::Serializers::JSON
+    include ActiveModel::Validations
+    include ActiveModel::Conversion
+    extend ActiveModel::Naming
+    extend ActiveModel::Translation
+
+    class_attribute :attribute_names
+    self.attribute_names = []
 
     def self.attributes(*names)
       attr_accessor(*names)
+      self.attribute_names = attribute_names | names.map(&:to_sym)
     end
 
-    attr_reader :attributes, :errors
-
-    def initialize(attributes = {})
-      @attributes = attributes && attributes.symbolize_keys
-      @errors = ActiveModel::Errors.new(self)
-      super
-    end
+    attributes :id
+    attr_writer :updated_at
 
     # Defaults to the downcased model name.
     def id
-      attributes.fetch(:id) { self.class.name.downcase }
+      @id ||= self.class.name.downcase
     end
 
     # Defaults to the downcased model name and updated_at
     def cache_key
-      attributes.fetch(:cache_key) { "#{self.class.name.downcase}/#{id}-#{updated_at.strftime('%Y%m%d%H%M%S%9N')}" }
+      "#{self.class.name.downcase}/#{id}-#{updated_at.strftime('%Y%m%d%H%M%S%9N')}"
     end
 
     # Defaults to the time the serializer file was modified.
     def updated_at
-      attributes.fetch(:updated_at) { File.mtime(__FILE__) }
+      defined?(@updated_at) ? @updated_at : File.mtime(__FILE__)
     end
 
-    def read_attribute_for_serialization(key)
-      if key == :id || key == 'id'
-        attributes.fetch(key) { id }
-      else
-        attributes[key]
-      end
+    attr_reader :errors
+
+    def initialize(attributes = {})
+      assign_attributes(attributes) if attributes
+      @errors = ActiveModel::Errors.new(self)
+      super()
+    end
+
+    def attributes
+      attribute_names.each_with_object({}) do |attribute_name, result|
+        result[attribute_name] = public_send(attribute_name)
+      end.with_indifferent_access
     end
 
     # The following methods are needed to be minimally implemented for ActiveModel::Errors
@@ -51,5 +59,43 @@ def self.lookup_ancestors
       [self]
     end
     # :nocov:
+
+    def assign_attributes(new_attributes)
+      unless new_attributes.respond_to?(:stringify_keys)
+        fail ArgumentError, 'When assigning attributes, you must pass a hash as an argument.'
+      end
+      return if new_attributes.blank?
+
+      attributes = new_attributes.stringify_keys
+      _assign_attributes(attributes)
+    end
+
+    private
+
+    def _assign_attributes(attributes)
+      attributes.each do |k, v|
+        _assign_attribute(k, v)
+      end
+    end
+
+    def _assign_attribute(k, v)
+      fail UnknownAttributeError.new(self, k) unless respond_to?("#{k}=")
+      public_send("#{k}=", v)
+    end
+
+    def persisted?
+      false
+    end
+
+    # Raised when unknown attributes are supplied via mass assignment.
+    class UnknownAttributeError < NoMethodError
+      attr_reader :record, :attribute
+
+      def initialize(record, attribute)
+        @record = record
+        @attribute = attribute
+        super("unknown attribute '#{attribute}' for #{@record.class}.")
+      end
+    end
   end
 end

test/action_controller/adapter_selector_test.rb

@@ -15,7 +15,7 @@ def render_using_adapter_override
         end
 
         def render_skipping_adapter
-          @profile = Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1')
+          @profile = Profile.new(id: 'render_skipping_adapter_id', name: 'Name 1', description: 'Description 1', comments: 'Comments 1')
           render json: @profile, adapter: false
         end
       end
@@ -46,7 +46,7 @@ def test_render_using_adapter_override
 
       def test_render_skipping_adapter
         get :render_skipping_adapter
-        assert_equal '{"name":"Name 1","description":"Description 1","comments":"Comments 1"}', response.body
+        assert_equal '{"id":"render_skipping_adapter_id","name":"Name 1","description":"Description 1","comments":"Comments 1"}', response.body
       end
     end
   end

test/action_controller/json_api/transform_test.rb

@@ -5,9 +5,9 @@ module Serialization
     class JsonApi
       class KeyTransformTest < ActionController::TestCase
         class KeyTransformTestController < ActionController::Base
-          class Post < ::Model; end
-          class Author < ::Model; end
-          class TopComment < ::Model; end
+          class Post < ::Model; attributes :title, :body, :author, :top_comments, :publish_at end
+          class Author < ::Model; attributes :first_name, :last_name end
+          class TopComment < ::Model; attributes :body, :author, :post end
           class PostSerializer < ActiveModel::Serializer
             type 'posts'
             attributes :title, :body, :publish_at

test/action_controller/namespace_lookup_test.rb

@@ -3,10 +3,9 @@
 module ActionController
   module Serialization
     class NamespaceLookupTest < ActionController::TestCase
-      class Book < ::Model; end
-      class Page < ::Model; end
-      class Chapter < ::Model; end
-      class Writer < ::Model; end
+      class Book < ::Model; attributes :title, :body, :writer, :chapters end
+      class Chapter < ::Model; attributes :title end
+      class Writer < ::Model; attributes :name end
 
       module Api
         module V2
@@ -50,7 +49,7 @@ class LookupTestController < ActionController::Base
 
             def implicit_namespaced_serializer
               writer = Writer.new(name: 'Bob')
-              book = Book.new(title: 'New Post', body: 'Body', writer: writer, chapters: [])
+              book = Book.new(id: 'bookid', title: 'New Post', body: 'Body', writer: writer, chapters: [])
 
               render json: book
             end
@@ -93,7 +92,7 @@ def explicit_namespace_as_symbol
             end
 
             def invalid_namespace
-              book = Book.new(title: 'New Post', body: 'Body')
+              book = Book.new(id: 'invalid_namespace_book_id', title: 'New Post', body: 'Body')
 
               render json: book, namespace: :api_v2
             end
@@ -205,7 +204,7 @@ def namespace_set_by_request_headers
 
         assert_serializer ActiveModel::Serializer::Null
 
-        expected = { 'title' => 'New Post', 'body' => 'Body' }
+        expected = { 'id' => 'invalid_namespace_book_id', 'title' => 'New Post', 'body' => 'Body', 'writer' => nil, 'chapters' => nil }
         actual = JSON.parse(@response.body)
 
         assert_equal expected, actual

test/adapter/json_api/fields_test.rb

@@ -4,9 +4,9 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi
       class FieldsTest < ActiveSupport::TestCase
-        class Post < ::Model; end
-        class Author < ::Model; end
-        class Comment < ::Model; end
+        class Post < ::Model; attributes :title, :body, :author, :comments end
+        class Author < ::Model; attributes :name, :birthday end
+        class Comment < ::Model; attributes :body, :author, :post end
 
         class PostSerializer < ActiveModel::Serializer
           type 'posts'

test/adapter/json_api/include_data_if_sideloaded_test.rb

@@ -5,7 +5,9 @@ class Serializer
     module Adapter
       class JsonApi
         class IncludeParamTest < ActiveSupport::TestCase
-          IncludeParamAuthor = Class.new(::Model)
+          IncludeParamAuthor = Class.new(::Model) do
+            attributes :tags, :posts
+          end
 
           class CustomCommentLoader
             def all

test/adapter/json_api/linked_test.rb

@@ -1,6 +1,6 @@
 require 'test_helper'
 
-class NestedPost < ::Model; end
+class NestedPost < ::Model; attributes :nested_posts end
 class NestedPostSerializer < ActiveModel::Serializer
   has_many :nested_posts
 end
@@ -301,8 +301,8 @@ def test_nil_link_with_specified_serializer
       end
 
       class NoDuplicatesTest < ActiveSupport::TestCase
-        class Post < ::Model; end
-        class Author < ::Model; end
+        class Post < ::Model; attributes :author end
+        class Author < ::Model; attributes :posts, :roles, :bio end
 
         class PostSerializer < ActiveModel::Serializer
           type 'posts'

test/adapter/json_api/links_test.rb

@@ -4,7 +4,7 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi
       class LinksTest < ActiveSupport::TestCase
-        class LinkAuthor < ::Model; end
+        class LinkAuthor < ::Model; attributes :posts end
         class LinkAuthorSerializer < ActiveModel::Serializer
           link :self do
             href "http://example.com/link_author/#{object.id}"

test/adapter/json_api/transform_test.rb

@@ -4,9 +4,9 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi
       class KeyCaseTest < ActiveSupport::TestCase
-        class Post < ::Model; end
-        class Author < ::Model; end
-        class Comment < ::Model; end
+        class Post < ::Model; attributes :title, :body, :publish_at, :author, :comments end
+        class Author < ::Model; attributes :first_name, :last_name end
+        class Comment < ::Model; attributes :body, :author, :post end
 
         class PostSerializer < ActiveModel::Serializer
           type 'posts'

test/cache_test.rb

@@ -34,6 +34,7 @@ class UncachedAuthor < Author
     end
 
     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
@@ -48,6 +49,10 @@ class InheritedRoleSerializer < RoleSerializer
       attribute :special_attribute
     end
 
+    class Comment < ::Model
+      attributes :body, :post, :author
+    end
+
     setup do
       cache_store.clear
       @comment        = Comment.new(id: 1, body: 'ZOMG A COMMENT')
@@ -271,7 +276,7 @@ def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attribut
         ended_at: nil,
         updated_at: alert.updated_at,
         created_at: alert.created_at
-      }
+      }.with_indifferent_access
       expected_cached_jsonapi_attributes = {
         id: '1',
         type: 'alerts',
@@ -283,15 +288,15 @@ def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attribut
           updated_at: alert.updated_at,
           created_at: alert.created_at
         }
-      }
+      }.with_indifferent_access
 
       # Assert attributes are serialized correctly
       serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :attributes)
-      attributes_serialization = serializable_alert.as_json
+      attributes_serialization = serializable_alert.as_json.with_indifferent_access
       assert_equal expected_fetch_attributes, alert.attributes
       assert_equal alert.attributes, attributes_serialization
       attributes_cache_key = serializable_alert.adapter.serializer.cache_key(serializable_alert.adapter)
-      assert_equal attributes_serialization, cache_store.fetch(attributes_cache_key)
+      assert_equal attributes_serialization, cache_store.fetch(attributes_cache_key).with_indifferent_access
 
       serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :json_api)
       jsonapi_cache_key = serializable_alert.adapter.serializer.cache_key(serializable_alert.adapter)
@@ -303,7 +308,7 @@ def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attribut
       serializable_alert = serializable(alert, serializer: UncachedAlertSerializer, adapter: :json_api)
       assert_equal serializable_alert.as_json, jsonapi_serialization
 
-      cached_serialization = cache_store.fetch(jsonapi_cache_key)
+      cached_serialization = cache_store.fetch(jsonapi_cache_key).with_indifferent_access
       assert_equal expected_cached_jsonapi_attributes, cached_serialization
     ensure
       Object.send(:remove_const, :Alert)
@@ -323,17 +328,26 @@ def test_cache_digest_definition
     end
 
     def test_object_cache_keys
+      class << @comment
+        def cache_key
+          "comment/#{id}"
+        end
+      end
       serializable = ActiveModelSerializers::SerializableResource.new([@comment, @comment])
       include_directive = JSONAPI::IncludeDirective.new('*', allow_wildcard: true)
 
       actual = ActiveModel::Serializer.object_cache_keys(serializable.adapter.serializer, serializable.adapter, include_directive)
 
       assert_equal 3, actual.size
-      assert actual.any? { |key| key == "comment/1/#{serializable.adapter.cache_key}" }
-      assert actual.any? { |key| key =~ %r{post/post-\d+} }
-      assert actual.any? { |key| key =~ %r{author/author-\d+} }
+      expected_key = "comment/1/#{serializable.adapter.cache_key}"
+      assert actual.any? { |key| key == expected_key }, "actual '#{actual}' should include #{expected_key}"
+      expected_key = %r{post/post-\d+}
+      assert actual.any? { |key| key =~ expected_key }, "actual '#{actual}' should match '#{expected_key}'"
+      expected_key = %r{author/author-\d+}
+      assert actual.any? { |key| key =~ expected_key }, "actual '#{actual}' should match '#{expected_key}'"
     end
 
+    # rubocop:disable Metrics/AbcSize
     def test_fetch_attributes_from_cache
       serializers = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
 
@@ -344,20 +358,21 @@ def test_fetch_attributes_from_cache
         adapter_options = {}
         adapter_instance = ActiveModelSerializers::Adapter::Attributes.new(serializers, adapter_options)
         serializers.serializable_hash(adapter_options, options, adapter_instance)
-        cached_attributes = adapter_options.fetch(:cached_attributes)
+        cached_attributes = adapter_options.fetch(:cached_attributes).with_indifferent_access
 
         include_directive = ActiveModelSerializers.default_include_directive
-        manual_cached_attributes = ActiveModel::Serializer.cache_read_multi(serializers, adapter_instance, include_directive)
+        manual_cached_attributes = ActiveModel::Serializer.cache_read_multi(serializers, adapter_instance, include_directive).with_indifferent_access
         assert_equal manual_cached_attributes, cached_attributes
 
-        assert_equal cached_attributes["#{@comment.cache_key}/#{adapter_instance.cache_key}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes
-        assert_equal cached_attributes["#{@comment.post.cache_key}/#{adapter_instance.cache_key}"], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes
+        assert_equal cached_attributes["#{@comment.cache_key}/#{adapter_instance.cache_key}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes.reject { |_, v| v.nil? }
+        assert_equal cached_attributes["#{@comment.post.cache_key}/#{adapter_instance.cache_key}"], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes.reject { |_, v| v.nil? }
 
         writer = @comment.post.blog.writer
         writer_cache_key = writer.cache_key
-        assert_equal cached_attributes["#{writer_cache_key}/#{adapter_instance.cache_key}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes
+        assert_equal cached_attributes["#{writer_cache_key}/#{adapter_instance.cache_key}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes.reject { |_, v| v.nil? }
       end
     end
+    # rubocop:enable Metrics/AbcSize
 
     def test_cache_read_multi_with_fragment_cache_enabled
       post_serializer = Class.new(ActiveModel::Serializer) do