diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 69 | ||||
| -rw-r--r-- | actionpack/lib/action_view/digestor.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/abstract_unit.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing/concerns_test.rb | 47 | ||||
| -rw-r--r-- | activerecord/lib/active_record/persistence.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/attribute_methods_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/base_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/inheritance_test.rb | 98 | ||||
| -rw-r--r-- | activerecord/test/cases/schema_dumper_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/fixtures/companies.yml | 6 | ||||
| -rw-r--r-- | activerecord/test/fixtures/vegetables.yml | 20 | ||||
| -rw-r--r-- | activerecord/test/models/company.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/models/vegetables.rb | 24 | ||||
| -rw-r--r-- | activerecord/test/schema/schema.rb | 9 | ||||
| -rw-r--r-- | activesupport/lib/active_support/dependencies.rb | 2 |
15 files changed, 208 insertions, 82 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index b52f66faf1..ddb34a2394 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1585,7 +1585,7 @@ module ActionDispatch end end - # Routing Concerns allows you to declare common routes that can be reused + # Routing Concerns allow you to declare common routes that can be reused # inside others resources and routes. # # concern :commentable do @@ -1608,13 +1608,63 @@ module ActionDispatch module Concerns # Define a routing concern using a name. # - # concern :commentable do - # resources :comments + # Concerns may be defined inline, using a block, or handled by + # another object, by passing that object as the second parameter. + # + # The concern object, if supplied, should respond to <tt>call</tt>, + # which will receive two parameters: + # + # * The current mapper + # * A hash of options which the concern object may use + # + # Options may also be used by concerns defined in a block by accepting + # a block parameter. So, using a block, you might do something as + # simple as limit the actions available on certain resources, passing + # standard resource options through the concern: + # + # concern :commentable do |options| + # resources :comments, options # end # - # Any routing helpers can be used inside a concern. - def concern(name, &block) - @concerns[name] = block + # resources :posts, concerns: :commentable + # resources :archived_posts do + # # Don't allow comments on archived posts + # concerns :commentable, only: [:index, :show] + # end + # + # Or, using a callable object, you might implement something more + # specific to your application, which would be out of place in your + # routes file. + # + # # purchasable.rb + # class Purchasable + # def initialize(defaults = {}) + # @defaults = defaults + # end + # + # def call(mapper, options = {}) + # options = @defaults.merge(options) + # mapper.resources :purchases + # mapper.resources :receipts + # mapper.resources :returns if options[:returnable] + # end + # end + # + # # routes.rb + # concern :purchasable, Purchasable.new(returnable: true) + # + # resources :toys, concerns: :purchasable + # resources :electronics, concerns: :purchasable + # resources :pets do + # concerns :purchasable, returnable: false + # end + # + # Any routing helpers can be used inside a concern. If using a + # callable, they're accessible from the Mapper that's passed to + # <tt>call</tt>. + def concern(name, callable = nil, &block) + callable ||= lambda { |mapper, options| mapper.instance_exec(options, &block) } + @concerns[name] = callable end # Use the named concerns @@ -1628,10 +1678,11 @@ module ActionDispatch # namespace :posts do # concerns :commentable # end - def concerns(*names) - names.flatten.each do |name| + def concerns(*args) + options = args.extract_options! + args.flatten.each do |name| if concern = @concerns[name] - instance_eval(&concern) + concern.call(self, options) else raise ArgumentError, "No concern named #{name} was found!" end diff --git a/actionpack/lib/action_view/digestor.rb b/actionpack/lib/action_view/digestor.rb index 899100d06c..2d7ba856d5 100644 --- a/actionpack/lib/action_view/digestor.rb +++ b/actionpack/lib/action_view/digestor.rb @@ -35,7 +35,7 @@ module ActionView end def digest - Digest::MD5.hexdigest("#{name}.#{format}-#{source}-#{dependency_digest}").tap do |digest| + Digest::MD5.hexdigest("#{source}-#{dependency_digest}").tap do |digest| logger.try :info, "Cache digest for #{name}.#{format}: #{digest}" end rescue ActionView::MissingTemplate diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index e5054a9eb8..4f5b2895c9 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -358,6 +358,7 @@ end class ThreadsController < ResourcesController; end class MessagesController < ResourcesController; end class CommentsController < ResourcesController; end +class ReviewsController < ResourcesController; end class AuthorsController < ResourcesController; end class LogosController < ResourcesController; end diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 21da3bd77a..9f37701656 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -1,18 +1,28 @@ require 'abstract_unit' class RoutingConcernsTest < ActionDispatch::IntegrationTest + class Reviewable + def self.call(mapper, options = {}) + mapper.resources :reviews, options + end + end + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do - concern :commentable do - resources :comments + concern :commentable do |options| + resources :comments, options end concern :image_attachable do resources :images, only: :index end - resources :posts, concerns: [:commentable, :image_attachable] do - resource :video, concerns: :commentable + concern :reviewable, Reviewable + + resources :posts, concerns: [:commentable, :image_attachable, :reviewable] do + resource :video, concerns: :commentable do + concerns :reviewable, as: :video_reviews + end end resource :picture, concerns: :commentable do @@ -20,7 +30,7 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest end scope "/videos" do - concerns :commentable + concerns :commentable, except: :destroy end end end @@ -63,11 +73,28 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "404", @response.code end + def test_accessing_callable_concern_ + get "/posts/1/reviews/1" + assert_equal "200", @response.code + assert_equal "/posts/1/reviews/1", post_review_path(post_id: 1, id: 1) + end + + def test_callable_concerns_accept_options + get "/posts/1/video/reviews/1" + assert_equal "200", @response.code + assert_equal "/posts/1/video/reviews/1", post_video_video_review_path(post_id: 1, id: 1) + end + def test_accessing_concern_from_a_scope get "/videos/comments" assert_equal "200", @response.code end + def test_concerns_accept_options + delete "/videos/comments/1" + assert_equal "404", @response.code + end + def test_with_an_invalid_concern_name e = assert_raise ArgumentError do ActionDispatch::Routing::RouteSet.new.tap do |app| @@ -79,4 +106,14 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "No concern named foo was found!", e.message end + + def test_concerns_executes_block_in_context_of_current_mapper + mapper = ActionDispatch::Routing::Mapper.new(ActionDispatch::Routing::RouteSet.new) + mapper.concern :test_concern do + resources :things + return self + end + + assert_equal mapper, mapper.concerns(:test_concern) + end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index ffb8513624..7bd65c180d 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -161,7 +161,7 @@ module ActiveRecord became.instance_variable_set("@new_record", new_record?) became.instance_variable_set("@destroyed", destroyed?) became.instance_variable_set("@errors", errors) - became.type = klass.name unless self.class.descends_from_active_record? + became.public_send("#{klass.inheritance_column}=", klass.name) unless self.class.descends_from_active_record? became end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index ea58a624a1..d08b157011 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -395,7 +395,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_query_attribute_with_custom_fields object = Company.find_by_sql(<<-SQL).first - SELECT c1.*, c2.ruby_type as string_value, c2.rating as int_value + SELECT c1.*, c2.type as string_value, c2.rating as int_value FROM companies c1, companies c2 WHERE c1.firm_id = c2.id AND c1.id = 2 diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index c6f7e8cf0f..9fcee99222 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1735,7 +1735,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_attribute_names - assert_equal ["id", "type", "ruby_type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"], + assert_equal ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"], Company.attribute_names end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index e80259a7f1..8fded9159f 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -5,9 +5,10 @@ require 'models/post' require 'models/project' require 'models/subscriber' require 'models/teapot' +require 'models/vegetables' class InheritanceTest < ActiveRecord::TestCase - fixtures :companies, :projects, :subscribers, :accounts + fixtures :companies, :projects, :subscribers, :accounts, :vegetables def test_class_with_store_full_sti_class_returns_full_name old = ActiveRecord::Base.store_full_sti_class @@ -122,9 +123,17 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_inheritance_find - switch_to_alt_inheritance_column - test_inheritance_find - switch_to_default_inheritance_column + assert_kind_of Cucumber, Vegetable.find(1) + assert_kind_of Cucumber, Cucumber.find(1) + assert_kind_of Cabbage, Vegetable.find(2) + assert_kind_of Cabbage, Cabbage.find(2) + end + + def test_alt_becomes_works_with_sti + vegetable = Vegetable.find(1) + assert_kind_of Vegetable, vegetable + cabbage = vegetable.becomes(Cabbage) + assert_kind_of Cabbage, cabbage end def test_inheritance_find_all @@ -134,9 +143,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_inheritance_find_all - switch_to_alt_inheritance_column - test_inheritance_find_all - switch_to_default_inheritance_column + companies = Vegetable.all.merge!(:order => 'id').to_a + assert_kind_of Cucumber, companies[0] + assert_kind_of Cabbage, companies[1] end def test_inheritance_save @@ -149,9 +158,11 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_inheritance_save - switch_to_alt_inheritance_column - test_inheritance_save - switch_to_default_inheritance_column + cabbage = Cabbage.new(:name => 'Savoy') + cabbage.save! + + savoy = Vegetable.find(cabbage.id) + assert_kind_of Cabbage, savoy end def test_inheritance_condition @@ -161,9 +172,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_inheritance_condition - switch_to_alt_inheritance_column - test_inheritance_condition - switch_to_default_inheritance_column + assert_equal 4, Vegetable.count + assert_equal 1, Cucumber.count + assert_equal 3, Cabbage.count end def test_finding_incorrect_type_data @@ -172,9 +183,8 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_finding_incorrect_type_data - switch_to_alt_inheritance_column - test_finding_incorrect_type_data - switch_to_default_inheritance_column + assert_raise(ActiveRecord::RecordNotFound) { Cucumber.find(2) } + assert_nothing_raised { Cucumber.find(1) } end def test_update_all_within_inheritance @@ -185,9 +195,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_update_all_within_inheritance - switch_to_alt_inheritance_column - test_update_all_within_inheritance - switch_to_default_inheritance_column + Cabbage.update_all "name = 'the cabbage'" + assert_equal "the cabbage", Cabbage.first.name + assert_equal ["my cucumber"], Cucumber.all.map(&:name).uniq end def test_destroy_all_within_inheritance @@ -197,9 +207,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_destroy_all_within_inheritance - switch_to_alt_inheritance_column - test_destroy_all_within_inheritance - switch_to_default_inheritance_column + Cabbage.destroy_all + assert_equal 0, Cabbage.count + assert_equal 1, Cucumber.count end def test_find_first_within_inheritance @@ -209,9 +219,9 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_find_first_within_inheritance - switch_to_alt_inheritance_column - test_find_first_within_inheritance - switch_to_default_inheritance_column + assert_kind_of Cabbage, Vegetable.all.merge!(:where => "name = 'his cabbage'").first + assert_kind_of Cabbage, Cabbage.all.merge!(:where => "name = 'his cabbage'").first + assert_nil Cucumber.all.merge!(:where => "name = 'his cabbage'").first end def test_complex_inheritance @@ -225,9 +235,13 @@ class InheritanceTest < ActiveRecord::TestCase end def test_alt_complex_inheritance - switch_to_alt_inheritance_column - test_complex_inheritance - switch_to_default_inheritance_column + king_cole = KingCole.create("name" => "uniform heads") + assert_equal king_cole, KingCole.where("name = 'uniform heads'").first + assert_equal king_cole, GreenCabbage.all.merge!(:where => "name = 'uniform heads'").first + assert_equal king_cole, Cabbage.all.merge!(:where => "name = 'uniform heads'").first + assert_equal king_cole, Vegetable.all.merge!(:where => "name = 'uniform heads'").first + assert_equal 1, Cabbage.all.merge!(:where => "name = 'his cabbage'").to_a.size + assert_equal king_cole, Cabbage.find(king_cole.id) end def test_eager_load_belongs_to_something_inherited @@ -235,6 +249,11 @@ class InheritanceTest < ActiveRecord::TestCase assert account.association_cache.key?(:firm), "nil proves eager load failed" end + def test_alt_eager_loading + cabbage = RedCabbage.all.merge!(:includes => :seller).find(4) + assert cabbage.association_cache.key?(:seller), "nil proves eager load failed" + end + def test_eager_load_belongs_to_primary_key_quoting con = Account.connection assert_sql(/#{con.quote_table_name('companies')}.#{con.quote_column_name('id')} IN \(1\)/) do @@ -242,12 +261,6 @@ class InheritanceTest < ActiveRecord::TestCase end end - def test_alt_eager_loading - switch_to_alt_inheritance_column - test_eager_load_belongs_to_something_inherited - switch_to_default_inheritance_column - end - def test_inherits_custom_primary_key assert_equal Subscriber.primary_key, SpecialSubscriber.primary_key end @@ -256,21 +269,6 @@ class InheritanceTest < ActiveRecord::TestCase assert_kind_of SpecialSubscriber, SpecialSubscriber.find("webster132") assert_nothing_raised { s = SpecialSubscriber.new("name" => "And breaaaaathe!"); s.id = 'roger'; s.save } end - - private - def switch_to_alt_inheritance_column - # we don't want misleading test results, so get rid of the values in the type column - Company.all.merge!(:order => 'id').to_a.each do |c| - c['type'] = nil - c.save - end - [ Company, Firm, Client].each { |klass| klass.reset_column_information } - Company.inheritance_column = 'ruby_type' - end - def switch_to_default_inheritance_column - [ Company, Firm, Client].each { |klass| klass.reset_column_information } - Company.inheritance_column = 'type' - end end @@ -290,7 +288,7 @@ class InheritanceComputeTypeTest < ActiveRecord::TestCase def test_instantiation_doesnt_try_to_require_corresponding_file ActiveRecord::Base.store_full_sti_class = false foo = Firm.first.clone - foo.ruby_type = foo.type = 'FirmOnTheFly' + foo.type = 'FirmOnTheFly' foo.save! # Should fail without FirmOnTheFly in the type condition. diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 01dd25a9df..92084d3eb6 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -182,7 +182,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dumps_index_columns_in_right_order index_definition = standard_dump.split(/\n/).grep(/add_index.*companies/).first.strip - assert_equal 'add_index "companies", ["firm_id", "type", "rating", "ruby_type"], :name => "company_index"', index_definition + assert_equal 'add_index "companies", ["firm_id", "type", "rating"], :name => "company_index"', index_definition end def test_schema_dumps_partial_indices diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml index a982d3921d..0766e92027 100644 --- a/activerecord/test/fixtures/companies.yml +++ b/activerecord/test/fixtures/companies.yml @@ -4,14 +4,12 @@ first_client: firm_id: 1 client_of: 2 name: Summit - ruby_type: Client firm_name: 37signals first_firm: id: 1 type: Firm name: 37signals - ruby_type: Firm firm_id: 1 second_client: @@ -20,13 +18,11 @@ second_client: firm_id: 1 client_of: 1 name: Microsoft - ruby_type: Client another_firm: id: 4 type: Firm name: Flamboyant Software - ruby_type: Firm another_client: id: 5 @@ -34,7 +30,6 @@ another_client: firm_id: 4 client_of: 4 name: Ex Nihilo - ruby_type: Client a_third_client: id: 10 @@ -42,7 +37,6 @@ a_third_client: firm_id: 4 client_of: 4 name: Ex Nihilo Part Deux - ruby_type: Client rails_core: id: 6 diff --git a/activerecord/test/fixtures/vegetables.yml b/activerecord/test/fixtures/vegetables.yml new file mode 100644 index 0000000000..b9afbfbb05 --- /dev/null +++ b/activerecord/test/fixtures/vegetables.yml @@ -0,0 +1,20 @@ +first_cucumber: + id: 1 + custom_type: Cucumber + name: 'my cucumber' + +first_cabbage: + id: 2 + custom_type: Cabbage + name: 'my cabbage' + +second_cabbage: + id: 3 + custom_type: Cabbage + name: 'his cabbage' + +red_cabbage: + id: 4 + custom_type: RedCabbage + name: 'red cabbage' + seller_id: 3
\ No newline at end of file diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 75f38d275c..9bdce6e729 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -173,10 +173,6 @@ class Client < Company before_destroy :overwrite_to_raise # Used to test that read and question methods are not generated for these attributes - def ruby_type - read_attribute :ruby_type - end - def rating? query_attribute :rating end diff --git a/activerecord/test/models/vegetables.rb b/activerecord/test/models/vegetables.rb new file mode 100644 index 0000000000..1f41cde3a5 --- /dev/null +++ b/activerecord/test/models/vegetables.rb @@ -0,0 +1,24 @@ +class Vegetable < ActiveRecord::Base + + validates_presence_of :name + + def self.inheritance_column + 'custom_type' + end +end + +class Cucumber < Vegetable +end + +class Cabbage < Vegetable +end + +class GreenCabbage < Cabbage +end + +class KingCole < GreenCabbage +end + +class RedCabbage < Cabbage + belongs_to :seller, :class_name => 'Company' +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7c45ca27c0..007349ea87 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -171,7 +171,6 @@ ActiveRecord::Schema.define do create_table :companies, :force => true do |t| t.string :type - t.string :ruby_type t.integer :firm_id t.string :firm_name t.string :name @@ -181,9 +180,15 @@ ActiveRecord::Schema.define do t.string :description, :default => "" end - add_index :companies, [:firm_id, :type, :rating, :ruby_type], :name => "company_index" + add_index :companies, [:firm_id, :type, :rating], :name => "company_index" add_index :companies, [:firm_id, :type], :name => "company_partial_index", :where => "rating > 10" + create_table :vegetables, :force => true do |t| + t.string :name + t.integer :seller_id + t.string :custom_type + end + create_table :computers, :force => true do |t| t.integer :developer, :null => false t.integer :extendedWarranty, :null => false diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 45c9f0472a..d6bc95a522 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -487,7 +487,7 @@ module ActiveSupport #:nodoc: raise "Circular dependency detected while autoloading constant #{qualified_name}" else require_or_load(expanded) - raise LoadError, "Expected #{file_path} to define #{qualified_name}" unless from_mod.const_defined?(const_name, false) + raise LoadError, "Unable to autoload constant #{qualified_name}, expected #{file_path} to define it" unless from_mod.const_defined?(const_name, false) return from_mod.const_get(const_name) end elsif mod = autoload_module!(from_mod, const_name, qualified_name, path_suffix) |