From 995bbcc18d6b270cdfb0ee443fc596bcc3d698f9 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Fri, 28 Aug 2015 21:06:10 +0200 Subject: [PATCH 1/6] Fix definition of serializer attributes with multiple calls to `attribute` instead of one single call to `attributes`. --- lib/active_model/serializer.rb | 2 +- test/fixtures/poro.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index a9f0e756..6e6833bb 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -70,7 +70,7 @@ module ActiveModel ActiveModelSerializers.silence_warnings do define_method key do object.read_attribute_for_serialization(attr) - end unless respond_to?(key, false) || _fragmented.respond_to?(attr) + end unless (key != :id && method_defined?(key)) || _fragmented.respond_to?(attr) end end diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 0c0e3a58..7293e546 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -134,7 +134,8 @@ end AuthorSerializer = Class.new(ActiveModel::Serializer) do cache key:'writer', skip_digest: true - attributes :id, :name + attribute :id + attribute :name has_many :posts, embed: :ids has_many :roles, embed: :ids From c5446d759fe5426ef68e779eed343fbf05e80267 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Mon, 31 Aug 2015 01:19:47 +0200 Subject: [PATCH 2/6] Remove traces of `embed` option. --- lib/active_model/serializer/association.rb | 2 +- test/fixtures/poro.rb | 4 ++-- test/serializers/association_macros_test.rb | 4 ++-- test/serializers/associations_test.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/active_model/serializer/association.rb b/lib/active_model/serializer/association.rb index af1cb914..bca03665 100644 --- a/lib/active_model/serializer/association.rb +++ b/lib/active_model/serializer/association.rb @@ -7,7 +7,7 @@ module ActiveModel # @param [Hash{Symbol => Object}] options # # @example - # Association.new(:comments, CommentSummarySerializer, embed: :ids) + # Association.new(:comments, CommentSummarySerializer) # Association = Struct.new(:name, :serializer, :options) do diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 0c0e3a58..0f099efe 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -136,8 +136,8 @@ AuthorSerializer = Class.new(ActiveModel::Serializer) do cache key:'writer', skip_digest: true attributes :id, :name - has_many :posts, embed: :ids - has_many :roles, embed: :ids + has_many :posts + has_many :roles has_one :bio end diff --git a/test/serializers/association_macros_test.rb b/test/serializers/association_macros_test.rb index f99e1980..30202f4a 100644 --- a/test/serializers/association_macros_test.rb +++ b/test/serializers/association_macros_test.rb @@ -6,7 +6,7 @@ module ActiveModel AuthorSummarySerializer = Class.new class AssociationsTestSerializer < Serializer belongs_to :author, serializer: AuthorSummarySerializer - has_many :comments, embed: :ids + has_many :comments has_one :category end @@ -21,7 +21,7 @@ module ActiveModel end def test_has_many_defines_reflection - has_many_reflection = HasManyReflection.new(:comments, embed: :ids) + has_many_reflection = HasManyReflection.new(:comments, {}) assert_includes(@reflections, has_many_reflection) end diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index b7b77cf9..bfc1b40c 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -52,13 +52,13 @@ module ActiveModel case key when :posts - assert_equal({ embed: :ids }, options) + assert_equal({}, options) assert_kind_of(ActiveModel::Serializer.config.array_serializer, serializer) when :bio assert_equal({}, options) assert_nil serializer when :roles - assert_equal({ embed: :ids }, options) + assert_equal({}, options) assert_kind_of(ActiveModel::Serializer.config.array_serializer, serializer) else flunk "Unknown association: #{key}" From d0d00d02a032b176284d2387a18663ce5c101f80 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Mon, 31 Aug 2015 05:11:32 +0200 Subject: [PATCH 3/6] Add ActiveRecord-backed fixtures. --- test/fixtures/active_record.rb | 58 ++++++++++++++++++++++++++++++++++ test/test_helper.rb | 6 ++-- 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/active_record.rb diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb new file mode 100644 index 00000000..e5029c30 --- /dev/null +++ b/test/fixtures/active_record.rb @@ -0,0 +1,58 @@ +require 'active_record' + +ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:') +ActiveRecord::Schema.define do + create_table :posts, force: true do |t| + t.string :title + t.text :body + t.references :author + t.timestamps null: false + end + create_table :authors, force: true do |t| + t.string :name + t.timestamps null: false + end + create_table :comments, force: true do |t| + t.text :contents + t.references :author + t.references :post + t.timestamp null: false + end +end + +module ARModels + class Post < ActiveRecord::Base + has_many :comments + belongs_to :author + end + + class Comment < ActiveRecord::Base + belongs_to :post + belongs_to :author + end + + class Author < ActiveRecord::Base + has_many :posts + end + + class PostSerializer < ActiveModel::Serializer + attributes :id, :title, :body + params :title, :body + + has_many :comments + belongs_to :author + url :comments + end + + class CommentSerializer < ActiveModel::Serializer + attributes :id, :contents + + belongs_to :author + end + + class AuthorSerializer < ActiveModel::Serializer + attributes :id, :name + + has_many :posts + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 1327188e..ce5164c3 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -34,6 +34,8 @@ require 'support/stream_capture' require 'support/rails_app' -require 'fixtures/poro' - require 'support/test_case' + +require 'fixtures/active_record' + +require 'fixtures/poro' From e3d3d92201564b900aa7452f3ba9f41da06202a4 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 27 Aug 2015 21:17:00 -0500 Subject: [PATCH 4/6] Clarify AMS dependencies --- Gemfile | 27 ++++++++++++------ active_model_serializers.gemspec | 49 +++++++++++++++++++++++--------- lib/active_model_serializers.rb | 24 +++++++--------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/Gemfile b/Gemfile index bdfff856..ad21ffae 100644 --- a/Gemfile +++ b/Gemfile @@ -3,17 +3,26 @@ source 'https://rubygems.org' # Specify your gem's dependencies in active_model_serializers.gemspec gemspec -gem "minitest" +version = ENV['RAILS_VERSION'] || '4.2' -version = ENV["RAILS_VERSION"] || "4.2" - -if version == "master" - gem "rails", github: "rails/rails" - - # ugh https://github.com/rails/rails/issues/16063#issuecomment-48090125 - gem "arel", github: "rails/arel" +if version == 'master' + gem 'rack', github: 'rack/rack' + git 'https://github.com/rails/rails.git' do + gem 'railties' + gem 'activesupport' + gem 'activemodel' + gem 'actionpack' + # Rails 5 + gem 'actionview' + end + # Rails 5 + gem 'rails-controller-testing', github: 'rails/rails-controller-testing' else - gem "rails", "~> #{version}.0" + gem_version = "~> #{version}.0" + gem 'railties', gem_version + gem 'activesupport', gem_version + gem 'activemodel', gem_version + gem 'actionpack', gem_version end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index eff27ca9..de385a97 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -4,26 +4,47 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'active_model/serializer/version' Gem::Specification.new do |spec| - spec.name = "active_model_serializers" + spec.name = 'active_model_serializers' spec.version = ActiveModel::Serializer::VERSION - spec.authors = ["Steve Klabnik"] - spec.email = ["steve@steveklabnik.com"] + spec.platform = Gem::Platform::RUBY + spec.authors = ['Steve Klabnik'] + spec.email = ['steve@steveklabnik.com'] spec.summary = %q{Conventions-based JSON generation for Rails.} spec.description = %q{ActiveModel::Serializers allows you to generate your JSON in an object-oriented and convention-driven manner.} - spec.homepage = "https://github.com/rails-api/active_model_serializers" - spec.license = "MIT" + spec.homepage = 'https://github.com/rails-api/active_model_serializers' + spec.license = 'MIT' spec.files = `git ls-files -z`.split("\x0") - spec.executables = spec.files.grep(%r{^bin/}) { |f| File.basename(f) } spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) - spec.require_paths = ["lib"] + spec.require_paths = ['lib'] - spec.add_dependency "activemodel", ">= 4.0" + spec.required_ruby_version = '>= 1.9.3' - spec.add_development_dependency "rails", ">= 4.0" - spec.add_development_dependency "bundler", "~> 1.6" - spec.add_development_dependency "timecop", ">= 0.7" - spec.add_development_dependency "rake" - spec.add_development_dependency "kaminari" - spec.add_development_dependency "will_paginate" + rails_versions = '>= 4.0' + spec.add_runtime_dependency 'activemodel', rails_versions + # 'activesupport', rails_versions + # 'builder' + + spec.add_runtime_dependency 'actionpack', rails_versions + # 'activesupport', rails_versions + # 'rack' + # 'rack-test', '~> 0.6.2' + + spec.add_runtime_dependency 'railties', rails_versions + # 'activesupport', rails_versions + # 'actionpack', rails_versions + # 'rake', '>= 0.8.7' + + # 'activesupport', rails_versions + # 'i18n, + # 'tzinfo' + # 'minitest' + # 'thread_safe' + + # Soft dependency for pagination + spec.add_development_dependency 'kaminari' + spec.add_development_dependency 'will_paginate' + + spec.add_development_dependency 'bundler', '~> 1.6' + spec.add_development_dependency 'timecop', '>= 0.7' end diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 01c23e5f..8b90ba7a 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -10,22 +10,20 @@ module ActiveModelSerializers end end + require 'active_model' -require 'active_model/serializer/version' +require 'action_controller' + require 'active_model/serializer' require 'active_model/serializable_resource' +require 'active_model/serializer/version' -begin - require 'active_model/serializer/railtie' - require 'action_controller' - require 'action_controller/serialization' - - ActiveSupport.on_load(:action_controller) do - include ::ActionController::Serialization - ActionDispatch::Reloader.to_prepare do - ActiveModel::Serializer.serializers_cache.clear - end +require 'action_controller/serialization' +ActiveSupport.on_load(:action_controller) do + include ::ActionController::Serialization + ActionDispatch::Reloader.to_prepare do + ActiveModel::Serializer.serializers_cache.clear end -rescue LoadError - # rails not installed, continuing end + +require 'active_model/serializer/railtie' From 83f11acd6607fd2383250ba884eb84202f07f266 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Mon, 31 Aug 2015 05:46:04 +0200 Subject: [PATCH 5/6] Add Gemfile dependencies to ActiveRecord and sqlite3. --- Gemfile | 6 ++++++ test/fixtures/active_record.rb | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index ad21ffae..e0a4364c 100644 --- a/Gemfile +++ b/Gemfile @@ -25,5 +25,11 @@ else gem 'actionpack', gem_version end +group :test do + gem 'activerecord' + gem 'sqlite3', platform: :ruby + gem 'activerecord-jdbcsqlite3-adapter', platform: :jruby +end + # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index e5029c30..ab3e4d85 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -37,7 +37,6 @@ module ARModels class PostSerializer < ActiveModel::Serializer attributes :id, :title, :body - params :title, :body has_many :comments belongs_to :author From 9673b6471c46719dcf443529fb38a5eb7d85f137 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 31 Aug 2015 00:04:23 -0500 Subject: [PATCH 6/6] Better lint Extracted from https://github.com/rails-api/active_model_serializers/pull/1004/files#diff-56455571c4ba7a2b4c640b9e8168f522R40 Correct cache_key lint for ActiveRecord 4.1+ https://github.com/rails/rails/blob/4-0-stable/activerecord/lib/active_record/integration.rb def cache_key https://github.com/rails/rails/blob/4-1-stable/activerecord/lib/active_record/integration.rb def cache_key(*timestamp_names) --- lib/active_model/serializer/lint.rb | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/active_model/serializer/lint.rb b/lib/active_model/serializer/lint.rb index 97ffcd7f..bf3b7a37 100644 --- a/lib/active_model/serializer/lint.rb +++ b/lib/active_model/serializer/lint.rb @@ -36,7 +36,17 @@ module ActiveModel::Serializer::Lint # Typically, it is implemented by including ActiveModel::Serialization. def test_read_attribute_for_serialization assert_respond_to resource, :read_attribute_for_serialization, "The resource should respond to read_attribute_for_serialization" - assert_equal resource.method(:read_attribute_for_serialization).arity, 1 + actual_arity = resource.method(:read_attribute_for_serialization).arity + if defined?(::Rubinius) + # 1 for def read_attribute_for_serialization(name); end + # -2 for alias :read_attribute_for_serialization :send for rbx because :shrug: + assert_includes [1, -2], actual_arity, "expected #{actual_arity.inspect} to be 1 or -2" + else + # using absolute value since arity is: + # 1 for def read_attribute_for_serialization(name); end + # -1 for alias :read_attribute_for_serialization :send + assert_includes [1, -1], actual_arity, "expected #{actual_arity.inspect} to be 1 or -1" + end end # Passes if the object responds to as_json and if it takes @@ -68,7 +78,7 @@ module ActiveModel::Serializer::Lint end # Passes if the object responds to cache_key and if it takes no - # arguments. + # arguments (Rails 4.0) or a splat (Rails 4.1+). # Fails otherwise. # # cache_key returns a (self-expiring) unique key for the object, @@ -76,7 +86,11 @@ module ActiveModel::Serializer::Lint # It is not required unless caching is enabled. def test_cache_key assert_respond_to resource, :cache_key - assert_equal resource.method(:cache_key).arity, 0 + actual_arity = resource.method(:cache_key).arity + # using absolute value since arity is: + # 0 for Rails 4.1+, *timestamp_names + # -1 for Rails 4.0, no arguments + assert_includes [-1, 0], actual_arity, "expected #{actual_arity.inspect} to be 0 or -1" end # Passes if the object responds to id and if it takes no