aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Allow using env var to specify pidfileBen Thorner2019-06-194-6/+32
| | | | | | | | | | | | | | | | Previously it was only possible to specify the location of the pidfile for the 'rails server' command with the '-P' flag. This adds support for specifying the pidfile using a PIDFILE env var, which can still be overridden by the '-P' flag and with the default pidfile path unchanged. The motivation for this feature comes from using Docker to run multiple instances of the same rails app. When developing a rails app with Docker, it's common to bind-mount the rails root directory in the running container, so that changes to files are shared between the container and the host. However, this doesn't work so well with the pidfile and it's necessary to (remember to) add a '-P' flag to the 'rails server' command line; being able to specify this flag using an env var would make developing with Rails+Docker a bit simpler.
* 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()`.
* | | | | | Remove redundant blank line at the bottom of the generated controller testyuuji.yaginuma2019-06-071-1/+1
| | | | | |
* | | | | | Merge pull request #36429 from bogdan/fix-preloading-duplicate-recordsRyuta Kamizono2019-06-072-1/+8
|\ \ \ \ \ \ | | | | | | | | | | | | | | Fix preloading on AR::Relation where records are duplicated by a join
| * | | | | | Fix preloading on AR::Relation where records are duplicated by a joinBogdan Gusiev2019-06-062-1/+8
| | | | | | |
* | | | | | | Merge pull request #36422 from jhawthorn/parallelize_actionviewJohn Hawthorn2019-06-063-39/+13
|\ \ \ \ \ \ \ | |_|_|_|_|_|/ |/| | | | | | Run actionview tests in parallel
| * | | | | | Run actionview tests in parallelJohn Hawthorn2019-06-052-0/+12
| | | | | | |
| * | | | | | Remove actionview tests which modify fixturesJohn Hawthorn2019-06-051-39/+1
| | |_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We shouldn't modify fixtures (or any files which are checked-in). It prevents us from parallelizing, and probably has other issues. We could fix these tests by copying the file to a tmpdir and modifying it there, but I don't think they are testing anything useful anymore. Re-initializing a resolver isn't representative of "uncached" rendering (either in dev-mode or using lookup_context.disable_cache).
* | | | | | Merge pull request #36431 from alexgomez54/patch-1Rafael França2019-06-061-0/+2
|\ \ \ \ \ \ | | | | | | | | | | | | | | Include warning in docs about polymorphism in underlying tables