diff options
17 files changed, 179 insertions, 129 deletions
diff --git a/actionpack/lib/action_dispatch/http/filter_redirect.rb b/actionpack/lib/action_dispatch/http/filter_redirect.rb index bf79963351..94c1f2b41f 100644 --- a/actionpack/lib/action_dispatch/http/filter_redirect.rb +++ b/actionpack/lib/action_dispatch/http/filter_redirect.rb @@ -5,8 +5,7 @@ module ActionDispatch FILTERED = '[FILTERED]'.freeze # :nodoc: def filtered_location # :nodoc: - filters = location_filter - if !filters.empty? && location_filter_match?(filters) + if location_filter_match? FILTERED else location @@ -15,7 +14,7 @@ module ActionDispatch private - def location_filter + def location_filters if request request.env['action_dispatch.redirect_filter'] || [] else @@ -23,12 +22,12 @@ module ActionDispatch end end - def location_filter_match?(filters) - filters.any? do |filter| + def location_filter_match? + location_filters.any? do |filter| if String === filter location.include?(filter) elsif Regexp === filter - location.match(filter) + location =~ filter end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 00fe350276..887b5957df 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -61,16 +61,9 @@ module ActionDispatch attr_reader :requirements, :conditions, :defaults attr_reader :to, :default_controller, :default_action, :as, :anchor - def self.build(scope, set, path, as, controller, default_action, to, via, options) - formatted = options.delete(:format) { scope.mapping_option(:format) } - + def self.build(scope, set, path, as, controller, default_action, to, via, formatted, options) options = scope[:options].merge(options) if scope[:options] - options.delete :shallow_path - options.delete :shallow_prefix - options.delete :shallow - options.delete :format - defaults = (scope[:defaults] || {}).dup scope_constraints = scope[:constraints] || {} @@ -128,14 +121,14 @@ module ActionDispatch @requirements = formats[:requirements].merge Hash[requirements] @conditions = Hash[conditions] - @defaults = formats[:defaults].merge @defaults + @defaults = formats[:defaults].merge(@defaults).merge(normalize_defaults(options)) @conditions[:required_defaults] = (split_options[:required_defaults] || []).map(&:first) @conditions[:path_info] = path @conditions[:parsed_path_info] = ast - - add_request_method(via, @conditions) - normalize_defaults!(options) + unless via == [:all] + @conditions[:request_method] = via.map { |m| m.to_s.dasherize.upcase } + end end def to_route @@ -226,17 +219,8 @@ module ActionDispatch end end - def normalize_defaults!(options) - options.each_pair do |key, default| - unless Regexp === default - @defaults[key] = default - end - end - end - - def add_request_method(via, conditions) - return if via == [:all] - conditions[:request_method] = via.map { |m| m.to_s.dasherize.upcase } + def normalize_defaults(options) + Hash[options.reject { |_, default| Regexp === default }] end def app(blocks) @@ -810,10 +794,10 @@ module ActionDispatch elsif option == :options value = options else - value = options.delete(option) + value = options.delete(option) { POISON } end - if value + unless POISON == value scope[option] = send("merge_#{option}_scope", @scope[option], value) end end @@ -825,6 +809,8 @@ module ActionDispatch @scope = @scope.parent end + POISON = Object.new # :nodoc: + # Scopes routes to a specific controller # # controller "food" do @@ -994,6 +980,10 @@ module ActionDispatch child end + def merge_format_scope(parent, child) #:nodoc: + child + end + def merge_path_names_scope(parent, child) #:nodoc: merge_options_scope(parent, child) end @@ -1551,29 +1541,30 @@ module ActionDispatch via = Mapping.check_via Array(options.delete(:via) { @scope[:via] }) + formatted = options.delete(:format) { @scope[:format] } path_types = paths.group_by(&:class) path_types.fetch(String, []).each do |_path| route_options = options.dup - process_path(route_options, controller, _path, option_path || _path, to, via) + process_path(route_options, controller, _path, option_path || _path, to, via, formatted) end path_types.fetch(Symbol, []).each do |action| route_options = options.dup - decomposed_match(action, controller, route_options, option_path, to, via) + decomposed_match(action, controller, route_options, option_path, to, via, formatted) end self end - def process_path(options, controller, path, option_path, to, via) + def process_path(options, controller, path, option_path, to, via, formatted) path_without_format = path.sub(/\(\.:format\)$/, '') if using_match_shorthand?(path_without_format, to, options[:action]) to ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1') to.tr!("-", "_") end - decomposed_match(path, controller, options, option_path, to, via) + decomposed_match(path, controller, options, option_path, to, via, formatted) end def using_match_shorthand?(path, to, action) @@ -1582,22 +1573,22 @@ module ActionDispatch path =~ %r{^/?[-\w]+/[-\w/]+$} end - def decomposed_match(path, controller, options, _path, to, via) # :nodoc: + def decomposed_match(path, controller, options, _path, to, via, formatted) # :nodoc: if on = options.delete(:on) - send(on) { decomposed_match(path, controller, options, _path, to, via) } + send(on) { decomposed_match(path, controller, options, _path, to, via, formatted) } else case @scope.scope_level when :resources - nested { decomposed_match(path, controller, options, _path, to, via) } + nested { decomposed_match(path, controller, options, _path, to, via, formatted) } when :resource - member { decomposed_match(path, controller, options, _path, to, via) } + member { decomposed_match(path, controller, options, _path, to, via, formatted) } else - add_route(path, controller, options, _path, to, via) + add_route(path, controller, options, _path, to, via, formatted) end end end - def add_route(action, controller, options, _path, to, via) # :nodoc: + def add_route(action, controller, options, _path, to, via, formatted) # :nodoc: path = path_for_action(action, _path) raise ArgumentError, "path is required" if path.blank? @@ -1617,7 +1608,7 @@ module ActionDispatch name_for_action(options.delete(:as), action) end - mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, controller, default_action, to, via, options) + mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, controller, default_action, to, via, formatted, options) app, conditions, requirements, defaults, as, anchor = mapping.to_route @set.add_route(app, conditions, requirements, defaults, as, anchor) end @@ -1943,7 +1934,7 @@ module ActionDispatch class Scope # :nodoc: OPTIONS = [:path, :shallow_path, :as, :shallow_prefix, :module, :controller, :action, :path_names, :constraints, - :shallow, :blocks, :defaults, :via, :options] + :shallow, :blocks, :defaults, :via, :format, :options] RESOURCE_SCOPES = [:resource, :resources] RESOURCE_METHOD_SCOPES = [:collection, :member, :new] @@ -1993,12 +1984,6 @@ module ActionDispatch OPTIONS end - def mapping_option(name) - options = self[:options] - return unless options - options[name] - end - def new(hash) self.class.new hash, self, scope_level end diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 3071890521..6ce0e34ec3 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -52,6 +52,26 @@ module ActionDispatch end end + def test_unscoped_formatted + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.get '/foo', :to => 'posts#index', :as => :main, :format => true + assert_equal({:controller=>"posts", :action=>"index"}, + fakeset.defaults.first) + assert_equal "/foo.:format", fakeset.conditions.first[:path_info] + end + + def test_scoped_formatted + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.scope(format: true) do + mapper.get '/foo', :to => 'posts#index', :as => :main + end + assert_equal({:controller=>"posts", :action=>"index"}, + fakeset.defaults.first) + assert_equal "/foo.:format", fakeset.conditions.first[:path_info] + end + def test_random_keys fakeset = FakeSet.new mapper = Mapper.new fakeset @@ -66,7 +86,7 @@ module ActionDispatch def test_mapping_requirements options = { } scope = Mapper::Scope.new({}) - m = Mapper::Mapping.build(scope, FakeSet.new, '/store/:name(*rest)', nil, 'foo', 'bar', nil, [:get], options) + m = Mapper::Mapping.build(scope, FakeSet.new, '/store/:name(*rest)', nil, 'foo', 'bar', nil, [:get], nil, options) _, _, requirements, _ = m.to_route assert_equal(/.+?/, requirements[:rest]) end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 7c729676a7..c7b396f3d4 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -251,6 +251,14 @@ module ActiveRecord initialize_attributes(record) end end + + # Returns true if statement cache should be skipped on the association reader. + def skip_statement_cache? + reflection.scope_chain.any?(&:any?) || + scope.eager_loading? || + klass.scope_attributes? || + reflection.source_reflection.active_record.default_scopes.any? + end end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0fc2b83b71..256df3ca11 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -442,12 +442,7 @@ module ActiveRecord private def get_records - if reflection.scope_chain.any?(&:any?) || - scope.eager_loading? || - klass.scope_attributes? - - return scope.to_a - end + return scope.to_a if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 30c5d72482..03cb8cb8c3 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -47,12 +47,7 @@ module ActiveRecord end def get_records - if reflection.scope_chain.any?(&:any?) || - scope.eager_loading? || - klass.scope_attributes? - - return scope.limit(1).to_a - end + return scope.limit(1).to_a if skip_statement_cache? conn = klass.connection sc = reflection.association_scope_cache(conn, owner) do diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ce2557339e..b22ce42696 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1171,4 +1171,32 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_deprecated { post.people(true) } end + + def test_has_many_through_do_not_cache_association_reader_if_the_though_method_has_default_scopes + member = Member.create! + club = Club.create! + TenantMembership.create!( + member: member, + club: club + ) + + TenantMembership.current_member = member + + tenant_clubs = member.tenant_clubs + assert_equal [club], tenant_clubs + + TenantMembership.current_member = nil + + other_member = Member.create! + other_club = Club.create! + TenantMembership.create!( + member: other_member, + club: other_club + ) + + tenant_clubs = other_member.tenant_clubs + assert_equal [other_club], tenant_clubs + ensure + TenantMembership.current_member = nil + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 2c8feb9152..b2b46812b9 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -16,6 +16,10 @@ require 'models/owner' require 'models/post' require 'models/comment' require 'models/categorization' +require 'models/customer' +require 'models/carrier' +require 'models/shop_account' +require 'models/customer_carrier' class HasOneThroughAssociationsTest < ActiveRecord::TestCase fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans, @@ -347,16 +351,33 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end end - def test_association_force_reload_with_only_true_is_deprecated - member = Member.find(1) + def test_has_one_through_do_not_cache_association_reader_if_the_though_method_has_default_scopes + customer = Customer.create! + carrier = Carrier.create! + customer_carrier = CustomerCarrier.create!( + customer: customer, + carrier: carrier, + ) + account = ShopAccount.create!(customer_carrier: customer_carrier) - assert_deprecated { member.club(true) } - end + CustomerCarrier.current_customer = customer - def test_has_one_through_associations_are_mutable_unless_through_belongs_to - member_detail = MemberDetail.new(member: @member) - assert_raise(ActiveRecord::HasOneThroughCantAssociateThroughHasOneOrManyReflection) do - member_detail.membership = Membership.new - end + account_carrier = account.carrier + assert_equal carrier, account_carrier + + CustomerCarrier.current_customer = nil + + other_carrier = Carrier.create! + other_customer = Customer.create! + other_customer_carrier = CustomerCarrier.create!( + customer: other_customer, + carrier: other_carrier, + ) + other_account = ShopAccount.create!(customer_carrier: other_customer_carrier) + + account_carrier = other_account.carrier + assert_equal other_carrier, account_carrier + ensure + CustomerCarrier.current_customer = nil end end diff --git a/activerecord/test/models/carrier.rb b/activerecord/test/models/carrier.rb new file mode 100644 index 0000000000..230be118c3 --- /dev/null +++ b/activerecord/test/models/carrier.rb @@ -0,0 +1,2 @@ +class Carrier < ActiveRecord::Base +end diff --git a/activerecord/test/models/customer_carrier.rb b/activerecord/test/models/customer_carrier.rb new file mode 100644 index 0000000000..37186903ff --- /dev/null +++ b/activerecord/test/models/customer_carrier.rb @@ -0,0 +1,14 @@ +class CustomerCarrier < ActiveRecord::Base + cattr_accessor :current_customer + + belongs_to :customer + belongs_to :carrier + + default_scope -> { + if current_customer + where(customer: current_customer) + else + all + end + } +end diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index dc0566d8a7..7693c6e515 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -26,6 +26,9 @@ class Member < ActiveRecord::Base has_many :current_memberships, -> { where :favourite => true } has_many :clubs, :through => :current_memberships + has_many :tenant_memberships + has_many :tenant_clubs, through: :tenant_memberships, class_name: 'Club', source: :club + has_one :club_through_many, :through => :current_memberships, :source => :club belongs_to :admittable, polymorphic: true diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index df7167ee93..e181ba1f11 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -18,3 +18,18 @@ class SelectedMembership < Membership select("'1' as foo") end end + +class TenantMembership < Membership + cattr_accessor :current_member + + belongs_to :member + belongs_to :club + + default_scope -> { + if current_member + where(member: current_member) + else + all + end + } +end diff --git a/activerecord/test/models/shop_account.rb b/activerecord/test/models/shop_account.rb new file mode 100644 index 0000000000..1580e8b20c --- /dev/null +++ b/activerecord/test/models/shop_account.rb @@ -0,0 +1,6 @@ +class ShopAccount < ActiveRecord::Base + belongs_to :customer + belongs_to :customer_carrier + + has_one :carrier, through: :customer_carrier +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 1fa824bf1d..08ac7749f9 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -131,6 +131,8 @@ ActiveRecord::Schema.define do t.timestamps null: false end + create_table :carriers, force: true + create_table :categories, force: true do |t| t.string :name, null: false t.string :type @@ -237,6 +239,11 @@ ActiveRecord::Schema.define do t.string :gps_location end + create_table :customer_carriers, force: true do |t| + t.references :customer + t.references :carrier + end + create_table :dashboards, force: true, id: false do |t| t.string :dashboard_id t.string :name @@ -683,6 +690,11 @@ ActiveRecord::Schema.define do t.belongs_to :ship end + create_table :shop_accounts, force: true do |t| + t.references :customer + t.references :customer_carrier + end + create_table :speedometers, force: true, id: false do |t| t.string :speedometer_id t.string :name diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index 0d5e02cd77..b2e713077c 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -172,7 +172,7 @@ class String # uses the +pluralize+ method on the last word in the string. # # 'RawScaledScorer'.tableize # => "raw_scaled_scorers" - # 'egg_and_ham'.tableize # => "egg_and_hams" + # 'ham_and_egg'.tableize # => "ham_and_eggs" # 'fancyCategory'.tableize # => "fancy_categories" def tableize ActiveSupport::Inflector.tableize(self) @@ -182,7 +182,7 @@ class String # Note that this returns a string and not a class. (To convert to an actual class # follow +classify+ with +constantize+.) # - # 'egg_and_hams'.classify # => "EggAndHam" + # 'ham_and_eggs'.classify # => "HamAndEgg" # 'posts'.classify # => "Post" def classify ActiveSupport::Inflector.classify(self) diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 899ba70af6..ab5372ac43 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -160,7 +160,7 @@ module ActiveSupport # This method uses the #pluralize method on the last word in the string. # # tableize('RawScaledScorer') # => "raw_scaled_scorers" - # tableize('egg_and_ham') # => "egg_and_hams" + # tableize('ham_and_egg') # => "ham_and_eggs" # tableize('fancyCategory') # => "fancy_categories" def tableize(class_name) pluralize(underscore(class_name)) @@ -170,7 +170,7 @@ module ActiveSupport # names to models. Note that this returns a string and not a Class (To # convert to an actual class follow +classify+ with #constantize). # - # classify('egg_and_hams') # => "EggAndHam" + # classify('ham_and_eggs') # => "HamAndEgg" # classify('posts') # => "Post" # # Singular names are not handled correctly: diff --git a/railties/lib/rails/generators/rails/app/templates/config/routes.rb b/railties/lib/rails/generators/rails/app/templates/config/routes.rb index 3f66539d54..787824f888 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/routes.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/routes.rb @@ -1,56 +1,3 @@ Rails.application.routes.draw do - # The priority is based upon order of creation: first created -> highest priority. - # See how all your routes lay out with "rake routes". - - # You can have the root of your site routed with "root" - # root 'welcome#index' - - # Example of regular route: - # get 'products/:id' => 'catalog#view' - - # Example of named route that can be invoked with purchase_url(id: product.id) - # get 'products/:id/purchase' => 'catalog#purchase', as: :purchase - - # Example resource route (maps HTTP verbs to controller actions automatically): - # resources :products - - # Example resource route with options: - # resources :products do - # member do - # get 'short' - # post 'toggle' - # end - # - # collection do - # get 'sold' - # end - # end - - # Example resource route with sub-resources: - # resources :products do - # resources :comments, :sales - # resource :seller - # end - - # Example resource route with more complex sub-resources: - # resources :products do - # resources :comments - # resources :sales do - # get 'recent', on: :collection - # end - # end - - # Example resource route with concerns: - # concern :toggleable do - # post 'toggle' - # end - # resources :posts, concerns: :toggleable - # resources :photos, concerns: :toggleable - - # Example resource route within a namespace: - # namespace :admin do - # # Directs /admin/products/* to Admin::ProductsController - # # (app/controllers/admin/products_controller.rb) - # resources :products - # end + # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html end |