-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix warnings #1067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix warnings #1067
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,23 @@ | ||
| require 'active_model/serializer/adapter/fragment_cache' | ||
|
|
||
| module ActiveModel | ||
| class Serializer | ||
| class Adapter | ||
| extend ActiveSupport::Autoload | ||
| autoload :Json | ||
| require 'active_model/serializer/adapter/json' | ||
| require 'active_model/serializer/adapter/json_api' | ||
| autoload :FlattenJson | ||
| autoload :Null | ||
| autoload :JsonApi | ||
| autoload :FragmentCache | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| def self.create(resource, options = {}) | ||
| override = options.delete(:adapter) | ||
| klass = override ? adapter_class(override) : ActiveModel::Serializer.adapter | ||
| klass.new(resource, options) | ||
| end | ||
|
|
||
| def self.adapter_class(adapter) | ||
| adapter_name = adapter.to_s.classify.sub("API", "Api") | ||
| "ActiveModel::Serializer::Adapter::#{adapter_name}".safe_constantize | ||
| end | ||
|
|
||
| attr_reader :serializer | ||
|
|
||
|
|
@@ -26,17 +36,6 @@ def as_json(options = nil) | |
| hash | ||
| end | ||
|
|
||
| def self.create(resource, options = {}) | ||
| override = options.delete(:adapter) | ||
| klass = override ? adapter_class(override) : ActiveModel::Serializer.adapter | ||
| klass.new(resource, options) | ||
| end | ||
|
|
||
| def self.adapter_class(adapter) | ||
| adapter_name = adapter.to_s.classify.sub("API", "Api") | ||
| "ActiveModel::Serializer::Adapter::#{adapter_name}".safe_constantize | ||
| end | ||
|
|
||
| def fragment_cache(*args) | ||
| raise NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.' | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,11 +73,9 @@ def has_one(name, options = {}) | |
| def associate(reflection) | ||
| self._reflections = _reflections.dup | ||
|
|
||
| unless method_defined?(reflection.name) | ||
| define_method reflection.name do | ||
| object.send reflection.name | ||
| end | ||
| end | ||
| define_method reflection.name do | ||
| object.send reflection.name | ||
| end unless method_defined?(reflection.name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| self._reflections << reflection | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,18 @@ | ||
| module ActiveModelSerializers | ||
| module_function | ||
|
|
||
| def silence_warnings | ||
| verbose = $VERBOSE | ||
| $VERBOSE = nil | ||
| yield | ||
| ensure | ||
| $VERBOSE = verbose | ||
| end | ||
|
|
||
| end | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. required because of a Ruby bug or as a 'safety valve' for unfixable or not-worth-fixing warnings. such is life e.g. private
ActiveModelSerializers.silence_warnings do
attr_reader :resource, :adapter_opts, :serializer_opts
end |
||
| require 'active_model' | ||
| require 'active_model/serializer/version' | ||
| require 'active_model/serializer' | ||
| require 'active_model/serializer/fieldset' | ||
| require 'active_model/serializable_resource' | ||
|
|
||
| begin | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,10 @@ | |
| class DefaultScopeNameTest < ActionController::TestCase | ||
| class UserSerializer < ActiveModel::Serializer | ||
| attributes :admin? | ||
| def admin? | ||
| current_user.admin | ||
| ActiveModelSerializers.silence_warnings do | ||
| def admin? | ||
| current_user.admin | ||
| end | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't figure out how to get around this 'method redefined?' warning. I think Rails silences these kind of warnings as well. |
||
| end | ||
| end | ||
|
|
||
|
|
@@ -34,8 +36,10 @@ def test_default_scope_name | |
| class SerializationScopeNameTest < ActionController::TestCase | ||
| class AdminUserSerializer < ActiveModel::Serializer | ||
| attributes :admin? | ||
| def admin? | ||
| current_admin.admin | ||
| ActiveModelSerializers.silence_warnings do | ||
| def admin? | ||
| current_admin.admin | ||
| end | ||
| end | ||
| end | ||
|
|
||
|
|
@@ -60,4 +64,4 @@ def test_override_scope_name_with_controller | |
| get :render_new_user | ||
| assert_equal '{"data":{"id":"1","type":"users","attributes":{"admin?":true}}}', @response.body | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| verbose = $VERBOSE | ||
| $VERBOSE = nil | ||
| class Model | ||
| FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) | ||
|
|
||
|
|
@@ -260,9 +262,4 @@ def maker | |
| cache only: [:id] | ||
| attributes :id | ||
| end | ||
|
|
||
| RaiseErrorSerializer = Class.new(ActiveModel::Serializer) do | ||
| def json_key | ||
| raise StandardError, 'Intentional error for rescue_from test' | ||
| end | ||
| end | ||
| $VERBOSE = verbose | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this file was just full of warnings that aren't really that important and were really hard to fix, mostly due to all the dynamic class generation. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,13 +47,13 @@ def test_cache_key_definition | |
| end | ||
|
|
||
| def test_cache_key_interpolation_with_updated_at | ||
| author = render_object_with_cache(@author) | ||
| render_object_with_cache(@author) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assert_equal(nil, ActionController::Base.cache_store.fetch(@author.cache_key)) | ||
| assert_equal(@author_serializer.attributes.to_json, ActionController::Base.cache_store.fetch("#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}").to_json) | ||
| end | ||
|
|
||
| def test_default_cache_key_fallback | ||
| comment = render_object_with_cache(@comment) | ||
| render_object_with_cache(@comment) | ||
| assert_equal(@comment_serializer.attributes.to_json, ActionController::Base.cache_store.fetch(@comment.cache_key).to_json) | ||
| end | ||
|
|
||
|
|
@@ -74,7 +74,7 @@ def test_associations_separately_cache | |
| assert_equal(nil, ActionController::Base.cache_store.fetch(@comment.cache_key)) | ||
|
|
||
| Timecop.freeze(Time.now) do | ||
| post = render_object_with_cache(@post) | ||
| render_object_with_cache(@post) | ||
|
|
||
| assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key)) | ||
| assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(@comment.cache_key)) | ||
|
|
@@ -122,7 +122,7 @@ def test_fragment_fetch_with_virtual_associations | |
| end | ||
|
|
||
| def test_uses_file_digest_in_cache_key | ||
| blog = render_object_with_cache(@blog) | ||
| render_object_with_cache(@blog) | ||
| assert_equal(@blog_serializer.attributes, ActionController::Base.cache_store.fetch(@blog.cache_key_with_digest)) | ||
| end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| class Foo < Rails::Application | ||
| if Rails::VERSION::MAJOR >= 4 | ||
| config.eager_load = false | ||
| config.secret_key_base = 'abc123' | ||
| config.action_controller.perform_caching = true | ||
| config.active_support.test_order = :random | ||
| config.logger = Logger.new(nil) | ||
| ActionController::Base.cache_store = :memory_store | ||
| end | ||
| end | ||
| Foo.initialize! | ||
|
|
||
| module TestHelper | ||
| Routes = ActionDispatch::Routing::RouteSet.new | ||
| Routes.draw do | ||
| get ':controller(/:action(/:id))' | ||
| get ':controller(/:action)' | ||
| end | ||
|
|
||
| ActionController::Base.send :include, Routes.url_helpers | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Use cleaner stream testing interface from Rails 5 if available | ||
| # see https://github.com/rails/rails/blob/29959eb59d/activesupport/lib/active_support/testing/stream.rb | ||
| begin | ||
| require "active_support/testing/stream" | ||
| rescue LoadError | ||
| module ActiveSupport | ||
| module Testing | ||
| module Stream #:nodoc: | ||
| private | ||
|
|
||
| def silence_stream(stream) | ||
| old_stream = stream.dup | ||
| stream.reopen(IO::NULL) | ||
| stream.sync = true | ||
| yield | ||
| ensure | ||
| stream.reopen(old_stream) | ||
| old_stream.close | ||
| end | ||
|
|
||
| def quietly | ||
| silence_stream(STDOUT) do | ||
| silence_stream(STDERR) do | ||
| yield | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def capture(stream) | ||
| stream = stream.to_s | ||
| captured_stream = Tempfile.new(stream) | ||
| stream_io = eval("$#{stream}") | ||
| origin_stream = stream_io.dup | ||
| stream_io.reopen(captured_stream) | ||
|
|
||
| yield | ||
|
|
||
| stream_io.rewind | ||
| return captured_stream.read | ||
| ensure | ||
| captured_stream.close | ||
| captured_stream.unlink | ||
| stream_io.reopen(origin_stream) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ActionController::TestCase.class_eval do | ||
| def setup | ||
| @routes = TestHelper::Routes | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍