diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a313e63..a1d5d990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,12 @@ Breaking changes: Features: -- [#2021](https://github.com/rails-api/active_model_serializers/pull/2021) ActiveModelSerializers::Model#attributes. (@bf4) +- [#2021](https://github.com/rails-api/active_model_serializers/pull/2021) ActiveModelSerializers::Model#attributes. Originally in [#1982](https://github.com/rails-api/active_model_serializers/pull/1982). (@bf4) Fixes: +- [#2022](https://github.com/rails-api/active_model_serializers/pull/2022) Mutation of ActiveModelSerializers::Model now changes the attributes. Originally in [#1984](https://github.com/rails-api/active_model_serializers/pull/1984). (@bf4) + Misc: - [#2021](https://github.com/rails-api/active_model_serializers/pull/2021) Make test attributes explicit. Tests have Model#associations. (@bf4) diff --git a/docs/howto/serialize_poro.md b/docs/howto/serialize_poro.md index 61696e86..20091c52 100644 --- a/docs/howto/serialize_poro.md +++ b/docs/howto/serialize_poro.md @@ -29,7 +29,7 @@ define and validate which methods ActiveModelSerializers expects to be implement An implementation of the complete spec is included either for use or as reference: [`ActiveModelSerializers::Model`](../../lib/active_model_serializers/model.rb). -You can use in production code that will make your PORO a lot cleaner. +You can use in production code that will make your PORO a lot cleaner. The above code now becomes: diff --git a/lib/active_model_serializers/model.rb b/lib/active_model_serializers/model.rb index e49bea33..b61661bc 100644 --- a/lib/active_model_serializers/model.rb +++ b/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] + 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] # @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 diff --git a/test/action_controller/adapter_selector_test.rb b/test/action_controller/adapter_selector_test.rb index 6f22aae2..db93573b 100644 --- a/test/action_controller/adapter_selector_test.rb +++ b/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') diff --git a/test/action_controller/namespace_lookup_test.rb b/test/action_controller/namespace_lookup_test.rb index f40cca11..b5c8f496 100644 --- a/test/action_controller/namespace_lookup_test.rb +++ b/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 @@ -86,7 +86,7 @@ module ActionController book = Book.new(title: 'New Post', body: 'Body') # because this is a string, ruby can't auto-lookup the constant, so otherwise - # the looku things we mean ::Api::V2 + # the lookup thinks we mean ::Api::V2 render json: book, namespace: 'ActionController::Serialization::NamespaceLookupTest::Api::V2' end @@ -94,7 +94,7 @@ module ActionController book = Book.new(title: 'New Post', body: 'Body') # because this is a string, ruby can't auto-lookup the constant, so otherwise - # the looku things we mean ::Api::V2 + # the lookup thinks we mean ::Api::V2 render json: book, namespace: :'ActionController::Serialization::NamespaceLookupTest::Api::V2' end diff --git a/test/active_model_serializers/model_test.rb b/test/active_model_serializers/model_test.rb index 235761c5..6a8a29af 100644 --- a/test/active_model_serializers/model_test.rb +++ b/test/active_model_serializers/model_test.rb @@ -49,6 +49,33 @@ module ActiveModelSerializers assert_equal 1, instance.read_attribute_for_serialization(:one) end + def test_attributes_can_be_read_for_serialization_with_attributes_accessors_fix + klass = Class.new(ActiveModelSerializers::Model) do + derive_attributes_from_names_and_fix_accessors + attributes :one, :two, :three + end + original_attributes = { one: 1, two: 2, three: 3 } + original_instance = klass.new(original_attributes) + + # Initial value + instance = original_instance + expected_attributes = { one: 1, two: 2, three: 3 }.with_indifferent_access + assert_equal expected_attributes, instance.attributes + assert_equal 1, instance.one + assert_equal 1, instance.read_attribute_for_serialization(:one) + + expected_attributes = { one: :not_one, two: 2, three: 3 }.with_indifferent_access + # Change via accessor + instance = original_instance.dup + instance.one = :not_one + assert_equal expected_attributes, instance.attributes + assert_equal :not_one, instance.one + assert_equal :not_one, instance.read_attribute_for_serialization(:one) + + # Attributes frozen + assert instance.attributes.frozen? + end + def test_id_attribute_can_be_read_for_serialization klass = Class.new(ActiveModelSerializers::Model) do attributes :id, :one, :two, :three @@ -81,5 +108,35 @@ module ActiveModelSerializers ensure self.class.send(:remove_const, :SomeTestModel) end + + def test_id_attribute_can_be_read_for_serialization_with_attributes_accessors_fix + klass = Class.new(ActiveModelSerializers::Model) do + derive_attributes_from_names_and_fix_accessors + attributes :id, :one, :two, :three + end + self.class.const_set(:SomeTestModel, klass) + original_attributes = { id: :ego, one: 1, two: 2, three: 3 } + original_instance = klass.new(original_attributes) + + # Initial value + instance = original_instance.dup + expected_attributes = { id: :ego, one: 1, two: 2, three: 3 }.with_indifferent_access + assert_equal expected_attributes, instance.attributes + assert_equal :ego, instance.id + assert_equal :ego, instance.read_attribute_for_serialization(:id) + + expected_attributes = { id: :superego, one: 1, two: 2, three: 3 }.with_indifferent_access + # Change via accessor + instance = original_instance.dup + instance.id = :superego + assert_equal expected_attributes, instance.attributes + assert_equal :superego, instance.id + assert_equal :superego, instance.read_attribute_for_serialization(:id) + + # Attributes frozen + assert instance.attributes.frozen? + ensure + self.class.send(:remove_const, :SomeTestModel) + end end end diff --git a/test/adapter/json/has_many_test.rb b/test/adapter/json/has_many_test.rb index 3f6fa546..feeec93c 100644 --- a/test/adapter/json/has_many_test.rb +++ b/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, diff --git a/test/adapter/json_api/has_many_test.rb b/test/adapter/json_api/has_many_test.rb index 9da73af9..a9fa9ac9 100644 --- a/test/adapter/json_api/has_many_test.rb +++ b/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({ diff --git a/test/adapter/json_api/include_data_if_sideloaded_test.rb b/test/adapter/json_api/include_data_if_sideloaded_test.rb index b5c81869..c0da9488 100644 --- a/test/adapter/json_api/include_data_if_sideloaded_test.rb +++ b/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 diff --git a/test/cache_test.rb b/test/cache_test.rb index f9a10464..f0958931 100644 --- a/test/cache_test.rb +++ b/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) diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 384610cc..6245ad23 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -1,5 +1,5 @@ class Model < ActiveModelSerializers::Model - FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) + rand(2).zero? && derive_attributes_from_names_and_fix_accessors attr_writer :id @@ -108,10 +108,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 +204,6 @@ module Spam end end -class Tag < Model - attributes :name -end - class VirtualValue < Model; end class VirtualValueSerializer < ActiveModel::Serializer attributes :id diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index 6d6447c3..90d213dc 100644 --- a/test/serializers/associations_test.rb +++ b/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