| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
This reverts commit dd779c9686f49f5ed6dda8ad5a1cb3b0788e1dd4.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before 34cc301, type casting by boolean attribute when querying is a
no-op, so finding by truthy boolean string (i.e.
`where(value: "true") # => value = 'true'`) didn't work as expected
(matches it to FALSE in MySQL #32624). By type casting is ensured, a
value on boolean attribute is always serialized to TRUE or FALSE.
In PostgreSQL, `where(value: :false) # => value = 'false'` was a valid
SQL, so 34cc301 is a regresson for PostgreSQL since all symbol values
are serialized as TRUE.
I'd say using `:false` is mostly a developer's mistake (user's input
basically comes as a string), but `:false` on boolean attribute is
serialized as TRUE is not a desirable behavior for anybody.
This allows falsy boolean symbols as false, i.e.
`klass.create(value: :false).value? # => false` and
`where(value: :false) # => value = FALSE`.
Fixes #35676.
|
|\
| |
| |
| |
| | |
kamipo/dont_allow_non_numeric_string_matches_to_zero
Don't allow `where` with non numeric string matches to 0 values
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This is a follow-up of #35310.
Currently `Topic.find_by(id: "not-a-number")` matches to a `id = 0`
record. That is considered as silently leaking information.
If non numeric string is given to find by an integer column, it should
not be matched to any record.
Related #12793.
|
|/
|
|
|
|
|
|
|
|
|
| |
This reverts commit 52fddcc653458456f98b3683dffd781cf00b35fe.
52fddcc was to short-circuit `ensure_in_range` in `cast_value`. But that
caused a regression for empty string deserialization.
Since 7c6f393, `ensure_in_range` is moved into `serialize`. As 52fddcc
said, the absolute gain is quite small. So I've reverted that commit to
fix the regression.
|
|\
| |
| |
| | |
Return correct date in ActiveModel for time to date conversions
|
| |
| |
| |
| |
| |
| |
| | |
time.to_date conversion happens considering leap years
so a conversion of "Day.new({'day(1i)'=>'1', 'day(2i)'=>'1', 'day(3i)'=>'1'})" results in saving the date as Mon, 03 Jan 0001
which might seem weird on the user level, hence falling back to parsing on string level resolves this data mismatch
Fixes #28521
|
| |
| |
| |
| |
| |
| |
| |
| | |
`value_from_multiparameter_assignment` defined by
`AcceptsMultiparameterTime` helper requires `default_timezone` method
which is defined at `TimeValue` helper.
Since `Date` type doesn't include `TimeValue`, I've extracted `Timezone`
helper to be shared by `Date`, `DateTime`, and `Time` types.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When assigning a hash to a time attribute that's missing a year
component (e.g. a `time_select` with `:ignore_date` set to `true`)
then the year defaults to 1970 instead of the expected 2000. This
results in the attribute changing as a result of the save.
Before:
event = Event.new(start_time: { 4 => 20, 5 => 30 })
event.start_time # => 1970-01-01 20:30:00 UTC
event.save
event.reload
event.start_time # => 2000-01-01 20:30:00 UTC
After:
event = Event.new(start_time: { 4 => 20, 5 => 30 })
event.start_time # => 2000-01-01 20:30:00 UTC
event.save
event.reload
event.start_time # => 2000-01-01 20:30:00 UTC
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since Rails 6.0 will support Ruby 2.4.1 or higher
`# frozen_string_literal: true` magic comment is enough to make string object frozen.
This magic comment is enabled by `Style/FrozenStringLiteralComment` cop.
* Exclude these files not to auto correct false positive `Regexp#freeze`
- 'actionpack/lib/action_dispatch/journey/router/utils.rb'
- 'activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb'
It has been fixed by https://github.com/rubocop-hq/rubocop/pull/6333
Once the newer version of RuboCop released and available at Code Climate these exclude entries should be removed.
* Replace `String#freeze` with `String#-@` manually if explicit frozen string objects are required
- 'actionpack/test/controller/test_case_test.rb'
- 'activemodel/test/cases/type/string_test.rb'
- 'activesupport/lib/active_support/core_ext/string/strip.rb'
- 'activesupport/test/core_ext/string_ext_test.rb'
- 'railties/test/generators/actions_test.rb'
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In Ruby 2.3 or later, `String#+@` is available and `+@` is faster than `dup`.
```ruby
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "benchmark-ips"
end
Benchmark.ips do |x|
x.report('+@') { +"" }
x.report('dup') { "".dup }
x.compare!
end
```
```
$ ruby -v benchmark.rb
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
Warming up --------------------------------------
+@ 282.289k i/100ms
dup 187.638k i/100ms
Calculating -------------------------------------
+@ 6.775M (± 3.6%) i/s - 33.875M in 5.006253s
dup 3.320M (± 2.2%) i/s - 16.700M in 5.032125s
Comparison:
+@: 6775299.3 i/s
dup: 3320400.7 i/s - 2.04x slower
```
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
For example, dirty checking was not right for the following case:
```
model.int_column = "+5"
model.float_column = "0.5E+1"
model.decimal_column = "0.5e-3"
```
It is enough to see whether leading character is a digit for avoiding
invalid numeric expression like 'wibble' to be type-casted to 0, as
this method's comment says.
Fixes #33801
|
| |
| |
| |
| | |
Before it was coercing an invalid string into "2000-01-01 00:00:00".
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`BigDecimal.new` has been deprecated in BigDecimal 1.3.3
which will be a default for Ruby 2.5.
Refer ruby/bigdecimal@5337373
* This commit has been made as follows:
```ruby
$ cd activemodel/
$ git grep -l BigDecimal.new | grep \.rb | xargs sed -i -e "s/BigDecimal.new/BigDecimal/g"
```
* This commit has been tested with these Ruby versions:
```
ruby 2.5.0dev (2017-12-15 trunk 61262) [x86_64-linux]
ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-linux]
ruby 2.3.5p376 (2017-09-14 revision 59905) [x86_64-linux]
ruby 2.2.8p477 (2017-09-14 revision 59906) [x86_64-linux]
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Attribute modules (`Attribute`, `Attributes`, `AttributeSet`) uses
`Type`, but referencing `Type` before the modules still fail.
```
% ./bin/test -w test/cases/attribute_test.rb -n test_with_value_from_user_validates_the_value
Run options: -n test_with_value_from_user_validates_the_value --seed 31876
E
Error:
ActiveModel::AttributeTest#test_with_value_from_user_validates_the_value:
NameError: uninitialized constant ActiveModel::AttributeTest::Type
/Users/kamipo/src/github.com/rails/rails/activemodel/test/cases/attribute_test.rb:233:in `block in <class:AttributeTest>'
bin/test test/cases/attribute_test.rb:232
Finished in 0.002985s, 335.0479 runs/s, 335.0479 assertions/s.
1 runs, 1 assertions, 0 failures, 1 errors, 0 skips
```
Probably we need more autoloading at least `Type`.
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
|\
| |
| |
| | |
Enforce frozen string in Rubocop
|
| | |
|
|/
|
|
| |
Includes two external changes because they're referenced within the ActiveModel test suite.
|
|
|
|
|
|
|
|
|
| |
See 34321e4a433bb7eef48fd743286601403f8f7d82 for background on
ImmutableString vs String.
Our String type cannot delegate typecasting to ImmutableString, because
the latter freezes its input: duplicating the value after that gives us
an unfrozen result, but still mutates the originally passed object.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In Ruby 2.4, BigDecimal(), as used by the Decimal cast, was changed so
that it will raise ArgumentError when passed an invalid string, in order
to be more consistent with Integer(), Float(), etc. The other numeric
types use ex. to_i and to_f.
Unfortunately, we can't simply change BigDecimal() to to_d. String#to_d
raises errors like BigDecimal(), unlike all the other to_* methods (this
should probably be filed as a ruby bug).
Instead, this simulates the existing behaviour and the behaviour of the
other to_* methods by finding a numeric string at the start of the
passed in value, and parsing that using BigDecimal().
See also
https://bugs.ruby-lang.org/issues/10286
https://github.com/ruby/bigdecimal/commit/3081a627cebdc1fc119425c7a9f009dbb6bec8e8
|
| |
|
|\
| |
| |
| | |
Moved database-specific ActiveModel types into ActiveRecord
|
| |
| |
| |
| | |
ie. DecimalWithoutScale, Text and UnsignedInteger
|
|\ \
| | |
| | | |
remove warning from big integer test
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This removes the following warnings.
```
activemodel/test/cases/type/big_integer_test.rb:15: warning: ambiguous first argument; put parentheses or a space even after `-' operator
```
|
|/ / |
|
|/ |
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
|
|
|
|
|
| |
The should make it easier for apps to rescue ActiveModel specific
errors without the need to wrap all method calls with a generic
rescue RangeError.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since precision is always larger than scale, it can actually change
rounding behavior. Given a precision of 5 and a scale of 3, when you
apply the precision of 5 to `1.25047`, the result is `1.2505`, which
when the scale is applied would be `1.251` instead of the expected
`1.250`.
This issue appears to only occur with floats, as scale doesn't apply to
other numeric types, and the bigdecimal constructor actually ignores
precision entirely when working with strings. There's no way we could
handle this for the "unknown object which responds to `to_d`" case, as
we can't assume an interface for applying the scale.
Fixes #24235
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This type adds an escape hatch to apps for which string duping causes
unacceptable memory growth. The reason we are duping them is in order to
detect mutation, which was a feature added to 4.2 in #15674. The string
type was modified to support this behavior in #15788.
Memory growth is really only a concern for string types, as it's the
only mutable type where the act of coersion does not create a new object
regardless (as we're usually returning an object of a different class).
I do feel strongly that if we are going to support detecting mutation,
we should do it universally for any type which is mutable. While it is
less common and ideomatic to mutate strings than arrays or hashes, there
shouldn't be rules or gotchas to understanding our behavior.
However, I also appreciate that for apps which are using a lot of string
columns, this would increase the number of allocations by a large
factor. To ensure that we keep our contract, if you'd like to opt out of
mutation detection on strings, you'll also be option out of mutation of
those strings.
I'm not completely married to the thought that strings coming out of
this actually need to be frozen -- and I think the name is correct
either way, as the purpose of this is to provide a string type which
does not detect mutation.
In the new implementation, I'm only overriding `cast_value`. I did not
port over the duping in `serialize`. I cannot think of a reason we'd
need to dup the string there, and the tests pass without it.
Unfortunately that line was introduced at a time where I was not nearly
as good about writing my commit messages, so I have no context as to
why I added it. Thanks past Sean. You are a jerk.
|
|
|
|
|
|
|
|
|
| |
Any tests for a type which is not overridden by Active Record, and does
not test the specifics of the attributes API interacting in more complex
ways have no reason to be in the Active Record suite. Doing this
revealed that the implementation of the date and time types in AM was
actually completely broken, and incapable of returning any value other
than `nil`.
|
|
Things like decorations, overrides, and priorities only matter for
Active Record, so the Active Model registry can be implemented much more
simply. At this point, I wonder if having Active Record's registry
inherit from Active Model's is even worth the trouble?
The Active Model class was also missing test cases, which have been
backfilled.
This removes the error when two types are registered with the same name,
but given that Active Model is meant to be significantly more generic, I
do not think this is an issue for now. If we want, we can raise an error
at the point that someone tries to register it.
|