From bea44cbaa48433d427a9418ff6b5c617b10e0585 Mon Sep 17 00:00:00 2001 From: Chulki Lee Date: Fri, 10 Jan 2014 16:27:18 -0800 Subject: [PATCH 01/10] Set NameError#name --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/inheritance.rb | 2 +- activerecord/test/cases/base_test.rb | 4 +++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f57158f38ec0a..7db700a27a8e8 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Set NameError#name with class name when failed to load the class for association. + + *Chulki Lee* + * Fix bug in `becomes!` when changing from the base model to a STI sub-class. Fixes #13272. diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 949e7678a57b0..69896f7219314 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -126,7 +126,7 @@ def compute_type(type_name) end end - raise NameError, "uninitialized constant #{candidates.first}" + raise NameError.new("uninitialized constant #{candidates.first}", candidates.first) end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index cb8e564da1013..4bc6002bfe1df 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1301,9 +1301,11 @@ def test_compute_type_success end def test_compute_type_nonexistent_constant - assert_raises NameError do + e = assert_raises NameError do ActiveRecord::Base.send :compute_type, 'NonexistentModel' end + assert_equal 'uninitialized constant ActiveRecord::Base::NonexistentModel', e.message + assert_equal 'ActiveRecord::Base::NonexistentModel', e.name end def test_compute_type_no_method_error From ad04c2e0b592b01f4e3b3ec2dcbb799b921a6fff Mon Sep 17 00:00:00 2001 From: Uday Kadaboina Date: Tue, 14 Jan 2014 01:31:23 -0500 Subject: [PATCH 02/10] [ci skip] Added alias to CSRF --- guides/source/security.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guides/source/security.md b/guides/source/security.md index 21cc3deb8ac0c..c367604d6f03d 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -150,7 +150,7 @@ Another countermeasure is to _save user-specific properties in the session_, ver ### Session Expiry -NOTE: _Sessions that never expire extend the time-frame for attacks such as cross-site reference forgery (CSRF), session hijacking and session fixation._ +NOTE: _Sessions that never expire extend the time-frame for attacks such as cross-site request forgery (CSRF), session hijacking and session fixation._ One possibility is to set the expiry time-stamp of the cookie with the session id. However the client can edit cookies that are stored in the web browser so expiring sessions on the server is safer. Here is an example of how to _expire sessions in a database table_. Call `Session.sweep("20 minutes")` to expire sessions that were used longer than 20 minutes ago. @@ -354,7 +354,7 @@ Having one single place in the admin interface or Intranet, where the input has Refer to the Injection section for countermeasures against XSS. It is _recommended to use the SafeErb plugin_ also in an Intranet or administration interface. -**CSRF** Cross-Site Reference Forgery (CSRF) is a gigantic attack method, it allows the attacker to do everything the administrator or Intranet user may do. As you have already seen above how CSRF works, here are a few examples of what attackers can do in the Intranet or admin interface. +**CSRF** Cross-Site Request Forgery (CSRF), also known as Cross-Site Reference Forgery (XSRF), is a gigantic attack method, it allows the attacker to do everything the administrator or Intranet user may do. As you have already seen above how CSRF works, here are a few examples of what attackers can do in the Intranet or admin interface. A real-world example is a [router reconfiguration by CSRF](http://www.h-online.com/security/Symantec-reports-first-active-attack-on-a-DSL-router--/news/102352). The attackers sent a malicious e-mail, with CSRF in it, to Mexican users. The e-mail claimed there was an e-card waiting for them, but it also contained an image tag that resulted in a HTTP-GET request to reconfigure the user's router (which is a popular model in Mexico). The request changed the DNS-settings so that requests to a Mexico-based banking site would be mapped to the attacker's site. Everyone who accessed the banking site through that router saw the attacker's fake web site and had their credentials stolen. From 9cdc7c0614fabc56d8560deeffae7e949bb9f464 Mon Sep 17 00:00:00 2001 From: Cristian Mircea Messel Date: Mon, 13 Jan 2014 21:11:15 +0200 Subject: [PATCH 03/10] single quotes for controller generated routes Write routes in route.rb with single quotes get 'welcome/index' instead of get "welcome/index" --- guides/source/getting_started.md | 10 +++++----- railties/CHANGELOG.md | 4 ++++ .../rails/controller/controller_generator.rb | 6 +++--- railties/test/generators/controller_generator_test.rb | 4 ++-- railties/test/generators/namespaced_generators_test.rb | 2 +- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index ebc56cba58e62..dbcedba800eb2 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -231,7 +231,7 @@ Rails will create several files and a route for you. ```bash create app/controllers/welcome_controller.rb - route get "welcome/index" + route get 'welcome/index' invoke erb create app/views/welcome create app/views/welcome/index.html.erb @@ -272,13 +272,13 @@ Open the file `config/routes.rb` in your editor. ```ruby Rails.application.routes.draw do - get "welcome/index" + get 'welcome/index' # The priority is based upon order of creation: # first created -> highest priority. # # You can have the root of your site routed with "root" - # root "welcome#index" + # root 'welcome#index' # # ... ``` @@ -295,7 +295,7 @@ root 'welcome#index' ``` `root 'welcome#index'` tells Rails to map requests to the root of the -application to the welcome controller's index action and `get "welcome/index"` +application to the welcome controller's index action and `get 'welcome/index'` tells Rails to map requests to to the welcome controller's index action. This was created earlier when you ran the controller generator (`rails generate controller welcome index`). @@ -328,7 +328,7 @@ Blog::Application.routes.draw do resources :posts - root "welcome#index" + root 'welcome#index' end ``` diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 83747470bf8bb..e314ab56373a3 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Write controller generated routes in routes.rb with single quotes. + + *Cristian Mircea Messel* + * Only lookup `config.log_level` for stdlib `::Logger` instances. Assign it as is for third party loggers like `Log4r::Logger`. diff --git a/railties/lib/rails/generators/rails/controller/controller_generator.rb b/railties/lib/rails/generators/rails/controller/controller_generator.rb index ef84447df9432..33a0d81bf6fd4 100644 --- a/railties/lib/rails/generators/rails/controller/controller_generator.rb +++ b/railties/lib/rails/generators/rails/controller/controller_generator.rb @@ -23,7 +23,7 @@ def add_routes # Will generate - # namespace :foo do # namespace :bar do - # get "baz/index" + # get 'baz/index' # end # end def generate_routing_code(action) @@ -36,8 +36,8 @@ def generate_routing_code(action) end.join # Create route - # get "baz/index" - route = indent(%{get "#{file_name}/#{action}"\n}, depth * 2) + # get 'baz/index' + route = indent(%{get '#{file_name}/#{action}'\n}, depth * 2) # Create `end` ladder # end diff --git a/railties/test/generators/controller_generator_test.rb b/railties/test/generators/controller_generator_test.rb index 2268f048397ca..4b2f8539d06e1 100644 --- a/railties/test/generators/controller_generator_test.rb +++ b/railties/test/generators/controller_generator_test.rb @@ -67,7 +67,7 @@ def test_invokes_default_template_engine def test_add_routes run_generator - assert_file "config/routes.rb", /get "account\/foo"/, /get "account\/bar"/ + assert_file "config/routes.rb", /get 'account\/foo'/, /get 'account\/bar'/ end def test_invokes_default_template_engine_even_with_no_action @@ -91,6 +91,6 @@ def test_actions_are_turned_into_methods def test_namespaced_routes_are_created_in_routes run_generator ["admin/dashboard", "index"] - assert_file "config/routes.rb", /namespace :admin do\n\s+get "dashboard\/index"\n/ + assert_file "config/routes.rb", /namespace :admin do\n\s+get 'dashboard\/index'\n/ end end diff --git a/railties/test/generators/namespaced_generators_test.rb b/railties/test/generators/namespaced_generators_test.rb index e17925ff65584..d677c21f152dd 100644 --- a/railties/test/generators/namespaced_generators_test.rb +++ b/railties/test/generators/namespaced_generators_test.rb @@ -63,7 +63,7 @@ def test_invokes_default_template_engine def test_routes_should_not_be_namespaced run_generator - assert_file "config/routes.rb", /get "account\/foo"/, /get "account\/bar"/ + assert_file "config/routes.rb", /get 'account\/foo'/, /get 'account\/bar'/ end def test_invokes_default_template_engine_even_with_no_action From a71a8e2a35e159b92d393228b935cee8301fce34 Mon Sep 17 00:00:00 2001 From: Akshay Vishnoi Date: Tue, 14 Jan 2014 13:43:47 +0530 Subject: [PATCH 04/10] [ci skip] Grammar correction --- activesupport/lib/active_support/core_ext/hash/conversions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 2684c772ea14f..7bea461c77140 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -105,7 +105,7 @@ class << self # hash = Hash.from_xml(xml) # # => {"hash"=>{"foo"=>1, "bar"=>2}} # - # DisallowedType is raise if the XML contains attributes with type="yaml" or + # DisallowedType is raised if the XML contains attributes with type="yaml" or # type="symbol". Use Hash.from_trusted_xml to parse this XML. def from_xml(xml, disallowed_types = nil) ActiveSupport::XMLConverter.new(xml, disallowed_types).to_h From 66f3d5bd5a0b6cda11a8fe8aa2ebcfc25cb8f53d Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Tue, 14 Jan 2014 09:19:37 +0100 Subject: [PATCH 05/10] quick pass through Active Record CHANGELOG. [ci skip] --- activerecord/CHANGELOG.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 7db700a27a8e8..ada5a57f3b582 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,4 @@ -* Set NameError#name with class name when failed to load the class for association. +* Set `NameError#name` when STI-class-lookup fails. *Chulki Lee* @@ -65,7 +65,7 @@ class: `ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig`. To understand the exact behavior of this class, it is best to review the - behavior in `activerecord/test/cases/connection_adapters/connection_handler_test.rb` + behavior in `activerecord/test/cases/connection_adapters/connection_handler_test.rb`. *Richard Schneeman* @@ -411,7 +411,7 @@ *kostya*, *Lauro Caetano* * `type_to_sql` returns a `String` for unmapped columns. This fixes an error - when using unmapped array types in PG + when using unmapped PostgreSQL array types. Example: @@ -450,7 +450,7 @@ * Update counter cache on a `has_many` relationship regardless of default scope. - Fix #12952. + Fixes #12952. *Uku Taht* @@ -461,9 +461,10 @@ *Cody Cutrer*, *Yves Senn* -* Raise `ActiveRecord::RecordNotDestroyed` when a replaced child marked with `dependent: destroy` fails to be destroyed. +* Raise `ActiveRecord::RecordNotDestroyed` when a replaced child + marked with `dependent: destroy` fails to be destroyed. - Fix #12812 + Fixex #12812. *Brian Thomas Storti* @@ -1369,6 +1370,7 @@ *Yves Senn* * Fix the `:primary_key` option for `has_many` associations. + Fixes #10693. *Yves Senn* @@ -1493,7 +1495,7 @@ * Trigger a save on `has_one association=(associate)` when the associate contents have changed. - Fix #8856. + Fixes #8856. *Chris Thompson* From 3cf5d2b75bca5eb0578643780bfeb50e75ba59be Mon Sep 17 00:00:00 2001 From: Prathamesh Sonpatki Date: Tue, 14 Jan 2014 10:46:29 +0530 Subject: [PATCH 06/10] Fix fields_for documentation with index option [ci skip] - fields_for documentation with index option was wrong. - It does not work with passing model as it is. - Changed the example by passing id of the address object. - Fixes #13125. --- guides/source/form_helpers.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index ec4a255398745..455dc7bebe3e4 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -751,7 +751,7 @@ You might want to render a form with a set of edit fields for each of a person's <%= form_for @person do |person_form| %> <%= person_form.text_field :name %> <% @person.addresses.each do |address| %> - <%= person_form.fields_for address, index: address do |address_form|%> + <%= person_form.fields_for address, index: address.id do |address_form|%> <%= address_form.text_field :city %> <% end %> <% end %> @@ -774,9 +774,16 @@ This will result in a `params` hash that looks like {'person' => {'name' => 'Bob', 'address' => {'23' => {'city' => 'Paris'}, '45' => {'city' => 'London'}}}} ``` -Rails knows that all these inputs should be part of the person hash because you called `fields_for` on the first form builder. By specifying an `:index` option you're telling Rails that instead of naming the inputs `person[address][city]` it should insert that index surrounded by [] between the address and the city. If you pass an Active Record object as we did then Rails will call `to_param` on it, which by default returns the database id. This is often useful as it is then easy to locate which Address record should be modified. You can pass numbers with some other significance, strings or even `nil` (which will result in an array parameter being created). +Rails knows that all these inputs should be part of the person hash because you +called `fields_for` on the first form builder. By specifying an `:index` option +you're telling Rails that instead of naming the inputs `person[address][city]` +it should insert that index surrounded by [] between the address and the city. +This is often useful as it is then easy to locate which Address record +should be modified. You can pass numbers with some other significance, +strings or even `nil` (which will result in an array parameter being created). -To create more intricate nestings, you can specify the first part of the input name (`person[address]` in the previous example) explicitly, for example +To create more intricate nestings, you can specify the first part of the input +name (`person[address]` in the previous example) explicitly: ```erb <%= fields_for 'person[address][primary]', address, index: address do |address_form| %> From b242b2dbe75f0b5e86e2ce9ef7c2c5ee96e17862 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 14 Jan 2014 03:58:22 -0800 Subject: [PATCH 07/10] Enum mappings are now exposed via class methods instead of constants. Example: class Conversation < ActiveRecord::Base enum status: [ :active, :archived ] end Before: Conversation::STATUS # => { "active" => 0, "archived" => 1 } After: Conversation.statuses # => { "active" => 0, "archived" => 1 } --- activerecord/CHANGELOG.md | 18 ++++++++++++++++++ activerecord/lib/active_record/enum.rb | 17 ++++++++++------- activerecord/test/cases/enum_test.rb | 6 +++--- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ada5a57f3b582..e85300c95155e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,21 @@ +* Enum mappings are now exposed via class methods instead of constants. + + Example: + + class Conversation < ActiveRecord::Base + enum status: [ :active, :archived ] + end + + Before: + + Conversation::STATUS # => { "active" => 0, "archived" => 1 } + + After: + + Conversation.statuses # => { "active" => 0, "archived" => 1 } + + *Godfrey Chan* + * Set `NameError#name` when STI-class-lookup fails. *Chulki Lee* diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index d1bc785bee62d..c34fc086e2d90 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -56,21 +56,24 @@ module ActiveRecord # In rare circumstances you might need to access the mapping directly. # The mappings are exposed through a constant with the attributes name: # - # Conversation::STATUS # => { "active" => 0, "archived" => 1 } + # Conversation.statuses # => { "active" => 0, "archived" => 1 } # # Use that constant when you need to know the ordinal value of an enum: # - # Conversation.where("status <> ?", Conversation::STATUS[:archived]) + # Conversation.where("status <> ?", Conversation.statuses[:archived]) module Enum def enum(definitions) klass = self definitions.each do |name, values| - # STATUS = { } - enum_values = _enum_methods_module.const_set name.to_s.upcase, ActiveSupport::HashWithIndifferentAccess.new + # statuses = { } + enum_values = ActiveSupport::HashWithIndifferentAccess.new name = name.to_sym + # def self.statuses statuses end + klass.singleton_class.send(:define_method, name.to_s.pluralize) { enum_values } + _enum_methods_module.module_eval do - # def status=(value) self[:status] = STATUS[value] end + # def status=(value) self[:status] = statuses[value] end define_method("#{name}=") { |value| if enum_values.has_key?(value) || value.blank? self[name] = enum_values[value] @@ -84,10 +87,10 @@ def enum(definitions) end } - # def status() STATUS.key self[:status] end + # def status() statuses.key self[:status] end define_method(name) { enum_values.key self[name] } - # def status_before_type_cast() STATUS.key self[:status] end + # def status_before_type_cast() statuses.key self[:status] end define_method("#{name}_before_type_cast") { enum_values.key self[name] } pairs = values.respond_to?(:each_pair) ? values.each_pair : values.each_with_index diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 0075df8b8d8f8..1f98801e93df5 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -74,9 +74,9 @@ class EnumTest < ActiveRecord::TestCase end test "constant to access the mapping" do - assert_equal 0, Book::STATUS[:proposed] - assert_equal 1, Book::STATUS["written"] - assert_equal 2, Book::STATUS[:published] + assert_equal 0, Book.statuses[:proposed] + assert_equal 1, Book.statuses["written"] + assert_equal 2, Book.statuses[:published] end test "building new objects with enum scopes" do From e8d1d84837a59ef7d73b29b16ee05cd610d30a90 Mon Sep 17 00:00:00 2001 From: Ujjwal Thaakar Date: Tue, 14 Jan 2014 18:53:45 +0530 Subject: [PATCH 08/10] Don't try to get the subclass if the inheritance column doesn't exist The `subclass_from_attrs` method is called even if the column specified by the `inheritance_column` setting doesn't exist. This prevents setting associations via the attributes hash if the association name clashes with the value of the setting, typically `:type`. This worked previously in Rails 3.2. --- activerecord/CHANGELOG.md | 9 ++++++++ activerecord/lib/active_record/inheritance.rb | 22 +++++++++++++------ activerecord/test/cases/inheritance_test.rb | 9 +++++++- activerecord/test/models/shop.rb | 5 +++++ activerecord/test/schema/schema.rb | 5 +++++ 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index e85300c95155e..cb42df5aa0743 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Don't try to get the subclass if the inheritance column doesn't exist + + The `subclass_from_attrs` method is called even if the column specified by + the `inheritance_column` setting doesn't exist. This prevents setting associations + via the attributes hash if the association name clashes with the value of the setting, + typically `:type`. This worked previously in Rails 3.2. + + *Ujjwal Thaakar* + * Enum mappings are now exposed via class methods instead of constants. Example: diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 69896f7219314..da73112e90422 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -18,13 +18,17 @@ def new(*args, &block) if abstract_class? || self == Base raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated." end - if (attrs = args.first).is_a?(Hash) - if subclass = subclass_from_attrs(attrs) - return subclass.new(*args, &block) - end + + attrs = args.first + if subclass_from_attributes?(attrs) + subclass = subclass_from_attributes(attrs) + end + + if subclass + subclass.new(*args, &block) + else + super end - # Delegate to the original .new - super end # Returns +true+ if this does not need STI type condition. Returns @@ -172,7 +176,11 @@ def type_condition(table = arel_table) # is not self or a valid subclass, raises ActiveRecord::SubclassNotFound # If this is a StrongParameters hash, and access to inheritance_column is not permitted, # this will ignore the inheritance column and return nil - def subclass_from_attrs(attrs) + def subclass_from_attributes?(attrs) + columns_hash.include?(inheritance_column) && attrs.is_a?(Hash) + end + + def subclass_from_attributes(attrs) subclass_name = attrs.with_indifferent_access[inheritance_column] if subclass_name.present? && subclass_name != self.name diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 7fd7d423545fa..d2b5a06b55d25 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -1,10 +1,11 @@ -require "cases/helper" +require 'cases/helper' require 'models/company' require 'models/person' require 'models/post' require 'models/project' require 'models/subscriber' require 'models/vegetables' +require 'models/shop' class InheritanceTest < ActiveRecord::TestCase fixtures :companies, :projects, :subscribers, :accounts, :vegetables @@ -367,4 +368,10 @@ def test_instantiation_doesnt_try_to_require_corresponding_file ensure ActiveRecord::Base.store_full_sti_class = true end + + def test_sti_type_from_attributes_disabled_in_non_sti_class + phone = Shop::Product::Type.new(name: 'Phone') + product = Shop::Product.new(:type => phone) + assert product.save + end end diff --git a/activerecord/test/models/shop.rb b/activerecord/test/models/shop.rb index 81414227ea832..607a0a5b413c7 100644 --- a/activerecord/test/models/shop.rb +++ b/activerecord/test/models/shop.rb @@ -5,6 +5,11 @@ class Collection < ActiveRecord::Base class Product < ActiveRecord::Base has_many :variants, :dependent => :delete_all + belongs_to :type + + class Type < ActiveRecord::Base + has_many :products + end end class Variant < ActiveRecord::Base diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index ddfc1ac0d63cd..9a7d918a25a9f 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -557,9 +557,14 @@ def create_table(*args, &block) create_table :products, force: true do |t| t.references :collection + t.references :type t.string :name end + create_table :product_types, force: true do |t| + t.string :name + end + create_table :projects, force: true do |t| t.string :name t.string :type From 547ed456337dda54799d9c5f316dcbce43cfce9d Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Tue, 17 Dec 2013 10:10:23 -0700 Subject: [PATCH 09/10] sqlite >= 3.8.0 supports partial indexes --- activerecord/CHANGELOG.md | 6 ++++++ .../connection_adapters/sqlite3_adapter.rb | 18 +++++++++++++++++- activerecord/test/cases/schema_dumper_test.rb | 2 ++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index cb42df5aa0743..5c5dd99ab7c05 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Enable partial indexes for sqlite >= 3.8.0 + + See http://www.sqlite.org/partialindex.html + + *Cody Cutrer* + * Don't try to get the subclass if the inheritance column doesn't exist The `subclass_from_attrs` method is called even if the column specified by diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 92bb70ba533fc..170dddb08e993 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -155,6 +155,10 @@ def supports_savepoints? true end + def supports_partial_index? + sqlite_version >= '3.8.0' + end + # Returns true, since this connection adapter supports prepared statement # caching. def supports_statement_cache? @@ -397,13 +401,25 @@ def columns(table_name) #:nodoc: # Returns an array of indexes for the given table. def indexes(table_name, name = nil) #:nodoc: exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", 'SCHEMA').map do |row| + sql = <<-SQL + SELECT sql + FROM sqlite_master + WHERE name=#{quote(row['name'])} AND type='index' + UNION ALL + SELECT sql + FROM sqlite_temp_master + WHERE name=#{quote(row['name'])} AND type='index' + SQL + index_sql = exec_query(sql).first['sql'] + match = /\sWHERE\s+(.+)$/i.match(index_sql) + where = match[1] if match IndexDefinition.new( table_name, row['name'], row['unique'] != 0, exec_query("PRAGMA index_info('#{row['name']}')", "SCHEMA").map { |col| col['name'] - }) + }, nil, nil, where) end end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 741827446dc25..c085663efbc9a 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -190,6 +190,8 @@ def test_schema_dumps_partial_indices assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", where: "(rating > 10)", using: :btree', index_definition elsif current_adapter?(:MysqlAdapter) || current_adapter?(:Mysql2Adapter) assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", using: :btree', index_definition + elsif current_adapter?(:SQLite3Adapter) && ActiveRecord::Base.connection.supports_partial_index? + assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", where: "rating > 10"', index_definition else assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index"', index_definition end From b23330745bddd6729f95fb8487e3ec4857e4bb58 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Tue, 14 Jan 2014 18:00:58 +0100 Subject: [PATCH 10/10] don't establish a new connection when testing with `sqlite3_mem`. This fixes broken `rake test_sqlite3_mem` suite for Active Record. The problem is that that the old database with the schema is lost when establishing a new connection. Upon reconnting we are left with a blank database and tests down the line start failing. --- .../adapters/sqlite3/sqlite3_adapter_test.rb | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 0598ff25f854f..082fe3cde5cb7 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -34,24 +34,30 @@ def test_bad_connection end def test_connect_with_url - original_connection = ActiveRecord::Base.remove_connection - tf = Tempfile.open 'whatever' - url = "sqlite3://#{tf.path}" - ActiveRecord::Base.establish_connection(url) - assert ActiveRecord::Base.connection - ensure - tf.close - tf.unlink - ActiveRecord::Base.establish_connection(original_connection) + skip "can't establish new connection when using memory db" if in_memory_db? + begin + original_connection = ActiveRecord::Base.remove_connection + tf = Tempfile.open 'whatever' + url = "sqlite3://#{tf.path}" + ActiveRecord::Base.establish_connection(url) + assert ActiveRecord::Base.connection + ensure + tf.close + tf.unlink + ActiveRecord::Base.establish_connection(original_connection) + end end def test_connect_memory_with_url - original_connection = ActiveRecord::Base.remove_connection - url = "sqlite3:///:memory:" - ActiveRecord::Base.establish_connection(url) - assert ActiveRecord::Base.connection - ensure - ActiveRecord::Base.establish_connection(original_connection) + skip "can't establish new connection when using memory db" if in_memory_db? + begin + original_connection = ActiveRecord::Base.remove_connection + url = "sqlite3:///:memory:" + ActiveRecord::Base.establish_connection(url) + assert ActiveRecord::Base.connection + ensure + ActiveRecord::Base.establish_connection(original_connection) + end end def test_valid_column