aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Fix TranslationHelper#translate handling of Hash defaultsJean Boussier2019-06-142-1/+6
| | | | | | | | | | | | | | It is sometimes expected of the `translate` methods to return a Hash, for instance it's the case of the `number.format` key. As such users might need to specify a Hash default, e.g. `translate(:'some.format', default: { separator: '.', delimiter: ',' })`. This works as expected with the `I18n.translate` methods, however `TranslationHelper#translate` apply `Array()` on the default value. As a result the default value end up as `[:separator, '.', :delimiter, ',']`.
* 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
| * | | | | | Include warning in docs about polymorphism in underlying tablesAlex Gomez2019-06-061-0/+2
|/ / / / / / | | | | | | | | | | | | We had a bug whereby we changed the namespace on a model using ActiveStorage, which resulted in broken download links. The reason this happened is because the `active_storage_attachments` table is a polymorphic join table that records the model's class name at the time of record creation, and uses this `record_type` in queries. Since the model namespace changed, the queries did not return the blob as expected. Discussed with @rafaelfranca, who suggested adding a warning about this in the docs.
* | | | | | Merge pull request #36270 from Edouard-chin/ec-on-rotation-constructorKasper Timm Hansen2019-06-063-4/+50
|\ \ \ \ \ \ | | | | | | | | | | | | | | Allow `on_rotation` in MessageEncryptor to be passed in constructor:
| * | | | | | Allow `on_rotation` in MessageEncryptor to be passed in constructor:Edouard CHIN2019-06-063-4/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Use case: I'm writing a wrapper around MessageEncryptor to make things easier to rotate a secret in our app. It works something like ```ruby crypt = RotatableSecret.new(['old_secret', 'new_secret']) crypt.decrypt_and_verify(message) ``` I'd like the caller to not have to care about passing the `on_rotation` option and have the wrapper deal with it when instantiating the MessageEncryptor object. Also, almost all of the time the on_rotation should be the same when rotating a secret (logging something or StatsD event) so I think it's not worth having to repeat ourselves each time we decrypt a message.