aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb4
-rw-r--r--activerecord/lib/active_record/associations.rb14
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_adapter.rb6
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb2
-rw-r--r--activerecord/lib/active_record/persistence.rb5
-rw-r--r--activerecord/lib/active_record/relation.rb4
-rw-r--r--activerecord/lib/active_record/validations/uniqueness.rb3
-rw-r--r--activerecord/test/cases/comment_test.rb6
-rw-r--r--activesupport/CHANGELOG.md32
-rw-r--r--activesupport/lib/active_support/duration.rb57
-rw-r--r--activesupport/test/core_ext/duration_test.rb67
-rw-r--r--guides/bug_report_templates/benchmark.rb1
-rw-r--r--guides/source/routing.md8
-rw-r--r--tasks/release.rb7
14 files changed, 179 insertions, 37 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb
index 2d21ae63f5..1fa0691303 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb
@@ -17,6 +17,10 @@
line-height: 15px;
}
+ #route_table thead tr.bottom th input#search {
+ -webkit-appearance: textfield;
+ }
+
#route_table tbody tr {
border-bottom: 1px solid #ddd;
}
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index e86e02c53a..840f71bef2 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -224,13 +224,6 @@ module ActiveRecord
autoload :CollectionAssociation
autoload :ForeignAssociation
autoload :CollectionProxy
-
- autoload :BelongsToAssociation
- autoload :BelongsToPolymorphicAssociation
- autoload :HasManyAssociation
- autoload :HasManyThroughAssociation
- autoload :HasOneAssociation
- autoload :HasOneThroughAssociation
autoload :ThroughAssociation
module Builder #:nodoc:
@@ -245,6 +238,13 @@ module ActiveRecord
end
eager_autoload do
+ autoload :BelongsToAssociation
+ autoload :BelongsToPolymorphicAssociation
+ autoload :HasManyAssociation
+ autoload :HasManyThroughAssociation
+ autoload :HasOneAssociation
+ autoload :HasOneThroughAssociation
+
autoload :Preloader
autoload :JoinDependency
autoload :AssociationScope
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index e699a77230..7645cf7825 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -421,14 +421,14 @@ module ActiveRecord
end
def case_sensitive_comparison(table, attribute, column, value) # :nodoc:
- table[attribute].eq(Arel::Nodes::BindParam.new(value))
+ table[attribute].eq(value)
end
def case_insensitive_comparison(table, attribute, column, value) # :nodoc:
if can_perform_case_insensitive_comparison_for?(column)
- table[attribute].lower.eq(table.lower(Arel::Nodes::BindParam.new(value)))
+ table[attribute].lower.eq(table.lower(value))
else
- table[attribute].eq(Arel::Nodes::BindParam.new(value))
+ table[attribute].eq(value)
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index e762abc00f..8a9c497918 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -495,7 +495,7 @@ module ActiveRecord
def case_sensitive_comparison(table, attribute, column, value) # :nodoc:
if column.collation && !column.case_sensitive?
- table[attribute].eq(Arel::Nodes::Bin.new(Arel::Nodes::BindParam.new(value)))
+ table[attribute].eq(Arel::Nodes::Bin.new(value))
else
super
end
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 4bd14ccf29..1297e0cde7 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -332,7 +332,7 @@ module ActiveRecord
verify_readonly_attribute(key.to_s)
end
- updated_count = self.class.unscoped.where(self.class.primary_key => id).update_all(attributes)
+ updated_count = _relation_for_itself.update_all(attributes)
attributes.each do |k, v|
write_attribute_without_type_cast(k, v)
@@ -523,8 +523,7 @@ module ActiveRecord
changes[column] = write_attribute(column, time)
end
- primary_key = self.class.primary_key
- scope = self.class.unscoped.where(primary_key => _read_attribute(primary_key))
+ scope = _relation_for_itself
if locking_enabled?
locking_column = self.class.locking_column
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 1c4011b75e..caabad6055 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -92,8 +92,8 @@ module ActiveRecord
def substitute_values(values) # :nodoc:
values.map do |arel_attr, value|
- bind = QueryAttribute.new(arel_attr.name, value, klass.type_for_attribute(arel_attr.name))
- [arel_attr, Arel::Nodes::BindParam.new(bind)]
+ bind = predicate_builder.build_bind_attribute(arel_attr.name, value)
+ [arel_attr, bind]
end
end
diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb
index 17662592ad..2677fade18 100644
--- a/activerecord/lib/active_record/validations/uniqueness.rb
+++ b/activerecord/lib/active_record/validations/uniqueness.rb
@@ -67,12 +67,11 @@ module ActiveRecord
end
attribute_name = attribute.to_s
+ value = klass.predicate_builder.build_bind_attribute(attribute_name, value)
table = klass.arel_table
column = klass.columns_hash[attribute_name]
- cast_type = klass.type_for_attribute(attribute_name)
- value = Relation::QueryAttribute.new(attribute_name, value, cast_type)
comparison = if !options[:case_sensitive]
# will use SQL LOWER function before comparison, unless it detects a case insensitive collation
klass.connection.case_insensitive_comparison(table, attribute, column, value)
diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb
index 27d042db47..9ab6607668 100644
--- a/activerecord/test/cases/comment_test.rb
+++ b/activerecord/test/cases/comment_test.rb
@@ -115,7 +115,11 @@ if ActiveRecord::Base.connection.supports_comments?
assert_match %r[t\.string\s+"name",\s+comment: "Comment should help clarify the column purpose"], output
assert_match %r[t\.string\s+"obvious"\n], output
assert_match %r[t\.string\s+"content",\s+comment: "Whoa, content describes itself!"], output
- assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output
+ if current_adapter?(:OracleAdapter)
+ assert_match %r[t\.integer\s+"rating",\s+precision: 38,\s+comment: "I am running out of imagination"], output
+ else
+ assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output
+ end
unless current_adapter?(:OracleAdapter)
assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output
assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 457eb84a62..938d4f9d31 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,35 @@
+* Fix modulo operations involving durations
+
+ Rails 5.1 introduce an `ActiveSupport::Duration::Scalar` class as a wrapper
+ around a numeric value as a way of ensuring a duration was the outcome of
+ an expression. However the implementation was missing support for modulo
+ operations. This support has now been added and should result in a duration
+ being returned from expressions involving modulo operations.
+
+ Prior to Rails 5.1:
+
+ 5.minutes % 2.minutes
+ => 60
+
+ Now:
+
+ 5.minutes % 2.minutes
+ => 1 minute
+
+ Fixes #29603 and #29743.
+
+ *Sayan Chakraborty*, *Andrew White*
+
+* Fix division where a duration is the denominator
+
+ PR #29163 introduced a change in behavior when a duration was the denominator
+ in a calculation - this was incorrect as dividing by a duration should always
+ return a `Numeric`. The behavior of previous versions of Rails has been restored.
+
+ Fixes #29592.
+
+ *Andrew White*
+
* Add purpose and expiry support to `ActiveSupport::MessageVerifier` &
`ActiveSupport::MessageEncryptor`.
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index 068adcea24..f411bb81df 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -76,15 +76,20 @@ module ActiveSupport
def /(other)
if Duration === other
- new_parts = other.parts.map { |part, other_value| [part, value / other_value] }.to_h
- new_value = new_parts.inject(0) { |total, (part, value)| total + value * Duration::PARTS_IN_SECONDS[part] }
-
- Duration.new(new_value, new_parts)
+ value / other.value
else
calculate(:/, other)
end
end
+ def %(other)
+ if Duration === other
+ Duration.build(value % other.value)
+ else
+ calculate(:%, other)
+ end
+ end
+
private
def calculate(op, other)
if Scalar === other
@@ -118,6 +123,8 @@ module ActiveSupport
years: SECONDS_PER_YEAR
}.freeze
+ PARTS = [:years, :months, :weeks, :days, :hours, :minutes, :seconds].freeze
+
attr_accessor :value, :parts
autoload :ISO8601Parser, "active_support/duration/iso8601_parser"
@@ -168,6 +175,30 @@ module ActiveSupport
new(value * SECONDS_PER_YEAR, [[:years, value]])
end
+ # Creates a new Duration from a seconds value that is converted
+ # to the individual parts:
+ #
+ # ActiveSupport::Duration.build(31556952).parts # => {:years=>1}
+ # ActiveSupport::Duration.build(2716146).parts # => {:months=>1, :days=>1}
+ #
+ def build(value)
+ parts = {}
+ remainder = value.to_f
+
+ PARTS.each do |part|
+ unless part == :seconds
+ part_in_seconds = PARTS_IN_SECONDS[part]
+ parts[part] = remainder.div(part_in_seconds)
+ remainder = (remainder % part_in_seconds).round(9)
+ end
+ end
+
+ parts[:seconds] = remainder
+ parts.reject! { |k, v| v.zero? }
+
+ new(value, parts)
+ end
+
private
def calculate_total_seconds(parts)
@@ -234,8 +265,10 @@ module ActiveSupport
# Divides this Duration by a Numeric and returns a new Duration.
def /(other)
- if Scalar === other || Duration === other
+ if Scalar === other
Duration.new(value / other.value, parts.map { |type, number| [type, number / other.value] })
+ elsif Duration === other
+ value / other.value
elsif Numeric === other
Duration.new(value / other, parts.map { |type, number| [type, number / other] })
else
@@ -243,6 +276,18 @@ module ActiveSupport
end
end
+ # Returns the modulo of this Duration by another Duration or Numeric.
+ # Numeric values are treated as seconds.
+ def %(other)
+ if Duration === other || Scalar === other
+ Duration.build(value % other.value)
+ elsif Numeric === other
+ Duration.build(value % other)
+ else
+ raise_type_error(other)
+ end
+ end
+
def -@ #:nodoc:
Duration.new(-value, parts.map { |type, number| [type, -number] })
end
@@ -327,7 +372,7 @@ module ActiveSupport
def inspect #:nodoc:
parts.
reduce(::Hash.new(0)) { |h, (l, r)| h[l] += r; h }.
- sort_by { |unit, _ | [:years, :months, :weeks, :days, :hours, :minutes, :seconds].index(unit) }.
+ sort_by { |unit, _ | PARTS.index(unit) }.
map { |unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}" }.
to_sentence(locale: ::I18n.default_locale)
end
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index c655f466f2..b3e0cd8bd0 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -111,7 +111,44 @@ class DurationTest < ActiveSupport::TestCase
def test_divide
assert_equal 1.day, 7.days / 7
assert_instance_of ActiveSupport::Duration, 7.days / 7
+
+ assert_equal 1.hour, 1.day / 24
+ assert_instance_of ActiveSupport::Duration, 1.day / 24
+
+ assert_equal 24, 86400 / 1.hour
+ assert_kind_of Integer, 86400 / 1.hour
+
+ assert_equal 24, 1.day / 1.hour
+ assert_kind_of Integer, 1.day / 1.hour
+
assert_equal 1, 1.day / 1.day
+ assert_kind_of Integer, 1.day / 1.hour
+ end
+
+ def test_modulo
+ assert_equal 1.minute, 5.minutes % 120
+ assert_instance_of ActiveSupport::Duration, 5.minutes % 120
+
+ assert_equal 1.minute, 5.minutes % 2.minutes
+ assert_instance_of ActiveSupport::Duration, 5.minutes % 2.minutes
+
+ assert_equal 1.minute, 5.minutes % 120.seconds
+ assert_instance_of ActiveSupport::Duration, 5.minutes % 120.seconds
+
+ assert_equal 5.minutes, 5.minutes % 1.hour
+ assert_instance_of ActiveSupport::Duration, 5.minutes % 1.hour
+
+ assert_equal 1.day, 36.days % 604800
+ assert_instance_of ActiveSupport::Duration, 36.days % 604800
+
+ assert_equal 1.day, 36.days % 7.days
+ assert_instance_of ActiveSupport::Duration, 36.days % 7.days
+
+ assert_equal 800.seconds, 8000 % 1.hour
+ assert_instance_of ActiveSupport::Duration, 8000 % 1.hour
+
+ assert_equal 1.month, 13.months % 1.year
+ assert_instance_of ActiveSupport::Duration, 13.months % 1.year
end
def test_date_added_with_multiplied_duration
@@ -410,9 +447,9 @@ class DurationTest < ActiveSupport::TestCase
assert_equal 5, scalar / 2
assert_instance_of ActiveSupport::Duration::Scalar, scalar / 2
assert_equal 10, 100.seconds / scalar
- assert_instance_of ActiveSupport::Duration, 2.seconds * scalar
+ assert_instance_of ActiveSupport::Duration, 100.seconds / scalar
assert_equal 5, scalar / 2.seconds
- assert_instance_of ActiveSupport::Duration, scalar / 2.seconds
+ assert_kind_of Integer, scalar / 2.seconds
exception = assert_raises(TypeError) do
scalar / "foo"
@@ -421,13 +458,29 @@ class DurationTest < ActiveSupport::TestCase
assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message
end
- def test_scalar_divide_parts
+ def test_scalar_modulo
scalar = ActiveSupport::Duration::Scalar.new(10)
- assert_equal({ days: 2 }, (scalar / 5.days).parts)
- assert_equal(172800, (scalar / 5.days).value)
- assert_equal({ days: -2 }, (scalar / -5.days).parts)
- assert_equal(-172800, (scalar / -5.days).value)
+ assert_equal 1, 31 % scalar
+ assert_instance_of ActiveSupport::Duration::Scalar, 31 % scalar
+ assert_equal 1, scalar % 3
+ assert_instance_of ActiveSupport::Duration::Scalar, scalar % 3
+ assert_equal 1, 31.seconds % scalar
+ assert_instance_of ActiveSupport::Duration, 31.seconds % scalar
+ assert_equal 1, scalar % 3.seconds
+ assert_instance_of ActiveSupport::Duration, scalar % 3.seconds
+
+ exception = assert_raises(TypeError) do
+ scalar % "foo"
+ end
+
+ assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message
+ end
+
+ def test_scalar_modulo_parts
+ scalar = ActiveSupport::Duration::Scalar.new(82800)
+ assert_equal({ hours: 1 }, (scalar % 2.hours).parts)
+ assert_equal(3600, (scalar % 2.hours).value)
end
def test_twelve_months_equals_one_year
diff --git a/guides/bug_report_templates/benchmark.rb b/guides/bug_report_templates/benchmark.rb
index 54433b34dd..a0b541d012 100644
--- a/guides/bug_report_templates/benchmark.rb
+++ b/guides/bug_report_templates/benchmark.rb
@@ -8,6 +8,7 @@ end
gemfile(true) do
source "https://rubygems.org"
gem "rails", github: "rails/rails"
+ gem "arel", github: "rails/arel"
gem "benchmark-ips"
end
diff --git a/guides/source/routing.md b/guides/source/routing.md
index 0d45d6c554..f52b1862a8 100644
--- a/guides/source/routing.md
+++ b/guides/source/routing.md
@@ -418,7 +418,7 @@ resources :articles do
end
```
-Also you can use them in any place that you want inside the routes, for example in a scope or namespace call:
+Also you can use them in any place that you want inside the routes, for example in a `scope` or `namespace` call:
```ruby
namespace :articles do
@@ -640,7 +640,7 @@ match 'photos', to: 'photos#show', via: :all
NOTE: Routing both `GET` and `POST` requests to a single action has security implications. In general, you should avoid routing all verbs to an action unless you have a good reason to.
-NOTE: 'GET' in Rails won't check for CSRF token. You should never write to the database from 'GET' requests, for more information see the [security guide](security.html#csrf-countermeasures) on CSRF countermeasures.
+NOTE: `GET` in Rails won't check for CSRF token. You should never write to the database from `GET` requests, for more information see the [security guide](security.html#csrf-countermeasures) on CSRF countermeasures.
### Segment Constraints
@@ -808,14 +808,14 @@ NOTE: For the curious, `'articles#index'` actually expands out to `ArticlesContr
If you specify a Rack application as the endpoint for a matcher, remember that
the route will be unchanged in the receiving application. With the following
-route your Rack application should expect the route to be '/admin':
+route your Rack application should expect the route to be `/admin`:
```ruby
match '/admin', to: AdminApp, via: :all
```
If you would prefer to have your Rack application receive requests at the root
-path instead, use mount:
+path instead, use `mount`:
```ruby
mount AdminApp, at: '/admin'
diff --git a/tasks/release.rb b/tasks/release.rb
index ac13612b91..b3bbbb0076 100644
--- a/tasks/release.rb
+++ b/tasks/release.rb
@@ -154,8 +154,13 @@ namespace :all do
task verify: :install do
app_name = "pkg/verify-#{version}-#{Time.now.to_i}"
- sh "rails new #{app_name}"
+ sh "rails _#{version}_ new #{app_name} --skip-bundle" # Generate with the right version.
cd app_name
+
+ # Replace the generated gemfile entry with the exact version.
+ File.write("Gemfile", File.read("Gemfile").sub(/^gem 'rails.*/, "gem 'rails', '#{version}'"))
+ sh "bundle"
+
sh "rails generate scaffold user name admin:boolean && rails db:migrate"
puts "Booting a Rails server. Verify the release by:"