From 1847d33b4bc776fbd4746c06db41965744377ed6 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 24 Aug 2010 22:16:34 +0100 Subject: [PATCH 01/28] Use nested scope for routes defined at the :resources scope level (as in Rails 2.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/routing/mapper.rb | 11 ++++++----- actionpack/test/dispatch/routing_test.rb | 7 +++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 9aa34e7ba5..9a92ed0b62 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -695,15 +695,14 @@ module ActionDispatch raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end - if @scope[:scope_level] == :resource + if @scope[:scope_level] == :resources + args.push(options) + return nested { match(*args) } + elsif @scope[:scope_level] == :resource args.push(options) return member { match(*args) } end - if resource_scope? - raise ArgumentError, "can't define route directly in resource(s) scope" - end - action = args.first path = path_for_action(action, options.delete(:path)) @@ -900,6 +899,8 @@ module ActionDispatch end name = case @scope[:scope_level] + when :nested + [member_name, prefix] when :collection [prefix, name_prefix, collection_name] when :new diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5b2547e700..a4b8fafa78 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -231,6 +231,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get "inactive", :on => :collection post "deactivate", :on => :member get "old", :on => :collection, :as => :stale + get "export" end namespace :api do @@ -2091,6 +2092,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/customers/1/invoices/aged/3', aged_customer_invoices_path(:customer_id => '1', :months => '3') end + def test_route_defined_in_resources_scope_level + get '/customers/1/export' + assert_equal 'customers#export', @response.body + assert_equal '/customers/1/export', customer_export_path(:customer_id => '1') + end + private def with_test_routes yield From c1b49f1e18e08580196f5acfaacebcf4c3aa17d3 Mon Sep 17 00:00:00 2001 From: Mikel Lindsaar Date: Wed, 25 Aug 2010 12:05:23 +1000 Subject: [PATCH 02/28] Make ActiveResource::InvalidRequestError more user friendly Signed-off-by: Xavier Noria --- activeresource/lib/active_resource/http_mock.rb | 8 ++++++-- activeresource/test/cases/http_mock_test.rb | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/activeresource/lib/active_resource/http_mock.rb b/activeresource/lib/active_resource/http_mock.rb index a98af88a37..8753a21835 100644 --- a/activeresource/lib/active_resource/http_mock.rb +++ b/activeresource/lib/active_resource/http_mock.rb @@ -126,7 +126,7 @@ module ActiveResource # if response = self.class.responses.assoc(request) # response[1] # else - # raise InvalidRequestError.new("No response recorded for #{request}") + # raise InvalidRequestError.new("Could not find a response recorded for #{request.to_s} - Responses recorded are: - #{inspect_responses}") # end # end module_eval <<-EOE, __FILE__, __LINE__ + 1 @@ -136,7 +136,7 @@ module ActiveResource if response = self.class.responses.assoc(request) response[1] else - raise InvalidRequestError.new("No response recorded for \#{request}") + raise InvalidRequestError.new("Could not find a response recorded for \#{request.to_s} - Responses recorded are: \#{inspect_responses}") end end EOE @@ -146,6 +146,10 @@ module ActiveResource def initialize(site) #:nodoc: @site = site end + + def inspect_responses #:nodoc: + self.class.responses.map { |r| r[0].to_s }.inspect + end end class Request diff --git a/activeresource/test/cases/http_mock_test.rb b/activeresource/test/cases/http_mock_test.rb index 5e032d03f1..a387cd20b1 100644 --- a/activeresource/test/cases/http_mock_test.rb +++ b/activeresource/test/cases/http_mock_test.rb @@ -59,6 +59,17 @@ class HttpMockTest < ActiveSupport::TestCase assert_equal "XML", request(method, "/people/1", FORMAT_HEADER[method] => "application/xml").body assert_equal "Json", request(method, "/people/1", FORMAT_HEADER[method] => "application/json").body end + + test "raises InvalidRequestError if no response found for the #{method} request" do + ActiveResource::HttpMock.respond_to do |mock| + mock.send(method, "/people/1", {FORMAT_HEADER[method] => "application/xml"}, "XML") + end + + assert_raise(::ActiveResource::InvalidRequestError) do + request(method, "/people/1", FORMAT_HEADER[method] => "application/json") + end + end + end def request(method, path, headers = {}, body = nil) From 0b9357d401677f289994369e48c2065880978a53 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 25 Aug 2010 06:31:11 +0100 Subject: [PATCH 03/28] Remove rails info route from rake routes output [#5452 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- railties/lib/rails/tasks/routes.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/tasks/routes.rake b/railties/lib/rails/tasks/routes.rake index 65cf79a0a2..306c88c261 100644 --- a/railties/lib/rails/tasks/routes.rake +++ b/railties/lib/rails/tasks/routes.rake @@ -23,7 +23,7 @@ task :routes => :environment do {:name => name, :verb => route.verb.to_s, :path => route.path, :reqs => reqs} end - routes.reject! { |r| r[:path] == "/rails/info/properties" } # Skip the route if it's internal info route + routes.reject! { |r| r[:path] =~ %r{/rails/info/properties} } # Skip the route if it's internal info route name_width = routes.map{ |r| r[:name] }.map(&:length).max verb_width = routes.map{ |r| r[:verb] }.map(&:length).max From 4f945e97e55b861ab014d71efa983ab183441031 Mon Sep 17 00:00:00 2001 From: Jakub Suder Date: Fri, 20 Aug 2010 20:05:38 +0200 Subject: [PATCH 04/28] better callstack reporting in deprecation messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit now the reported line is the first line in the stack that's outside Rails, which is the one that actually caused the problem in the first place [#5231 state:resolved] Signed-off-by: José Valim --- .../lib/active_support/deprecation/reporting.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/deprecation/reporting.rb b/activesupport/lib/active_support/deprecation/reporting.rb index 49d58cd3a1..6a7b11c7e0 100644 --- a/activesupport/lib/active_support/deprecation/reporting.rb +++ b/activesupport/lib/active_support/deprecation/reporting.rb @@ -46,10 +46,14 @@ module ActiveSupport end def extract_callstack(callstack) - if md = callstack.first.match(/^(.+?):(\d+)(?::in `(.*?)')?/) - md.captures - else - callstack.first + rails_gem_root = File.expand_path("../../../../..", __FILE__) + "/" + offending_line = callstack.find { |line| !line.start_with?(rails_gem_root) } || callstack.first + if offending_line + if md = offending_line.match(/^(.+?):(\d+)(?::in `(.*?)')?/) + md.captures + else + offending_line + end end end end From e1e712227197bcfb9564113eb3dd8ef6ffac243f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 24 Aug 2010 17:15:46 -0700 Subject: [PATCH 05/28] mark SQL literals as SQL literals --- .../associations/has_and_belongs_to_many_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index e983f86f9e..c0ec65bd40 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -26,7 +26,7 @@ module ActiveRecord def construct_find_options!(options) options[:joins] = Arel::SqlLiteral.new @join_sql options[:readonly] = finding_with_ambiguous_select?(options[:select] || @reflection.options[:select]) - options[:select] ||= (@reflection.options[:select] || '*') + options[:select] ||= (@reflection.options[:select] || Arel::SqlLiteral.new('*')) end def count_records From ce7c2f72f42f8f89517810fc6e51c21118af6a39 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 24 Aug 2010 17:19:59 -0700 Subject: [PATCH 06/28] refactor select { is_a? } to grep() --- activerecord/lib/active_record/relation/query_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 8ccc62c9d1..6fb97c6eb4 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -188,7 +188,7 @@ module ActiveRecord association_joins << join if [Hash, Array, Symbol].include?(join.class) && !array_of_strings?(join) end - stashed_association_joins = joins.select {|j| j.is_a?(ActiveRecord::Associations::ClassMethods::JoinDependency::JoinAssociation)} + stashed_association_joins = joins.grep(ActiveRecord::Associations::ClassMethods::JoinDependency::JoinAssociation) non_association_joins = (joins - association_joins - stashed_association_joins) custom_joins = custom_join_sql(*non_association_joins) From b28cafe01a8165f0e9401225b3702a504ea1d8e9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 24 Aug 2010 18:08:36 -0700 Subject: [PATCH 07/28] no need to send on a public method --- activerecord/lib/active_record/relation/query_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6fb97c6eb4..321beadb0c 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -96,7 +96,7 @@ module ActiveRecord end def reverse_order - order_clause = arel.send(:order_clauses).join(', ') + order_clause = arel.order_clauses.join(', ') relation = except(:order) unless order_clauses.blank? From ffdda4ddfb76fec92faea714f03a37baf75351ef Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 24 Aug 2010 18:29:53 -0700 Subject: [PATCH 08/28] use blank? instead of present?, mark SQL literals as SQL literals Conflicts: activerecord/lib/active_record/relation/query_methods.rb --- .../lib/active_record/relation/query_methods.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 321beadb0c..bb9cdaaefe 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -99,11 +99,11 @@ module ActiveRecord order_clause = arel.order_clauses.join(', ') relation = except(:order) - unless order_clauses.blank? - relation.order(reverse_sql_order(order_clause)) - else - relation.order("#{@klass.table_name}.#{@klass.primary_key} DESC") - end + order = order_clause.blank? ? + "#{@klass.table_name}.#{@klass.primary_key} DESC" : + reverse_sql_order(order_clause) + + relation.order Arel::SqlLiteral.new order end def arel From 1fbb0684d267ca61e9409206c9939d0aeab609c9 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 24 Aug 2010 12:33:11 +0100 Subject: [PATCH 09/28] Catch mysql2 access denied errors in rake db:create [#5432 state:resolved] Signed-off-by: Xavier Noria --- activerecord/lib/active_record/railties/databases.rake | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ae605d3e7a..b46c4b59a2 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -61,12 +61,14 @@ namespace :db do @charset = ENV['CHARSET'] || 'utf8' @collation = ENV['COLLATION'] || 'utf8_unicode_ci' creation_options = {:charset => (config['charset'] || @charset), :collation => (config['collation'] || @collation)} + error_class = config['adapter'] == 'mysql2' ? Mysql2::Error : Mysql::Error + access_denied_error = 1045 begin ActiveRecord::Base.establish_connection(config.merge('database' => nil)) ActiveRecord::Base.connection.create_database(config['database'], creation_options) ActiveRecord::Base.establish_connection(config) - rescue Mysql::Error => sqlerr - if sqlerr.errno == Mysql::Error::ER_ACCESS_DENIED_ERROR + rescue error_class => sqlerr + if sqlerr.errno == access_denied_error print "#{sqlerr.error}. \nPlease provide the root password for your mysql installation\n>" root_password = $stdin.gets.strip grant_statement = "GRANT ALL PRIVILEGES ON #{config['database']}.* " \ From d2f55e7eee3f82a9a8cc53a2c48294685d75b15e Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Wed, 25 Aug 2010 20:30:06 +0200 Subject: [PATCH 10/28] AS guide: Array.wrap vs splat is only valid for 1.8 --- railties/guides/source/active_support_core_extensions.textile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/guides/source/active_support_core_extensions.textile b/railties/guides/source/active_support_core_extensions.textile index 9b3bb1da15..561bae3be8 100644 --- a/railties/guides/source/active_support_core_extensions.textile +++ b/railties/guides/source/active_support_core_extensions.textile @@ -2226,7 +2226,7 @@ There's also a related idiom that uses the splat operator: [*object] -which returns +[nil]+ for +nil+, and calls to Array(object) otherwise +which in Ruby 1.8 returns +[nil]+ for +nil+, and calls to Array(object) otherwise. (Please if you know the exact behavior in 1.9 contact fxn.) Thus, in this case the behavior is different for +nil+, and the differences with Kernel#Array explained above apply to the rest of +object+s. From cd0267cd33d1792348f758044a8b3c301b875dcd Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 26 Aug 2010 01:56:10 +0200 Subject: [PATCH 11/28] adds missing require for #parameterize --- activesupport/lib/active_support/core_ext/string/inflections.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index 9a4e63672f..55b24b0925 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -1,5 +1,7 @@ require 'active_support/inflector/methods' require 'active_support/inflector/inflections' +require 'active_support/inflector/transliterate' + # String inflections define new methods on the String class to transform names for different purposes. # For instance, you can figure out the name of a database from the name of a class. # From 5e73ab0936b492aec24f41be269e0ae481287df0 Mon Sep 17 00:00:00 2001 From: Jaime Iniesta Date: Thu, 26 Aug 2010 09:01:59 +0200 Subject: [PATCH 12/28] Fix capture_helper.rb api documentation, unescaped script tag was breaking it on the content_for explanation --- actionpack/lib/action_view/helpers/capture_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index 89e95e8694..52e71a4c3a 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -106,7 +106,7 @@ module ActionView # <%= javascript_include_tag :defaults %> # <% end %> # - # That will place