aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_controller/metal/request_forgery_protection.rb2
-rw-r--r--actionview/lib/action_view/helpers/url_helper.rb3
-rw-r--r--actionview/test/template/url_helper_test.rb7
-rw-r--r--activerecord/CHANGELOG.md13
-rw-r--r--activerecord/Rakefile9
-rw-r--r--activerecord/lib/active_record/callbacks.rb3
-rw-r--r--activerecord/lib/active_record/persistence.rb9
-rw-r--r--activerecord/lib/active_record/validations/uniqueness.rb4
-rw-r--r--activerecord/test/cases/adapters/mysql/quoting_test.rb4
-rw-r--r--activerecord/test/cases/adapters/mysql2/quoting_test.rb4
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb30
-rw-r--r--activerecord/test/cases/validations/uniqueness_validation_test.rb19
-rw-r--r--activesupport/lib/active_support/concurrency/share_lock.rb60
-rw-r--r--activesupport/lib/active_support/dependencies.rb9
-rw-r--r--activesupport/lib/active_support/dependencies/interlock.rb16
-rw-r--r--activesupport/test/share_lock_test.rb333
-rw-r--r--guides/source/credits.html.erb2
-rw-r--r--railties/CHANGELOG.md5
-rw-r--r--railties/lib/rails/application/finisher.rb2
-rw-r--r--railties/lib/rails/generators/rails/plugin/plugin_generator.rb31
-rw-r--r--railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt2
-rw-r--r--railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/engine.rb1
-rw-r--r--railties/test/generators/plugin_generator_test.rb54
23 files changed, 560 insertions, 62 deletions
diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index 4cb634477e..9e09242872 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -20,7 +20,7 @@ module ActionController #:nodoc:
# Since HTML and JavaScript requests are typically made from the browser, we
# need to ensure to verify request authenticity for the web browser. We can
# use session-oriented authentication for these types of requests, by using
- # the `protect_form_forgery` method in our controllers.
+ # the `protect_from_forgery` method in our controllers.
#
# GET requests are not protected since they don't have side effects like writing
# to the database and don't leak sensitive information. JavaScript requests are
diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb
index afb1265ad9..7d92651183 100644
--- a/actionview/lib/action_view/helpers/url_helper.rb
+++ b/actionview/lib/action_view/helpers/url_helper.rb
@@ -468,7 +468,8 @@ module ActionView
}.compact
extras = extras.empty? ? '' : '?' + extras.join('&')
- html_options["href"] = "mailto:#{email_address}#{extras}"
+ encoded_email_address = ERB::Util.url_encode(email_address).gsub("%40", "@")
+ html_options["href"] = "mailto:#{encoded_email_address}#{extras}"
content_tag(:a, name || email_address, html_options, &block)
end
diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb
index 0e35c67516..416d30938a 100644
--- a/actionview/test/template/url_helper_test.rb
+++ b/actionview/test/template/url_helper_test.rb
@@ -500,6 +500,13 @@ class UrlHelperTest < ActiveSupport::TestCase
mail_to("david@loudthinking.com", "David Heinemeier Hansson", class: "admin")
end
+ def test_mail_to_with_special_characters
+ assert_dom_equal(
+ %{<a href="mailto:%23%21%24%25%26%27%2A%2B-%2F%3D%3F%5E_%60%7B%7D%7C%7E@example.org">#!$%&amp;&#39;*+-/=?^_`{}|~@example.org</a>},
+ mail_to("#!$%&'*+-/=?^_`{}|~@example.org")
+ )
+ end
+
def test_mail_with_options
assert_dom_equal(
%{<a href="mailto:me@example.com?cc=ccaddress%40example.com&amp;bcc=bccaddress%40example.com&amp;body=This%20is%20the%20body%20of%20the%20message.&amp;subject=This%20is%20an%20example%20email&amp;reply-to=foo%40bar.com">My email</a>},
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8676f95d6d..4cf54ffeb1 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,16 @@
+* Properly allow uniqueness validations on primary keys.
+
+ Fixes #20966.
+
+ *Sean Griffin & presskey*
+
+* Don't raise an error if an association failed to destroy when `destroy` was
+ called on the parent (as opposed to `destroy!`).
+
+ Fixes #20991.
+
+ *Sean Griffin*
+
* ActiveRecord::RecordNotFound modified to store model name, primary_key and
id of the caller model. It allows the catcher of this exception to make
a better decision to what to do with it. For example consider this simple
diff --git a/activerecord/Rakefile b/activerecord/Rakefile
index a619204e6f..8ea22fd901 100644
--- a/activerecord/Rakefile
+++ b/activerecord/Rakefile
@@ -83,15 +83,6 @@ end
task "isolated_test_#{adapter}" => ["#{adapter}:env", "test:isolated:#{adapter}"]
end
-rule '.sqlite3' do |t|
- sh %Q{sqlite3 "#{t.name}" "create table a (a integer); drop table a;"}
-end
-
-task :test_sqlite3 => [
- 'test/fixtures/fixture_database.sqlite3',
- 'test/fixtures/fixture_database_2.sqlite3'
-]
-
namespace :db do
namespace :mysql do
desc 'Build the MySQL test databases'
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 19f0dca5a6..c7c769b283 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -290,6 +290,9 @@ module ActiveRecord
def destroy #:nodoc:
_run_destroy_callbacks { super }
+ rescue RecordNotDestroyed => e
+ @_association_destroy_exception = e
+ false
end
def touch(*) #:nodoc:
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 0a6e4ac0bd..09c36d7b4d 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -193,7 +193,7 @@ module ActiveRecord
# and #destroy! raises ActiveRecord::RecordNotDestroyed.
# See ActiveRecord::Callbacks for further details.
def destroy!
- destroy || raise(RecordNotDestroyed.new("Failed to destroy the record", self))
+ destroy || _raise_record_not_destroyed
end
# Returns an instance of the specified +klass+ with the attributes of the
@@ -548,5 +548,12 @@ module ActiveRecord
def verify_readonly_attribute(name)
raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name)
end
+
+ def _raise_record_not_destroyed
+ @_association_destroy_exception ||= nil
+ raise @_association_destroy_exception || RecordNotDestroyed.new("Failed to destroy the record", self)
+ ensure
+ @_association_destroy_exception = nil
+ end
end
end
diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb
index 5106f4e127..32d17a1392 100644
--- a/activerecord/lib/active_record/validations/uniqueness.rb
+++ b/activerecord/lib/active_record/validations/uniqueness.rb
@@ -17,7 +17,9 @@ module ActiveRecord
value = map_enum_attribute(finder_class, attribute, value)
relation = build_relation(finder_class, table, attribute, value)
- relation = relation.where.not(finder_class.primary_key => record.id) if record.persisted?
+ if record.persisted? && finder_class.primary_key.to_s != attribute.to_s
+ relation = relation.where.not(finder_class.primary_key => record.id)
+ end
relation = scope_relation(record, table, relation)
relation = relation.merge(options[:conditions]) if options[:conditions]
diff --git a/activerecord/test/cases/adapters/mysql/quoting_test.rb b/activerecord/test/cases/adapters/mysql/quoting_test.rb
index ca476947fc..426b088e2f 100644
--- a/activerecord/test/cases/adapters/mysql/quoting_test.rb
+++ b/activerecord/test/cases/adapters/mysql/quoting_test.rb
@@ -15,14 +15,14 @@ class MysqlQuotingTest < ActiveRecord::MysqlTestCase
def test_quoted_date_precision_for_gte_564
@conn.stubs(:full_version).returns('5.6.4')
- @conn.remove_instance_variable(:@version)
+ @conn.remove_instance_variable(:@version) if @conn.instance_variable_defined?(:@version)
t = Time.now.change(usec: 1)
assert_match(/\.000001\z/, @conn.quoted_date(t))
end
def test_quoted_date_precision_for_lt_564
@conn.stubs(:full_version).returns('5.6.3')
- @conn.remove_instance_variable(:@version)
+ @conn.remove_instance_variable(:@version) if @conn.instance_variable_defined?(:@version)
t = Time.now.change(usec: 1)
refute_match(/\.000001\z/, @conn.quoted_date(t))
end
diff --git a/activerecord/test/cases/adapters/mysql2/quoting_test.rb b/activerecord/test/cases/adapters/mysql2/quoting_test.rb
index a49cbba4b4..92221e61bf 100644
--- a/activerecord/test/cases/adapters/mysql2/quoting_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/quoting_test.rb
@@ -7,14 +7,14 @@ class Mysql2QuotingTest < ActiveRecord::Mysql2TestCase
test 'quoted date precision for gte 5.6.4' do
@connection.stubs(:full_version).returns('5.6.4')
- @connection.remove_instance_variable(:@version)
+ @connection.remove_instance_variable(:@version) if @connection.instance_variable_defined?(:@version)
t = Time.now.change(usec: 1)
assert_match(/\.000001\z/, @connection.quoted_date(t))
end
test 'quoted date precision for lt 5.6.4' do
@connection.stubs(:full_version).returns('5.6.3')
- @connection.remove_instance_variable(:@version)
+ @connection.remove_instance_variable(:@version) if @connection.instance_variable_defined?(:@version)
t = Time.now.change(usec: 1)
refute_match(/\.000001\z/, @connection.quoted_date(t))
end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index c2ef59d0f7..21e2ee3b11 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2278,4 +2278,34 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_deprecated { company.clients_of_firm(true) }
end
+
+ class AuthorWithErrorDestroyingAssociation < ActiveRecord::Base
+ self.table_name = "authors"
+ has_many :posts_with_error_destroying,
+ class_name: "PostWithErrorDestroying",
+ foreign_key: :author_id,
+ dependent: :destroy
+ end
+
+ class PostWithErrorDestroying < ActiveRecord::Base
+ self.table_name = "posts"
+ self.inheritance_column = nil
+ before_destroy -> { throw :abort }
+ end
+
+ def test_destroy_does_not_raise_when_association_errors_on_destroy
+ assert_no_difference "AuthorWithErrorDestroyingAssociation.count" do
+ author = AuthorWithErrorDestroyingAssociation.first
+
+ assert_not author.destroy
+ end
+ end
+
+ def test_destroy_with_bang_bubbles_errors_from_associations
+ error = assert_raises ActiveRecord::RecordNotDestroyed do
+ AuthorWithErrorDestroyingAssociation.first.destroy!
+ end
+
+ assert_instance_of PostWithErrorDestroying, error.record
+ end
end
diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb
index 2608c84be2..ceca2c8366 100644
--- a/activerecord/test/cases/validations/uniqueness_validation_test.rb
+++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb
@@ -427,4 +427,23 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert reply.valid?
assert topic.valid?
end
+
+ def test_validate_uniqueness_of_custom_primary_key
+ klass = Class.new(ActiveRecord::Base) do
+ self.table_name = "keyboards"
+ self.primary_key = :key_number
+
+ validates_uniqueness_of :key_number
+
+ def self.name
+ "Keyboard"
+ end
+ end
+
+ klass.create!(key_number: 10)
+ key2 = klass.create!(key_number: 11)
+
+ key2.key_number = 10
+ assert_not key2.valid?
+ end
end
diff --git a/activesupport/lib/active_support/concurrency/share_lock.rb b/activesupport/lib/active_support/concurrency/share_lock.rb
index f1c6230084..ca48164c54 100644
--- a/activesupport/lib/active_support/concurrency/share_lock.rb
+++ b/activesupport/lib/active_support/concurrency/share_lock.rb
@@ -9,7 +9,7 @@ module ActiveSupport
#--
# Note that a pending Exclusive lock attempt does not block incoming
# Share requests (i.e., we are "read-preferring"). That seems
- # consistent with the behavior of +loose_upgrades+, but may be the
+ # consistent with the behavior of "loose" upgrades, but may be the
# wrong choice otherwise: it nominally reduces the possibility of
# deadlock by risking starvation instead.
class ShareLock
@@ -20,47 +20,48 @@ module ActiveSupport
# to upgrade share locks to exclusive.
- # If +loose_upgrades+ is false (the default), then a thread that
- # is waiting on an Exclusive lock will continue to hold any Share
- # lock that it has already established. This is safer, but can
- # lead to deadlock.
- #
- # If +loose_upgrades+ is true, a thread waiting on an Exclusive
- # lock will temporarily relinquish its Share lock. Being less
- # strict, this behavior prevents some classes of deadlocks. For
- # many resources, loose upgrades are sufficient: if a thread is
- # awaiting a lock, it is not running any other code.
- attr_reader :loose_upgrades
-
- def initialize(loose_upgrades = false)
- @loose_upgrades = loose_upgrades
-
+ def initialize
super()
@cv = new_cond
@sharing = Hash.new(0)
+ @waiting = {}
@exclusive_thread = nil
@exclusive_depth = 0
end
- # Returns false if +no_wait+ is specified and the lock is not
+ # Returns false if +no_wait+ is set and the lock is not
# immediately available. Otherwise, returns true after the lock
# has been acquired.
- def start_exclusive(no_wait=false)
+ #
+ # +purpose+ and +compatible+ work together; while this thread is
+ # waiting for the exclusive lock, it will yield its share (if any)
+ # to any other attempt whose +purpose+ appears in this attempt's
+ # +compatible+ list. This allows a "loose" upgrade, which, being
+ # less strict, prevents some classes of deadlocks.
+ #
+ # For many resources, loose upgrades are sufficient: if a thread
+ # is awaiting a lock, it is not running any other code. With
+ # +purpose+ matching, it is possible to yield only to other
+ # threads whose activity will not interfere.
+ def start_exclusive(purpose: nil, compatible: [], no_wait: false)
synchronize do
unless @exclusive_thread == Thread.current
- return false if no_wait && busy?
+ if busy?(purpose)
+ return false if no_wait
- loose_shares = nil
- if @loose_upgrades
loose_shares = @sharing.delete(Thread.current)
+ @waiting[Thread.current] = compatible if loose_shares
+
+ begin
+ @cv.wait_while { busy?(purpose) }
+ ensure
+ @waiting.delete Thread.current
+ @sharing[Thread.current] = loose_shares if loose_shares
+ end
end
-
- @cv.wait_while { busy? } if busy?
-
@exclusive_thread = Thread.current
- @sharing[Thread.current] = loose_shares if loose_shares
end
@exclusive_depth += 1
@@ -106,8 +107,10 @@ module ActiveSupport
# +no_wait+ is set and the lock is not immediately available,
# returns +nil+ without yielding. Otherwise, returns the result of
# the block.
- def exclusive(no_wait=false)
- if start_exclusive(no_wait)
+ #
+ # See +start_exclusive+ for other options.
+ def exclusive(purpose: nil, compatible: [], no_wait: false)
+ if start_exclusive(purpose: purpose, compatible: compatible, no_wait: no_wait)
begin
yield
ensure
@@ -129,8 +132,9 @@ module ActiveSupport
private
# Must be called within synchronize
- def busy?
+ def busy?(purpose)
(@exclusive_thread && @exclusive_thread != Thread.current) ||
+ @waiting.any? { |k, v| k != Thread.current && !v.include?(purpose) } ||
@sharing.size > (@sharing[Thread.current] > 0 ? 1 : 0)
end
end
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index be03f65e6e..f76ef04f49 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -37,6 +37,13 @@ module ActiveSupport #:nodoc:
Dependencies.interlock.loading { yield }
end
+ # Execute the supplied block while holding an exclusive lock,
+ # preventing any other thread from being inside a #run_interlock
+ # block at the same time
+ def self.unload_interlock
+ Dependencies.interlock.unloading { yield }
+ end
+
# :nodoc:
# Should we turn on Ruby warnings on the first load of dependent files?
@@ -346,7 +353,7 @@ module ActiveSupport #:nodoc:
def clear
log_call
- Dependencies.load_interlock do
+ Dependencies.unload_interlock do
loaded.clear
loading.clear
remove_unloadable_constants!
diff --git a/activesupport/lib/active_support/dependencies/interlock.rb b/activesupport/lib/active_support/dependencies/interlock.rb
index 148212c951..fbeb904684 100644
--- a/activesupport/lib/active_support/dependencies/interlock.rb
+++ b/activesupport/lib/active_support/dependencies/interlock.rb
@@ -4,21 +4,27 @@ module ActiveSupport #:nodoc:
module Dependencies #:nodoc:
class Interlock
def initialize # :nodoc:
- @lock = ActiveSupport::Concurrency::ShareLock.new(true)
+ @lock = ActiveSupport::Concurrency::ShareLock.new
end
def loading
- @lock.exclusive do
+ @lock.exclusive(purpose: :load, compatible: [:load]) do
yield
end
end
- # Attempt to obtain a "loading" (exclusive) lock. If possible,
+ def unloading
+ @lock.exclusive(purpose: :unload, compatible: [:load, :unload]) do
+ yield
+ end
+ end
+
+ # Attempt to obtain an "unloading" (exclusive) lock. If possible,
# execute the supplied block while holding the lock. If there is
# concurrent activity, return immediately (without executing the
# block) instead of waiting.
- def attempt_loading
- @lock.exclusive(true) do
+ def attempt_unloading
+ @lock.exclusive(purpose: :unload, compatible: [:load, :unload], no_wait: true) do
yield
end
end
diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb
new file mode 100644
index 0000000000..ad41db608b
--- /dev/null
+++ b/activesupport/test/share_lock_test.rb
@@ -0,0 +1,333 @@
+require 'abstract_unit'
+require 'concurrent/atomics'
+require 'active_support/concurrency/share_lock'
+
+class ShareLockTest < ActiveSupport::TestCase
+ def setup
+ @lock = ActiveSupport::Concurrency::ShareLock.new
+ end
+
+ def test_reentrancy
+ thread = Thread.new do
+ @lock.sharing { @lock.sharing {} }
+ @lock.exclusive { @lock.exclusive {} }
+ end
+ assert_threads_not_stuck thread
+ end
+
+ def test_sharing_doesnt_block
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_latch|
+ assert_threads_not_stuck(Thread.new {@lock.sharing {} })
+ end
+ end
+
+ def test_sharing_blocks_exclusive
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ @lock.exclusive(no_wait: true) { flunk } # polling should fail
+ exclusive_thread = Thread.new { @lock.exclusive {} }
+ assert_threads_stuck_but_releasable_by_latch exclusive_thread, sharing_thread_release_latch
+ end
+ end
+
+ def test_exclusive_blocks_sharing
+ with_thread_waiting_in_lock_section(:exclusive) do |exclusive_thread_release_latch|
+ sharing_thread = Thread.new { @lock.sharing {} }
+ assert_threads_stuck_but_releasable_by_latch sharing_thread, exclusive_thread_release_latch
+ end
+ end
+
+ def test_multiple_exlusives_are_able_to_progress
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ exclusive_threads = (1..2).map do
+ Thread.new do
+ @lock.exclusive {}
+ end
+ end
+
+ assert_threads_stuck_but_releasable_by_latch exclusive_threads, sharing_thread_release_latch
+ end
+ end
+
+ def test_sharing_is_upgradeable_to_exclusive
+ upgrading_thread = Thread.new do
+ @lock.sharing do
+ @lock.exclusive {}
+ end
+ end
+ assert_threads_not_stuck upgrading_thread
+ end
+
+ def test_exclusive_upgrade_waits_for_other_sharers_to_leave
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ in_sharing = Concurrent::CountDownLatch.new
+
+ upgrading_thread = Thread.new do
+ @lock.sharing do
+ in_sharing.count_down
+ @lock.exclusive {}
+ end
+ end
+
+ in_sharing.wait
+ assert_threads_stuck_but_releasable_by_latch upgrading_thread, sharing_thread_release_latch
+ end
+ end
+
+ def test_exclusive_matching_purpose
+ [true, false].each do |use_upgrading|
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ exclusive_threads = (1..2).map do
+ Thread.new do
+ @lock.send(use_upgrading ? :sharing : :tap) do
+ @lock.exclusive(purpose: :load, compatible: [:load, :unload]) {}
+ end
+ end
+ end
+
+ assert_threads_stuck_but_releasable_by_latch exclusive_threads, sharing_thread_release_latch
+ end
+ end
+ end
+
+ def test_killed_thread_loses_lock
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ thread = Thread.new do
+ @lock.sharing do
+ @lock.exclusive {}
+ end
+ end
+
+ assert_threads_stuck thread
+ thread.kill
+
+ sharing_thread_release_latch.count_down
+
+ thread = Thread.new do
+ @lock.exclusive {}
+ end
+
+ assert_threads_not_stuck thread
+ end
+ end
+
+ def test_exclusive_conflicting_purpose
+ [true, false].each do |use_upgrading|
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ begin
+ conflicting_exclusive_threads = [
+ Thread.new do
+ @lock.send(use_upgrading ? :sharing : :tap) do
+ @lock.exclusive(purpose: :red, compatible: [:green, :purple]) {}
+ end
+ end,
+ Thread.new do
+ @lock.send(use_upgrading ? :sharing : :tap) do
+ @lock.exclusive(purpose: :blue, compatible: [:green]) {}
+ end
+ end
+ ]
+
+ assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks
+
+ # This thread will be stuck as long as any other thread is in
+ # a sharing block. While it's blocked, it holds no lock, so it
+ # doesn't interfere with any other attempts.
+ no_purpose_thread = Thread.new do
+ @lock.exclusive {}
+ end
+ assert_threads_stuck no_purpose_thread
+
+ # This thread is compatible with both of the "primary"
+ # attempts above. It's initially stuck on the outer share
+ # lock, but as soon as that's released, it can run --
+ # regardless of whether those threads hold share locks.
+ compatible_thread = Thread.new do
+ @lock.exclusive(purpose: :green, compatible: []) {}
+ end
+ assert_threads_stuck compatible_thread
+
+ assert_threads_stuck conflicting_exclusive_threads
+
+ sharing_thread_release_latch.count_down
+
+ assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through
+
+ if use_upgrading
+ # The "primary" threads both each hold a share lock, and are
+ # mutually incompatible; they're still stuck.
+ assert_threads_stuck conflicting_exclusive_threads
+
+ # The thread without a specified purpose is also stuck; it's
+ # not compatible with anything.
+ assert_threads_stuck no_purpose_thread
+ else
+ # As the primaries didn't hold a share lock, as soon as the
+ # outer one was released, all the exclusive locks are free
+ # to be acquired in turn.
+
+ assert_threads_not_stuck conflicting_exclusive_threads
+ assert_threads_not_stuck no_purpose_thread
+ end
+ ensure
+ conflicting_exclusive_threads.each(&:kill)
+ no_purpose_thread.kill
+ end
+ end
+ end
+ end
+
+ def test_exclusive_ordering
+ scratch_pad = []
+ scratch_pad_mutex = Mutex.new
+
+ load_params = [:load, [:load]]
+ unload_params = [:unload, [:unload, :load]]
+
+ [load_params, load_params, unload_params, unload_params].permutation do |thread_params|
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ threads = thread_params.map do |purpose, compatible|
+ Thread.new do
+ @lock.sharing do
+ @lock.exclusive(purpose: purpose, compatible: compatible) do
+ scratch_pad_mutex.synchronize { scratch_pad << purpose }
+ end
+ end
+ end
+ end
+
+ sleep(0.01)
+ scratch_pad_mutex.synchronize { assert_empty scratch_pad }
+
+ sharing_thread_release_latch.count_down
+
+ assert_threads_not_stuck threads
+ scratch_pad_mutex.synchronize do
+ assert_equal [:load, :load, :unload, :unload], scratch_pad
+ scratch_pad.clear
+ end
+ end
+ end
+ end
+
+ def test_in_shared_section_incompatible_non_upgrading_threads_cannot_preempt_upgrading_threads
+ scratch_pad = []
+ scratch_pad_mutex = Mutex.new
+
+ upgrading_load_params = [:load, [:load], true]
+ non_upgrading_unload_params = [:unload, [:load, :unload], false]
+
+ [upgrading_load_params, non_upgrading_unload_params].permutation do |thread_params|
+ with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
+ threads = thread_params.map do |purpose, compatible, use_upgrading|
+ Thread.new do
+ @lock.send(use_upgrading ? :sharing : :tap) do
+ @lock.exclusive(purpose: purpose, compatible: compatible) do
+ scratch_pad_mutex.synchronize { scratch_pad << purpose }
+ end
+ end
+ end
+ end
+
+ assert_threads_stuck threads
+ scratch_pad_mutex.synchronize { assert_empty scratch_pad }
+
+ sharing_thread_release_latch.count_down
+
+ assert_threads_not_stuck threads
+ scratch_pad_mutex.synchronize do
+ assert_equal [:load, :unload], scratch_pad
+ scratch_pad.clear
+ end
+ end
+ end
+ end
+
+ private
+
+ module CustomAssertions
+ SUFFICIENT_TIMEOUT = 0.2
+
+ private
+
+ def assert_threads_stuck_but_releasable_by_latch(threads, latch)
+ assert_threads_stuck threads
+ latch.count_down
+ assert_threads_not_stuck threads
+ end
+
+ def assert_threads_stuck(threads)
+ sleep(SUFFICIENT_TIMEOUT) # give threads time to do their business
+ assert(Array(threads).all? { |t| t.join(0.001).nil? })
+ end
+
+ def assert_threads_not_stuck(threads)
+ assert(Array(threads).all? { |t| t.join(SUFFICIENT_TIMEOUT) })
+ end
+ end
+
+ class CustomAssertionsTest < ActiveSupport::TestCase
+ include CustomAssertions
+
+ def setup
+ @latch = Concurrent::CountDownLatch.new
+ @thread = Thread.new { @latch.wait }
+ end
+
+ def teardown
+ @latch.count_down
+ @thread.join
+ end
+
+ def test_happy_path
+ assert_threads_stuck_but_releasable_by_latch @thread, @latch
+ end
+
+ def test_detects_stuck_thread
+ assert_raises(Minitest::Assertion) do
+ assert_threads_not_stuck @thread
+ end
+ end
+
+ def test_detects_free_thread
+ @latch.count_down
+ assert_raises(Minitest::Assertion) do
+ assert_threads_stuck @thread
+ end
+ end
+
+ def test_detects_already_released
+ @latch.count_down
+ assert_raises(Minitest::Assertion) do
+ assert_threads_stuck_but_releasable_by_latch @thread, @latch
+ end
+ end
+
+ def test_detects_remains_latched
+ another_latch = Concurrent::CountDownLatch.new
+ assert_raises(Minitest::Assertion) do
+ assert_threads_stuck_but_releasable_by_latch @thread, another_latch
+ end
+ end
+ end
+
+ include CustomAssertions
+
+ def with_thread_waiting_in_lock_section(lock_section)
+ in_section = Concurrent::CountDownLatch.new
+ section_release = Concurrent::CountDownLatch.new
+
+ stuck_thread = Thread.new do
+ @lock.send(lock_section) do
+ in_section.count_down
+ section_release.wait
+ end
+ end
+
+ in_section.wait
+
+ yield section_release
+ ensure
+ section_release.count_down
+ stuck_thread.join # clean up
+ end
+end
diff --git a/guides/source/credits.html.erb b/guides/source/credits.html.erb
index 61ea0b44ef..1d995581fa 100644
--- a/guides/source/credits.html.erb
+++ b/guides/source/credits.html.erb
@@ -28,7 +28,7 @@ Ruby on Rails Guides: Credits
<h3 class="section">Rails Guides Authors</h3>
<%= author('Ryan Bigg', 'radar', 'radar.png') do %>
- Ryan Bigg works as the Community Manager at <a href="http://spreecommerce.com">Spree Commerce</a> and has been working with Rails since 2006. He's the author of <a href="https://leanpub.com/multi-tenancy-rails">Multi Tenancy With Rails</a> and co-author of <a href="http://manning.com/bigg2">Rails 4 in Action</a>. He's written many gems which can be seen on <a href="https://github.com/radar">his GitHub page</a> and he also tweets prolifically as <a href="http://twitter.com/ryanbigg">@ryanbigg</a>.
+ Ryan Bigg works as a Rails developer at <a href="http://marketplacer.com">Marketplacer</a> and has been working with Rails since 2006. He's the author of <a href="https://leanpub.com/multi-tenancy-rails">Multi Tenancy With Rails</a> and co-author of <a href="http://manning.com/bigg2">Rails 4 in Action</a>. He's written many gems which can be seen on <a href="https://github.com/radar">his GitHub page</a> and he also tweets prolifically as <a href="http://twitter.com/ryanbigg">@ryanbigg</a>.
<% end %>
<%= author('Oscar Del Ben', 'oscardelben', 'oscardelben.jpg') do %>
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index 573b1f8f69..30449aa670 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Add a `--api` option in order to generate plugins that can be added
+ inside an API application.
+
+ *Robin Dupret*
+
* Fix `NoMethodError` when generating a scaffold inside a full engine.
*Yuji Yaginuma*
diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb
index f8f92792a7..404e3c3e23 100644
--- a/railties/lib/rails/application/finisher.rb
+++ b/railties/lib/rails/application/finisher.rb
@@ -86,7 +86,7 @@ module Rails
# added in the hook are taken into account.
initializer :set_clear_dependencies_hook, group: :all do
callback = lambda do
- ActiveSupport::Dependencies.interlock.attempt_loading do
+ ActiveSupport::Dependencies.interlock.attempt_unloading do
ActiveSupport::DescendantsTracker.clear
ActiveSupport::Dependencies.clear
end
diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
index 66111004aa..0e1326ce4f 100644
--- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
+++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb
@@ -17,15 +17,22 @@ module Rails
def app
if mountable?
- directory 'app'
- empty_directory_with_keep_file "app/assets/images/#{namespaced_name}"
+ if api?
+ directory 'app', exclude_pattern: %r{app/(views|helpers)}
+ else
+ directory 'app'
+ empty_directory_with_keep_file "app/assets/images/#{namespaced_name}"
+ end
elsif full?
empty_directory_with_keep_file 'app/models'
empty_directory_with_keep_file 'app/controllers'
- empty_directory_with_keep_file 'app/views'
- empty_directory_with_keep_file 'app/helpers'
empty_directory_with_keep_file 'app/mailers'
- empty_directory_with_keep_file "app/assets/images/#{namespaced_name}"
+
+ unless api?
+ empty_directory_with_keep_file "app/assets/images/#{namespaced_name}"
+ empty_directory_with_keep_file 'app/helpers'
+ empty_directory_with_keep_file 'app/views'
+ end
end
end
@@ -82,6 +89,7 @@ task default: :test
opts = (options || {}).slice(*PASSTHROUGH_OPTIONS)
opts[:force] = force
opts[:skip_bundle] = true
+ opts[:api] = options.api?
invoke Rails::Generators::AppGenerator,
[ File.expand_path(dummy_path, destination_root) ], opts
@@ -176,6 +184,9 @@ task default: :test
desc: "If creating plugin in application's directory " +
"skip adding entry to Gemfile"
+ class_option :api, type: :boolean, default: false,
+ desc: "Generate a smaller stack for API application plugins"
+
def initialize(*args)
@dummy_path = nil
super
@@ -210,15 +221,15 @@ task default: :test
end
def create_public_stylesheets_files
- build(:stylesheets)
+ build(:stylesheets) unless api?
end
def create_javascript_files
- build(:javascripts)
+ build(:javascripts) unless api?
end
def create_images_directory
- build(:images)
+ build(:images) unless api?
end
def create_bin_files
@@ -305,6 +316,10 @@ task default: :test
options[:skip_test].blank? || options[:dummy_path] != 'test/dummy'
end
+ def api?
+ options[:api]
+ end
+
def self.banner
"rails plugin new #{self.arguments.map(&:usage).join(' ')} [options]"
end
diff --git a/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt b/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt
index 7157e48c42..7fe4e5034d 100644
--- a/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt
+++ b/railties/lib/rails/generators/rails/plugin/templates/app/controllers/%namespaced_name%/application_controller.rb.tt
@@ -1,5 +1,5 @@
<%= wrap_in_modules <<-rb.strip_heredoc
- class ApplicationController < ActionController::Base
+ class ApplicationController < ActionController::#{api? ? "API" : "Base"}
end
rb
%>
diff --git a/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/engine.rb b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/engine.rb
index 17afd52177..8938770fc4 100644
--- a/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/engine.rb
+++ b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/engine.rb
@@ -1,6 +1,7 @@
<%= wrap_in_modules <<-rb.strip_heredoc
class Engine < ::Rails::Engine
#{mountable? ? ' isolate_namespace ' + camelized_modules : ' '}
+ #{api? ? " config.generators.api_only = true" : ' '}
end
rb
%>
diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb
index 73e68863f4..6f5a815173 100644
--- a/railties/test/generators/plugin_generator_test.rb
+++ b/railties/test/generators/plugin_generator_test.rb
@@ -544,6 +544,60 @@ class PluginGeneratorTest < Rails::Generators::TestCase
end
end
+ def test_skipping_useless_folders_generation_for_api_engines
+ ['--full', '--mountable'].each do |option|
+ run_generator [destination_root, option, '--api']
+
+ assert_no_directory "app/assets"
+ assert_no_directory "app/helpers"
+ assert_no_directory "app/views"
+
+ FileUtils.rm_rf destination_root
+ end
+ end
+
+ def test_application_controller_parent_for_mountable_api_plugins
+ run_generator [destination_root, '--mountable', '--api']
+
+ assert_file "app/controllers/bukkits/application_controller.rb" do |content|
+ assert_match "ApplicationController < ActionController::API", content
+ end
+ end
+
+ def test_dummy_api_application_for_api_plugins
+ run_generator [destination_root, '--api']
+
+ assert_file "test/dummy/config/application.rb" do |content|
+ assert_match "config.api_only = true", content
+ end
+ end
+
+
+ def test_api_generators_configuration_for_api_engines
+ run_generator [destination_root, '--full', '--api']
+
+ assert_file "lib/bukkits/engine.rb" do |content|
+ assert_match "config.generators.api_only = true", content
+ end
+ end
+
+ def test_scaffold_generator_for_mountable_api_plugins
+ run_generator [destination_root, '--mountable', '--api']
+
+ capture(:stdout) do
+ `#{destination_root}/bin/rails g scaffold article`
+ end
+
+ assert_file "app/models/bukkits/article.rb"
+ assert_file "app/controllers/bukkits/articles_controller.rb" do |content|
+ assert_match "only: [:show, :update, :destroy]", content
+ end
+
+ assert_no_directory "app/assets"
+ assert_no_directory "app/helpers"
+ assert_no_directory "app/views"
+ end
+
protected
def action(*args, &block)
silence(:stdout){ generator.send(*args, &block) }