Commit Graph

20694 Commits

Author SHA1 Message Date
Jean Boussier
de3d1dd8f9
Merge pull request #42015 from Shopify/refactor-inheritance-column
Make ModelSchema.inheritance_column a class_attribute
2021-04-20 19:48:38 +02:00
Ryuta Kamizono
5d7b86b698
Merge pull request #42023 from caffkane/remove-unused-ar-disable-join-constant
Remove unused constant from disable_joins_association_relation
2021-04-20 17:51:45 +09:00
Ryuta Kamizono
f02a136f89 Merge pull request #41990 from p8/autosave-association-callbacks-get-called-once
Ensure has_one autosave association callbacks get called once
2021-04-20 17:50:16 +09:00
Logan Kane
6f23b69a1e Remove unused constant from disable_joins_association_relation 2021-04-19 22:26:33 -07:00
Yasuo Honda
662f7c14c6 Address non-deteristic CI failure of HasManyThroughDisableJoinsAssociationsTest
Addressed CI failure at https://buildkite.com/rails/rails/builds/76751#fa683e3c-b213-4474-8faf-dd37e6444b76
and modified two tests to avoid similar failures.

- HasManyThroughDisableJoinsAssociationsTest#test_to_a_on_disable_joins_through

```
Failure:
HasManyThroughDisableJoinsAssociationsTest#test_to_a_on_disable_joins_through [/rails/activerecord/test/cases/associations/has_many_through_disable_joins_associations_test.rb:71]:
--- expected
+++ actual
@@ -1 +1 @@
-[#<SpecialComment id: 11, post_id: 7, body: "go crazy", type: "SpecialComment", label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: nil, origin_type: nil, developer_id: nil, updated_at: "2021-04-19 22:54:11.359941000 +0000", deleted_at: nil, comments: nil, company: nil>, #<Comment id: 71, post_id: 65, body: "text", type: nil, label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: 70, origin_type: "Member", developer_id: nil, updated_at: "2021-04-19 22:54:12.138620000 +0000", deleted_at: nil, comments: nil, company: nil>, #<Comment id: 70, post_id: 64, body: "text", type: nil, label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: 69, origin_type: "Member", developer_id: nil, updated_at: "2021-04-19 22:54:12.131832000 +0000", deleted_at: nil, comments: nil, company: nil>]
+[#<SpecialComment id: 11, post_id: 7, body: "go crazy", type: "SpecialComment", label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: nil, origin_type: nil, developer_id: nil, updated_at: "2021-04-19 22:54:11.359941000 +0000", deleted_at: nil, comments: nil, company: nil>, #<Comment id: 70, post_id: 64, body: "text", type: nil, label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: 69, origin_type: "Member", developer_id: nil, updated_at: "2021-04-19 22:54:12.131832000 +0000", deleted_at: nil, comments: nil, company: nil>, #<Comment id: 71, post_id: 65, body: "text", type: nil, label: "default", tags_count: 0, children_count: 0, parent_id: nil, author_type: nil, author_id: nil, resource_id: nil, resource_type: nil, origin_id: 70, origin_type: "Member", developer_id: nil, updated_at: "2021-04-19 22:54:12.138620000 +0000", deleted_at: nil, comments: nil, company: nil>]
```

- HasManyThroughDisableJoinsAssociationsTest#test_pluck_on_disable_joins_through
Reproduced one time at local environment.

```ruby
...........F

Failure:
HasManyThroughDisableJoinsAssociationsTest#test_pluck_on_disable_joins_through [/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/associations/has_many_through_disable_joins_associations_test.rb:47]:
Expected: [31, 11, 32]
  Actual: [11, 31, 32]

bin/test test/cases/associations/has_many_through_disable_joins_associations_test.rb:46

```

-  HasManyThroughDisableJoinsAssociationsTest#test_pluck_on_disable_joins_through_using_custom_foreign_key

Were not reproduced locally but it could also fail due to
non-determistic order.
2021-04-20 11:56:34 +09:00
eileencodes
de6b4efa3e
Add option to skip joins for associations.
In a multiple database application, associations can't join across
databases. When set, this option tells Rails to make 2 or more queries
rather than using joins for associations.

Set the option on a has many through association:

```ruby
class Dog
  has_many :treats, through: :humans, disable_joins: true
  has_many :humans
end
```

Then instead of generating join SQL, two queries are used for `@dog.treats`:

```
SELECT "humans"."id" FROM "humans" WHERE "humans"."dog_id" = ?  [["dog_id", 1]]
SELECT "treats".* FROM "treats" WHERE "treats"."human_id" IN (?, ?, ?)  [["human_id", 1], ["human_id", 2], ["human_id", 3]]
```

This code is extracted from a gem we use internally at GitHub which
means the implementation here is used in production daily and isn't
experimental.

I often get the question "why can't Rails do this automatically" so I
figured I'd include the answer in the commit. Rails can't do this
automatically because associations are lazily loaded. `dog.treats` needs
to load `Dog`, then `Human` and then `Treats`. When `dog.treats` is
called Rails pre-generates the SQL that will be run and puts that
information into a reflection object. Because the SQL parts are pre-generated,
as soon as `dog.treats` is loaded it's too late to skip a join. The join
is already available on the object and that join is what's run to load
`treats` from `dog` through `humans`. I think the only way to avoid setting
an option on the association is to rewrite how and when the SQL is
generated for associations which is a large undertaking. Basically the
way that Active Record associations are designed, it is currently
impossible to have Rails figure out to not join (loading the association
will cause the join to occur, and that join will raise an error if the
models don't live in the same db).

The original implementation was written by me and Aaron. Lee helped port
over tests, and I refactored the extraction to better match Rails style.

Co-authored-by: Lee Quarella <leequarella@gmail.com>
Co-authored-by: Aaron Patterson <aaron@rubyonrails.org>
2021-04-19 11:17:31 -04:00
Jean Boussier
5877562f56 Make ModelSchema.inheritance_column a class_attribute
Ultimately it's exactly the same semantic that was implemented
using `superclass`, except it avoid recursion and benefit from
method cache, so much faster the longer the inheritance chain.

The use of an alias_method chain is unfortunate, but I don't
see any alternative to cast the provided value to string.
2021-04-19 11:17:39 +02:00
Ryuta Kamizono
9a263e9a0f
Merge pull request #42001 from ricardotk002/fix-encrypted-fixtures-test
Fix test `EncryptableFixtureTest` that fails intermittently
2021-04-17 13:10:01 +09:00
Ricardo Díaz
edc481a66d Fix test EncryptableFixtureTest that fails intermittently
Reference:

https://buildkite.com/rails/rails/builds/76506#f42a5ab3-1923-4a9f-b843-56c4098b6bef
https://buildkite.com/rails/rails/builds/76590#04721d73-9505-4c29-82bf-0a1be1f41649
https://buildkite.com/rails/rails/builds/76556#aa284ad0-0eab-48e9-a50b-60e3b5c7ccac

Apparently when loading different fixtures that reference the same
table, the files that are loaded last run a `DELETE FROM` statement
that removes previous data.

Reproduction command using `minitest_bisect`:

```
activerecord $ bin/test -a mysql2 --seed 22031 -n "/^(?:ActiveRecord::Encryption::EncryptableRecordTest#(?:test_when_downcase:_true_it_creates_content_downcased)|ActiveRecord::Encryption::EncryptableFixtureTest#(?:test_fixtures_get_encrypted_automatically))$/"
```
2021-04-16 13:06:05 -05:00
Ashik Salman
8e1b191b37 Updated abbreviation for single table inheritance. 2021-04-16 11:53:05 +05:30
Rafael Mendonça França
6d624c648c
Make sure the config values is set back to the original value
If the test fails the config should be reset as well.
2021-04-15 23:36:26 +00:00
Rafael Mendonça França
7b198e6024
Copy edit #41718 2021-04-15 23:34:30 +00:00
Rafael França
7ce853ecb0
Merge pull request #41718 from dzunk/enumerate_columns
Add setting for enumerating column names in SELECT statements
2021-04-15 19:28:41 -04:00
Rafael Mendonça França
8f07a124ef
Eager load ActiveRecord::StatementCache 2021-04-15 22:09:21 +00:00
Petrik
2a786431e2 Ensure has_one autosave association callbacks get called once
When saving a record, autosave adds callbacks to save its' associations.
Since the associations can have similar callbacks for the inverse,
endless loops could occur.

To prevent these endless loops, the callbacks for `has_many` and
`belongs_to` are defined as methods that only execute once.
This is implemented in the `define_non_cyclic_method` method.
However, this wasn't used for the `has_one` callbacks.

While `has_one` association callbacks didn't result in endless loops,
they could execute multiple times.
For example for a bidirectional `has_one` with autosave enabled,
the `save_has_one_association` gets called twice:

    class Pirate < ActiveRecord::Base
      has_one :ship, autosave: true

      def save_has_one_association(reflection)
        @count ||= 0
        @count += 1 if reflection.name == :ship
        super
      end
    end

    class Ship < ActiveRecord::Base
      belongs_to :pirate, autosave: true
    end

    pirate = Pirate.new(catchphrase: "Aye")
    pirate.build_ship(name: "Nights Dirty Lightning")
    pirate.save!
    # this returns 2 instead of 1.
    assert_equal 1, pirate.instance_variable_get(:@count)

This commit changes `has_one` autosave callbacks to be non-cyclic as
well. By doing this the autosave callback are made more consistent for
all 3 cases: `has_many`, `has_one` and `belongs_to`.
2021-04-15 22:30:59 +02:00
Eileen M. Uchitelle
80a23227ea
Merge pull request #41839 from abhaynikam/fix-return-type-for-strict-loading
ActiveRecord#strict_loading! should return boolean instead of current mode set.
2021-04-15 08:49:33 -04:00
Abhay Nikam
b841b2a84e Adds documentation for strict_loading_n_plus_one_only? [ci skip] 2021-04-15 10:35:11 +05:30
Abhay Nikam
2629f48ced ActiveRecord#strict_loading! should return boolean instead of mode set.
The return type was changed in the PR #41704 after addition of mode
option. The current documentation is misleading since
documentation puropose strict_loading! would return boolean whereas
it returns the current mode set.

I can across this issue while debugging issue: #41827 and thought
this should be brought to the attention.

PR fixes the issue and would always return boolean based on
strict_loading is enabled or disabled.

```
user.strict_loading! # => true
user.strict_loading!(false) # => false
user.strict_loading!(mode: :n_plus_one_only) # => true
```
2021-04-15 10:22:18 +05:30
John Bampton
cf05afc990 chore: fix spelling 2021-04-15 14:14:59 +10:00
John Bampton
5afc11f897 chore: fix spelling 2021-04-15 12:32:57 +10:00
Matt Duszynski
726abeaab4 Add setting for enumerating column names in SELECT statements 2021-04-14 15:48:17 -07:00
Rafael França
16c0f388be
Merge pull request #41860 from p8/fix-previously-saved-for-autosaved-record
Only update dirty attributes once for cyclic autosave callbacks
2021-04-14 16:05:28 -04:00
Bastian Bartmann
0daf81f69a upsert_all fails cleanly for MySQL 2021-04-14 19:33:42 +02:00
Zachary Scott
43e29f0f5d
Merge pull request #41945 from jbampton/fix-grammar
chore: fix grammar, spelling and minor whitespace fix
2021-04-14 09:19:15 +09:00
Petrik
ae56ecd467 Only update dirty attributes once for cyclic autosave callbacks
Calling save on a record with cyclic autosave callbacks, can call other
callbacks and hooks multiple times. This can lead to unexpected
behaviour.

For example `save` gets called twice on Post in the following example.
This results in `changes_applied` getting called twice.

    class Post < ApplicationRecord
      belongs_to :postable, polymorphic: true, inverse_of: :post
    end

    class Message < ApplicationRecord
      has_one :post, as: :postable
    end

    post = Post.create!(postable: Message.new(subject: "Hello, world!"))
    # the following would return false when true is expected
    post.id_previously_changed?

`save` gets called twice because Post autosaves Message, which
autosaves Post again.

Instead of calling `changes_applied` everytime `save` is called,
we can skip it if it has already been called once in the current saving
cycle. This requires us to track the `@_saving` state of a record.
if `@_saving` is true we know we the record is being saved.

To track if a method has already been called we reuse the
@_already_called hash that is already used for this purpose.
2021-04-13 21:39:29 +02:00
Petrik
406374de09 Add regression test for #41714
Commit a1a5d37749964b1e1a23914ef13da327403e34cb tried to fix doubles
saves in autosaved associations. However, it didn't work properly for
non-nullable foreign_keys and had to be reverted.

To prevent this in the future, add a regression test.
2021-04-13 20:45:13 +02:00
Rafael Mendonça França
94b954576a
Autocorrect Rubocop roles 2021-04-13 18:32:25 +00:00
Ryuta Kamizono
e68cd6fe96 Remove unused HstorePair constant
It is no longer used since 98bf64bcb9648f88bff4cb59a7ae4db2b6410241.
2021-04-14 00:51:36 +09:00
John Bampton
a0b8ac3eb2
Fix spelling in HasOneAssociationsTest 2021-04-13 10:46:55 -04:00
Jean Boussier
67c0f9aa25
Merge pull request #41930 from Shopify/optimize-hstore-parser
Optimize the HStore parser
2021-04-13 15:46:26 +02:00
Andrew White
98bf64bcb9 Use StringScanner to parse Hstore payloads 2021-04-13 15:32:09 +02:00
John Bampton
c5fe7b8d9a
Fix spelling in AsynchronousQueryInsideTransactionError docs [ci skip] 2021-04-13 09:17:47 -04:00
John Bampton
33baf0dabd
Fix spelling of EnvelopeEncryptionPerformanceTest 2021-04-13 09:17:11 -04:00
Jorge Manrubia
170bc08eae Remove fixed "id" in encrypted book fixtures
Hopefully this will help with flaky encrypted_fixture_test failing from
time to time due to not finding book with id=1.
2021-04-13 08:55:44 -04:00
Jorge Manrubia
ae7f693647 Fix: assign previous encryption schemes via previous: config option.
The previous: config option was being skipped because it was checking
the existence of a reader method instead of the accessor itself.

This also adds a test for the .configure option that was missing.
2021-04-13 08:55:44 -04:00
John Bampton
bf79740600
Fix spelling in Encryptor.encrypt docs [ci skip] 2021-04-13 08:39:38 -04:00
John Bampton
97cf9c21fb
Fix spelling in EncryptionSchemesTest 2021-04-13 08:39:01 -04:00
John Bampton
54e526e473 chore: fix grammar, spelling and minor whitespace fix 2021-04-13 21:35:50 +10:00
John Bampton
285d7d4eaf docs: change user name to the more used username
Remove an unneeded comma
2021-04-13 15:54:49 +10:00
Rafael Mendonça França
14bca25975
Make sure establish_connection with symbol work properly
Before when calling `preventing_writes?` in the connection the code
would raise a `NoMethodError` when calling `current_preventing_writes`
in a String.

This problem only happen when `establish_connection` is called on the
`ConnectionHandler` and not in the model itself.

This also removes the conditional from the `PoolConfig`.
2021-04-12 22:27:16 +00:00
Jean Boussier
6a5fb7dbd4
Merge pull request #41911 from Shopify/simplify-proxy-call
Allow to pass the method signature when defining attribute methods
2021-04-12 22:36:32 +02:00
Rafael França
a18f8a9a17
Merge pull request #41899 from sebastian-palma/add-for-docs
Add QueryMethods#for docs [ci skip]
2021-04-12 16:23:36 -04:00
Rafael França
32db8149ed
Merge pull request #41926 from jbampton/fix-favorite
chore: fix spelling change `favourite` to the more used `favorite`
2021-04-12 16:14:56 -04:00
Rafael França
b171b842da
Merge pull request #41933 from palkan/feat/upsert-all-returning-update-sql
Add ability to provide raw SQL as returning and update to #upsert_all
2021-04-12 15:29:52 -04:00
Rafael Mendonça França
4354e3ae49
Don't define methods using the method modifier in the same line as the method
Our style guide use block method modifiers, not inline method modifiers.
2021-04-12 18:49:54 +00:00
Vladimir Dementyev
4bef82217c Force Arel.sql for returning and on_duplicate 2021-04-12 20:23:46 +03:00
Vladimir Dementyev
8f3c12f880 Add update_sql option to #upsert_all 2021-04-12 19:09:17 +03:00
Vladimir Dementyev
c7613dde53 Support string returning clause for AR#insert_all
(cherry picked from commit 15b3310a4d6b2044da4ff79737e2b19fef9b6267)
2021-04-12 18:08:04 +03:00
Ryuta Kamizono
9aed3dcdfe Remove unused Category.has_and_belongs_to_many :popular_grouped_posts definition
This was added at 97403ad but it was never used.
2021-04-12 15:05:42 +09:00
John Bampton
11557e6cec chore: fix spelling change favourite to the more used favorite 2021-04-12 12:35:12 +10:00