aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Ensure to reset actually used `@connection.schema_migration`'s table nameRyuta Kamizono2019-06-151-4/+4
| | | | https://buildkite.com/rails/rails/builds/61744#f12cc6cf-7458-4131-917a-9735615f6259/999-1010
* Fix `test_schema_names` to include "hint_plan" schemaRyuta Kamizono2019-06-151-1/+5
|
* Merge pull request #36469 from ↵Eileen M. Uchitelle2019-06-148-49/+111
|\ | | | | | | | | eileencodes/move-while_preventing_writes-to-handler Move while_preventing_writes from conn to handler
| * Move while_preventing_writes from conn to handlereileencodes2019-06-148-49/+111
|/ | | | | | | | | | | | | | | | | If we put the `while_preventing_writes` on the connection then the middleware that sends reads to the primary and ensures they can't write will not work. The `while_preventing_writes` will only be applied to the connection which it's called on - which in the case of the middleware is Ar::Base. This worked fine if you called it directly like `OtherDbConn.connection.while_preventing_writes` but Rails didn't have a way of knowing you wanted to call it on all the connections. The change here moves the `while_preventing_writes` method from the connection to the handler so that it can block writes to all queries for that handler. This will apply to all the connections associated with that handler.
* Merge pull request #36488 from eileencodes/update-multi-db-docsEileen M. Uchitelle2019-06-141-1/+20
|\ | | | | Update multi-db docs
| * Update multi-db docseileencodes2019-06-141-1/+20
|/ | | | | | * Add note about schema cache * Add note about opening too many connections * Improve headers in caveats section
* Merge pull request #36439 from ↵Eileen M. Uchitelle2019-06-1417-150/+412
|\ | | | | | | | | eileencodes/move-schema-migration-to-migration-context Move SchemaMigration to migration_context
| * Move SchemaMigration to migration_contexteileencodes2019-06-1417-150/+412
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR moves the `schema_migration` to `migration_context` so that we can access the `schema_migration` per connection. This does not change behavior of the SchemaMigration if you are using one database. This also does not change behavior of any public APIs. `Migrator` is private as is `MigrationContext` so we can change these as needed. We now need to pass a `schema_migration` to `Migrator` so that we can run migrations on the right connection outside the context of a rake task. The bugs this fixes were discovered while debugging the issues around the SchemaCache on initialization with multiple database. It was clear that `get_all_versions` wouldn't work without these changes outside the context of a rake task (because in the rake task we establish a connection and change AR::Base.connection to the db we're running on). Because the `SchemaCache` relies on the `SchemaMigration` information we need to make sure we store it per-connection rather than on ActiveRecord::Base. [Eileen M. Uchitelle & Aaron Patterson]
* | Merge pull request #36484 from ↵Ryuta Kamizono2019-06-141-3/+3
|\ \ | |/ |/| | | | | albertoalmagro/alberto/reverse-column-is-reversible [ci skip] Update docs as `remove_column` can be reversed
| * [ci skip] Update docs as `remove_column` can be reversedAlberto Almagro2019-06-141-3/+3
| | | | | | | | | | As `remove_column` can be reversed when a type is provided this example was not accurate anymore.
* | Fix rubocop violationsRyuta Kamizono2019-06-142-3/+2
| |
* | Merge pull request #36478 from kamipo/allocation_on_demand_in_transactionRyuta Kamizono2019-06-141-20/+35
|\ \ | | | | | | Allocation on demand in transactions
| * | Allocation on demand in transactionsRyuta Kamizono2019-06-141-20/+35
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently 1,000 transactions creates 10,000 objects regardless whether it is necessary or not. This makes allocation on demand in transactions, now 1,000 transactions creates required 5,000 objects only by default. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.create } }.select { |k, _| k[0].end_with?("transaction.rb") } ``` Before (95d038f): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 209, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 80, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 8, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 289, :T_STRING]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 116, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 120, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 121, :T_HASH]=>[1000, 0, 714, 0, 1, 0]} ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 213, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 304, :T_STRING]=>[1000, 0, 738, 0, 1, 0]} ```
* | Merge pull request #36477 from albertoalmagro/alberto/button-to-default-pathRafael França2019-06-131-1/+1
|\ \ | | | | | | [ci skip] Use default path in button_to documentation
| * | [ci skip] Use default path in button_to documentationAlberto Almagro2019-06-131-1/+1
| |/ | | | | | | | | | | | | | | | | | | This is really a nit pick, but as this is the framework's documentation I think it should follow standards as many times as possible to avoid confusion in new users. If we were using `resources :articles` in routes. which is what scaffold adds, the generated helper would be `new_article_path` instead of `new_articles_path`.
* | Merge pull request #36437 from sudara/fix_programmatic_clicks_with_data_remoteGannon McGibbon2019-06-132-2/+16
|\ \ | |/ |/| Fix programmatic clicks with data-remote
| * Ensure non-mouse/programmatic clicks work with data-remoteSudara2019-06-132-2/+16
| |
* | Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)jeffdoering2019-06-133-1/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Make ActiveRecord `ConnectionPool.connections` thread-safe. ConnectionPool documentation is clear on the need to synchronize access to @connections but also states that public methods do not require synchronization. Existing code exposed @connections directly via attr_reader. The fix uses synchronize() to lock @connections then returns a copy to the caller using Array.dup(). Includes comments on the connections method that thread-safe access to the connections array does not imply thread-safety of accessing methods on the actual connections. Adds a test-case that modifies the pool using a supported method in one thread while a second thread accesses pool.connections. The test fails without this patch. Fixes #36465. * Update activerecord/test/cases/connection_pool_test.rb [jeffdoering + Rafael Mendonça França]
* | Merge pull request #36466 from wbnns/update-missing-create-action-screenshotRafael França2019-06-131-0/+0
|\ \ | | | | | | images/getting_started: Update screenshot for missing action
| * | [ci skip] images/getting_started: Update screenshot for missing actionWill Binns2019-06-121-0/+0
| | |
* | | Merge pull request #36388 from joelhawksley/actionview-componentAaron Patterson2019-06-134-1/+72
|\ \ \ | | | | | | | | Introduce ActionView::Component
| * | | `RenderingHelper` supports rendering objects that `respond_to?` `:render_in`Joel Hawksley2019-06-124-1/+72
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Co-authored-by: Natasha Umer <natashau@github.com> Co-authored-by: Aaron Patterson <tenderlove@github.com> Co-authored-by: Shawn Allen <shawnbot@github.com> Co-authored-by: Emily Plummer <emplums@github.com> Co-authored-by: Diana Mounter <broccolini@github.com> Co-authored-by: John Hawthorn <jhawthorn@github.com> Co-authored-by: Nathan Herald <myobie@github.com> Co-authored-by: Zaid Zawaideh <zawaideh@github.com> Co-authored-by: Zach Ahn <engineering@zachahn.com>
| * | | Merge pull request #2 from rails/masterJoel Hawksley2019-06-0365-293/+486
| |\ \ \ | | | | | | | | | | merge master
* | | | | Ensure to reset migration version after testing migrationRyuta Kamizono2019-06-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "schema_migrations" table may be hard dropped before, so the reset migration version should be done in ensure block. https://buildkite.com/rails/rails/builds/61697#18d6f3ac-2257-4f4b-8efc-4010464c4d9a/999-1011
* | | | | Reset migration version before testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | | | | | | | | | https://buildkite.com/rails/rails/builds/61695#373bb1a7-677f-49ec-95e7-a92467fefd60/1076-1084
* | | | | Avoid implicit rollback when testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "schema_migrations" is hard dropped by some existing tests, so testing migration in using transactional tests may cause implicit creation and rollback "schema_migrations" table, it makes migration tests flaky. https://buildkite.com/rails/rails/builds/61692#42383249-30be-4508-b1fb-a7bb27600c8e/999-1010 https://buildkite.com/rails/rails/builds/61694#6e462ad3-41d8-4e26-95ce-728495b0ac64/999-1010
* | | | | Merge pull request #36472 from kamipo/empty_line_only_before_access_modifierRyuta Kamizono2019-06-13434-514/+8
|\ \ \ \ \ | | | | | | | | | | | | Enable `Layout/EmptyLinesAroundAccessModifier` cop
| * | | | | Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-13434-514/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | | | Remove duplicated `table_exists?`Ryuta Kamizono2019-06-132-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `table_exists?` is already exist in `ModelSchema`. https://github.com/rails/rails/blob/5cab344494c340ea82a35b46efa06b94f0b7730b/activerecord/lib/active_record/model_schema.rb#L339-L341
* | | | | | Don't `drop_table` before schema cache testsRyuta Kamizono2019-06-131-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | Since 5cab34449, `drop_table` clears schema cache.
* | | | | | Clear schema cache when a table is created/dropped/renamedRyuta Kamizono2019-06-136-4/+18
|/ / / / / | | | | | | | | | | | | | | | Otherwise `Model.table_exists?` returns the staled cache result.
* | | | | Merge pull request #36470 from lucasprag/activesupport/missing-require-digestRafael França2019-06-121-0/+3
|\ \ \ \ \ | |_|_|/ / |/| | | | Add missing file to require digest/uuid on active_support core ext
| * | | | Add missing file to require digest/uuid on active_support core extensionsLucas Arantes2019-06-121-0/+3
|/ / / /
* | | | Merge pull request #36447 from holyketzer/masterRafael França2019-06-111-1/+1
|\ \ \ \ | | | | | | | | | | ActionCable: optimise logger.debug calling
| * | | | ActionCable: optimize logger.debug callingAlex Emelyanov2019-06-081-1/+1
| | | | |
* | | | | Merge pull request #36440 from malept/multi-db-abort_if_pending_migrations-taskEileen M. Uchitelle2019-06-113-1/+55
|\ \ \ \ \ | | | | | | | | | | | | Add support for multiple databases to `rails db:abort_if_pending_migrations`
| * | | | | Convert the db:abort_if_pending_migrations task to be multi-DB awareMark Lee2019-06-103-1/+55
| | | | | |
* | | | | | Merge pull request #36448 from ↵Ryuta Kamizono2019-06-117-42/+67
|\ \ \ \ \ \ | |/ / / / / |/| | | | | | | | | | | | | | | | | kamipo/allow_column_name_with_simple_function_call Allow column name with function (e.g. `length(title)`) as safe SQL string
| * | | | | Allow column name with function (e.g. `length(title)`) as safe SQL stringRyuta Kamizono2019-06-107-42/+67
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, almost all "Dangerous query method" warnings are false alarm. As long as almost all the warnings are false alarm, developers think "Let's ignore the warnings by using `Arel.sql()`, it actually is false alarm in practice.", so I think we should effort to reduce false alarm in order to make the warnings valuable. This allows column name with function (e.g. `length(title)`) as safe SQL string, which is very common false alarm pattern, even in the our codebase. Related 6c82b6c99, 6607ecb2a, #36420. Fixes #32995.
* | | | | | All modern adapters returns a numeric value as the result of numeric calculationRyuta Kamizono2019-06-112-10/+2
| | | | | |
* | | | | | Merge pull request #36452 from kojoma/kojoma/update_guidesKasper Timm Hansen2019-06-101-2/+2
|\ \ \ \ \ \ | | | | | | | | | | | | | | [ci skip] Fix rails/command.rb document
| * | | | | | [ci skip] Fix rails/command.rb documentkojoma2019-06-101-2/+2
| | | | | | |
* | | | | | | Bump `webdrivers`yuuji.yaginuma2019-06-101-7/+5
| |/ / / / / |/| | | | | | | | | | | | | | | | | Latest `webdrivers` is necessary to run ujs test.
* | | | | | Allow `column_name AS alias` as safe SQL stringRyuta Kamizono2019-06-105-0/+14
| | | | | |
* | | | | | Refactor `disallow_raw_sql!` to avoid `split(/\s*,\s*/)` to order argsRyuta Kamizono2019-06-096-21/+56
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `split(/\s*,\s*/)` to order args and then `permit.match?` one by one is much slower than `permit.match?` once.
* | | | | | Merge pull request #36117 from shioimm/fix_action_text_overviewYuji Yaginuma2019-06-091-0/+2
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | Fix installation on guides/source/action_text_overview.md
| * | | | | Fix installation on guides/source/action_text_overview.mdMisaki Shioi2019-06-081-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | gem 'image_proccessing' is required.
* | | | | | Merge pull request #36438 from freeletics/nodocEileen M. Uchitelle2019-06-071-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | Add forgotten nodoc to dump_schema method.
| * | | | | | Add forgotten nodoc to dump_schema method.Wojciech Wnętrzak2019-06-071-1/+1
|/ / / / / / | | | | | | | | | | | | | | | | | | Method added in https://github.com/rails/rails/pull/36416
* | | | | | NULLS { FIRST | LAST } is safe SQL string since ↵Ryuta Kamizono2019-06-071-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 6c82b6c99d86f37e61f935fb342cccd725d6c7d4 There is no need to be wrapped by `Arel.sql()`.