diff options
23 files changed, 405 insertions, 201 deletions
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 8a9d7982d3..e059f37d38 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -4,10 +4,6 @@ require "abstract_unit" require "active_model" require "controller/fake_models" -class ApplicationController < ActionController::Base - self.view_paths = File.join(FIXTURE_LOAD_PATH, "actionpack") -end - module Quiz # Models Question = Struct.new(:name, :id) do @@ -20,7 +16,7 @@ module Quiz end # Controller - class QuestionsController < ApplicationController + class QuestionsController < ActionController::Base def new render partial: Quiz::Question.new("Namespaced Partial") end @@ -28,7 +24,7 @@ module Quiz end module Fun - class GamesController < ApplicationController + class GamesController < ActionController::Base def hello_world; end def nested_partial_with_form_builder @@ -37,7 +33,7 @@ module Fun end end -class TestController < ApplicationController +class TestController < ActionController::Base protect_from_forgery before_action :set_variable_for_layout @@ -640,10 +636,15 @@ class RenderTest < ActionController::TestCase ActionView::Base.logger = ActiveSupport::Logger.new(nil) @request.host = "www.nextangle.com" + + @old_view_paths = ActionController::Base.view_paths + ActionController::Base.view_paths = File.join(FIXTURE_LOAD_PATH, "actionpack") end def teardown ActionView::Base.logger = nil + + ActionController::Base.view_paths = @old_view_paths end # :ported: diff --git a/actionview/test/template/date_helper_test.rb b/actionview/test/template/date_helper_test.rb index 94357d5f90..4b4939d705 100644 --- a/actionview/test/template/date_helper_test.rb +++ b/actionview/test/template/date_helper_test.rb @@ -141,10 +141,10 @@ class DateHelperTest < ActionView::TestCase end def test_distance_in_words_doesnt_use_the_quotient_operator - rubinius_skip "Date is written in Ruby and relies on Fixnum#/" - jruby_skip "Date is written in Ruby and relies on Fixnum#/" + rubinius_skip "Date is written in Ruby and relies on Integer#/" + jruby_skip "Date is written in Ruby and relies on Integer#/" - # Make sure that we avoid {Integer,Fixnum}#/ (redefined by mathn) + # Make sure that we avoid Integer#/ (redefined by mathn) Integer.send :private, :/ from = Time.utc(2004, 6, 6, 21, 45, 0) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 842f407517..0ca0e5dcdb 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -438,24 +438,18 @@ module ActiveRecord defined?(@attributes) && @attributes.key?(attr_name) end - def arel_attributes_with_values_for_create(attribute_names) - arel_attributes_with_values(attributes_for_create(attribute_names)) + def attributes_with_values_for_create(attribute_names) + attributes_with_values(attributes_for_create(attribute_names)) end - def arel_attributes_with_values_for_update(attribute_names) - arel_attributes_with_values(attributes_for_update(attribute_names)) + def attributes_with_values_for_update(attribute_names) + attributes_with_values(attributes_for_update(attribute_names)) end - # Returns a Hash of the Arel::Attributes and attribute values that have been - # typecasted for use in an Arel insert/update method. - def arel_attributes_with_values(attribute_names) - attrs = {} - arel_table = self.class.arel_table - - attribute_names.each do |name| - attrs[arel_table[name]] = _read_attribute(name) + def attributes_with_values(attribute_names) + attribute_names.each_with_object({}) do |name, attrs| + attrs[name] = _read_attribute(name) end - attrs end # Filters the primary keys and readonly attributes from the attribute names. diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index bf17f27e68..af361a8d2d 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -91,15 +91,10 @@ module ActiveRecord attribute_names.push(lock_col) - relation = self.class.unscoped - - affected_rows = relation.where( - self.class.primary_key => id, + affected_rows = self.class._update_record( + attributes_with_values_for_update(attribute_names), + self.class.primary_key => id_in_database, lock_col => previous_lock_value - ).update_all( - attributes_for_update(attribute_names).map do |name| - [name, _read_attribute(name)] - end.to_h ) unless affected_rows == 1 @@ -116,25 +111,52 @@ module ActiveRecord end end - def destroy_row - affected_rows = super + def _update_row(attribute_names, attempted_action) + return super unless locking_enabled? - if locking_enabled? && affected_rows != 1 - raise ActiveRecord::StaleObjectError.new(self, "destroy") - end + begin + locking_column = self.class.locking_column + previous_lock_value = read_attribute_before_type_cast(locking_column) + attribute_names << locking_column - affected_rows + self[locking_column] += 1 + + affected_rows = self.class._update_record( + attributes_with_values(attribute_names), + self.class.primary_key => id_in_database, + locking_column => previous_lock_value + ) + + if affected_rows != 1 + raise ActiveRecord::StaleObjectError.new(self, attempted_action) + end + + affected_rows + + # If something went wrong, revert the locking_column value. + rescue Exception + self[locking_column] = previous_lock_value + raise + ensure + clear_attribute_change(locking_column) + end end - def relation_for_destroy - relation = super + def destroy_row + return super unless locking_enabled? - if locking_enabled? - locking_column = self.class.locking_column - relation = relation.where(locking_column => read_attribute_before_type_cast(locking_column)) + locking_column = self.class.locking_column + + affected_rows = self.class._delete_record( + self.class.primary_key => id_in_database, + locking_column => read_attribute_before_type_cast(locking_column) + ) + + if affected_rows != 1 + raise ActiveRecord::StaleObjectError.new(self, "destroy") end - relation + affected_rows end module ClassMethods diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index c9f929680b..ce16587326 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -169,12 +169,11 @@ module ActiveRecord primary_key_value = nil if primary_key && Hash === values - arel_primary_key = arel_attribute(primary_key) - primary_key_value = values[arel_primary_key] + primary_key_value = values[primary_key] if !primary_key_value && prefetch_primary_key? primary_key_value = next_sequence_value - values[arel_primary_key] = primary_key_value + values[primary_key] = primary_key_value end end @@ -188,15 +187,26 @@ module ActiveRecord connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) end - def _update_record(values, id) # :nodoc: - bind = predicate_builder.build_bind_attribute(primary_key, id) + def _update_record(values, constraints) # :nodoc: + constraints = _substitute_values(constraints).map { |attr, bind| attr.eq(bind) } + um = arel_table.where( - arel_attribute(primary_key).eq(bind) + constraints.reduce(&:and) ).compile_update(_substitute_values(values), primary_key) connection.update(um, "#{self} Update") end + def _delete_record(constraints) # :nodoc: + constraints = _substitute_values(constraints).map { |attr, bind| attr.eq(bind) } + + dm = Arel::DeleteManager.new + dm.from(arel_table) + dm.wheres = constraints + + connection.delete(dm, "#{self} Destroy") + end + private # Called by +instantiate+ to decide which class to use for a new # record instance. @@ -208,8 +218,9 @@ module ActiveRecord end def _substitute_values(values) - values.map do |attr, value| - bind = predicate_builder.build_bind_attribute(attr.name, value) + values.map do |name, value| + attr = arel_attribute(name) + bind = predicate_builder.build_bind_attribute(name, value) [attr, bind] end end @@ -310,7 +321,7 @@ module ActiveRecord # callbacks or any <tt>:dependent</tt> association # options, use <tt>#destroy</tt>. def delete - _relation_for_itself.delete_all if persisted? + _delete_row if persisted? @destroyed = true freeze end @@ -463,13 +474,16 @@ module ActiveRecord verify_readonly_attribute(key.to_s) end - updated_count = _relation_for_itself.update_all(attributes) + affected_rows = self.class._update_record( + attributes, + self.class.primary_key => id_in_database + ) attributes.each do |k, v| write_attribute_without_type_cast(k, v) end - updated_count == 1 + affected_rows == 1 end # Initializes +attribute+ to zero if +nil+ and adds the value passed as +by+ (default is 1). @@ -643,34 +657,18 @@ module ActiveRecord end time ||= current_time_from_proper_timezone - attributes = timestamp_attributes_for_update_in_model - attributes.concat(names) - - unless attributes.empty? - changes = {} + attribute_names = timestamp_attributes_for_update_in_model + attribute_names.concat(names) - attributes.each do |column| - column = column.to_s - changes[column] = write_attribute(column, time) + unless attribute_names.empty? + attribute_names.each do |attr_name| + write_attribute(attr_name, time) + clear_attribute_change(attr_name) end - scope = _relation_for_itself + affected_rows = _update_row(attribute_names, "touch") - if locking_enabled? - locking_column = self.class.locking_column - scope = scope.where(locking_column => read_attribute_before_type_cast(locking_column)) - changes[locking_column] = increment_lock - end - - clear_attribute_changes(changes.keys) - result = scope.update_all(changes) == 1 - - if !result && locking_enabled? - raise ActiveRecord::StaleObjectError.new(self, "touch") - end - - @_trigger_update_callback = result - result + @_trigger_update_callback = affected_rows == 1 else true end @@ -683,15 +681,18 @@ module ActiveRecord end def destroy_row - relation_for_destroy.delete_all + _delete_row end - def relation_for_destroy - _relation_for_itself + def _delete_row + self.class._delete_record(self.class.primary_key => id_in_database) end - def _relation_for_itself - self.class.unscoped.where(self.class.primary_key => id) + def _update_row(attribute_names, attempted_action) + self.class._update_record( + attributes_with_values(attribute_names), + self.class.primary_key => id_in_database + ) end def create_or_update(*args, &block) @@ -705,25 +706,26 @@ module ActiveRecord # Returns the number of affected rows. def _update_record(attribute_names = self.attribute_names) attribute_names &= self.class.column_names - attributes_values = arel_attributes_with_values_for_update(attribute_names) - if attributes_values.empty? - rows_affected = 0 + attribute_names = attributes_for_update(attribute_names) + + if attribute_names.empty? + affected_rows = 0 @_trigger_update_callback = true else - rows_affected = self.class._update_record(attributes_values, id_in_database) - @_trigger_update_callback = rows_affected > 0 + affected_rows = _update_row(attribute_names, "update") + @_trigger_update_callback = affected_rows == 1 end yield(self) if block_given? - rows_affected + affected_rows end # Creates a record with values matching those of the instance attributes # and returns its id. def _create_record(attribute_names = self.attribute_names) attribute_names &= self.class.column_names - attributes_values = arel_attributes_with_values_for_create(attribute_names) + attributes_values = attributes_with_values_for_create(attribute_names) new_id = self.class._insert_record(attributes_values) self.id ||= new_id if self.class.primary_key diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 02be0aa9e3..9d04750712 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -194,6 +194,45 @@ class OptimisticLockingTest < ActiveRecord::TestCase end end + def test_update_with_dirty_primary_key + assert_raises(ActiveRecord::RecordNotUnique) do + person = Person.find(1) + person.id = 2 + person.save! + end + + person = Person.find(1) + person.id = 42 + person.save! + + assert Person.find(42) + assert_raises(ActiveRecord::RecordNotFound) do + Person.find(1) + end + end + + def test_delete_with_dirty_primary_key + person = Person.find(1) + person.id = 2 + person.delete + + assert Person.find(2) + assert_raises(ActiveRecord::RecordNotFound) do + Person.find(1) + end + end + + def test_destroy_with_dirty_primary_key + person = Person.find(1) + person.id = 2 + person.destroy + + assert Person.find(2) + assert_raises(ActiveRecord::RecordNotFound) do + Person.find(1) + end + end + def test_explicit_update_lock_column_raise_error person = Person.find(1) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index c241ddc807..f77a275544 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -177,7 +177,7 @@ class MigrationTest < ActiveRecord::TestCase assert BigNumber.create( bank_balance: 1586.43, big_bank_balance: BigDecimal("1000234000567.95"), - world_population: 6000000000, + world_population: 2**62, my_house_population: 3, value_of_e: BigDecimal("2.7182818284590452353602875") ) @@ -191,10 +191,8 @@ class MigrationTest < ActiveRecord::TestCase assert_not_nil b.my_house_population assert_not_nil b.value_of_e - # TODO: set world_population >= 2**62 to cover 64-bit platforms and test - # is_a?(Bignum) assert_kind_of Integer, b.world_population - assert_equal 6000000000, b.world_population + assert_equal 2**62, b.world_population assert_kind_of Integer, b.my_house_population assert_equal 3, b.my_house_population assert_kind_of BigDecimal, b.bank_balance diff --git a/activerecord/test/cases/numeric_data_test.rb b/activerecord/test/cases/numeric_data_test.rb index f917c8f727..14db63890e 100644 --- a/activerecord/test/cases/numeric_data_test.rb +++ b/activerecord/test/cases/numeric_data_test.rb @@ -19,7 +19,7 @@ class NumericDataTest < ActiveRecord::TestCase m = NumericData.new( bank_balance: 1586.43, big_bank_balance: BigDecimal("1000234000567.95"), - world_population: 6000000000, + world_population: 2**62, my_house_population: 3 ) assert m.save @@ -27,11 +27,8 @@ class NumericDataTest < ActiveRecord::TestCase m1 = NumericData.find(m.id) assert_not_nil m1 - # As with migration_test.rb, we should make world_population >= 2**62 - # to cover 64-bit platforms and test it is a Bignum, but the main thing - # is that it's an Integer. assert_kind_of Integer, m1.world_population - assert_equal 6000000000, m1.world_population + assert_equal 2**62, m1.world_population assert_kind_of Integer, m1.my_house_population assert_equal 3, m1.my_house_population @@ -47,7 +44,7 @@ class NumericDataTest < ActiveRecord::TestCase m = NumericData.new( bank_balance: 1586.43122334, big_bank_balance: BigDecimal("234000567.952344"), - world_population: 6000000000, + world_population: 2**62, my_house_population: 3 ) assert m.save @@ -55,11 +52,8 @@ class NumericDataTest < ActiveRecord::TestCase m1 = NumericData.find(m.id) assert_not_nil m1 - # As with migration_test.rb, we should make world_population >= 2**62 - # to cover 64-bit platforms and test it is a Bignum, but the main thing - # is that it's an Integer. assert_kind_of Integer, m1.world_population - assert_equal 6000000000, m1.world_population + assert_equal 2**62, m1.world_population assert_kind_of Integer, m1.my_house_population assert_equal 3, m1.my_house_population diff --git a/activerecord/test/migrations/decimal/1_give_me_big_numbers.rb b/activerecord/test/migrations/decimal/1_give_me_big_numbers.rb index b892f50e41..7d4233fe31 100644 --- a/activerecord/test/migrations/decimal/1_give_me_big_numbers.rb +++ b/activerecord/test/migrations/decimal/1_give_me_big_numbers.rb @@ -5,7 +5,7 @@ class GiveMeBigNumbers < ActiveRecord::Migration::Current create_table :big_numbers do |table| table.column :bank_balance, :decimal, precision: 10, scale: 2 table.column :big_bank_balance, :decimal, precision: 15, scale: 2 - table.column :world_population, :decimal, precision: 10 + table.column :world_population, :decimal, precision: 20 table.column :my_house_population, :decimal, precision: 2 table.column :value_of_e, :decimal end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 3f4fe16678..8b0106dbf0 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -547,7 +547,7 @@ ActiveRecord::Schema.define do create_table :numeric_data, force: true do |t| t.decimal :bank_balance, precision: 10, scale: 2 t.decimal :big_bank_balance, precision: 15, scale: 2 - t.decimal :world_population, precision: 10, scale: 0 + t.decimal :world_population, precision: 20, scale: 0 t.decimal :my_house_population, precision: 2, scale: 0 t.decimal :decimal_number_with_default, precision: 3, scale: 2, default: 2.78 t.float :temperature diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index 0244232b2c..008f5a796b 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -64,16 +64,25 @@ module ActiveStorage private def replace(attachable) - blob.tap do + blob_was = blob + blob = create_blob_from(attachable) + + unless blob == blob_was transaction do detach - write_attachment build_attachment_from(attachable) + write_attachment build_attachment(blob: blob) end - end.purge_later + + blob_was.purge_later + end end def build_attachment_from(attachable) - ActiveStorage::Attachment.new(record: record, name: name, blob: create_blob_from(attachable)) + build_attachment blob: create_blob_from(attachable) + end + + def build_attachment(blob:) + ActiveStorage::Attachment.new(record: record, name: name, blob: blob) end def write_attachment(attachment) diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index 9ba2759893..25e0352eca 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -56,6 +56,30 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase assert ActiveStorage::Blob.service.exist?(@user.avatar.key) end + test "replace attached blob with itself" do + @user.avatar.attach create_blob(filename: "funky.jpg") + + assert_no_changes -> { @user.reload.avatar.blob } do + assert_no_changes -> { @user.reload.avatar.attachment } do + assert_no_enqueued_jobs do + @user.avatar.attach @user.avatar.blob + end + end + end + end + + test "replaced attached blob with itself by signed ID" do + @user.avatar.attach create_blob(filename: "funky.jpg") + + assert_no_changes -> { @user.reload.avatar.blob } do + assert_no_changes -> { @user.reload.avatar.attachment } do + assert_no_enqueued_jobs do + @user.avatar.attach @user.avatar.blob.signed_id + end + end + end + end + test "attach blob to new record" do user = User.new(name: "Jason") diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 45d16515d2..ec3497173f 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -114,23 +114,22 @@ class Module # invoice.customer_name # => 'John Doe' # invoice.customer_address # => 'Vimmersvej 13' # - # If you want the delegate methods to be a private, - # use the <tt>:private</tt> option. + # The delegated methods are public by default. + # Pass <tt>private: true</tt> to change that. # # class User < ActiveRecord::Base # has_one :profile # delegate :first_name, to: :profile - # delegate :date_of_birth, :religion, to: :profile, private: true + # delegate :date_of_birth, to: :profile, private: true # # def age # Date.today.year - date_of_birth.year # end # end # - # User.new.age # => 2 # User.new.first_name # => "Tomas" # User.new.date_of_birth # => NoMethodError: private method `date_of_birth' called for #<User:0x00000008221340> - # User.new.religion # => NoMethodError: private method `religion' called for #<User:0x00000008221340> + # User.new.age # => 2 # # If the target is +nil+ and does not respond to the delegated method a # +Module::DelegationError+ is raised. If you wish to instead return +nil+, diff --git a/guides/source/active_record_postgresql.md b/guides/source/active_record_postgresql.md index 58c61f0864..6c6c6a1ded 100644 --- a/guides/source/active_record_postgresql.md +++ b/guides/source/active_record_postgresql.md @@ -84,7 +84,7 @@ Book.where("array_length(ratings, 1) >= 3") ### Hstore * [type definition](https://www.postgresql.org/docs/current/static/hstore.html) -* [functions and operators](https://www.postgresql.org/docs/current/static/hstore.html#AEN179902) +* [functions and operators](https://www.postgresql.org/docs/current/static/hstore.html#id-1.11.7.26.5) NOTE: You need to enable the `hstore` extension to use hstore. @@ -290,7 +290,7 @@ SELECT n.nspname AS enum_schema, ### UUID * [type definition](https://www.postgresql.org/docs/current/static/datatype-uuid.html) -* [pgcrypto generator function](https://www.postgresql.org/docs/current/static/pgcrypto.html#AEN182570) +* [pgcrypto generator function](https://www.postgresql.org/docs/current/static/pgcrypto.html#id-1.11.7.35.7) * [uuid-ossp generator functions](https://www.postgresql.org/docs/current/static/uuid-ossp.html) NOTE: You need to enable the `pgcrypto` (only PostgreSQL >= 9.4) or `uuid-ossp` diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index a38888afe5..335eb6c4e3 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,5 +1,23 @@ ## Rails 6.0.0.alpha (Unreleased) ## +* Deprecate passing Rack server name as a regular argument to `rails server`. + + Previously: + + $ bin/rails server thin + + There wasn't an explicit option for the Rack server to use, now we have the + `--using` option with the `-u` short switch. + + Now: + + $ bin/rails server -u thin + + This change also improves the error message if a missing or mistyped rack + server is given. + + *Genadi Samokovarov* + * Add "rails routes --expanded" option to output routes in expanded mode like "psql --expanded". Result looks like: diff --git a/railties/lib/rails/command.rb b/railties/lib/rails/command.rb index 078e9f937f..6d99ac9936 100644 --- a/railties/lib/rails/command.rb +++ b/railties/lib/rails/command.rb @@ -11,6 +11,7 @@ module Rails module Command extend ActiveSupport::Autoload + autoload :Spellchecker autoload :Behavior autoload :Base diff --git a/railties/lib/rails/command/behavior.rb b/railties/lib/rails/command/behavior.rb index 7a6dd28e1a..718e2d9ab2 100644 --- a/railties/lib/rails/command/behavior.rb +++ b/railties/lib/rails/command/behavior.rb @@ -19,46 +19,6 @@ module Rails end private - - # This code is based directly on the Text gem implementation. - # Copyright (c) 2006-2013 Paul Battley, Michael Neumann, Tim Fletcher. - # - # Returns a value representing the "cost" of transforming str1 into str2. - def levenshtein_distance(str1, str2) # :doc: - s = str1 - t = str2 - n = s.length - m = t.length - - return m if (0 == n) - return n if (0 == m) - - d = (0..m).to_a - x = nil - - # avoid duplicating an enumerable object in the loop - str2_codepoint_enumerable = str2.each_codepoint - - str1.each_codepoint.with_index do |char1, i| - e = i + 1 - - str2_codepoint_enumerable.with_index do |char2, j| - cost = (char1 == char2) ? 0 : 1 - x = [ - d[j + 1] + 1, # insertion - e + 1, # deletion - d[j] + cost # substitution - ].min - d[j] = e - e = x - end - - d[m] = x - end - - x - end - # Prints a list of generators. def print_list(base, namespaces) return if namespaces.empty? diff --git a/railties/lib/rails/command/spellchecker.rb b/railties/lib/rails/command/spellchecker.rb new file mode 100644 index 0000000000..59ccab4ea2 --- /dev/null +++ b/railties/lib/rails/command/spellchecker.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Rails + module Command + module Spellchecker # :nodoc: + class << self + def suggest(word, from:, count: 3) + from.sort_by { |w| levenshtein_distance(word, w) }.take(count) + end + + private + # This code is based directly on the Text gem implementation. + # Copyright (c) 2006-2013 Paul Battley, Michael Neumann, Tim Fletcher. + # + # Returns a value representing the "cost" of transforming str1 into str2. + def levenshtein_distance(str1, str2) # :doc: + s = str1 + t = str2 + n = s.length + m = t.length + + return m if (0 == n) + return n if (0 == m) + + d = (0..m).to_a + x = nil + + # avoid duplicating an enumerable object in the loop + str2_codepoint_enumerable = str2.each_codepoint + + str1.each_codepoint.with_index do |char1, i| + e = i + 1 + + str2_codepoint_enumerable.with_index do |char2, j| + cost = (char1 == char2) ? 0 : 1 + x = [ + d[j + 1] + 1, # insertion + e + 1, # deletion + d[j] + cost # substitution + ].min + d[j] = e + e = x + end + + d[m] = x + end + + x + end + end + end + end +end diff --git a/railties/lib/rails/commands/server/server_command.rb b/railties/lib/rails/commands/server/server_command.rb index e546fe3e4b..64fb912b51 100644 --- a/railties/lib/rails/commands/server/server_command.rb +++ b/railties/lib/rails/commands/server/server_command.rb @@ -44,7 +44,6 @@ module Rails end def start - print_boot_information trap(:INT) { exit } create_tmp_directories setup_dev_caching @@ -52,9 +51,14 @@ module Rails super ensure - # The '-h' option calls exit before @options is set. - # If we call 'options' with it unset, we get double help banners. - puts "Exiting" unless @options && options[:daemonize] + yield + end + + def serveable? # :nodoc: + server + true + rescue LoadError, NameError + false end def middleware @@ -65,6 +69,10 @@ module Rails super.merge(@default_options) end + def served_url + "#{options[:SSLEnable] ? 'https' : 'http'}://#{options[:Host]}:#{options[:Port]}" unless use_puma? + end + private def setup_dev_caching if options[:environment] == "development" @@ -72,13 +80,6 @@ module Rails end end - def print_boot_information - url = "on #{options[:SSLEnable] ? 'https' : 'http'}://#{options[:Host]}:#{options[:Port]}" unless use_puma? - puts "=> Booting #{ActiveSupport::Inflector.demodulize(server)}" - puts "=> Rails #{Rails.version} application starting in #{Rails.env} #{url}" - puts "=> Run `rails server -h` for more startup options" - end - def create_tmp_directories %w(cache pids sockets).each do |dir_to_make| FileUtils.mkdir_p(File.join(Rails.root, "tmp", dir_to_make)) @@ -108,9 +109,15 @@ module Rails module Command class ServerCommand < Base # :nodoc: + # Hard-coding a bunch of handlers here as we don't have a public way of + # querying them from the Rack::Handler registry. + RACK_SERVERS = %w(cgi fastcgi webrick lsws scgi thin puma unicorn) + DEFAULT_PORT = 3000 DEFAULT_PID_PATH = "tmp/pids/server.pid".freeze + argument :using, optional: true + class_option :port, aliases: "-p", type: :numeric, desc: "Runs Rails on the specified port - defaults to 3000.", banner: :port class_option :binding, aliases: "-b", type: :string, @@ -122,6 +129,8 @@ module Rails desc: "Runs server as a Daemon." class_option :environment, aliases: "-e", type: :string, desc: "Specifies the environment to run this server under (development/test/production).", banner: :name + class_option :using, aliases: "-u", type: :string, + desc: "Specifies the Rack server used to run the application (thin/puma/webrick).", banner: :name class_option :pid, aliases: "-P", type: :string, default: DEFAULT_PID_PATH, desc: "Specifies the PID file." class_option "dev-caching", aliases: "-C", type: :boolean, default: nil, @@ -132,19 +141,28 @@ module Rails def initialize(args = [], local_options = {}, config = {}) @original_options = local_options super - @server = self.args.shift + @using = deprecated_positional_rack_server(using) || options[:using] @log_stdout = options[:daemon].blank? && (options[:environment] || Rails.env) == "development" end def perform set_application_directory! prepare_restart + Rails::Server.new(server_options).tap do |server| # Require application after server sets environment to propagate # the --environment option. require APP_PATH Dir.chdir(Rails.application.root) - server.start + + if server.serveable? + print_boot_information(server.server, server.served_url) + server.start do + say "Exiting" unless options[:daemon] + end + else + say rack_server_suggestion(using) + end end end @@ -152,7 +170,7 @@ module Rails def server_options { user_supplied_options: user_supplied_options, - server: @server, + server: using, log_stdout: @log_stdout, Port: port, Host: host, @@ -226,7 +244,7 @@ module Rails end def restart_command - "bin/rails server #{@server} #{@original_options.join(" ")} --restart" + "bin/rails server #{using} #{@original_options.join(" ")} --restart" end def early_hints @@ -238,12 +256,50 @@ module Rails end def self.banner(*) - "rails server [puma, thin etc] [options]" + "rails server [thin/puma/webrick] [options]" end def prepare_restart FileUtils.rm_f(options[:pid]) if options[:restart] end + + def deprecated_positional_rack_server(value) + if value + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing the Rack server name as a regular argument is deprecated + and will be removed in the next Rails version. Please, use the -u + option instead. + MSG + value + end + end + + def rack_server_suggestion(server) + if server.in?(RACK_SERVERS) + <<~MSG + Could not load server "#{server}". Maybe you need to the add it to the Gemfile? + + gem "#{server}" + + Run `rails server --help` for more options. + MSG + else + suggestions = Rails::Command::Spellchecker.suggest(server, from: RACK_SERVERS).map(&:inspect) + + <<~MSG + Could not find server "#{server}". Maybe you meant #{suggestions.first} or #{suggestions.second}? + Run `rails server --help` for more options. + MSG + end + end + + def print_boot_information(server, url) + say <<~MSG + => Booting #{ActiveSupport::Inflector.demodulize(server)} + => Rails #{Rails.version} application starting in #{Rails.env} #{url} + => Run `rails server --help` for more startup options + MSG + end end end end diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 6a7175c02b..7248fbbc94 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -276,7 +276,7 @@ module Rails klass.start(args, config) else options = sorted_groups.flat_map(&:last) - suggestions = options.sort_by { |suggested| levenshtein_distance(namespace.to_s, suggested) }.first(3) + suggestions = Rails::Command::Spellchecker.suggest(namespace.to_s, from: options, count: 3) suggestions.map! { |s| "'#{s}'" } msg = "Could not find generator '#{namespace}'. ".dup msg << "Maybe you meant #{ suggestions[0...-1].join(', ')} or #{suggestions[-1]}\n" diff --git a/railties/test/command/spellchecker_test.rb b/railties/test/command/spellchecker_test.rb new file mode 100644 index 0000000000..aff50a3e73 --- /dev/null +++ b/railties/test/command/spellchecker_test.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "rails/command/spellchecker" + +class Rails::Command::SpellcheckerTest < ActiveSupport::TestCase + test "suggests a word correction from dictionary" do + assert_equal %w(thin cgi puma), Rails::Command::Spellchecker.suggest("tin", from: %w(puma thin cgi)) + end +end diff --git a/railties/test/commands/routes_test.rb b/railties/test/commands/routes_test.rb index b3d85882ab..409a6f3417 100644 --- a/railties/test/commands/routes_test.rb +++ b/railties/test/commands/routes_test.rb @@ -39,12 +39,11 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase output = run_routes_command(["-g", "show"]) assert_equal <<~MESSAGE, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show MESSAGE output = run_routes_command(["-g", "POST"]) @@ -108,13 +107,12 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase RUBY assert_equal <<~MESSAGE, run_routes_command - Prefix Verb URI Pattern Controller#Action - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_blob_preview GET /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) active_storage/previews#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show - update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update - rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + Prefix Verb URI Pattern Controller#Action + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_representation GET /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) active_storage/representations#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE end @@ -138,26 +136,21 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase URI | /rails/active_storage/blobs/:signed_id/*filename(.:format) Controller#Action | active_storage/blobs#show --[ Route 3 ]------------------------------------------------------------ - Prefix | rails_blob_variation + Prefix | rails_blob_representation Verb | GET - URI | /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) - Controller#Action | active_storage/variants#show + URI | /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) + Controller#Action | active_storage/representations#show --[ Route 4 ]------------------------------------------------------------ - Prefix | rails_blob_preview - Verb | GET - URI | /rails/active_storage/previews/:signed_blob_id/:variation_key/*filename(.:format) - Controller#Action | active_storage/previews#show - --[ Route 5 ]------------------------------------------------------------ Prefix | rails_disk_service Verb | GET URI | /rails/active_storage/disk/:encoded_key/*filename(.:format) Controller#Action | active_storage/disk#show - --[ Route 6 ]------------------------------------------------------------ + --[ Route 5 ]------------------------------------------------------------ Prefix | update_rails_disk_service Verb | PUT URI | /rails/active_storage/disk/:encoded_token(.:format) Controller#Action | active_storage/disk#update - --[ Route 7 ]------------------------------------------------------------ + --[ Route 6 ]------------------------------------------------------------ Prefix | rails_direct_uploads Verb | POST URI | /rails/active_storage/direct_uploads(.:format) diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb index 33715ea75f..467afe7cea 100644 --- a/railties/test/commands/server_test.rb +++ b/railties/test/commands/server_test.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true -require "abstract_unit" +require "isolation/abstract_unit" require "env_helpers" require "rails/command" require "rails/commands/server/server_command" -class Rails::ServerTest < ActiveSupport::TestCase +class Rails::Command::ServerCommandTest < ActiveSupport::TestCase include EnvHelpers def test_environment_with_server_option - args = ["thin", "-e", "production"] + args = ["-u", "thin", "-e", "production"] options = parse_arguments(args) assert_equal "production", options[:environment] assert_equal "thin", options[:server] @@ -22,6 +22,24 @@ class Rails::ServerTest < ActiveSupport::TestCase assert_nil options[:server] end + def test_explicit_using_option + args = ["-u", "thin"] + options = parse_arguments(args) + assert_equal "thin", options[:server] + end + + def test_using_server_mistype + assert_match(/Could not find server "tin". Maybe you meant "thin" or "cgi"/, run_command("--using", "tin")) + end + + def test_using_positional_argument_deprecation + assert_match(/DEPRECATION WARNING/, run_command("tin")) + end + + def test_using_known_server_that_isnt_in_the_gemfile + assert_match(/Could not load server "unicorn". Maybe you need to the add it to the Gemfile/, run_command("-u", "unicorn")) + end + def test_daemon_with_option args = ["-d"] options = parse_arguments(args) @@ -35,7 +53,7 @@ class Rails::ServerTest < ActiveSupport::TestCase end def test_server_option_without_environment - args = ["thin"] + args = ["-u", "thin"] with_rack_env nil do with_rails_env nil do options = parse_arguments(args) @@ -221,7 +239,20 @@ class Rails::ServerTest < ActiveSupport::TestCase ARGV.replace original_args end + def test_served_url + args = %w(-u webrick -b 127.0.0.1 -p 4567) + server = Rails::Server.new(parse_arguments(args)) + assert_equal "http://127.0.0.1:4567", server.served_url + end + private + def run_command(*args) + build_app + rails "server", *args + ensure + teardown_app + end + def parse_arguments(args = []) Rails::Command::ServerCommand.new([], args).server_options end |