An Active Storage `Blob` must be identified before it can be reliably
validated. For direct uploads, a `Blob` is identified when it is
attached, rather than when it is created.
Before this commit, the sequence of events when attaching a `Blob` was:
1. Find the `Blob`.
2. Assign the `Blob` to an `Attachment`.
3. Save the owner record.
4. Save the `Attachment`.
5. Identify the `Blob`'s true `content_type` from its file.
6. Save the `Blob`.
This meant that the owner record's validations might not see the
`Blob`'s true `content_type`.
After this commit, the sequence of events will be:
1. Find the `Blob`.
2. Identify the `Blob`'s true `content_type` from its file.
3. Assign the `Blob` to an `Attachment`.
4. Save the owner record.
5. Save the `Attachment`.
6. Save the `Blob`.
Thus the `Blob`'s true `content_type` will be available when running the
owner record's validations.
In `connected_to` one of the deprecated arguments wasn't well tested so
the incorrect methods signature wasn't caught by the tests.
This change updates the caller when `connected_to` uses the database
key.
I've also cleaned up a few arguments that weren't necessary. Since
the handler methods set defaults for the `shard` key, we don't need to
pass that in `establish_connection` when not using the sharding API.
This change makes a helper method for resetting connection handlers in
the Active Record tests. The change here is relatively small and may
seem unnecessary. The reason we're pushing this change is for upcoming
refactoring to connection management. This change will mean that we can
update one location instead of 9+ files to reset connections. It will
reduce the diff of our refactoring and make reusing this code easier in
the future.
The method name chosen is purposefully `clean_up_connection_handler`
over `clean_up_connection_handlers` because in the future there will
only be one handler.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
This change ensures that the connection methods are using kwargs instead
of positional arguments. This change may look unnecessary but we're
working on refactoring connection management to make it more robust and
flexible so the method signatures of the methods changed here will
continue to evolve and change.
This commit does not change any released public APIs. The `shard` and
`owner_name` arguments were added in 6.1 which is not released yet.
Using kwargs will allow these methods to be more flexible and not get
super ugly as we change their underlying behavior. The kwargs let us
support multiple non-positional arguments with default.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
This change renames the following:
* `current_pool_key` -> `current_shard`
* `default_pool_key` -> `default_shard`
* `pool_key` -> `shard`
Originally we had intended to name the `shard` as `pool_key` because
when we implemented the internal private API we weren't sure how it was
going to be used for sharding and wanted to implement behavior without
promising a public API. Now that we have a public API for sharding it's
better to use the same name everywhere rather than have one name for
private APIs and one name for public APIs. This should make
contributions and tracking down bugs easier in the future.
This PR doesn't require any deprecations because the sharding API is
unreleased and so is all the internal code that was using `pool_key`.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
While debugging a different problem I'm working on I realized that this
method `with_temporary_connection_pool` isn't necessary in most of the
cases we're using it for.
Anywhere we establish new connections inside the block won't throw away
those new connections. I also removed this from places that can use the
existing connection and don't need a new temporary pool. I'm not sure if
this file was using it in many places because of copy / paste or real
issues that are no longer present.
I had found the issue while working on fixing #33525.
That is if duplicated association has a scope which has `where` with
explicit table name condition (e.g. `where("categories.name": "General")`),
that condition in all duplicated associations will filter the first one
only, other all duplicated associations are not filtered, since
duplicated joins will be aliased except the first one (e.g.
`INNER JOIN "categories" "categories_categorizations"`).
```ruby
class Author < ActiveRecord::Base
has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }, class_name: "Categorization"
has_many :general_posts, through: :general_categorizations, source: :post
end
authors = Author.eager_load(:general_categorizations, :general_posts).to_a
```
Generated eager loading query:
```sql
SELECT "authors"."id" AS t0_r0, ... FROM "authors"
-- `has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }`
LEFT OUTER JOIN "categorizations" ON "categorizations"."author_id" = "authors"."id"
INNER JOIN "categories" ON "categories"."id" = "categorizations"."category_id" AND "categories"."name" = ?
-- `has_many :general_posts, through: :general_categorizations, source: :post`
---- duplicated `through: :general_categorizations` part
LEFT OUTER JOIN "categorizations" "general_categorizations_authors_join" ON "general_categorizations_authors_join"."author_id" = "authors"."id"
INNER JOIN "categories" "categories_categorizations" ON "categories_categorizations"."id" = "general_categorizations_authors_join"."category_id" AND "categories"."name" = ? -- <-- filtering `"categories"."name" = ?` won't work
---- `source: :post` part
LEFT OUTER JOIN "posts" ON "posts"."id" = "general_categorizations_authors_join"."post_id"
```
Originally eager loading with join scope didn't work before Rails 5.2
(#29413), and duplicated through association with join scope raised a
duplicated alias error before alias tracking is improved in 590b045.
But now it will potentially be got incorrect result instead of an error,
it is worse than an error.
To fix the issue, it makes eager loading to deduplicate / re-use
duplicated through association if possible, like as `preload`.
```sql
SELECT "authors"."id" AS t0_r0, ... FROM "authors"
-- `has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }`
LEFT OUTER JOIN "categorizations" ON "categorizations"."author_id" = "authors"."id"
INNER JOIN "categories" ON "categories"."id" = "categorizations"."category_id" AND "categories"."name" = ?
-- `has_many :general_posts, through: :general_categorizations, source: :post`
---- `through: :general_categorizations` part is deduplicated / re-used
LEFT OUTER JOIN "posts" ON "posts"."id" = "categorizations"."post_id"
```
Fixes#32819.
The GitHub gist API page is out of date. This commit replaces it with
the new link.
Also, removed unnecessary commas, added missing fullstop and fixed
a ruby snippet which wasn't rendered correctly before.
Removes the use of `ActiveRecord::AdvisoryLockBase` since it inherits
from `ActiveRecord::Base` and hence share module attributes that are defined in `ActiveRecord::Base`.
This is problematic because establishing connections through
`ActiveRecord::AdvisoryLockBase` can end up changing state of the default
connection handler of `ActiveRecord::Base` leading to unexpected
behaviors in a Rails application.
In the case of https://github.com/rails/rails/issues/39157,
Running migrations with `rails db:migrate:primary_shard_one` was not working as
the application itself defined the following
```
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
connects_to shards: {
default: { writing: :primary },
shard_one: { writing: :primary_shard_one }
}
end
```
In the database migrate rake task, the default connection was
established with the database config of `primary_shard_one`. However,
the default connection was altered to that of `primary` because
`ActiveRecord::AdvisoryLockBase.establish_connection` ended up loading
`ApplicationRecord` which calls `connects_to shards:`. Since all we
really need here is just a normal database connection, we can avoid
accidentally altering the default connection handler state during the migration
by creating a custom connection handler used for retrieving a connection.
Currently, the file generated via the generator is pluralized
but the parent class is singluar.
example: bundle exec rails g scaffold Pet name:string --database=animals
The above command should generate: apps/models/animals_record.rb
but the pets model would inherit from: `AnimalRecord` as
`"animals".classify` would be `Animal`
This will throw the `uninitialized constant AnimalRecord Did you mean? AnimalsRecord`
error.
Regardless of whether doing implicit `create_with` or not, `sti_name` is
set by `ensure_proper_type`.
11f54e12b9/activerecord/lib/active_record/inheritance.rb (L289-L304)
The purpose of the `create_with` is to suppress/override `type_condition`,
since `type_condition` will be an array of all sti subclass names for
`scope_for_create`, it is not desired behavior for `scope_for_create`,
and it will cause `find_sti_class` to fail.
That undesired behavior was derived from `In` arel node was accidentally
a subclass of `Equality` node, IMHO it is considered as almost a bug.
But someone depends on the behavior for now (see also #39288).
Unfortunately that implicit `create_with` had an unwanted side effect to
fail `or` with STI subclass relation #39956.
This changes a way to suppress/override `type_condition`, from doing
implicit `create_with` to excepting `type_condition` in `scope_for_create`,
to fix the `or` issue #39956.
For example, for a plugin like `my_plugin`, this makes the
`MyPlugin::VERSION` constant available to all consumers by default.
This commit also replaces the default generated test with a test that
the developer is less likely to delete.
