From 91d199259bc7a85763d487a1d0fbf8cd50fe29a0 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 27 Jun 2013 23:18:30 -0500 Subject: Encapsulate rake lines from ActiveRecord/ActionPack as CodeTools::LineStatistics [ci skip] --- actionpack/Rakefile | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) (limited to 'actionpack') diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 7eab972595..d12213a2d8 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -40,27 +40,9 @@ task :release => :package do end task :lines do - lines, codelines, total_lines, total_codelines = 0, 0, 0, 0 - - FileList["lib/**/*.rb"].each do |file_name| - next if file_name =~ /vendor/ - File.open(file_name, 'r') do |f| - while line = f.gets - lines += 1 - next if line =~ /^\s*$/ - next if line =~ /^\s*#/ - codelines += 1 - end - end - puts "L: #{sprintf("%4d", lines)}, LOC #{sprintf("%4d", codelines)} | #{file_name}" - - total_lines += lines - total_codelines += codelines - - lines, codelines = 0, 0 - end - - puts "Total: Lines #{total_lines}, LOC #{total_codelines}" + load File.expand_path('..', File.dirname(__FILE__)) + '/tools/line_statistics' + files = FileList["lib/**/*.rb"] + CodeTools::LineStatistics.new(files).print_loc end rule '.rb' => '.y' do |t| -- cgit v1.2.3 From 29be3f5d8386fc9a8a67844fa9b7d6860574e715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Strza=C5=82kowski?= Date: Tue, 12 Aug 2014 21:57:51 +0200 Subject: Add config option for cookies digest You can now configure custom digest for cookies in the same way as `serializer`: config.action_dispatch.cookies_digest = 'SHA256' --- actionpack/CHANGELOG.md | 5 +++ .../lib/action_dispatch/middleware/cookies.rb | 12 ++++-- actionpack/test/dispatch/cookies_test.rb | 50 ++++++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 44b8fa843d..253b647aca 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Add `config.action_dispatch.cookies_digest` option for setting custom + digest. The default remains the same - 'SHA1'. + + *Łukasz Strzałkowski* + * Extract source code for the entire exception stack trace for better debugging and diagnosis. diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index ac9e5effe2..0f088c5539 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -90,6 +90,7 @@ module ActionDispatch SECRET_TOKEN = "action_dispatch.secret_token".freeze SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze + COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -212,7 +213,8 @@ module ActionDispatch secret_token: env[SECRET_TOKEN], secret_key_base: env[SECRET_KEY_BASE], upgrade_legacy_signed_cookies: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?, - serializer: env[COOKIES_SERIALIZER] + serializer: env[COOKIES_SERIALIZER], + digest: env[COOKIES_DIGEST] } end @@ -441,6 +443,10 @@ module ActionDispatch serializer end end + + def digest + @options[:digest] || 'SHA1' + end end class SignedCookieJar #:nodoc: @@ -451,7 +457,7 @@ module ActionDispatch @parent_jar = parent_jar @options = options secret = key_generator.generate_key(@options[:signed_cookie_salt]) - @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: NullSerializer) + @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: NullSerializer) end def [](name) @@ -508,7 +514,7 @@ module ActionDispatch @options = options secret = key_generator.generate_key(@options[:encrypted_cookie_salt]) sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt]) - @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: NullSerializer) + @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, digest: digest, serializer: NullSerializer) end def [](name) diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 0f145666d1..b40c21c067 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -369,6 +369,35 @@ class CookiesTest < ActionController::TestCase assert_equal 'Jamie', @controller.send(:cookies).permanent[:user_name] end + def test_signed_cookie_using_default_digest + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + + verifier = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal, digest: 'SHA1') + assert_equal verifier.generate(45), cookies[:user_id] + end + + def test_signed_cookie_using_custom_digest + @request.env["action_dispatch.cookies_digest"] = 'SHA256' + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + + verifier = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal, digest: 'SHA256') + assert_equal verifier.generate(45), cookies[:user_id] + end + def test_signed_cookie_using_default_serializer get :set_signed_cookie cookies = @controller.send :cookies @@ -481,6 +510,27 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] end + def test_encrypted_cookie_using_custom_digest + @request.env["action_dispatch.cookies_digest"] = 'SHA256' + get :set_encrypted_cookie + cookies = @controller.send :cookies + assert_not_equal 'bar', cookies[:foo] + assert_equal 'bar', cookies.encrypted[:foo] + + sign_secret = @request.env["action_dispatch.key_generator"].generate_key(@request.env["action_dispatch.encrypted_signed_cookie_salt"]) + + sha1_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActionDispatch::Cookies::NullSerializer, digest: 'SHA1') + sha256_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActionDispatch::Cookies::NullSerializer, digest: 'SHA256') + + assert_raises(ActiveSupport::MessageVerifier::InvalidSignature) do + sha1_verifier.verify(cookies[:foo]) + end + + assert_nothing_raised do + sha256_verifier.verify(cookies[:foo]) + end + end + def test_encrypted_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json @request.env["action_dispatch.cookies_serializer"] = :hybrid -- cgit v1.2.3 From 12ce5b1cf6be926d6ca22474d1d77ca83b9da2bf Mon Sep 17 00:00:00 2001 From: Akshay Vishnoi Date: Wed, 13 Aug 2014 16:47:28 +0530 Subject: [ci skip] fix spelling of override --- actionpack/test/dispatch/request_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 6737609567..fe9ee6f73d 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -640,7 +640,7 @@ end class RequestMethod < BaseRequestTest test "method returns environment's request method when it has not been - overriden by middleware".squish do + overridden by middleware".squish do ActionDispatch::Request::HTTP_METHODS.each do |method| request = stub_request('REQUEST_METHOD' => method) -- cgit v1.2.3 From bc116a55ca3dd9f63a1f1ca7ade3623885adcc57 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Wed, 13 Aug 2014 18:26:41 +0900 Subject: AM, AP, AV, and AMo tests are already order_independent! --- actionpack/test/abstract_unit.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 376add06ca..4e17d57dad 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -497,8 +497,3 @@ if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0 # Use N processes (N defaults to 4) Minitest.parallel_executor = ForkingExecutor.new(PROCESS_COUNT) end - -# FIXME: we have tests that depend on run order, we should fix that and -# remove this method call. -require 'active_support/test_case' -ActiveSupport::TestCase.my_tests_are_order_dependent! -- cgit v1.2.3 From 2c8714568b8ef43523505cfd897ca6c569cac4ac Mon Sep 17 00:00:00 2001 From: Aditya Kapoor Date: Wed, 13 Aug 2014 18:56:03 +0530 Subject: [ci skip] correct default cache store class --- actionpack/lib/action_controller/caching.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 12d798d0c1..de85e0c1a7 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -16,7 +16,7 @@ module ActionController # All the caching stores from ActiveSupport::Cache are available to be used as backends # for Action Controller caching. # - # Configuration examples (MemoryStore is the default): + # Configuration examples (FileStore is the default): # # config.action_controller.cache_store = :memory_store # config.action_controller.cache_store = :file_store, '/path/to/cache/directory' -- cgit v1.2.3 From b1ba333ea72b8aa8fc5ffeda4067c128afb026e4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 2 Jul 2014 15:36:23 -0700 Subject: Fix assert_template for files. The test was not failing for `assert_template file: nil` when a file has been rendered. --- actionpack/CHANGELOG.md | 4 ++++ actionpack/lib/action_controller/test_case.rb | 9 +++++++++ .../test/controller/action_pack_assertions_test.rb | 23 ++++++++++++++++++++++ 3 files changed, 36 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 44b8fa843d..6ea7276ac6 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix `assert_template` not being able to assert that no files were rendered. + + *Guo Xiang Tan* + * Extract source code for the entire exception stack trace for better debugging and diagnosis. diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index eb5d824cbc..152420a54c 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -91,6 +91,13 @@ module ActionController # # assert that no partials were rendered # assert_template partial: false # + # # assert that a file was rendered + # assert_template file: "README.rdoc" + # + # # assert that no file was rendered + # assert_template file: nil + # assert_template file: false + # # In a view test case, you can also assert that specific locals are passed # to partials: # @@ -140,6 +147,8 @@ module ActionController if options[:file] assert_includes @_files.keys, options[:file] + elsif options.key?(:file) + assert @_files.blank?, "expected no files but #{@_files.keys} was rendered" end if expected_partial = options[:partial] diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index b6b5a218cc..311302819e 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -488,6 +488,11 @@ class AssertTemplateTest < ActionController::TestCase assert_raise(ActiveSupport::TestCase::Assertion) do assert_template :file => 'test/hello_world' end + + get :render_file_absolute_path + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template file: nil + end end def test_with_nil_passes_when_no_template_rendered @@ -612,6 +617,24 @@ class AssertTemplateTest < ActionController::TestCase get :nothing assert_template nil + + get :partial + assert_template partial: 'test/_partial' + + get :nothing + assert_template partial: nil + + get :render_with_layout + assert_template layout: 'layouts/standard' + + get :nothing + assert_template layout: nil + + get :render_file_relative_path + assert_template file: 'README.rdoc' + + get :nothing + assert_template file: nil end end -- cgit v1.2.3 From 1a6ca03aecccea7ad73fff8b4429f578c6d68290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Strza=C5=82kowski?= Date: Wed, 13 Aug 2014 22:51:53 +0200 Subject: Remove redundant NullSerializer Use one from ActiveSupport::MessageEncryptor module. --- .../lib/action_dispatch/middleware/cookies.rb | 23 +++++++--------------- 1 file changed, 7 insertions(+), 16 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index ac9e5effe2..d1cc881a62 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -173,10 +173,14 @@ module ActionDispatch end end + # Passing the ActiveSupport::MessageEncryptor::NullSerializer downstream + # to the Message{Encryptor,Verifier} allows us to handle the + # (de)serialization step within the cookie jar, which gives us the + # opportunity to detect and migrate legacy cookies. module VerifyAndUpgradeLegacySignedMessage def initialize(*args) super - @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token], serializer: NullSerializer) + @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token], serializer: ActiveSupport::MessageEncryptor::NullSerializer) end def verify_and_upgrade_legacy_signed_message(name, signed_message) @@ -393,19 +397,6 @@ module ActionDispatch end end - # Passing the NullSerializer downstream to the Message{Encryptor,Verifier} - # allows us to handle the (de)serialization step within the cookie jar, - # which gives us the opportunity to detect and migrate legacy cookies. - class NullSerializer - def self.load(value) - value - end - - def self.dump(value) - value - end - end - module SerializedCookieJars MARSHAL_SIGNATURE = "\x04\x08".freeze @@ -451,7 +442,7 @@ module ActionDispatch @parent_jar = parent_jar @options = options secret = key_generator.generate_key(@options[:signed_cookie_salt]) - @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: NullSerializer) + @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end def [](name) @@ -508,7 +499,7 @@ module ActionDispatch @options = options secret = key_generator.generate_key(@options[:encrypted_cookie_salt]) sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt]) - @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: NullSerializer) + @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end def [](name) -- cgit v1.2.3 From efb835c9c0aea9d2a6487a6fdd31d459f1771879 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 14:22:29 -0700 Subject: UnexpectedErrors may reference exceptions that can't be dumped UnexpectedError exceptions wrap the original exception, and the original exception may contain a reference to something that can't be marshal dumped which will cause the process to die. --- actionpack/test/abstract_unit.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 4e17d57dad..674fb253f4 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -484,6 +484,9 @@ class ForkingExecutor method = job[1] reporter = job[2] result = Minitest.run_one_method klass, method + if result.error? + translate_exceptions result + end queue.record reporter, result end } @@ -491,6 +494,20 @@ class ForkingExecutor @size.times { @queue << nil } pool.each { |pid| Process.waitpid pid } end + + private + def translate_exceptions(result) + result.failures.map! { |e| + begin + Marshal.dump e + e + rescue TypeError + ex = Exception.new e.message + ex.set_backtrace e.backtrace + Minitest::UnexpectedError.new ex + end + } + end end if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0 -- cgit v1.2.3 From d4981c393bb086eb7d8a6696528891927398b073 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 14:31:55 -0700 Subject: this should be accessing the hash, not calling a method --- .../lib/action_dispatch/middleware/templates/rescues/_trace.text.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb index 36b01bf952..c0b53068f7 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb @@ -3,7 +3,7 @@ Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unse <% @traces.each do |name, trace| %> <% if trace.any? %> <%= name %> -<%= trace.map(&:trace).join("\n") %> +<%= trace.map { |t| t[:trace] }.join("\n") %> <% end %> <% end %> -- cgit v1.2.3 From 3b908cba28552604f8d85a236f2d1c82f7589d80 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 18:34:21 -0700 Subject: fewer operations on the options hash since we pass `as` down, then we won't have to do an insert / delete dance with the options hash --- actionpack/lib/action_dispatch/routing/mapper.rb | 20 ++++++++++---------- actionpack/test/dispatch/mapper_test.rb | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index cd94f35e8f..e977d769e0 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -63,7 +63,7 @@ module ActionDispatch attr_reader :requirements, :conditions, :defaults attr_reader :to, :default_controller, :default_action, :as, :anchor - def self.build(scope, set, path, options) + def self.build(scope, set, path, as, options) options = scope[:options].merge(options) if scope[:options] options.delete :only @@ -74,10 +74,10 @@ module ActionDispatch defaults = (scope[:defaults] || {}).merge options.delete(:defaults) || {} - new scope, set, path, defaults, options + new scope, set, path, defaults, as, options end - def initialize(scope, set, path, defaults, options) + def initialize(scope, set, path, defaults, as, options) @requirements, @conditions = {}, {} @defaults = defaults @set = set @@ -85,7 +85,7 @@ module ActionDispatch @to = options.delete :to @default_controller = options.delete(:controller) || scope[:controller] @default_action = options.delete(:action) || scope[:action] - @as = options.delete :as + @as = as @anchor = options.delete :anchor formatted = options.delete :format @@ -1544,13 +1544,13 @@ module ActionDispatch action = nil end - if !options.fetch(:as, true) # if it's set to nil or false - options.delete(:as) - else - options[:as] = name_for_action(options[:as], action) - end + as = if !options.fetch(:as, true) # if it's set to nil or false + options.delete(:as) + else + name_for_action(options.delete(:as), action) + end - mapping = Mapping.build(@scope, @set, URI.parser.escape(path), options) + mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, options) app, conditions, requirements, defaults, as, anchor = mapping.to_route @set.add_route(app, conditions, requirements, defaults, as, anchor) end diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 3e554a9cf6..889f9a4736 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -38,7 +38,7 @@ module ActionDispatch def test_mapping_requirements options = { :controller => 'foo', :action => 'bar', :via => :get } - m = Mapper::Mapping.build({}, FakeSet.new, '/store/:name(*rest)', options) + m = Mapper::Mapping.build({}, FakeSet.new, '/store/:name(*rest)', nil, options) _, _, requirements, _ = m.to_route assert_equal(/.+?/, requirements[:rest]) end -- cgit v1.2.3 From 318eea062ec3aa95929cfb516a273e732e22a9d1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 18:42:26 -0700 Subject: pass consistent parameters to canonical_action? now we only have to look up @scope[:scope_level] once per call to canonical_action? and we don't have a variable named "flag" --- actionpack/lib/action_dispatch/routing/mapper.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e977d769e0..a3a22bceb1 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1634,8 +1634,8 @@ module ActionDispatch RESOURCE_SCOPES.include? @scope[:scope_level] end - def resource_method_scope? #:nodoc: - RESOURCE_METHOD_SCOPES.include? @scope[:scope_level] + def resource_method_scope?(scope_level) #:nodoc: + RESOURCE_METHOD_SCOPES.include? scope_level end def nested_scope? #:nodoc: @@ -1699,8 +1699,8 @@ module ActionDispatch @scope[:constraints][parent_resource.param] end - def canonical_action?(action, flag) #:nodoc: - flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) + def canonical_action?(action, scope_level) #:nodoc: + scope_level && resource_method_scope?(scope_level) && CANONICAL_ACTIONS.include?(action.to_s) end def shallow_scope(path, options = {}) #:nodoc: @@ -1714,7 +1714,7 @@ module ActionDispatch end def path_for_action(action, path) #:nodoc: - if canonical_action?(action, path.blank?) + if path.blank? && canonical_action?(action, @scope[:scope_level]) @scope[:path].to_s else "#{@scope[:path]}/#{action_path(action, path)}" -- cgit v1.2.3 From 91608dc342237372548ccbe403ef06c56c2755f2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 18:47:16 -0700 Subject: only test `prefix` once we don't need to repeat if statements --- actionpack/lib/action_dispatch/routing/mapper.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index a3a22bceb1..37f781a7dd 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1732,12 +1732,14 @@ module ActionDispatch elsif !canonical_action?(action, @scope[:scope_level]) prefix = action end - prefix.to_s.tr('-', '_') if prefix + + if prefix + Mapper.normalize_name prefix.to_s.tr('-', '_') + end end def name_for_action(as, action) #:nodoc: prefix = prefix_name_for_action(as, action) - prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] if parent_resource -- cgit v1.2.3 From 0127f02826fec6641c21779c4109a1d2ccb700fe Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:13:58 -0700 Subject: only look up scope level once avoid hash lookups and remove depency on the instance --- actionpack/lib/action_dispatch/routing/mapper.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 37f781a7dd..2f03e22be3 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1726,20 +1726,21 @@ module ActionDispatch path || @scope[:path_names][name] || name.to_s end - def prefix_name_for_action(as, action) #:nodoc: + def prefix_name_for_action(as, action, scope_level) #:nodoc: if as prefix = as - elsif !canonical_action?(action, @scope[:scope_level]) + elsif !canonical_action?(action, scope_level) prefix = action end - if prefix + if prefix && prefix != '/' && !prefix.empty? Mapper.normalize_name prefix.to_s.tr('-', '_') end end def name_for_action(as, action) #:nodoc: - prefix = prefix_name_for_action(as, action) + scope_level = @scope[:scope_level] + prefix = prefix_name_for_action(as, action, scope_level) name_prefix = @scope[:as] if parent_resource @@ -1749,7 +1750,7 @@ module ActionDispatch member_name = parent_resource.member_name end - name = case @scope[:scope_level] + name = case scope_level when :nested [name_prefix, prefix] when :collection @@ -1764,7 +1765,7 @@ module ActionDispatch [name_prefix, member_name, prefix] end - if candidate = name.select(&:present?).join("_").presence + if candidate = name.compact.join("_").presence # If a name was not explicitly given, we check if it is valid # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. -- cgit v1.2.3 From 911ef972a540686fd7fe7a3dee659ce3bb1fd12d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:22:30 -0700 Subject: move scope_level to a method on the scope object now we don't have to have a hard coded key --- actionpack/lib/action_dispatch/routing/mapper.rb | 27 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 2f03e22be3..77d9889f28 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1047,7 +1047,6 @@ module ActionDispatch RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except, :param, :concerns] CANONICAL_ACTIONS = %w(index create new show update destroy) RESOURCE_METHOD_SCOPES = [:collection, :member, :new] - RESOURCE_SCOPES = [:resource, :resources] class Resource #:nodoc: attr_reader :controller, :path, :options, :param @@ -1521,7 +1520,7 @@ module ActionDispatch if on = options.delete(:on) send(on) { decomposed_match(path, options) } else - case @scope[:scope_level] + case @scope.scope_level when :resources nested { decomposed_match(path, options) } when :resource @@ -1564,7 +1563,7 @@ module ActionDispatch raise ArgumentError, "must be called with a path and/or options" end - if @scope[:scope_level] == :resources + if @scope.scope_level == :resources with_scope_level(:root) do scope(parent_resource.path) do super(options) @@ -1631,7 +1630,7 @@ module ActionDispatch end def resource_scope? #:nodoc: - RESOURCE_SCOPES.include? @scope[:scope_level] + @scope.resource_scope? end def resource_method_scope?(scope_level) #:nodoc: @@ -1639,7 +1638,7 @@ module ActionDispatch end def nested_scope? #:nodoc: - @scope[:scope_level] == :nested + @scope.nested? end def with_exclusive_scope @@ -1714,7 +1713,7 @@ module ActionDispatch end def path_for_action(action, path) #:nodoc: - if path.blank? && canonical_action?(action, @scope[:scope_level]) + if path.blank? && canonical_action?(action, @scope.scope_level) @scope[:path].to_s else "#{@scope[:path]}/#{action_path(action, path)}" @@ -1739,7 +1738,7 @@ module ActionDispatch end def name_for_action(as, action) #:nodoc: - scope_level = @scope[:scope_level] + scope_level = @scope.scope_level prefix = prefix_name_for_action(as, action, scope_level) name_prefix = @scope[:as] @@ -1900,6 +1899,8 @@ module ActionDispatch :controller, :action, :path_names, :constraints, :shallow, :blocks, :defaults, :options] + RESOURCE_SCOPES = [:resource, :resources] + attr_reader :parent def initialize(hash, parent = {}) @@ -1907,6 +1908,18 @@ module ActionDispatch @parent = parent end + def scope_level + self[:scope_level] + end + + def nested? + scope_level == :nested + end + + def resource_scope? + RESOURCE_SCOPES.include? scope_level + end + def options OPTIONS end -- cgit v1.2.3 From 19bb6770c0b4a7970a8c6daa2a1ed0f926e721f2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:24:40 -0700 Subject: move the scope level key fully inside the scope object --- actionpack/lib/action_dispatch/routing/mapper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 77d9889f28..9c9c56fbce 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1654,7 +1654,7 @@ module ActionDispatch end def with_scope_level(kind) - @scope = @scope.new(:scope_level => kind) + @scope = @scope.new_level(kind) yield ensure @scope = @scope.parent @@ -1928,6 +1928,10 @@ module ActionDispatch self.class.new hash, self end + def new_level(level) + new(:scope_level => level) + end + def [](key) @hash.fetch(key) { @parent[key] } end -- cgit v1.2.3 From 677bc212eb7f35ad0f8808f56450a4b6b2340023 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:29:24 -0700 Subject: scope_level is no longer a hash key, just use the ivar --- actionpack/lib/action_dispatch/routing/mapper.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 9c9c56fbce..fa273ac201 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1903,13 +1903,14 @@ module ActionDispatch attr_reader :parent - def initialize(hash, parent = {}) + def initialize(hash, parent = {}, scope_level = nil) @hash = hash @parent = parent + @scope_level = scope_level end def scope_level - self[:scope_level] + @scope_level end def nested? @@ -1925,11 +1926,15 @@ module ActionDispatch end def new(hash) - self.class.new hash, self + self.class.new hash, self, scope_level end def new_level(level) - new(:scope_level => level) + self.class.new(self, self, level) + end + + def fetch(key, &block) + @hash.fetch(key, &block) end def [](key) -- cgit v1.2.3 From 047af8dd3c304fbfe625be8b7c600005c69fa593 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:29:39 -0700 Subject: change to attr_reader --- actionpack/lib/action_dispatch/routing/mapper.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index fa273ac201..209af7a16d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1901,7 +1901,7 @@ module ActionDispatch RESOURCE_SCOPES = [:resource, :resources] - attr_reader :parent + attr_reader :parent, :scope_level def initialize(hash, parent = {}, scope_level = nil) @hash = hash @@ -1909,10 +1909,6 @@ module ActionDispatch @scope_level = scope_level end - def scope_level - @scope_level - end - def nested? scope_level == :nested end -- cgit v1.2.3 From 374d66be3e9e994bbe8ad2702ee2209c24b581b0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:40:57 -0700 Subject: reduce calls to scope_level this will help us to encapsulate magical symbols so hopefully we can eliminate hardcoded magic symbols --- actionpack/lib/action_dispatch/routing/mapper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 209af7a16d..46d83f7fd7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1563,7 +1563,7 @@ module ActionDispatch raise ArgumentError, "must be called with a path and/or options" end - if @scope.scope_level == :resources + if @scope.resources? with_scope_level(:root) do scope(parent_resource.path) do super(options) @@ -1913,6 +1913,10 @@ module ActionDispatch scope_level == :nested end + def resources? + scope_level == :resources + end + def resource_scope? RESOURCE_SCOPES.include? scope_level end -- cgit v1.2.3 From e4cb3819dfe36cc9a8396fb207b74980d7bd0cd5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:44:05 -0700 Subject: ask the scope for the action name --- actionpack/lib/action_dispatch/routing/mapper.rb | 32 +++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 46d83f7fd7..1a7e9d32e6 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1749,20 +1749,7 @@ module ActionDispatch member_name = parent_resource.member_name end - name = case scope_level - when :nested - [name_prefix, prefix] - when :collection - [prefix, name_prefix, collection_name] - when :new - [prefix, :new, name_prefix, member_name] - when :member - [prefix, name_prefix, member_name] - when :root - [name_prefix, collection_name, prefix] - else - [name_prefix, member_name, prefix] - end + name = @scope.action_name(name_prefix, prefix, collection_name, member_name) if candidate = name.compact.join("_").presence # If a name was not explicitly given, we check if it is valid @@ -1917,6 +1904,23 @@ module ActionDispatch scope_level == :resources end + def action_name(name_prefix, prefix, collection_name, member_name) + case scope_level + when :nested + [name_prefix, prefix] + when :collection + [prefix, name_prefix, collection_name] + when :new + [prefix, :new, name_prefix, member_name] + when :member + [prefix, name_prefix, member_name] + when :root + [name_prefix, collection_name, prefix] + else + [name_prefix, member_name, prefix] + end + end + def resource_scope? RESOURCE_SCOPES.include? scope_level end -- cgit v1.2.3 From 43ce6e22b19245318ff154f859c82272130ac238 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 13 Aug 2014 19:47:26 -0700 Subject: ask the scope object if it is a resource_method_scope --- actionpack/lib/action_dispatch/routing/mapper.rb | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 1a7e9d32e6..e92baa5aa7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1046,7 +1046,6 @@ module ActionDispatch VALID_ON_OPTIONS = [:new, :collection, :member] RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except, :param, :concerns] CANONICAL_ACTIONS = %w(index create new show update destroy) - RESOURCE_METHOD_SCOPES = [:collection, :member, :new] class Resource #:nodoc: attr_reader :controller, :path, :options, :param @@ -1633,8 +1632,8 @@ module ActionDispatch @scope.resource_scope? end - def resource_method_scope?(scope_level) #:nodoc: - RESOURCE_METHOD_SCOPES.include? scope_level + def resource_method_scope? #:nodoc: + @scope.resource_method_scope? end def nested_scope? #:nodoc: @@ -1698,8 +1697,8 @@ module ActionDispatch @scope[:constraints][parent_resource.param] end - def canonical_action?(action, scope_level) #:nodoc: - scope_level && resource_method_scope?(scope_level) && CANONICAL_ACTIONS.include?(action.to_s) + def canonical_action?(action) #:nodoc: + resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) end def shallow_scope(path, options = {}) #:nodoc: @@ -1713,7 +1712,7 @@ module ActionDispatch end def path_for_action(action, path) #:nodoc: - if path.blank? && canonical_action?(action, @scope.scope_level) + if path.blank? && canonical_action?(action) @scope[:path].to_s else "#{@scope[:path]}/#{action_path(action, path)}" @@ -1725,10 +1724,10 @@ module ActionDispatch path || @scope[:path_names][name] || name.to_s end - def prefix_name_for_action(as, action, scope_level) #:nodoc: + def prefix_name_for_action(as, action) #:nodoc: if as prefix = as - elsif !canonical_action?(action, scope_level) + elsif !canonical_action?(action) prefix = action end @@ -1738,8 +1737,7 @@ module ActionDispatch end def name_for_action(as, action) #:nodoc: - scope_level = @scope.scope_level - prefix = prefix_name_for_action(as, action, scope_level) + prefix = prefix_name_for_action(as, action) name_prefix = @scope[:as] if parent_resource @@ -1887,6 +1885,7 @@ module ActionDispatch :shallow, :blocks, :defaults, :options] RESOURCE_SCOPES = [:resource, :resources] + RESOURCE_METHOD_SCOPES = [:collection, :member, :new] attr_reader :parent, :scope_level @@ -1904,6 +1903,10 @@ module ActionDispatch scope_level == :resources end + def resource_method_scope? + RESOURCE_METHOD_SCOPES.include? scope_level + end + def action_name(name_prefix, prefix, collection_name, member_name) case scope_level when :nested -- cgit v1.2.3 From a1ddde15ae0d612ff2973de9cf768ed701b594e8 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Thu, 14 Aug 2014 09:35:32 +0200 Subject: remove deprecated `MissingHelperError` proxy. The error was moved outside of the `ClassMethods` module. --- actionpack/CHANGELOG.md | 5 +++++ actionpack/lib/abstract_controller/helpers.rb | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6ea7276ac6..7c460fbaef 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Remove deprecated `AbstractController::Helpers::ClassMethods::MissingHelperError` + in favor of `AbstractController::Helpers::MissingHelperError`. + + *Yves Senn* + * Fix `assert_template` not being able to assert that no files were rendered. *Guo Xiang Tan* diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index e77e4e01e9..95c67d482b 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -27,9 +27,6 @@ module AbstractController end module ClassMethods - MissingHelperError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('AbstractController::Helpers::ClassMethods::MissingHelperError', - 'AbstractController::Helpers::MissingHelperError') - # When a class is inherited, wrap its helper module in a new module. # This ensures that the parent class's module can be changed # independently of the child class's. -- cgit v1.2.3 From 6c51cc85da458969299ae3d0565dccc3a8530df9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 14 Aug 2014 10:23:14 -0700 Subject: extract methods and metaprogram less. --- .../action_dispatch/routing/polymorphic_routes.rb | 27 +++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index bd3696cda1..cd8b1ab066 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -142,22 +142,27 @@ module ActionDispatch %w(edit new).each do |action| module_eval <<-EOT, __FILE__, __LINE__ + 1 - def #{action}_polymorphic_url(record_or_hash, options = {}) # def edit_polymorphic_url(record_or_hash, options = {}) - polymorphic_url( # polymorphic_url( - record_or_hash, # record_or_hash, - options.merge(:action => "#{action}")) # options.merge(:action => "edit")) - end # end - # - def #{action}_polymorphic_path(record_or_hash, options = {}) # def edit_polymorphic_path(record_or_hash, options = {}) - polymorphic_url( # polymorphic_url( - record_or_hash, # record_or_hash, - options.merge(:action => "#{action}", :routing_type => :path)) # options.merge(:action => "edit", :routing_type => :path)) - end # end + def #{action}_polymorphic_url(record_or_hash, options = {}) + polymorphic_url_for_action("#{action}", record_or_hash, options) + end + + def #{action}_polymorphic_path(record_or_hash, options = {}) + polymorphic_path_for_action("#{action}", record_or_hash, options) + end EOT end private + def polymorphic_url_for_action(action, record_or_hash, options) + polymorphic_url(record_or_hash, options.merge(:action => action)) + end + + def polymorphic_path_for_action(action, record_or_hash, options) + options = options.merge(:action => action, :routing_type => :path) + polymorphic_path(record_or_hash, options) + end + class HelperMethodBuilder # :nodoc: CACHE = { 'path' => {}, 'url' => {} } -- cgit v1.2.3 From 6c96602bc1480b41f5fd20aef46fc70bcf582aab Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 16 Aug 2014 15:06:20 -0700 Subject: When your templates change, browser caches bust automatically. New default: the template digest is automatically included in your ETags. When you call `fresh_when @post`, the digest for `posts/show.html.erb` is mixed in so future changes to the HTML will blow HTTP caches for you. This makes it easy to HTTP-cache many more of your actions. If you render a different template, you can now pass the `:template` option to include its digest instead: fresh_when @post, template: 'widgets/show' Pass `template: false` to skip the lookup. To turn this off entirely, set: config.action_controller.etag_with_template_digest = false --- actionpack/CHANGELOG.md | 18 ++++++++ actionpack/lib/action_controller.rb | 1 + actionpack/lib/action_controller/base.rb | 1 + .../lib/action_controller/metal/conditional_get.rb | 37 +++++++++++++--- .../metal/etag_with_template_digest.rb | 50 ++++++++++++++++++++++ actionpack/test/controller/live_stream_test.rb | 2 +- actionpack/test/controller/render_test.rb | 36 +++++++++++++++- 7 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 actionpack/lib/action_controller/metal/etag_with_template_digest.rb (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7c460fbaef..90fb99ad3a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,21 @@ +* When your templates change, browser caches bust automatically. + + New default: the template digest is automatically included in your ETags. + When you call `fresh_when @post`, the digest for `posts/show.html.erb` + is mixed in so future changes to the HTML will blow HTTP caches for you. + This makes it easy to HTTP-cache many more of your actions. + + If you render a different template, you can now pass the `:template` + option to include its digest instead: + + fresh_when @post, template: 'widgets/show' + + Pass `template: false` to skip the lookup. To turn this off entirely, set: + + config.action_controller.etag_with_template_digest = false + + *Jeremy Kemper* + * Remove deprecated `AbstractController::Helpers::ClassMethods::MissingHelperError` in favor of `AbstractController::Helpers::MissingHelperError`. diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 50bc26a80f..7f1aeafe8b 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -17,6 +17,7 @@ module ActionController autoload :ConditionalGet autoload :Cookies autoload :DataStreaming + autoload :EtagWithTemplateDigest autoload :Flash autoload :ForceSSL autoload :Head diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index e6fe6b0b00..7bbf938987 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -213,6 +213,7 @@ module ActionController Rendering, Renderers::All, ConditionalGet, + EtagWithTemplateDigest, RackDelegation, Caching, MimeResponds, diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 6e0cd51d8b..a93727df90 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -41,6 +41,11 @@ module ActionController # * :last_modified. # * :public By default the Cache-Control header is private, set this to # +true+ if you want your application to be cachable by other devices (proxy caches). + # * :template By default, the template digest for the current + # controller/action is included in ETags. If the action renders a + # different template, you can include its digest instead. If the action + # doesn't render a template at all, you can pass template: false + # to skip any attempt to check for a template digest. # # === Example: # @@ -66,18 +71,24 @@ module ActionController # @article = Article.find(params[:id]) # fresh_when(@article, public: true) # end + # + # When rendering a different template than the default controller/action + # style, you can indicate which digest to include in the ETag: + # + # before_action { fresh_when @article, template: 'widgets/show' } + # def fresh_when(record_or_options, additional_options = {}) if record_or_options.is_a? Hash options = record_or_options - options.assert_valid_keys(:etag, :last_modified, :public) + options.assert_valid_keys(:etag, :last_modified, :public, :template) else record = record_or_options options = { etag: record, last_modified: record.try(:updated_at) }.merge!(additional_options) end - response.etag = combine_etags(options[:etag]) if options[:etag] - response.last_modified = options[:last_modified] if options[:last_modified] - response.cache_control[:public] = true if options[:public] + response.etag = combine_etags(options) if options[:etag] || options[:template] + response.last_modified = options[:last_modified] if options[:last_modified] + response.cache_control[:public] = true if options[:public] head :not_modified if request.fresh?(response) end @@ -93,6 +104,11 @@ module ActionController # * :last_modified. # * :public By default the Cache-Control header is private, set this to # +true+ if you want your application to be cachable by other devices (proxy caches). + # * :template By default, the template digest for the current + # controller/action is included in ETags. If the action renders a + # different template, you can include its digest instead. If the action + # doesn't render a template at all, you can pass template: false + # to skip any attempt to check for a template digest. # # === Example: # @@ -133,6 +149,14 @@ module ActionController # end # end # end + # + # When rendering a different template than the default controller/action + # style, you can indicate which digest to include in the ETag: + # + # def show + # super if stale? @article, template: 'widgets/show' + # end + # def stale?(record_or_options, additional_options = {}) fresh_when(record_or_options, additional_options) !request.fresh?(response) @@ -168,8 +192,9 @@ module ActionController end private - def combine_etags(etag) - [ etag, *etaggers.map { |etagger| instance_exec(&etagger) }.compact ] + def combine_etags(options) + etags = etaggers.map { |etagger| instance_exec(options, &etagger) }.compact + etags.unshift options[:etag] end end end diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb new file mode 100644 index 0000000000..3ca0c6837a --- /dev/null +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -0,0 +1,50 @@ +module ActionController + # When our views change, they should bubble up into HTTP cache freshness + # and bust browser caches. So the template digest for the current action + # is automatically included in the ETag. + # + # Enabled by default for apps that use Action View. Disable by setting + # + # config.action_controller.etag_with_template_digest = false + # + # Override the template to digest by passing `:template` to `fresh_when` + # and `stale?` calls. For example: + # + # # We're going to render widgets/show, not posts/show + # fresh_when @post, template: 'widgets/show' + # + # # We're not going to render a template, so omit it from the ETag. + # fresh_when @post, template: false + # + module EtagWithTemplateDigest + extend ActiveSupport::Concern + + include ActionController::ConditionalGet + + included do + class_attribute :etag_with_template_digest + self.etag_with_template_digest = true + + ActiveSupport.on_load :action_view, yield: true do |action_view_base| + etag do |options| + determine_template_etag(options) if etag_with_template_digest + end + end + end + + private + def determine_template_etag(options) + if template = pick_template_for_etag(options) + lookup_and_digest_template(template) + end + end + + def pick_template_for_etag(options) + options.fetch(:template) { "#{controller_name}/#{action_name}" } + end + + def lookup_and_digest_template(template) + ActionView::Digestor.digest name: template, finder: lookup_context + end + end +end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 0500b7c789..7fd1276e98 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -162,7 +162,7 @@ module ActionController end def with_stale - render :text => 'stale' if stale?(:etag => "123") + render text: 'stale' if stale?(etag: "123", template: false) end def exception_in_view diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 9926130c02..b036b6c08e 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -10,11 +10,17 @@ class TestControllerWithExtraEtags < ActionController::Base etag { nil } def fresh - render text: "stale" if stale?(etag: '123') + render text: "stale" if stale?(etag: '123', template: false) end def array - render text: "stale" if stale?(etag: %w(1 2 3)) + render text: "stale" if stale?(etag: %w(1 2 3), template: false) + end + + def with_template + if stale? template: 'test/hello_world' + render text: 'stale' + end end end @@ -409,6 +415,32 @@ class EtagRenderTest < ActionController::TestCase assert_response :success end + def test_etag_reflects_template_digest + get :with_template + assert_response :ok + assert_not_nil etag = @response.etag + + request.if_none_match = etag + get :with_template + assert_response :not_modified + + # Modify the template digest + path = File.expand_path('../../fixtures/test/hello_world.erb', __FILE__) + old = File.read(path) + + begin + File.write path, 'foo' + ActionView::Digestor.cache.clear + + request.if_none_match = etag + get :with_template + assert_response :ok + assert_not_equal etag, @response.etag + ensure + File.write path, old + end + end + def etag(record) Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record)).inspect end -- cgit v1.2.3 From ee77770d57de9da87b05a2fe84b9d46ec6852c62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 16 Aug 2014 16:24:08 -0400 Subject: Move respond_with to the responders gem respond_with (and consequently the class-level respond_to) are being removed from Rails. Instead of moving it to a 3rd library, the functionality will be moved to responders gem (at github.com/plataformatec/responders) which already provides some responders extensions. --- actionpack/CHANGELOG.md | 5 + .../lib/action_controller/metal/mime_responds.rb | 234 +------ .../lib/action_controller/metal/responder.rb | 297 --------- .../test/controller/mime/respond_with_test.rb | 737 --------------------- 4 files changed, 8 insertions(+), 1265 deletions(-) delete mode 100644 actionpack/lib/action_controller/metal/responder.rb delete mode 100644 actionpack/test/controller/mime/respond_with_test.rb (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 90fb99ad3a..84ea8f4b22 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Move `respond_with` (and the class-level `respond_to`) to + the `responders` gem. + + *José Valim* + * When your templates change, browser caches bust automatically. New default: the template digest is automatically included in your ETags. diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 00e7e980f8..a533f03b6d 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -5,58 +5,6 @@ module ActionController #:nodoc: module MimeResponds extend ActiveSupport::Concern - included do - class_attribute :responder, :mimes_for_respond_to - self.responder = ActionController::Responder - clear_respond_to - end - - module ClassMethods - # Defines mime types that are rendered by default when invoking - # respond_with. - # - # respond_to :html, :xml, :json - # - # Specifies that all actions in the controller respond to requests - # for :html, :xml and :json. - # - # To specify on per-action basis, use :only and - # :except with an array of actions or a single action: - # - # respond_to :html - # respond_to :xml, :json, except: [ :edit ] - # - # This specifies that all actions respond to :html - # and all actions except :edit respond to :xml and - # :json. - # - # respond_to :json, only: :create - # - # This specifies that the :create action and no other responds - # to :json. - def respond_to(*mimes) - options = mimes.extract_options! - - only_actions = Array(options.delete(:only)).map(&:to_s) - except_actions = Array(options.delete(:except)).map(&:to_s) - - new = mimes_for_respond_to.dup - mimes.each do |mime| - mime = mime.to_sym - new[mime] = {} - new[mime][:only] = only_actions unless only_actions.empty? - new[mime][:except] = except_actions unless except_actions.empty? - end - self.mimes_for_respond_to = new.freeze - end - - # Clear all mime types in respond_to. - # - def clear_respond_to - self.mimes_for_respond_to = Hash.new.freeze - end - end - # Without web-service support, an action which collects the data for displaying a list of people # might look something like this: # @@ -253,189 +201,13 @@ module ActionController #:nodoc: def respond_to(*mimes, &block) raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? - if collector = retrieve_collector_from_mimes(mimes, &block) - response = collector.response - response ? response.call : render({}) - end - end - - # For a given controller action, respond_with generates an appropriate - # response based on the mime-type requested by the client. - # - # If the method is called with just a resource, as in this example - - # - # class PeopleController < ApplicationController - # respond_to :html, :xml, :json - # - # def index - # @people = Person.all - # respond_with @people - # end - # end - # - # then the mime-type of the response is typically selected based on the - # request's Accept header and the set of available formats declared - # by previous calls to the controller's class method +respond_to+. Alternatively - # the mime-type can be selected by explicitly setting request.format in - # the controller. - # - # If an acceptable format is not identified, the application returns a - # '406 - not acceptable' status. Otherwise, the default response is to render - # a template named after the current action and the selected format, - # e.g. index.html.erb. If no template is available, the behavior - # depends on the selected format: - # - # * for an html response - if the request method is +get+, an exception - # is raised but for other requests such as +post+ the response - # depends on whether the resource has any validation errors (i.e. - # assuming that an attempt has been made to save the resource, - # e.g. by a +create+ action) - - # 1. If there are no errors, i.e. the resource - # was saved successfully, the response +redirect+'s to the resource - # i.e. its +show+ action. - # 2. If there are validation errors, the response - # renders a default action, which is :new for a - # +post+ request or :edit for +patch+ or +put+. - # Thus an example like this - - # - # respond_to :html, :xml - # - # def create - # @user = User.new(params[:user]) - # flash[:notice] = 'User was successfully created.' if @user.save - # respond_with(@user) - # end - # - # is equivalent, in the absence of create.html.erb, to - - # - # def create - # @user = User.new(params[:user]) - # respond_to do |format| - # if @user.save - # flash[:notice] = 'User was successfully created.' - # format.html { redirect_to(@user) } - # format.xml { render xml: @user } - # else - # format.html { render action: "new" } - # format.xml { render xml: @user } - # end - # end - # end - # - # * for a JavaScript request - if the template isn't found, an exception is - # raised. - # * for other requests - i.e. data formats such as xml, json, csv etc, if - # the resource passed to +respond_with+ responds to to_, - # the method attempts to render the resource in the requested format - # directly, e.g. for an xml request, the response is equivalent to calling - # render xml: resource. - # - # === Nested resources - # - # As outlined above, the +resources+ argument passed to +respond_with+ - # can play two roles. It can be used to generate the redirect url - # for successful html requests (e.g. for +create+ actions when - # no template exists), while for formats other than html and JavaScript - # it is the object that gets rendered, by being converted directly to the - # required format (again assuming no template exists). - # - # For redirecting successful html requests, +respond_with+ also supports - # the use of nested resources, which are supplied in the same way as - # in form_for and polymorphic_url. For example - - # - # def create - # @project = Project.find(params[:project_id]) - # @task = @project.comments.build(params[:task]) - # flash[:notice] = 'Task was successfully created.' if @task.save - # respond_with(@project, @task) - # end - # - # This would cause +respond_with+ to redirect to project_task_url - # instead of task_url. For request formats other than html or - # JavaScript, if multiple resources are passed in this way, it is the last - # one specified that is rendered. - # - # === Customizing response behavior - # - # Like +respond_to+, +respond_with+ may also be called with a block that - # can be used to overwrite any of the default responses, e.g. - - # - # def create - # @user = User.new(params[:user]) - # flash[:notice] = "User was successfully created." if @user.save - # - # respond_with(@user) do |format| - # format.html { render } - # end - # end - # - # The argument passed to the block is an ActionController::MimeResponds::Collector - # object which stores the responses for the formats defined within the - # block. Note that formats with responses defined explicitly in this way - # do not have to first be declared using the class method +respond_to+. - # - # Also, a hash passed to +respond_with+ immediately after the specified - # resource(s) is interpreted as a set of options relevant to all - # formats. Any option accepted by +render+ can be used, e.g. - # respond_with @people, status: 200 - # However, note that these options are ignored after an unsuccessful attempt - # to save a resource, e.g. when automatically rendering :new - # after a post request. - # - # Two additional options are relevant specifically to +respond_with+ - - # 1. :location - overwrites the default redirect location used after - # a successful html +post+ request. - # 2. :action - overwrites the default render action used after an - # unsuccessful html +post+ request. - def respond_with(*resources, &block) - if self.class.mimes_for_respond_to.empty? - raise "In order to use respond_with, first you need to declare the " \ - "formats your controller responds to in the class level." - end - - if collector = retrieve_collector_from_mimes(&block) - options = resources.size == 1 ? {} : resources.extract_options! - options = options.clone - options[:default_response] = collector.response - (options.delete(:responder) || self.class.responder).call(self, resources, options) - end - end - - protected - - # Collect mimes declared in the class method respond_to valid for the - # current action. - def collect_mimes_from_class_level #:nodoc: - action = action_name.to_s - - self.class.mimes_for_respond_to.keys.select do |mime| - config = self.class.mimes_for_respond_to[mime] - - if config[:except] - !config[:except].include?(action) - elsif config[:only] - config[:only].include?(action) - else - true - end - end - end - - # Returns a Collector object containing the appropriate mime-type response - # for the current request, based on the available responses defined by a block. - # In typical usage this is the block passed to +respond_with+ or +respond_to+. - # - # Sends :not_acceptable to the client and returns nil if no suitable format - # is available. - def retrieve_collector_from_mimes(mimes=nil, &block) #:nodoc: - mimes ||= collect_mimes_from_class_level collector = Collector.new(mimes, request.variant) block.call(collector) if block_given? - format = collector.negotiate_format(request) - if format + if format = collector.negotiate_format(request) _process_format(format) - collector + response = collector.response + response ? response.call : render({}) else raise ActionController::UnknownFormat end diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb deleted file mode 100644 index 5096558c67..0000000000 --- a/actionpack/lib/action_controller/metal/responder.rb +++ /dev/null @@ -1,297 +0,0 @@ -require 'active_support/json' - -module ActionController #:nodoc: - # Responsible for exposing a resource to different mime requests, - # usually depending on the HTTP verb. The responder is triggered when - # respond_with is called. The simplest case to study is a GET request: - # - # class PeopleController < ApplicationController - # respond_to :html, :xml, :json - # - # def index - # @people = Person.all - # respond_with(@people) - # end - # end - # - # When a request comes in, for example for an XML response, three steps happen: - # - # 1) the responder searches for a template at people/index.xml; - # - # 2) if the template is not available, it will invoke #to_xml on the given resource; - # - # 3) if the responder does not respond_to :to_xml, call #to_format on it. - # - # === Built-in HTTP verb semantics - # - # The default \Rails responder holds semantics for each HTTP verb. Depending on the - # content type, verb and the resource status, it will behave differently. - # - # Using \Rails default responder, a POST request for creating an object could - # be written as: - # - # def create - # @user = User.new(params[:user]) - # flash[:notice] = 'User was successfully created.' if @user.save - # respond_with(@user) - # end - # - # Which is exactly the same as: - # - # def create - # @user = User.new(params[:user]) - # - # respond_to do |format| - # if @user.save - # flash[:notice] = 'User was successfully created.' - # format.html { redirect_to(@user) } - # format.xml { render xml: @user, status: :created, location: @user } - # else - # format.html { render action: "new" } - # format.xml { render xml: @user.errors, status: :unprocessable_entity } - # end - # end - # end - # - # The same happens for PATCH/PUT and DELETE requests. - # - # === Nested resources - # - # You can supply nested resources as you do in form_for and polymorphic_url. - # Consider the project has many tasks example. The create action for - # TasksController would be like: - # - # def create - # @project = Project.find(params[:project_id]) - # @task = @project.tasks.build(params[:task]) - # flash[:notice] = 'Task was successfully created.' if @task.save - # respond_with(@project, @task) - # end - # - # Giving several resources ensures that the responder will redirect to - # project_task_url instead of task_url. - # - # Namespaced and singleton resources require a symbol to be given, as in - # polymorphic urls. If a project has one manager which has many tasks, it - # should be invoked as: - # - # respond_with(@project, :manager, @task) - # - # Note that if you give an array, it will be treated as a collection, - # so the following is not equivalent: - # - # respond_with [@project, :manager, @task] - # - # === Custom options - # - # respond_with also allows you to pass options that are forwarded - # to the underlying render call. Those options are only applied for success - # scenarios. For instance, you can do the following in the create method above: - # - # def create - # @project = Project.find(params[:project_id]) - # @task = @project.tasks.build(params[:task]) - # flash[:notice] = 'Task was successfully created.' if @task.save - # respond_with(@project, @task, status: 201) - # end - # - # This will return status 201 if the task was saved successfully. If not, - # it will simply ignore the given options and return status 422 and the - # resource errors. You can also override the location to redirect to: - # - # respond_with(@project, location: root_path) - # - # To customize the failure scenario, you can pass a block to - # respond_with: - # - # def create - # @project = Project.find(params[:project_id]) - # @task = @project.tasks.build(params[:task]) - # respond_with(@project, @task, status: 201) do |format| - # if @task.save - # flash[:notice] = 'Task was successfully created.' - # else - # format.html { render "some_special_template" } - # end - # end - # end - # - # Using respond_with with a block follows the same syntax as respond_to. - class Responder - attr_reader :controller, :request, :format, :resource, :resources, :options - - DEFAULT_ACTIONS_FOR_VERBS = { - :post => :new, - :patch => :edit, - :put => :edit - } - - def initialize(controller, resources, options={}) - @controller = controller - @request = @controller.request - @format = @controller.formats.first - @resource = resources.last - @resources = resources - @options = options - @action = options.delete(:action) - @default_response = options.delete(:default_response) - end - - delegate :head, :render, :redirect_to, :to => :controller - delegate :get?, :post?, :patch?, :put?, :delete?, :to => :request - - # Undefine :to_json and :to_yaml since it's defined on Object - undef_method(:to_json) if method_defined?(:to_json) - undef_method(:to_yaml) if method_defined?(:to_yaml) - - # Initializes a new responder and invokes the proper format. If the format is - # not defined, call to_format. - # - def self.call(*args) - new(*args).respond - end - - # Main entry point for responder responsible to dispatch to the proper format. - # - def respond - method = "to_#{format}" - respond_to?(method) ? send(method) : to_format - end - - # HTML format does not render the resource, it always attempt to render a - # template. - # - def to_html - default_render - rescue ActionView::MissingTemplate => e - navigation_behavior(e) - end - - # to_js simply tries to render a template. If no template is found, raises the error. - def to_js - default_render - end - - # All other formats follow the procedure below. First we try to render a - # template, if the template is not available, we verify if the resource - # responds to :to_format and display it. - # - def to_format - if get? || !has_errors? || response_overridden? - default_render - else - display_errors - end - rescue ActionView::MissingTemplate => e - api_behavior(e) - end - - protected - - # This is the common behavior for formats associated with browsing, like :html, :iphone and so forth. - def navigation_behavior(error) - if get? - raise error - elsif has_errors? && default_action - render :action => default_action - else - redirect_to navigation_location - end - end - - # This is the common behavior for formats associated with APIs, such as :xml and :json. - def api_behavior(error) - raise error unless resourceful? - raise MissingRenderer.new(format) unless has_renderer? - - if get? - display resource - elsif post? - display resource, :status => :created, :location => api_location - else - head :no_content - end - end - - # Checks whether the resource responds to the current format or not. - # - def resourceful? - resource.respond_to?("to_#{format}") - end - - # Returns the resource location by retrieving it from the options or - # returning the resources array. - # - def resource_location - options[:location] || resources - end - alias :navigation_location :resource_location - alias :api_location :resource_location - - # If a response block was given, use it, otherwise call render on - # controller. - # - def default_render - if @default_response - @default_response.call(options) - else - controller.default_render(options) - end - end - - # Display is just a shortcut to render a resource with the current format. - # - # display @user, status: :ok - # - # For XML requests it's equivalent to: - # - # render xml: @user, status: :ok - # - # Options sent by the user are also used: - # - # respond_with(@user, status: :created) - # display(@user, status: :ok) - # - # Results in: - # - # render xml: @user, status: :created - # - def display(resource, given_options={}) - controller.render given_options.merge!(options).merge!(format => resource) - end - - def display_errors - controller.render format => resource_errors, :status => :unprocessable_entity - end - - # Check whether the resource has errors. - # - def has_errors? - resource.respond_to?(:errors) && !resource.errors.empty? - end - - # Check whether the necessary Renderer is available - def has_renderer? - Renderers::RENDERERS.include?(format) - end - - # By default, render the :edit action for HTML requests with errors, unless - # the verb was POST. - # - def default_action - @action ||= DEFAULT_ACTIONS_FOR_VERBS[request.request_method_symbol] - end - - def resource_errors - respond_to?("#{format}_resource_errors", true) ? send("#{format}_resource_errors") : resource.errors - end - - def json_resource_errors - {:errors => resource.errors} - end - - def response_overridden? - @default_response.present? - end - end -end diff --git a/actionpack/test/controller/mime/respond_with_test.rb b/actionpack/test/controller/mime/respond_with_test.rb deleted file mode 100644 index 115f3b2f41..0000000000 --- a/actionpack/test/controller/mime/respond_with_test.rb +++ /dev/null @@ -1,737 +0,0 @@ -require 'abstract_unit' -require 'controller/fake_models' - -class RespondWithController < ActionController::Base - class CustomerWithJson < Customer - def to_json; super; end - end - - respond_to :html, :json, :touch - respond_to :xml, :except => :using_resource_with_block - respond_to :js, :only => [ :using_resource_with_block, :using_resource, 'using_hash_resource' ] - - def using_resource - respond_with(resource) - end - - def using_hash_resource - respond_with({:result => resource}) - end - - def using_resource_with_block - respond_with(resource) do |format| - format.csv { render :text => "CSV" } - end - end - - def using_resource_with_overwrite_block - respond_with(resource) do |format| - format.html { render :text => "HTML" } - end - end - - def using_resource_with_collection - respond_with([resource, Customer.new("jamis", 9)]) - end - - def using_resource_with_parent - respond_with(Quiz::Store.new("developer?", 11), Customer.new("david", 13)) - end - - def using_resource_with_status_and_location - respond_with(resource, :location => "http://test.host/", :status => :created) - end - - def using_resource_with_json - respond_with(CustomerWithJson.new("david", request.delete? ? nil : 13)) - end - - def using_invalid_resource_with_template - respond_with(resource) - end - - def using_options_with_template - @customer = resource - respond_with(@customer, :status => 123, :location => "http://test.host/") - end - - def using_resource_with_responder - responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } - respond_with(resource, :responder => responder) - end - - def using_resource_with_action - respond_with(resource, :action => :foo) do |format| - format.html { raise ActionView::MissingTemplate.new([], "bar", ["foo"], {}, false) } - end - end - - def using_responder_with_respond - responder = Class.new(ActionController::Responder) do - def respond; @controller.render :text => "respond #{format}"; end - end - respond_with(resource, :responder => responder) - end - - def respond_with_additional_params - @params = RespondWithController.params - respond_with({:result => resource}, @params) - end - -protected - def self.params - { - :foo => 'bar' - } - end - - def resource - Customer.new("david", request.delete? ? nil : 13) - end -end - -class InheritedRespondWithController < RespondWithController - clear_respond_to - respond_to :xml, :json - - def index - respond_with(resource) do |format| - format.json { render :text => "JSON" } - end - end -end - -class RenderJsonRespondWithController < RespondWithController - clear_respond_to - respond_to :json - - def index - respond_with(resource) do |format| - format.json { render :json => RenderJsonTestException.new('boom') } - end - end - - def create - resource = ValidatedCustomer.new(params[:name], 1) - respond_with(resource) do |format| - format.json do - if resource.errors.empty? - render :json => { :valid => true } - else - render :json => { :valid => false } - end - end - end - end -end - -class CsvRespondWithController < ActionController::Base - respond_to :csv - - class RespondWithCsv - def to_csv - "c,s,v" - end - end - - def index - respond_with(RespondWithCsv.new) - end -end - -class EmptyRespondWithController < ActionController::Base - def index - respond_with(Customer.new("david", 13)) - end -end - -class RespondWithControllerTest < ActionController::TestCase - def setup - super - @request.host = "www.example.com" - Mime::Type.register_alias('text/html', :iphone) - Mime::Type.register_alias('text/html', :touch) - Mime::Type.register('text/x-mobile', :mobile) - end - - def teardown - super - Mime::Type.unregister(:iphone) - Mime::Type.unregister(:touch) - Mime::Type.unregister(:mobile) - end - - def test_respond_with_shouldnt_modify_original_hash - get :respond_with_additional_params - assert_equal RespondWithController.params, assigns(:params) - end - - def test_using_resource - @request.accept = "application/xml" - get :using_resource - assert_equal "application/xml", @response.content_type - assert_equal "david", @response.body - - @request.accept = "application/json" - assert_raise ActionView::MissingTemplate do - get :using_resource - end - end - - def test_using_resource_with_js_simply_tries_to_render_the_template - @request.accept = "text/javascript" - get :using_resource - assert_equal "text/javascript", @response.content_type - assert_equal "alert(\"Hi\");", @response.body - end - - def test_using_hash_resource_with_js_raises_an_error_if_template_cant_be_found - @request.accept = "text/javascript" - assert_raise ActionView::MissingTemplate do - get :using_hash_resource - end - end - - def test_using_hash_resource - @request.accept = "application/xml" - get :using_hash_resource - assert_equal "application/xml", @response.content_type - assert_equal "\n\n david\n\n", @response.body - - @request.accept = "application/json" - get :using_hash_resource - assert_equal "application/json", @response.content_type - assert @response.body.include?("result") - assert @response.body.include?('"name":"david"') - assert @response.body.include?('"id":13') - end - - def test_using_hash_resource_with_post - @request.accept = "application/json" - assert_raise ArgumentError, "Nil location provided. Can't build URI." do - post :using_hash_resource - end - end - - def test_using_resource_with_block - @request.accept = "*/*" - get :using_resource_with_block - assert_equal "text/html", @response.content_type - assert_equal 'Hello world!', @response.body - - @request.accept = "text/csv" - get :using_resource_with_block - assert_equal "text/csv", @response.content_type - assert_equal "CSV", @response.body - - @request.accept = "application/xml" - get :using_resource - assert_equal "application/xml", @response.content_type - assert_equal "david", @response.body - end - - def test_using_resource_with_overwrite_block - get :using_resource_with_overwrite_block - assert_equal "text/html", @response.content_type - assert_equal "HTML", @response.body - end - - def test_not_acceptable - @request.accept = "application/xml" - assert_raises(ActionController::UnknownFormat) do - get :using_resource_with_block - end - - @request.accept = "text/javascript" - assert_raises(ActionController::UnknownFormat) do - get :using_resource_with_overwrite_block - end - end - - def test_using_resource_for_post_with_html_redirects_on_success - with_test_route_set do - post :using_resource - assert_equal "text/html", @response.content_type - assert_equal 302, @response.status - assert_equal "http://www.example.com/customers/13", @response.location - assert @response.redirect? - end - end - - def test_using_resource_for_post_with_html_rerender_on_failure - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - post :using_resource - assert_equal "text/html", @response.content_type - assert_equal 200, @response.status - assert_equal "New world!\n", @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_post_with_xml_yields_created_on_success - with_test_route_set do - @request.accept = "application/xml" - post :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 201, @response.status - assert_equal "david", @response.body - assert_equal "http://www.example.com/customers/13", @response.location - end - end - - def test_using_resource_for_post_with_xml_yields_unprocessable_entity_on_failure - with_test_route_set do - @request.accept = "application/xml" - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - post :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 422, @response.status - assert_equal errors.to_xml, @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_post_with_json_yields_unprocessable_entity_on_failure - with_test_route_set do - @request.accept = "application/json" - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - post :using_resource - assert_equal "application/json", @response.content_type - assert_equal 422, @response.status - errors = {:errors => errors} - assert_equal errors.to_json, @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_patch_with_html_redirects_on_success - with_test_route_set do - patch :using_resource - assert_equal "text/html", @response.content_type - assert_equal 302, @response.status - assert_equal "http://www.example.com/customers/13", @response.location - assert @response.redirect? - end - end - - def test_using_resource_for_patch_with_html_rerender_on_failure - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - patch :using_resource - assert_equal "text/html", @response.content_type - assert_equal 200, @response.status - assert_equal "Edit world!\n", @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_patch_with_html_rerender_on_failure_even_on_method_override - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - @request.env["rack.methodoverride.original_method"] = "POST" - patch :using_resource - assert_equal "text/html", @response.content_type - assert_equal 200, @response.status - assert_equal "Edit world!\n", @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_put_with_html_redirects_on_success - with_test_route_set do - put :using_resource - assert_equal "text/html", @response.content_type - assert_equal 302, @response.status - assert_equal "http://www.example.com/customers/13", @response.location - assert @response.redirect? - end - end - - def test_using_resource_for_put_with_html_rerender_on_failure - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - put :using_resource - - assert_equal "text/html", @response.content_type - assert_equal 200, @response.status - assert_equal "Edit world!\n", @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_put_with_html_rerender_on_failure_even_on_method_override - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - @request.env["rack.methodoverride.original_method"] = "POST" - put :using_resource - assert_equal "text/html", @response.content_type - assert_equal 200, @response.status - assert_equal "Edit world!\n", @response.body - assert_nil @response.location - end - end - - def test_using_resource_for_put_with_xml_yields_no_content_on_success - @request.accept = "application/xml" - put :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 204, @response.status - assert_equal "", @response.body - end - - def test_using_resource_for_put_with_json_yields_no_content_on_success - @request.accept = "application/json" - put :using_resource_with_json - assert_equal "application/json", @response.content_type - assert_equal 204, @response.status - assert_equal "", @response.body - end - - def test_using_resource_for_put_with_xml_yields_unprocessable_entity_on_failure - @request.accept = "application/xml" - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - put :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 422, @response.status - assert_equal errors.to_xml, @response.body - assert_nil @response.location - end - - def test_using_resource_for_put_with_json_yields_unprocessable_entity_on_failure - @request.accept = "application/json" - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - put :using_resource - assert_equal "application/json", @response.content_type - assert_equal 422, @response.status - errors = {:errors => errors} - assert_equal errors.to_json, @response.body - assert_nil @response.location - end - - def test_using_resource_for_delete_with_html_redirects_on_success - with_test_route_set do - Customer.any_instance.stubs(:destroyed?).returns(true) - delete :using_resource - assert_equal "text/html", @response.content_type - assert_equal 302, @response.status - assert_equal "http://www.example.com/customers", @response.location - end - end - - def test_using_resource_for_delete_with_xml_yields_no_content_on_success - Customer.any_instance.stubs(:destroyed?).returns(true) - @request.accept = "application/xml" - delete :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 204, @response.status - assert_equal "", @response.body - end - - def test_using_resource_for_delete_with_json_yields_no_content_on_success - Customer.any_instance.stubs(:destroyed?).returns(true) - @request.accept = "application/json" - delete :using_resource_with_json - assert_equal "application/json", @response.content_type - assert_equal 204, @response.status - assert_equal "", @response.body - end - - def test_using_resource_for_delete_with_html_redirects_on_failure - with_test_route_set do - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - Customer.any_instance.stubs(:destroyed?).returns(false) - delete :using_resource - assert_equal "text/html", @response.content_type - assert_equal 302, @response.status - assert_equal "http://www.example.com/customers", @response.location - end - end - - def test_using_resource_with_parent_for_get - @request.accept = "application/xml" - get :using_resource_with_parent - assert_equal "application/xml", @response.content_type - assert_equal 200, @response.status - assert_equal "david", @response.body - end - - def test_using_resource_with_parent_for_post - with_test_route_set do - @request.accept = "application/xml" - - post :using_resource_with_parent - assert_equal "application/xml", @response.content_type - assert_equal 201, @response.status - assert_equal "david", @response.body - assert_equal "http://www.example.com/quiz_stores/11/customers/13", @response.location - - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - post :using_resource - assert_equal "application/xml", @response.content_type - assert_equal 422, @response.status - assert_equal errors.to_xml, @response.body - assert_nil @response.location - end - end - - def test_using_resource_with_collection - @request.accept = "application/xml" - get :using_resource_with_collection - assert_equal "application/xml", @response.content_type - assert_equal 200, @response.status - assert_match(/david<\/name>/, @response.body) - assert_match(/jamis<\/name>/, @response.body) - end - - def test_using_resource_with_action - @controller.instance_eval do - def render(params={}) - self.response_body = "#{params[:action]} - #{formats}" - end - end - - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - - post :using_resource_with_action - assert_equal "foo - #{[:html].to_s}", @controller.response.body - end - - def test_respond_as_responder_entry_point - @request.accept = "text/html" - get :using_responder_with_respond - assert_equal "respond html", @response.body - - @request.accept = "application/xml" - get :using_responder_with_respond - assert_equal "respond xml", @response.body - end - - def test_clear_respond_to - @controller = InheritedRespondWithController.new - @request.accept = "text/html" - assert_raises(ActionController::UnknownFormat) do - get :index - end - end - - def test_first_in_respond_to_has_higher_priority - @controller = InheritedRespondWithController.new - @request.accept = "*/*" - get :index - assert_equal "application/xml", @response.content_type - assert_equal "david", @response.body - end - - def test_block_inside_respond_with_is_rendered - @controller = InheritedRespondWithController.new - @request.accept = "application/json" - get :index - assert_equal "JSON", @response.body - end - - def test_render_json_object_responds_to_str_still_produce_json - @controller = RenderJsonRespondWithController.new - @request.accept = "application/json" - get :index, :format => :json - assert_match(/"message":"boom"/, @response.body) - assert_match(/"error":"RenderJsonTestException"/, @response.body) - end - - def test_api_response_with_valid_resource_respect_override_block - @controller = RenderJsonRespondWithController.new - post :create, :name => "sikachu", :format => :json - assert_equal '{"valid":true}', @response.body - end - - def test_api_response_with_invalid_resource_respect_override_block - @controller = RenderJsonRespondWithController.new - post :create, :name => "david", :format => :json - assert_equal '{"valid":false}', @response.body - end - - def test_no_double_render_is_raised - @request.accept = "text/html" - assert_raise ActionView::MissingTemplate do - get :using_resource - end - end - - def test_using_resource_with_status_and_location - @request.accept = "text/html" - post :using_resource_with_status_and_location - assert @response.redirect? - assert_equal "http://test.host/", @response.location - - @request.accept = "application/xml" - get :using_resource_with_status_and_location - assert_equal 201, @response.status - end - - def test_using_resource_with_status_and_location_with_invalid_resource - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - - @request.accept = "text/xml" - - post :using_resource_with_status_and_location - assert_equal errors.to_xml, @response.body - assert_equal 422, @response.status - assert_equal nil, @response.location - - put :using_resource_with_status_and_location - assert_equal errors.to_xml, @response.body - assert_equal 422, @response.status - assert_equal nil, @response.location - end - - def test_using_invalid_resource_with_template - errors = { :name => :invalid } - Customer.any_instance.stubs(:errors).returns(errors) - - @request.accept = "text/xml" - - post :using_invalid_resource_with_template - assert_equal errors.to_xml, @response.body - assert_equal 422, @response.status - assert_equal nil, @response.location - - put :using_invalid_resource_with_template - assert_equal errors.to_xml, @response.body - assert_equal 422, @response.status - assert_equal nil, @response.location - end - - def test_using_options_with_template - @request.accept = "text/xml" - - post :using_options_with_template - assert_equal "david", @response.body - assert_equal 123, @response.status - assert_equal "http://test.host/", @response.location - - put :using_options_with_template - assert_equal "david", @response.body - assert_equal 123, @response.status - assert_equal "http://test.host/", @response.location - end - - def test_using_resource_with_responder - get :using_resource_with_responder - assert_equal "Resource name is david", @response.body - end - - def test_using_resource_with_set_responder - RespondWithController.responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } - get :using_resource - assert_equal "Resource name is david", @response.body - ensure - RespondWithController.responder = ActionController::Responder - end - - def test_uses_renderer_if_an_api_behavior - ActionController::Renderers.add :csv do |obj, options| - send_data obj.to_csv, type: Mime::CSV - end - @controller = CsvRespondWithController.new - get :index, format: 'csv' - assert_equal Mime::CSV, @response.content_type - assert_equal "c,s,v", @response.body - ensure - ActionController::Renderers.remove :csv - end - - def test_raises_missing_renderer_if_an_api_behavior_with_no_renderer - @controller = CsvRespondWithController.new - assert_raise ActionController::MissingRenderer do - get :index, format: 'csv' - end - end - - def test_removing_renderers - ActionController::Renderers.add :csv do |obj, options| - send_data obj.to_csv, type: Mime::CSV - end - @controller = CsvRespondWithController.new - @request.accept = "text/csv" - get :index, format: 'csv' - assert_equal Mime::CSV, @response.content_type - - ActionController::Renderers.remove :csv - assert_raise ActionController::MissingRenderer do - get :index, format: 'csv' - end - ensure - ActionController::Renderers.remove :csv - end - - def test_error_is_raised_if_no_respond_to_is_declared_and_respond_with_is_called - @controller = EmptyRespondWithController.new - @request.accept = "*/*" - assert_raise RuntimeError do - get :index - end - end - - private - def with_test_route_set - with_routing do |set| - set.draw do - resources :customers - resources :quiz_stores do - resources :customers - end - get ":controller/:action" - end - yield - end - end -end - -class FlashResponder < ActionController::Responder - def initialize(controller, resources, options={}) - super - end - - def to_html - controller.flash[:notice] = 'Success' - super - end -end - -class FlashResponderController < ActionController::Base - self.responder = FlashResponder - respond_to :html - - def index - respond_with Object.new do |format| - format.html { render :text => 'HTML' } - end - end -end - -class FlashResponderControllerTest < ActionController::TestCase - tests FlashResponderController - - def test_respond_with_block_executed - get :index - assert_equal 'HTML', @response.body - end - - def test_flash_responder_executed - get :index - assert_equal 'Success', flash[:notice] - end -end -- cgit v1.2.3 From 57f5b00ba4f675873d5bab3bc88dae55d4fe7857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 16 Aug 2014 17:04:26 -0400 Subject: Remove more references to respond_with --- actionpack/test/fixtures/respond_with/edit.html.erb | 1 - actionpack/test/fixtures/respond_with/new.html.erb | 1 - .../test/fixtures/respond_with/respond_with_additional_params.html.erb | 0 .../fixtures/respond_with/using_invalid_resource_with_template.xml.erb | 1 - .../test/fixtures/respond_with/using_options_with_template.xml.erb | 1 - actionpack/test/fixtures/respond_with/using_resource.js.erb | 1 - actionpack/test/fixtures/respond_with/using_resource_with_block.html.erb | 1 - 7 files changed, 6 deletions(-) delete mode 100644 actionpack/test/fixtures/respond_with/edit.html.erb delete mode 100644 actionpack/test/fixtures/respond_with/new.html.erb delete mode 100644 actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb delete mode 100644 actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb delete mode 100644 actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb delete mode 100644 actionpack/test/fixtures/respond_with/using_resource.js.erb delete mode 100644 actionpack/test/fixtures/respond_with/using_resource_with_block.html.erb (limited to 'actionpack') diff --git a/actionpack/test/fixtures/respond_with/edit.html.erb b/actionpack/test/fixtures/respond_with/edit.html.erb deleted file mode 100644 index ae82dfa4fc..0000000000 --- a/actionpack/test/fixtures/respond_with/edit.html.erb +++ /dev/null @@ -1 +0,0 @@ -Edit world! diff --git a/actionpack/test/fixtures/respond_with/new.html.erb b/actionpack/test/fixtures/respond_with/new.html.erb deleted file mode 100644 index 96c8f1b88b..0000000000 --- a/actionpack/test/fixtures/respond_with/new.html.erb +++ /dev/null @@ -1 +0,0 @@ -New world! diff --git a/actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb b/actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb deleted file mode 100644 index bf5869ed22..0000000000 --- a/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb +++ /dev/null @@ -1 +0,0 @@ -I should not be displayed \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb deleted file mode 100644 index b313017913..0000000000 --- a/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb +++ /dev/null @@ -1 +0,0 @@ -<%= @customer.name %> \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_resource.js.erb b/actionpack/test/fixtures/respond_with/using_resource.js.erb deleted file mode 100644 index 4417680bce..0000000000 --- a/actionpack/test/fixtures/respond_with/using_resource.js.erb +++ /dev/null @@ -1 +0,0 @@ -alert("Hi"); \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_resource_with_block.html.erb b/actionpack/test/fixtures/respond_with/using_resource_with_block.html.erb deleted file mode 100644 index 6769dd60bd..0000000000 --- a/actionpack/test/fixtures/respond_with/using_resource_with_block.html.erb +++ /dev/null @@ -1 +0,0 @@ -Hello world! \ No newline at end of file -- cgit v1.2.3 From 69ed422a9c534224eb818636d79d4263e2abd0a2 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 17 Aug 2014 11:44:31 -0700 Subject: Fixed broken reference caused by 14965ba --- actionpack/test/dispatch/cookies_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index b40c21c067..7e7dd94425 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -519,8 +519,8 @@ class CookiesTest < ActionController::TestCase sign_secret = @request.env["action_dispatch.key_generator"].generate_key(@request.env["action_dispatch.encrypted_signed_cookie_salt"]) - sha1_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActionDispatch::Cookies::NullSerializer, digest: 'SHA1') - sha256_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActionDispatch::Cookies::NullSerializer, digest: 'SHA256') + sha1_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActiveSupport::MessageEncryptor::NullSerializer, digest: 'SHA1') + sha256_verifier = ActiveSupport::MessageVerifier.new(sign_secret, serializer: ActiveSupport::MessageEncryptor::NullSerializer, digest: 'SHA256') assert_raises(ActiveSupport::MessageVerifier::InvalidSignature) do sha1_verifier.verify(cookies[:foo]) -- cgit v1.2.3 From 24226c51f075ed8d8e721cdefb6d2661c0a1f53a Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 17 Aug 2014 11:54:09 -0700 Subject: Raise a more helpful error for people who are using these extracted features --- .../lib/action_controller/metal/mime_responds.rb | 17 +++++++++++- actionpack/test/controller/mime/responder_test.rb | 30 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/controller/mime/responder_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index a533f03b6d..f5565947aa 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -5,6 +5,21 @@ module ActionController #:nodoc: module MimeResponds extend ActiveSupport::Concern + module ClassMethods + def respond_to(*) + raise NoMethodError, "The controller-level `respond_to' feature has " \ + "been extracted to the `responder` gem. Add it to your Gemfile to " \ + "continue using this feature. Consult the Rails upgrade guide for " \ + "details." + end + end + + def respond_with(*) + raise NoMethodError, "The `respond_with' feature has been extracted " \ + "to the `responder` gem. Add it to your Gemfile to continue using " \ + "this feature. Consult the Rails upgrade guide for details." + end + # Without web-service support, an action which collects the data for displaying a list of people # might look something like this: # @@ -165,7 +180,7 @@ module ActionController #:nodoc: # format.html.phone { redirect_to progress_path } # format.html.none { render "trash" } # end - # + # # Variants also support common `any`/`all` block that formats have. # # It works for both inline: diff --git a/actionpack/test/controller/mime/responder_test.rb b/actionpack/test/controller/mime/responder_test.rb new file mode 100644 index 0000000000..6201af3299 --- /dev/null +++ b/actionpack/test/controller/mime/responder_test.rb @@ -0,0 +1,30 @@ +require 'abstract_unit' +require 'controller/fake_models' + +class ResponderTest < ActionController::TestCase + def test_class_level_respond_to + e = assert_raises(NoMethodError) do + Class.new(ActionController::Base) do + respond_to :json + end + end + + assert_includes e.message, '`responder` gem' + end + + def test_respond_with + klass = Class.new(ActionController::Base) do + def index + respond_with Customer.new("david", 13) + end + end + + @controller = klass.new + + e = assert_raises(NoMethodError) do + get :index + end + + assert_includes e.message, '`responder` gem' + end +end -- cgit v1.2.3 From b662273df3d546a1fdd2de79005fd802f0e12643 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 17 Aug 2014 11:58:17 -0700 Subject: The gem is called 'responders' --- .../lib/action_controller/metal/mime_responds.rb | 4 +-- actionpack/test/controller/mime/responder_test.rb | 30 ---------------------- actionpack/test/controller/mime/responders_test.rb | 30 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 32 deletions(-) delete mode 100644 actionpack/test/controller/mime/responder_test.rb create mode 100644 actionpack/test/controller/mime/responders_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index f5565947aa..5c869d4ce2 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -8,7 +8,7 @@ module ActionController #:nodoc: module ClassMethods def respond_to(*) raise NoMethodError, "The controller-level `respond_to' feature has " \ - "been extracted to the `responder` gem. Add it to your Gemfile to " \ + "been extracted to the `responders` gem. Add it to your Gemfile to " \ "continue using this feature. Consult the Rails upgrade guide for " \ "details." end @@ -16,7 +16,7 @@ module ActionController #:nodoc: def respond_with(*) raise NoMethodError, "The `respond_with' feature has been extracted " \ - "to the `responder` gem. Add it to your Gemfile to continue using " \ + "to the `responders` gem. Add it to your Gemfile to continue using " \ "this feature. Consult the Rails upgrade guide for details." end diff --git a/actionpack/test/controller/mime/responder_test.rb b/actionpack/test/controller/mime/responder_test.rb deleted file mode 100644 index 6201af3299..0000000000 --- a/actionpack/test/controller/mime/responder_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'abstract_unit' -require 'controller/fake_models' - -class ResponderTest < ActionController::TestCase - def test_class_level_respond_to - e = assert_raises(NoMethodError) do - Class.new(ActionController::Base) do - respond_to :json - end - end - - assert_includes e.message, '`responder` gem' - end - - def test_respond_with - klass = Class.new(ActionController::Base) do - def index - respond_with Customer.new("david", 13) - end - end - - @controller = klass.new - - e = assert_raises(NoMethodError) do - get :index - end - - assert_includes e.message, '`responder` gem' - end -end diff --git a/actionpack/test/controller/mime/responders_test.rb b/actionpack/test/controller/mime/responders_test.rb new file mode 100644 index 0000000000..416e3191e6 --- /dev/null +++ b/actionpack/test/controller/mime/responders_test.rb @@ -0,0 +1,30 @@ +require 'abstract_unit' +require 'controller/fake_models' + +class ResponderTest < ActionController::TestCase + def test_class_level_respond_to + e = assert_raises(NoMethodError) do + Class.new(ActionController::Base) do + respond_to :json + end + end + + assert_includes e.message, '`responders` gem' + end + + def test_respond_with + klass = Class.new(ActionController::Base) do + def index + respond_with Customer.new("david", 13) + end + end + + @controller = klass.new + + e = assert_raises(NoMethodError) do + get :index + end + + assert_includes e.message, '`responders` gem' + end +end -- cgit v1.2.3 From a485633b16abd64b0955010fa93e0bc4894c7b7c Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 17 Aug 2014 12:19:06 -0700 Subject: `responders` 1.x won't do it. Told you to RTFM for details! --- actionpack/lib/action_controller/metal/mime_responds.rb | 9 ++++++--- actionpack/test/controller/mime/responders_test.rb | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 5c869d4ce2..dc572f13d2 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -9,15 +9,18 @@ module ActionController #:nodoc: def respond_to(*) raise NoMethodError, "The controller-level `respond_to' feature has " \ "been extracted to the `responders` gem. Add it to your Gemfile to " \ - "continue using this feature. Consult the Rails upgrade guide for " \ - "details." + "continue using this feature:\n" \ + " gem 'responders', '~> 2.0'\n" \ + "Consult the Rails upgrade guide for details." end end def respond_with(*) raise NoMethodError, "The `respond_with' feature has been extracted " \ "to the `responders` gem. Add it to your Gemfile to continue using " \ - "this feature. Consult the Rails upgrade guide for details." + "this feature:\n" \ + " gem 'responders', '~> 2.0'\n" \ + "Consult the Rails upgrade guide for details." end # Without web-service support, an action which collects the data for displaying a list of people diff --git a/actionpack/test/controller/mime/responders_test.rb b/actionpack/test/controller/mime/responders_test.rb index 416e3191e6..032b4c0ab1 100644 --- a/actionpack/test/controller/mime/responders_test.rb +++ b/actionpack/test/controller/mime/responders_test.rb @@ -10,6 +10,7 @@ class ResponderTest < ActionController::TestCase end assert_includes e.message, '`responders` gem' + assert_includes e.message, '~> 2.0' end def test_respond_with @@ -26,5 +27,6 @@ class ResponderTest < ActionController::TestCase end assert_includes e.message, '`responders` gem' + assert_includes e.message, '~> 2.0' end end -- cgit v1.2.3 From e158ee50e64e2ae2460cd26be53039922f588300 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 17 Aug 2014 12:40:24 -0700 Subject: Use AS::JSON for (de)serializing cookies Use the Active Support JSON encoder for cookie jars using the `:json` or `:hybrid` serializer. This allows you to serialize custom Ruby objects into cookies by defining the `#as_json` hook on such objects. Fixes #16520. --- actionpack/CHANGELOG.md | 8 +++++ .../lib/action_dispatch/middleware/cookies.rb | 5 +-- actionpack/test/dispatch/cookies_test.rb | 39 ++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 17617206cb..67f117454f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Use the Active Support JSON encoder for cookie jars using the `:json` or + `:hybrid` serializer. This allows you to serialize custom Ruby objects into + cookies by defining the `#as_json` hook on such objects. + + Fixes #16520. + + *Godfrey Chan* + * Add `config.action_dispatch.cookies_digest` option for setting custom digest. The default remains the same - 'SHA1'. diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 5b3c0e7316..83ac62a83d 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -3,6 +3,7 @@ require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/object/blank' require 'active_support/key_generator' require 'active_support/message_verifier' +require 'active_support/json' module ActionDispatch class Request < Rack::Request @@ -391,11 +392,11 @@ module ActionDispatch class JsonSerializer def self.load(value) - JSON.parse(value, quirks_mode: true) + ActiveSupport::JSON.decode(value) end def self.dump(value) - JSON.generate(value, quirks_mode: true) + ActiveSupport::JSON.encode(value) end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 7e7dd94425..9b03c805a0 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -21,6 +21,16 @@ class CookiesTest < ActionController::TestCase end end + class JSONWrapper + def initialize(obj) + @obj = obj + end + + def as_json(options = nil) + "wrapped: #{@obj.as_json(options)}" + end + end + class TestController < ActionController::Base def authenticate cookies["user_name"] = "david" @@ -85,6 +95,11 @@ class CookiesTest < ActionController::TestCase head :ok end + def set_wrapped_signed_cookie + cookies.signed[:user_id] = JSONWrapper.new(45) + head :ok + end + def get_signed_cookie cookies.signed[:user_id] head :ok @@ -95,6 +110,11 @@ class CookiesTest < ActionController::TestCase head :ok end + def set_wrapped_encrypted_cookie + cookies.encrypted[:foo] = JSONWrapper.new('bar') + head :ok + end + def get_encrypted_cookie cookies.encrypted[:foo] head :ok @@ -421,6 +441,14 @@ class CookiesTest < ActionController::TestCase assert_equal 45, cookies.signed[:user_id] end + def test_wrapped_signed_cookie_using_json_serializer + @request.env["action_dispatch.cookies_serializer"] = :json + get :set_wrapped_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 'wrapped: 45', cookies[:user_id] + assert_equal 'wrapped: 45', cookies.signed[:user_id] + end + def test_signed_cookie_using_custom_serializer @request.env["action_dispatch.cookies_serializer"] = CustomSerializer get :set_signed_cookie @@ -503,6 +531,17 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar', cookies.encrypted[:foo] end + def test_wrapped_encrypted_cookie_using_json_serializer + @request.env["action_dispatch.cookies_serializer"] = :json + get :set_wrapped_encrypted_cookie + cookies = @controller.send :cookies + assert_not_equal 'wrapped: bar', cookies[:foo] + assert_raises ::JSON::ParserError do + cookies.signed[:foo] + end + assert_equal 'wrapped: bar', cookies.encrypted[:foo] + end + def test_encrypted_cookie_using_custom_serializer @request.env["action_dispatch.cookies_serializer"] = CustomSerializer get :set_encrypted_cookie -- cgit v1.2.3