Permalink
Browse files

Merge branch 'master' into adequaterecord

* master:
  Make AR::Base#touch fire the after_commit and after_rollback callbacks
  Fix test for cache_key + touched
  Revert "methods are defined right after the module_eval, so we don't need to do"
  Revert "Don't remove trailing slash from PATH_INFO for mounted apps"
  Add failing test for #13369
  reset column information after fiddling with `Encoding.default_internal`
  we have `with_env_tz` as global test helper. Remove duplicate.
  isolate class attribute assignment in `migration_test.rb`
  use `teardown` for cleanup, not `setup`.
  tests without transactional fixtures need to cleanup afterwards.
  no need to `return skip` in tests. `skip` is enough.
  methods are defined right after the module_eval, so we don't need to do any line number maths
  Get rid of unused TransactionError constant
  Avoid converting :on option to array twice when defining commit/rollback callbacks
  Unify changelog entries about single quotes [ci skip]
  Use single quotes in generated files
  • Loading branch information...
tenderlove committed Jan 16, 2014
2 parents f3e379f + 73ba2c1 commit f19ba681a80c0b0be604c58389ce2892c623d027
Showing with 168 additions and 100 deletions.
  1. +0 −6 actionpack/CHANGELOG.md
  2. +1 −7 actionpack/lib/action_dispatch/journey/router.rb
  3. +0 −5 actionpack/test/dispatch/mount_test.rb
  4. +18 −0 actionpack/test/dispatch/routing_test.rb
  5. +4 −0 activerecord/CHANGELOG.md
  6. +6 −6 activerecord/lib/active_record/transactions.rb
  7. +1 −1 activerecord/test/cases/adapters/postgresql/json_test.rb
  8. +1 −1 activerecord/test/cases/adapters/postgresql/range_test.rb
  9. +1 −1 activerecord/test/cases/adapters/postgresql/xml_test.rb
  10. +7 −0 activerecord/test/cases/autosave_association_test.rb
  11. +2 −1 activerecord/test/cases/base_test.rb
  12. +0 −7 activerecord/test/cases/finder_test.rb
  13. +9 −9 activerecord/test/cases/integration_test.rb
  14. +31 −27 activerecord/test/cases/migration_test.rb
  15. +2 −5 activerecord/test/cases/query_cache_test.rb
  16. +46 −1 activerecord/test/cases/transaction_callbacks_test.rb
  17. +0 −1 activerecord/test/models/car.rb
  18. +17 −0 activerecord/test/models/owner.rb
  19. +2 −2 guides/code/getting_started/app/views/layouts/application.html.erb
  20. +1 −1 guides/code/getting_started/config/environments/test.rb
  21. +5 −5 guides/source/upgrading_ruby_on_rails.md
  22. +2 −2 railties/CHANGELOG.md
  23. +6 −6 railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt
  24. +1 −1 railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt
  25. +1 −1 railties/lib/rails/generators/rails/app/templates/test/test_helper.rb
  26. +4 −4 railties/test/generators/app_generator_test.rb
View
@@ -431,10 +431,4 @@
*Piotr Sarnacki*, *Łukasz Strzałkowski*
-* Fix removing trailing slash for mounted apps.
-
- Fixes #3215.
-
- *Piotr Sarnacki*
-
Please check [4-0-stable](https://github.com/rails/rails/blob/4-0-stable/actionpack/CHANGELOG.md) for previous changes.
@@ -54,7 +54,7 @@ def initialize(routes, options)
end
def call(env)
- env['PATH_INFO'] = normalize_path(env['PATH_INFO'])
+ env['PATH_INFO'] = Utils.normalize_path(env['PATH_INFO'])
find_routes(env).each do |match, parameters, route|
script_name, path_info, set_params = env.values_at('SCRIPT_NAME',
@@ -103,12 +103,6 @@ def visualizer
private
- def normalize_path(path)
- path = "/#{path}"
- path.squeeze!('/')
- path
- end
-
def partitioned_routes
routes.partitioned_routes
end
@@ -44,11 +44,6 @@ def test_app_name_is_properly_generated_when_engine_is_mounted_in_resources
"A named route should be defined with a parent's prefix"
end
- def test_trailing_slash_is_not_removed_from_path_info
- get "/sprockets/omg/"
- assert_equal "/sprockets -- /omg/", response.body
- end
-
def test_mounting_sets_script_name
get "/sprockets/omg"
assert_equal "/sprockets -- /omg", response.body
@@ -2894,6 +2894,24 @@ def test_absolute_controller_namespace
assert_equal '/foo', foo_root_path
end
+ def test_trailing_slash
+ draw do
+ resources :streams
+ end
+
+ get '/streams'
+ assert @response.ok?, 'route without trailing slash should work'
+
+ get '/streams/'
+ assert @response.ok?, 'route with trailing slash should work'
+
+ get '/streams?foobar'
+ assert @response.ok?, 'route without trailing slash and with QUERY_STRING should work'
+
+ get '/streams/?foobar'
+ assert @response.ok?, 'route with trailing slash and with QUERY_STRING should work'
+ end
+
private
def draw(&block)
@@ -1,3 +1,7 @@
+* Make `touch` fire the `after_commit` and `after_rollback` callbacks.
+
+ *Harry Brundage*
+
* Enable partial indexes for sqlite >= 3.8.0
See http://www.sqlite.org/partialindex.html
@@ -6,9 +6,6 @@ module Transactions
extend ActiveSupport::Concern
ACTIONS = [:create, :destroy, :update]
- class TransactionError < ActiveRecordError # :nodoc:
- end
-
included do
define_callbacks :commit, :rollback,
terminator: ->(_, result) { result == false },
@@ -243,15 +240,14 @@ def after_rollback(*args, &block)
def set_options_for_callbacks!(args)
options = args.last
if options.is_a?(Hash) && options[:on]
- assert_valid_transaction_action(options[:on])
- options[:if] = Array(options[:if])
fire_on = Array(options[:on])
+ assert_valid_transaction_action(fire_on)
+ options[:if] = Array(options[:if])
options[:if] << "transaction_include_any_action?(#{fire_on})"
end
end
def assert_valid_transaction_action(actions)
- actions = Array(actions)
if (actions - ACTIONS).any?
raise ArgumentError, ":on conditions for after_commit and after_rollback callbacks have to be one of #{ACTIONS.join(",")}"
end
@@ -277,6 +273,10 @@ def save!(*) #:nodoc:
with_transaction_returning_status { super }
end
+ def touch(*) #:nodoc:
+ with_transaction_returning_status { super }
+ end
+
# Reset id and @new_record if the transaction rolls back.
def rollback_active_record_state!
remember_transaction_record_state
@@ -21,7 +21,7 @@ def setup
end
end
rescue ActiveRecord::StatementInvalid
- return skip "do not test on PG without json"
+ skip "do not test on PG without json"
end
@column = JsonDataType.columns.find { |c| c.name == 'payload' }
end
@@ -26,7 +26,7 @@ def setup
end
end
rescue ActiveRecord::StatementInvalid
- return skip "do not test on PG without range"
+ skip "do not test on PG without range"
end
insert_range(id: 101,
@@ -18,7 +18,7 @@ def setup
end
end
rescue ActiveRecord::StatementInvalid
- return skip "do not test on PG without xml"
+ skip "do not test on PG without xml"
end
@column = XmlDataType.columns.find { |c| c.name == 'payload' }
end
@@ -574,6 +574,13 @@ def setup
@ship = @pirate.create_ship(:name => 'Nights Dirty Lightning')
end
+ def teardown
+ # We are running without transactional fixtures and need to cleanup.
+ Bird.delete_all
+ @ship.delete
+ @pirate.delete
+ end
+
# reload
def test_a_marked_for_destruction_record_should_not_be_be_marked_after_reload
@pirate.mark_for_destruction
@@ -626,6 +626,7 @@ def test_respect_internal_encoding
assert_equal ["EUC-JP"], Weird.columns.map {|c| c.name.encoding.name }.uniq
ensure
silence_warnings { Encoding.default_internal = old_default_internal }
+ Weird.reset_column_information
end
end
@@ -1129,7 +1130,7 @@ def test_sequence_name_with_abstract_class
k = Class.new(ak)
k.table_name = "projects"
orig_name = k.sequence_name
- return skip "sequences not supported by db" unless orig_name
+ skip "sequences not supported by db" unless orig_name
assert_equal k.reset_sequence_name, orig_name
end
@@ -891,11 +891,4 @@ def bind(statement, *vars)
ActiveRecord::Base.send(:replace_bind_variables, statement, vars)
end
end
-
- def with_env_tz(new_tz = 'US/Eastern')
- old_tz, ENV['TZ'] = ENV['TZ'], new_tz
- yield
- ensure
- old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
- end
end
@@ -3,12 +3,11 @@
require 'cases/helper'
require 'models/company'
require 'models/developer'
-require 'models/car'
-require 'models/bulb'
require 'models/owner'
+require 'models/pet'
class IntegrationTest < ActiveRecord::TestCase
- fixtures :companies, :developers, :owners
+ fixtures :companies, :developers, :owners, :pets
def test_to_param_should_return_string
assert_kind_of String, Client.first.to_param
@@ -91,13 +90,14 @@ def test_cache_key_format_for_existing_record_with_updated_at_and_custom_cache_t
end
def test_cache_key_changes_when_child_touched
- car = Car.create
- Bulb.create(car: car)
+ owner = owners(:blackbeard)
+ pet = pets(:parrot)
+
+ owner.update_column :updated_at, Time.current
+ key = owner.cache_key
- key = car.cache_key
- car.bulb.touch
- car.reload
- assert_not_equal key, car.cache_key
+ assert pet.touch
+ assert_not_equal key, owner.reload.cache_key
end
def test_cache_key_format_for_existing_record_with_nil_updated_timestamps
@@ -328,37 +328,38 @@ def test_schema_migrations_table_name
end
def test_proper_table_name_on_migrator
+ reminder_class = new_isolated_reminder_class
assert_deprecated do
assert_equal "table", ActiveRecord::Migrator.proper_table_name('table')
end
assert_deprecated do
assert_equal "table", ActiveRecord::Migrator.proper_table_name(:table)
end
assert_deprecated do
- assert_equal "reminders", ActiveRecord::Migrator.proper_table_name(Reminder)
+ assert_equal "reminders", ActiveRecord::Migrator.proper_table_name(reminder_class)
end
- Reminder.reset_table_name
+ reminder_class.reset_table_name
assert_deprecated do
- assert_equal Reminder.table_name, ActiveRecord::Migrator.proper_table_name(Reminder)
+ assert_equal reminder_class.table_name, ActiveRecord::Migrator.proper_table_name(reminder_class)
end
# Use the model's own prefix/suffix if a model is given
ActiveRecord::Base.table_name_prefix = "ARprefix_"
ActiveRecord::Base.table_name_suffix = "_ARsuffix"
- Reminder.table_name_prefix = 'prefix_'
- Reminder.table_name_suffix = '_suffix'
- Reminder.reset_table_name
+ reminder_class.table_name_prefix = 'prefix_'
+ reminder_class.table_name_suffix = '_suffix'
+ reminder_class.reset_table_name
assert_deprecated do
- assert_equal "prefix_reminders_suffix", ActiveRecord::Migrator.proper_table_name(Reminder)
+ assert_equal "prefix_reminders_suffix", ActiveRecord::Migrator.proper_table_name(reminder_class)
end
- Reminder.table_name_prefix = ''
- Reminder.table_name_suffix = ''
- Reminder.reset_table_name
+ reminder_class.table_name_prefix = ''
+ reminder_class.table_name_suffix = ''
+ reminder_class.reset_table_name
# Use AR::Base's prefix/suffix if string or symbol is given
ActiveRecord::Base.table_name_prefix = "prefix_"
ActiveRecord::Base.table_name_suffix = "_suffix"
- Reminder.reset_table_name
+ reminder_class.reset_table_name
assert_deprecated do
assert_equal "prefix_table_suffix", ActiveRecord::Migrator.proper_table_name('table')
end
@@ -368,28 +369,29 @@ def test_proper_table_name_on_migrator
end
def test_proper_table_name_on_migration
+ reminder_class = new_isolated_reminder_class
migration = ActiveRecord::Migration.new
assert_equal "table", migration.proper_table_name('table')
assert_equal "table", migration.proper_table_name(:table)
- assert_equal "reminders", migration.proper_table_name(Reminder)
- Reminder.reset_table_name
- assert_equal Reminder.table_name, migration.proper_table_name(Reminder)
+ assert_equal "reminders", migration.proper_table_name(reminder_class)
+ reminder_class.reset_table_name
+ assert_equal reminder_class.table_name, migration.proper_table_name(reminder_class)
# Use the model's own prefix/suffix if a model is given
ActiveRecord::Base.table_name_prefix = "ARprefix_"
ActiveRecord::Base.table_name_suffix = "_ARsuffix"
- Reminder.table_name_prefix = 'prefix_'
- Reminder.table_name_suffix = '_suffix'
- Reminder.reset_table_name
- assert_equal "prefix_reminders_suffix", migration.proper_table_name(Reminder)
- Reminder.table_name_prefix = ''
- Reminder.table_name_suffix = ''
- Reminder.reset_table_name
+ reminder_class.table_name_prefix = 'prefix_'
+ reminder_class.table_name_suffix = '_suffix'
+ reminder_class.reset_table_name
+ assert_equal "prefix_reminders_suffix", migration.proper_table_name(reminder_class)
+ reminder_class.table_name_prefix = ''
+ reminder_class.table_name_suffix = ''
+ reminder_class.reset_table_name
# Use AR::Base's prefix/suffix if string or symbol is given
ActiveRecord::Base.table_name_prefix = "prefix_"
ActiveRecord::Base.table_name_suffix = "_suffix"
- Reminder.reset_table_name
+ reminder_class.reset_table_name
assert_equal "prefix_table_suffix", migration.proper_table_name('table', migration.table_name_options)
assert_equal "prefix_table_suffix", migration.proper_table_name(:table, migration.table_name_options)
end
@@ -532,11 +534,13 @@ def test_out_of_range_limit_should_raise
end
protected
- def with_env_tz(new_tz = 'US/Eastern')
- old_tz, ENV['TZ'] = ENV['TZ'], new_tz
- yield
- ensure
- old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
+ # This is needed to isolate class_attribute assignments like `table_name_prefix`
+ # for each test case.
+ def new_isolated_reminder_class
+ Class.new(Reminder) {
+ def self.name; "Reminder"; end
+ def self.base_class; self; end
+ }
end
end
@@ -8,7 +8,7 @@
class QueryCacheTest < ActiveRecord::TestCase
fixtures :tasks, :topics, :categories, :posts, :categories_posts
- def setup
+ teardown do
Task.connection.clear_query_cache
ActiveRecord::Base.connection.disable_query_cache!
end
@@ -214,7 +214,7 @@ def test_cache_gets_cleared_after_migration
Post.find(1)
# change the column definition
- Post.connection.change_column :posts, :title, :string, :limit => 80
+ Post.connection.change_column :posts, :title, :string, limit: 80
assert_nothing_raised { Post.find(1) }
# restore the old definition
@@ -241,7 +241,6 @@ def test_find
def test_update
Task.connection.expects(:clear_query_cache).times(2)
-
Task.cache do
task = Task.find(1)
task.starting = Time.now.utc
@@ -251,15 +250,13 @@ def test_update
def test_destroy
Task.connection.expects(:clear_query_cache).times(2)
-
Task.cache do
Task.find(1).destroy
end
end
def test_insert
ActiveRecord::Base.connection.expects(:clear_query_cache).times(2)
-
Task.cache do
Task.create!
end
Oops, something went wrong.

0 comments on commit f19ba68

Please sign in to comment.