Commit Graph

1581 Commits

Author SHA1 Message Date
Ryuta Kamizono
d29d459897 Avoid redundant time.getutc call if it is already utc time object
Currently `type.serialize` and `connection.{quote|type_cast}` for a time
object always does `time.getutc` call regardless of whether it is
already utc time object or not, that duplicated proccess
(`connection.type_cast(type.serialize(time))`) allocates extra/useless
time objects for each type casting.

This avoids that redundant `time.getutc` call if it is already utc time
object. In the case of a model has timestamps (`created_at` and
`updated_at`), it avoids 6,000 time objects allocation for 1,000 times
`model.save`.

```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})

pp ObjectSpace::AllocationTracer.trace {
  1_000.times { User.create }
}.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") }
```

Before (c104bfe424e6cebe9c8e85a38515327a6c88b1f8):

```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  203,
  :T_ARRAY]=>[1004, 0, 778, 0, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  220,
  :T_STRING]=>[2, 0, 2, 1, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  209,
  :T_ARRAY]=>[8, 0, 8, 1, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  57,
  :T_ARRAY]=>[4, 0, 4, 1, 1, 0],
 ["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb",
  17,
  :T_DATA]=>[4000, 0, 3096, 0, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  120,
  :T_DATA]=>[2000, 0, 1548, 0, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  126,
  :T_STRING]=>[4000, 0, 3096, 0, 1, 0]}
```

After (this change):

```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  203,
  :T_ARRAY]=>[1004, 0, 823, 0, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  220,
  :T_STRING]=>[2, 0, 2, 1, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  209,
  :T_ARRAY]=>[8, 0, 8, 1, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  57,
  :T_ARRAY]=>[4, 0, 4, 1, 1, 0],
 ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
  126,
  :T_STRING]=>[2000, 0, 1638, 0, 1, 0]}
```
2019-06-18 01:03:21 +09:00
Ryuta Kamizono
c81af6ae72 Enable Layout/EmptyLinesAroundAccessModifier cop
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.
2019-06-13 12:00:45 +09:00
Ryuta Kamizono
497e52f8c2 Don't round off subseconds unless necessary
Currently, if `:datetime` type has a precision, type casting always does
round off subseconds regardless of whether necessary or not, it is a bit
slow.

Since #34970, `t.timestamps` with `precision: 6` by default, so
`pluck(:created_at)` in newly created app will always be affected by the
round off.

This avoids the round off if possible, it makes `pluck(:created_at)`
about 25% faster.

https://gist.github.com/kamipo/e029539f2632aee9f5e711fe66fc8842

Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with postgresql adapter):

```
Warming up --------------------------------------
    users.pluck(:id)      344.000  i/100ms
  users.pluck(:name)      336.000  i/100ms
users.pluck(:created_at)  206.000  i/100ms
Calculating -------------------------------------
    users.pluck(:id)      3.620k (± 8.5%) i/s -     18.232k in   5.077316s
  users.pluck(:name)      3.579k (± 9.4%) i/s -     17.808k in   5.020230s
users.pluck(:created_at)  2.069k (± 8.0%) i/s -     10.300k in   5.019284s
```

Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with mysql2 adapter):

```
Warming up --------------------------------------
    users.pluck(:id)      245.000  i/100ms
  users.pluck(:name)      240.000  i/100ms
users.pluck(:created_at)  180.000  i/100ms
Calculating -------------------------------------
    users.pluck(:id)      2.548k (± 9.4%) i/s -     12.740k in   5.066574s
  users.pluck(:name)      2.513k (± 8.0%) i/s -     12.480k in   5.011260s
users.pluck(:created_at)  1.771k (±11.2%) i/s -      8.820k in   5.084473s
```

After (this change with postgresql adapter):

```
Warming up --------------------------------------
    users.pluck(:id)      348.000  i/100ms
  users.pluck(:name)      357.000  i/100ms
users.pluck(:created_at)  254.000  i/100ms
Calculating -------------------------------------
    users.pluck(:id)      3.628k (± 8.2%) i/s -     18.096k in   5.024748s
  users.pluck(:name)      3.624k (±12.4%) i/s -     17.850k in   5.020959s
users.pluck(:created_at)  2.567k (± 7.0%) i/s -     12.954k in   5.081153s
```

After (this change with mysql2 adapter):

```
Warming up --------------------------------------
    users.pluck(:id)      268.000  i/100ms
  users.pluck(:name)      265.000  i/100ms
users.pluck(:created_at)  207.000  i/100ms
Calculating -------------------------------------
    users.pluck(:id)      2.586k (±10.9%) i/s -     12.864k in   5.050546s
  users.pluck(:name)      2.543k (±10.2%) i/s -     12.720k in   5.067726s
users.pluck(:created_at)  2.263k (±14.5%) i/s -     10.971k in   5.004039s
```
2019-05-28 22:50:36 +09:00
Gannon McGibbon
93ef75615e
Merge pull request #36092 from imechemi/update-doc
Improve doc for :root option in as_json()
2019-05-13 12:06:49 -04:00
Tenzin Chemi
42a3ec8fee Improve doc for :root option in as_json() [ci skip]
Remove trailing whitespace [ci skip]

Reword

Root value should be string [ci skip]
2019-05-13 15:35:50 +05:30
Guo Xiang Tan
25f1e0e3ea Recover perf for pluck by reverting 9c9c950d02af83742a5f76302d0faa99508f242c.
This reverts commit 9c9c950d02af83742a5f76302d0faa99508f242c.
2019-05-03 22:36:23 +08:00
Aaron Patterson
2ada222f1e
any? should be delegated to the errors list
Otherwise we get deprecation warnings in the generated scaffold template files
2019-04-30 16:54:40 -05:00
Abhay Nikam
1cd24e5d00 Change the deprecation for Enumerating ActiveModel::Errors to Rails 6.1 instead of 6.0 (#36087)
* Change the deprecation for Enumerating ActiveModel::Errors to Rails 6.1 instead of 6.0

* Changed the deprecation message for ActiveModel::Errors methods: slice, values, keys and to_xml
2019-04-25 13:13:01 +09:00
Rafael França
d4d145a679
Merge pull request #32313 from lulalala/model_error_as_object
Model error as object
2019-04-24 16:16:00 -04:00
Rafael Mendonça França
9834be6565
Start Rails 6.1 development 2019-04-24 15:57:14 -04:00
Ryuta Kamizono
5575bd7b22 Avoid redundant attribute_alias? before attribute_alias
If we want to get alias resolved attribute finally, we can use
`attribute_alias` directly.

For that purpose, avoiding redundant `attribute_alias?` makes alias
attribute access 40% faster.

https://gist.github.com/kamipo/e427f080a27b46f50bc508fae3612a0e

Before (2c0729d8):

```
Warming up --------------------------------------
          user['id']   102.668k i/100ms
      user['new_id']    80.660k i/100ms
        user['name']    99.368k i/100ms
    user['new_name']    81.626k i/100ms
Calculating -------------------------------------
          user['id']      1.431M (± 4.0%) i/s -      7.187M in   5.031985s
      user['new_id']      1.042M (± 4.2%) i/s -      5.243M in   5.039858s
        user['name']      1.406M (± 5.6%) i/s -      7.055M in   5.036743s
    user['new_name']      1.074M (± 3.6%) i/s -      5.387M in   5.024152s
```

After (this change):

```
Warming up --------------------------------------
          user['id']   109.775k i/100ms
      user['new_id']   103.303k i/100ms
        user['name']   105.988k i/100ms
    user['new_name']    99.618k i/100ms
Calculating -------------------------------------
          user['id']      1.520M (± 6.7%) i/s -      7.574M in   5.011496s
      user['new_id']      1.485M (± 6.2%) i/s -      7.438M in   5.036252s
        user['name']      1.538M (± 5.4%) i/s -      7.737M in   5.049765s
    user['new_name']      1.516M (± 4.6%) i/s -      7.571M in   5.007293s
```
2019-04-24 18:46:23 +09:00
Rafael França
6a4eb3e75e
Merge pull request #36061 from shioyama/update_comment
Update comment in attribute_method_matchers_matching
2019-04-23 11:16:53 -04:00
Chris Salzberg
14ff93c4bc
Update comment in attribute_method_matchers_matching
The current comment here is from 2011 and its original context has
changed completely. The plain matcher will not "match every time"
anymore since the code now filters all matchers, and only selects those
for which the captured attribute is valid.

To avoid confusion, I updated the comment. For more discussion, see:

https://github.com/rails/rails/pull/36005
2019-04-23 14:24:40 +09:00
Daniel Colson
e7a28c3990
Add attribute_names to ActiveModel::Attributes
This adds `.attribute_names` and `#attribute_names` to
`ActiveModel::Attributes` along the same lines as the corresponding
methods in `ActiveRecord::AttributeMethods` (see
[`.attribute_names`][class_method] and
[`#attribute_names`][instance_method].

While I was here I also added documentation for '#attributes', which I
added in 043ce35b186. The whole class is still `#:nodoc:` so I don't
think this will have any effect for now.

[class_method]: cc834db1d0/activerecord/lib/active_record/attribute_methods.rb (L154-L160)
[instance_method]: cc834db1d0/activerecord/lib/active_record/attribute_methods.rb (L299-L301)
2019-04-22 19:48:17 -04:00
Chris Salzberg
c9d75177fe
Improve wording of comments
Most of the time, these methods are called from actual methods defined
from columns in the schema, not from method_missing, so the current
wording is misleading.
2019-04-13 11:12:39 +09:00
Chris Salzberg
bdcdb2b35f
Rename method_missing_target to target
The target of matchers is used in two contexts: to define attribute
methods which dispatch to handlers like attribute_was, and to match
method calls in method_missing and dispatch to those same handler
methods.

Only in the latter context does the term "method_missing_target" make
any sense; in the former context it is just confusing. "target" is not
ideal as a term but at least it avoids this confusion.
2019-04-13 10:41:57 +09:00
Chris Salzberg
63b5bdea51
Remove unused method_name from AttributeMethodMatch 2019-04-12 23:35:26 +09:00
Chris Salzberg
0bd4552e02
Rename "method" to "matcher" in map block 2019-04-12 15:19:09 +09:00
Ryuta Kamizono
6b0a9de906 PERF: 2x ~ 30x faster dirty tracking
Currently, although using both dirty tracking (ivar backed and
attributes backed) on one model is not supported (doesn't fully work at
least), both dirty tracking are being performed, that is very slow.

As long as attributes backed dirty tracking is used, ivar backed dirty
tracking should not need to be performed.

I've refactored to extract new `ForcedMutationTracker` which only tracks
`force_change` to be performed for ivar backed dirty tracking, that
makes dirty tracking on Active Record 2x ~ 30x faster.

https://gist.github.com/kamipo/971dfe0891f0fe1ec7db8ab31f016435

Before:

```
Warming up --------------------------------------
            changed?     4.467k i/100ms
             changed     5.134k i/100ms
             changes     3.023k i/100ms
  changed_attributes     4.358k i/100ms
        title_change     3.185k i/100ms
           title_was     3.381k i/100ms
Calculating -------------------------------------
            changed?     42.197k (±28.5%) i/s -    187.614k in   5.050446s
             changed     50.481k (±16.0%) i/s -    246.432k in   5.045759s
             changes     30.799k (± 7.2%) i/s -    154.173k in   5.030765s
  changed_attributes     51.530k (±14.2%) i/s -    252.764k in   5.041106s
        title_change     44.667k (± 9.0%) i/s -    222.950k in   5.040646s
           title_was     44.635k (±16.6%) i/s -    216.384k in   5.051098s
```

After:

```
Warming up --------------------------------------
            changed?    24.130k i/100ms
             changed    13.503k i/100ms
             changes     6.511k i/100ms
  changed_attributes     9.226k i/100ms
        title_change    48.221k i/100ms
           title_was    96.060k i/100ms
Calculating -------------------------------------
            changed?    245.478k (±16.1%) i/s -      1.182M in   5.015837s
             changed    157.641k (± 4.9%) i/s -    796.677k in   5.066734s
             changes     70.633k (± 5.7%) i/s -    358.105k in   5.086553s
  changed_attributes     95.155k (±13.6%) i/s -    470.526k in   5.082841s
        title_change    566.481k (± 3.5%) i/s -      2.845M in   5.028852s
           title_was      1.487M (± 3.9%) i/s -      7.493M in   5.046774s
```
2019-04-11 16:30:40 +09:00
Ryuta Kamizono
50fba828d5 Refactor has_secure_password to extract dedicated attribute module
Follow up of #26764 and #35700.

And add test case for #35700.
2019-04-05 01:55:00 +09:00
Ryuta Kamizono
dc45130c44
Merge pull request #35700 from Futurelearn/seb-secure-password-fix
Reintroduce support for overriding `has_secure_password` attributes
2019-04-05 01:19:08 +09:00
lulalala
aaa0c32797 Set default array to details
maintaining behavior errors.details[:foo].any?
2019-03-31 22:59:13 +08:00
lulalala
f7f42a2d0e Fix messages[]= does not override value 2019-03-31 22:59:13 +08:00
lulalala
514c4b4d53 Freeze DeprecationHandling array and hash 2019-03-31 22:59:13 +08:00
lulalala
e7834214a6 Fix equality comparison raising error bug 2019-03-31 22:59:13 +08:00
lulalala
90815b12c5 Fix spec 2019-03-31 22:59:12 +08:00
lulalala
ba38b40e83 Split messages and to_hash
Fix double wrapping issue
Revert messages_for wrapping. It's a new method so no need to put
deprecation warnings.
2019-03-31 22:59:12 +08:00
lulalala
abee034368 Raise deprecation for calling [:f] = 'b' or [:f] << 'b'
Revert some tests to ensure back compatibility
2019-03-31 22:59:12 +08:00
lulalala
67d262f70f Add deprecation to slice! 2019-03-31 22:59:12 +08:00
lulalala
be1585fca0 Nested attribute error's attribute name to use different key:
To keep the same as SHA dcafe995bfe51e53dd04607956be9b54073e9cb6
2019-03-31 22:59:12 +08:00
lulalala
582a8e2f94 String override options in #import to convert to symbol 2019-03-31 22:59:12 +08:00
lulalala
86620cc3aa Allow errors to remove duplicates, and ensure cyclic associations w/ autosave duplicate errors can be removed
See SHA 7550f0a016ee6647aaa76c0c0ae30bebc3867288
2019-03-31 22:59:12 +08:00
lulalala
2a06f13099 Add messages_for 2019-03-31 22:59:12 +08:00
lulalala
cccbac6df6 Add a transitional method objects, for accessing the array directly.
This is because we try to accommodate old hash behavior, so `first` and `last` now does not return Error object.
2019-03-31 22:59:12 +08:00
lulalala
86b4aa1175 Backward compatibility for errors.collect/select etc.
All enumerable methods must go through the `each` so it retain old hash behavior.
Revert this after Rails 6.1 in order to speed up enumerable methods.
2019-03-31 22:59:12 +08:00
lulalala
ea77205a9f Add convenience method group_by_attribute
Many operations need grouping of errors by attributes, e.g. ActiveRecord::AutosaveAssociation#association_valid?

Refactor other methods using group_by_attribute
2019-03-31 22:59:12 +08:00
lulalala
d9011e3935 Change errors
Allow `each` to behave in new way if block arity is 1

Ensure dumped marshal from Rails 5 can be loaded

Make errors compatible with marshal and YAML dumps from previous versions of Rails

Add deprecation warnings

Ensure each behave like the past, sorted by attribute
2019-03-31 22:59:12 +08:00
lulalala
ef68d3e35c Add ActiveModel::Error and NestedError
Add initialize_dup to deep dup.

Move proc eval and flexible message position out to Errors,
because proc eval is needed for Errors#added? and Errors#delete
2019-03-31 22:59:12 +08:00
Ryuta Kamizono
406d3a926c
Merge pull request #35794 from kamipo/type_cast_symbol_false
Type cast falsy boolean symbols on boolean attribute as false
2019-03-30 05:07:07 +09:00
Ryuta Kamizono
2d12f800f1 Type cast falsy boolean symbols on boolean attribute as false
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.
2019-03-30 04:18:25 +09:00
Prathamesh Sonpatki
d8ba2f7c56
Rename i18n_full_message config option to i18n_customize_full_message
- I feel `i18n_customize_full_messages` explains the meaning of the
  config better.
- Followup of https://github.com/rails/rails/pull/32956
2019-03-29 21:38:48 +05:30
Seb Jacobs
4733e04dfa Reintroduce support for overriding has_secure_password attributes
In Rails 5.2.x calling `has_secure_password` would define attribute
readers and writers on the superclass of the model, which meant that you
could override these attributes in a model and call the superclass for
example:

```
class Dog < ApplicationRecord
  has_secure_password

  def password=(new_password)
    @password_set = new_password.present?
    super
  end
end
```

However this behaviour was broken in Rails 6 when the ability to
customise the name of the attribute was introduced [1] since they are no
longer being defined on the superclass you will now see the following
error:

```
NoMethodError:
super: no superclass method `password=' for #<Dog:0x00007ffbbc7ce290>
Did you mean?  password
```

In order to resolve this issue and retain support for setting a custom
attribute name we can define these attribute readers/writers in a module
and then ensure that the module is included in the inheritance chain.

[1] https://www.github.com/rails/rails/commit/86a48b4da3
    https://www.github.com/rails/rails/commit/9b63bf1dfd
2019-03-22 08:28:13 +00:00
eileencodes
a2bd669ed2 v6.0.0.beta3 release
-----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEEvJkGf0BARV+D0L2ulxXUSC76N8FAlyJN4cACgkQulxXUSC7
 6N9ZXAf/Wx7edIct8kZzcC6irlROx4DzpNbrrH792sO1OAcnoFDE7DPkokllTEP/
 4kzC42lca/XG27MCl7E0dtVD8hIyAl89nxid6cwKFVZVTPIRVc1wjXkoiWy/cvd7
 6+9IjxhlgrzxGnw3aWZJG7H3iqz69yr55aoSDU/TbMqq5kQrqNF95vr2nc8LEUco
 SLQj0pO/tfJdHquSeX0JiXn3VSEHT+5TdLGQ3J/w0wFU6mkecH4MJMJvMwLFx/v4
 llnvF6HyfSLASWbrpdD3h6MQHpImDoee5vILXAHzPdSaEVcVa1cDFtMcPMYiu8Dw
 AGdCAaHQhZFFGoYK472+o6pur0dxEA==
 =5dET
 -----END PGP SIGNATURE-----

Merge tag 'v6.0.0.beta3'

v6.0.0.beta3 release
2019-03-13 13:11:10 -04:00
eileencodes
7c87fd5635 Prep release
* Update RAILS_VERSION
* Bundle
* rake update_versions
* rake changelog:header
2019-03-11 11:58:15 -04:00
Hugo Vacher
2176f4b30c Fall back to parent locale before it falls back to the :errors namespace 2019-03-04 16:54:06 -05:00
Rafael Mendonça França
5e6e505083
Preparing for 6.0.0.beta2 release 2019-02-25 17:45:04 -05:00
Ryuta Kamizono
f8a798c8e6
Merge pull request #35336 from kamipo/dont_allow_non_numeric_string_matches_to_zero
Don't allow `where` with non numeric string matches to 0 values
2019-02-21 18:58:44 +09:00
Ryuta Kamizono
9c9c950d02 Revert "Speed up integer casting from DB"
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.
2019-02-21 13:11:42 +09:00
Ryuta Kamizono
357cd23d3a 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.
2019-02-20 22:00:56 +09:00
Ryuta Kamizono
b09d8f6bb3 Don't allow where with invalid value matches to nil values
That is considered as silently leaking information.
If type casting doesn't return any actual value, it should not be
matched to any record.

Fixes #33624.
Closes #33946.
2019-02-18 16:57:10 +09:00