If one of the locals conflict with a keyword, typically `class`.
The potentially confusing part however is that if you define a default
value, `local_assigns` won't respect it.
Previously, it was very easy to accidentally leak a database password in
production logs if an error ends up calling inspect on a ConnectionPool
or an individual connection (Adapter). This is due to the default
`#inspect` output for Pools and Adapters being unnecessarily large, and
both currently including passwords (through the DatabaseConfig of a
Pool, and the internal configuration of an Adapter).
This commit addresses these issues by defining a custom `#inspect` for
ConnectionPool, AbstractAdapter, and DatabaseConfig. The condensed
`#inspect` only includes a few valuable fields instead of all of the
internals, which prevents both the large output and passwords from being
included.
Currently, we are using dynamic inclusions to
guarantee that the list of MODULES is always up to
date with what gets included into Base. However,
that prevents static analysis tools from
understanding the ancestors of controllers, which
prevents completion and other editor features from
working correctly. We can instead use a unit test
to verify that both lists are synchronized, which
retains the original behavior while allowing for
more accurate static analysis.
References https://github.com/rails/rails/pull/52012#issuecomment-2183415161
Revert "Merge pull request #52033 from Shopify/amend_lazy_routes_changelog"
This reverts commit 743128b2307b6e1bd59acb9dc8358592d264c573, reversing
changes made to 6622075802bdcca22ab3e32ef6e3f6d2b9a881f8.
Revert "Merge pull request #52012 from Shopify/defer_route_drawing"
This reverts commit 6622075802bdcca22ab3e32ef6e3f6d2b9a881f8, reversing
changes made to 5dabff4b7bf4cc5e2e552efb78c6a3f3e44bed37.
Since `__dir__` uses real path, when we load the gems from a symlinked path, Zeitwek loader
tries to load the ignored files and throws warnings or errors.
When there are no fields:
* Omit blank line in migration prior to "t.timestamps"
* Omit leading and trailing spaced in empty hashes in
create and update controller and api functional tests
Co-authored-by: zzak <zzakscott@gmail.com>
Some of them can be a bit costly to generate, particularly
when inspecting very large objects or when accessing the AST
of procs.
Minitest supports passing the message as a callable, which allow
us to defer all these computations.
Rails 7 apps require running in `zeitwerk` mode, so we no longer need
the migration guide. The "Autoloading and Reloading Constants" guide
also handles autoloading with Zeitwerk, including `autoload_paths`, the
`zeitwerk:check` command and more.
- [x] We talk about transactions and how to disable them early on, as part of the first few paragraphs, that could be a subsection to add separation.
- [x] The guide goes into reversible early, before even talking about generating migrations and other basic stuff, and then later talks more about it. I think it's important to lay the foundation of change vs up/down, and the fact that migrations should always try to be able to be rolled back, but maybe we can do that without going into reversible that early in the guide, which complicates things further.
- [x] We mention bin/rails generate --help to look for more details, but we could mention specific generators also offer help too, e.g. bin/rails generate model --help or bin/rails generate migration --help
- [x] There's a small section about composite PKs, but there's also [a whole guide on it](https://edgeguides.rubyonrails.org/active_record_composite_primary_keys.html), we can add a link to it from the migrations guide at least for more info on the topic.
- [x] The section that talks about execute shows an example of a specific model Product.connection.execute, but we can call execute directly within a migration, that might be simpler to show.
- [x] We talk about "seed data " in setup & preparing sections, but never mention it before, or explain what seed data is. There's a section about seed data later in the guide, perhaps we can link to it to facilitate.
- [x] In "running specific migrations", bin/rails db:migrate VERSION=zomg actually raises a different error: Invalid format of target version: `VERSION=zomg`... might need a different example there for the unknown migration version error. (no need to show this invalid format one)
- [x] The section about referential integrity can likely be better explained, in particular I think it's worth mentioning that even though the "AR method/way" (the pattern, I mean) doesn't think such "intelligence" belongs to the database, foreign keys and unique indexes are generally safer at the database level. (and should likely have their counterparts explicitly added in code with associations and validations). I just don't want to convey that someone shouldn't be adding FKs & unique constraints, because not adding these can definitely bite you.
Other changes:
- [x] Moved sections around
- [x] Added a section about "Rails Migration Version Control"
- [x] Added a section about "Using UUIDs instead of IDs for Primary Keys"
Co-authored-by: bhumi1102 <bhumi1102@gmail.com>
Co-authored-by: Bartosz Łęcki <bart.lecki@gmail.com>
Co-authored-by: Cecile Veneziani <cveneziani@users.noreply.github.com>
Co-authored-by: hatsu <hajiwata0308@gmail.com>
Co-authored-by: Petrik de Heus <petrik@deheus.net>
Co-authored-by: Ahmad hamza <ahmad.hamza@toptal.com>
Co-authored-by: Bart de Water <496367+bdewater@users.noreply.github.com>
Co-authored-by: Amanda Perino <58528404+AmandaPerino@users.noreply.github.com>
Co-authored-by: Andy Atkinson <andyatkinson@gmail.com>
Co-authored-by: Jamie Gaskins <jgaskins@gmail.com>
Currently, there is no (simple) way to ask a model if it connects to a
single database or to multiple shards. Furthermore, without looping
through a model's connections, I don't believe there's an easy way to
return a list of shards a model can connect to.
This commit adds a `@shard_keys` ivar that's set whenever `.connects_to`
is called. It sets the ivar to the result of `shards.keys`. `shards` in
`.connects_to` defaults to an empty hash and therefore when calling
`connects_to database: {...}` `@shard_keys` will be set to an empty array.
`@shard_keys` is set _before_ the following lines:
```
if shards.empty?
shards[:default] = database
end
```
This conditional sets the one and only shard (`:default`) to the value of `database`
that we pass to `.connects_to`. This allows for calling
`connected_to(shard: :default)` on models configured to only connect to
a database e.g.:
```ruby
class UnshardedBase < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary }
end
class UnshardedModel < UnshardedBase
end
UnshardedBase.connected_to(shard: :default) {
UnshardedBase.connection_pool.db_config.name } => primary
```
This is ultimately still an _unsharded_ model which is why `@shard_keys`
gets set before the conditional.
With the new `@shard_keys` ivar we need a way for descendants of the
abstract AR model to return that same value. For that we leverage the
existing `.connection_class_for_self` method. That method returns the
ancestor of the model where `.connects_to` was called, or returns self if
it's the connection class:
```ruby
class UnshardedBase < ActiveRecord::Base
self.abstract_class = true
connects_to database: { writing: :primary }
end
class UnshardedModel < UnshardedBase
end
ActiveRecord::Base.connection_class_for_self => ActiveRecord::Base
UnshardedBase.connection_class_for_self => UnshardedBase(abstract)
UnshardedModel.connection_class_for_self => UnshardedBase(abstract)
```
The new `.shard_keys` method is a getter which returns the value of
`@shard_keys` from the connection class or it returns an empty array.
The empty array is necessary in cases where `connects_to` was never
called.
Finally, I've added an `.connected_to_all_shards` method which takes all of the
arguments for `.connected_to` except for `shard`. Instead, it loops through
every shard key and then delegates everything else to `.connected_to`. I've
used `.map` instead of `.each` so that we can collect the results of each block.
Fix#51284
Both Proxy controllers in Active Storage set the caching headers early
before streaming.
In some instances (see #51284), it is possible for the file
download (from the service to server) to fail before we send the first
byte to the client (response not yet committed).
In those instances, this change would invalidate the cache and return
a better response status before closing the stream.
