aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #36424 from y-yagi/unlock_selenium-webdriverYuji Yaginuma2019-06-063-14/+14
|\ | | | | Unlock `selenium-webdriver` gem version
| * Fix broken driver testyuuji.yaginuma2019-06-061-2/+2
| | | | | | | | | | | | Since `selenium-webdrive` v3.1.30, use `goog:chromeOptions'` key for sending chrome options. Ref: https://github.com/SeleniumHQ/selenium/commit/0ba8188b1a26ff3587f08afa6b6182c32479e980
| * Unlock `selenium-webdriver` versionyuuji.yaginuma2019-06-062-12/+12
|/ | | | | | `selenium-webdriver` is deprecateing various features for improvement in 4.0. I want to test with the latest version to check if Rails uses deprecated features or not.
* Merge pull request #36371 from eileencodes/move-schema-cache-to-poolEileen M. Uchitelle2019-06-056-7/+33
|\ | | | | Move schema cache to pool
| * Move schema cache from connection to pooleileencodes2019-06-056-7/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR proposes moving the schema cache from the connection to the pool so the connection can ask the pool for the cache. In a future PR our goal is to be able to read the yaml file from the pool so we can get rid of the `active_record.check_schema_cache_dump` initializer. This will fix the issues surrounding dumping the schema cache and mulitple databases. Why do we want to get rid of the initializer you ask? Well I was looking at #34449 and trying to make it work for our usecase and it revealed A LOT of problems. There are a few issues that I will fix in remaining PRs with SchemaMigration, but there's a big glaring issue with this initializer. When you have an application with multiple databases we'll need to loop through all the configurations and set the schema cache on those connections. The problem is on initialization we only have one connection - the one for Ar::Base. This is fine in a single db application but not fine in multi-db. If we follow the pattern in #34449 and establish a connection to those other dbs we will end up setting the cache on the _connection object_ rather than on all connections that connect for that config. So even though we looped through the configs and assigned the cache the cache will not be set (or will be set wrong) once the app is booted because the connection objects after boot are _different_ than the connection objects we assigned the cache to. After trying many different ways to set the schema cache `@tenderlove` and I came to the conclusion that the initializer is problematic, as is setting the schema cache twice. This is part 1 to move the cache to the pool so the cache can read from the schema cache yaml file instead of setting it when initializing the app. To do this we have created a `NullPool` that initializes an empty cache. I put the `get_schema_cache` and `set_schema_cache` in an `AbstractPool` so we can share code between `ConnectionPool` and `NullPool` instead of duplicating code. Now we only need to set the schema_cache on the pool rather than the connection. In `discard!` we need to unset the connection from the schema_cache - we still want the cache just not the connection.
* | Fix period positionRyuta Kamizono2019-06-064-5/+5
| |
* | Merge pull request #36399 from jhawthorn/named_controller_helper_moduleJohn Hawthorn2019-06-052-9/+25
|\ \ | |/ |/| Name helper_method module and improve source location
| * Use file/line from call to helper_moduleJohn Hawthorn2019-06-031-5/+10
| |
| * Give HelperMethods module a nameJohn Hawthorn2019-06-032-4/+15
| |
* | Merge pull request #36416 from freeletics/fix-db-prepareEileen M. Uchitelle2019-06-054-31/+99
|\ \ | | | | | | Fixed db:prepare task for multiple databases.
| * | Fixed db:prepare task for multiple databases.Wojciech Wnętrzak2019-06-054-31/+99
| | | | | | | | | | | | | | | When one database existed already, but not the other, during setup of missing one, existing database was wiped out.
* | | Merge pull request #36394 from eileencodes/treat-application-record-as-primaryEileen M. Uchitelle2019-06-053-5/+22
|\ \ \ | | | | | | | | Treat ActiveRecord::Base and ApplicationRecord as "primary"
| * | | Treat ActiveRecord::Base and ApplicationRecord as "primary"eileencodes2019-06-053-5/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When someone has a multi-db application their `ApplicationRecord` will look like: ```ruby class ApplicationRecord < ActiveRecord::Base self.abstract_class = true connects_to database: { writing: :primary, reading: :replica } end ``` This will cause us to open 2 connections to ActiveRecord::Base's database when we actually only want 1. This is because Rails sees `ApplicationRecord` and thinks it's a new connection, not the existing `ActiveRecord::Base` connection because the `connection_specification_name` is different. This PR changes `ApplicationRecord` classes to consider themselves the same as the "primary" connection. Fixes #36382
* | | | Merge pull request #36415 from Aupajo/patch-1Xavier Noria2019-06-051-1/+1
|\ \ \ \ | |_|/ / |/| | | Guides 6.0 release notes: fix typo
| * | | 6.0 release notes: fix typoPete Nicholls2019-06-051-1/+1
|/ / / | | | | | | An unfinished parenthetical.
* | | Specify a config name from `config` as well as other configs [ci skip]yuuji.yaginuma2019-06-051-1/+1
| | |
* | | Merge pull request #36407 from y-yagi/fixes_32211Yuji Yaginuma2019-06-053-10/+21
|\ \ \ | | | | | | | | Extract default log file open operation to method
| * | | Extract default log file open operation to methodyuuji.yaginuma2019-06-053-10/+21
|/ / / | | | | | | | | | | | | | | | This allows customize a default log file(e.g. `reopen`) by an application. Fixes #32211.
* | | Fix rubocop violationyuuji.yaginuma2019-06-051-2/+1
| | |
* | | Unify to use 4 spaces indentation in CHANGELOGs [ci skip]Ryuta Kamizono2019-06-055-63/+68
| | | | | | | | | | | | | | | Especially, somehow `CHANGELOG.md` in actiontext and activestorage in master branch had used 3 spaces indentation.
* | | Merge pull request #36376 from sharang-d/reword-content_type-changeVipul A M2019-06-051-4/+4
|\ \ \ | | | | | | | | Reword the ActionDispatch::Response#content_type change [ci skip]
| * | | Reword the ActionDispatch::Response#content_type change [ci skip]Sharang Dashputre2019-06-021-4/+4
| | | |
* | | | Merge pull request #36404 from mrschuster/sqlite3_collation_bugRyuta Kamizono2019-06-053-2/+15
|\ \ \ \ | | | | | | | | | | Fix sqlite3 collation parsing when using decimal columns.
| * | | | Fix sqlite3 collation parsing when using decimal columns.Martin Schuster2019-06-043-2/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If an sqlite3 table contains a decimal column behind columns with a collation definition, then parsing the collation of all preceeding columns will fail -- the collation will be missed without notice.
* | | | | Merge pull request #36398 from jhawthorn/file_update_checker_emptyJohn Hawthorn2019-06-041-1/+4
|\ \ \ \ \ | | | | | | | | | | | | Don't call listen with empty directory list
| * | | | | Don't call listen with empty directory listJohn Hawthorn2019-06-031-1/+4
| | |_|_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | Listen interprets an empty list of directories as "watch the current directory". Since EventedFileUpdateChecker doesn't share these semantics, we should not initialize listen if we end up with an empty directory list.
* | | | | Merge pull request #36405 from ↵Eileen M. Uchitelle2019-06-041-2/+2
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | abhaynikam/36389-follow-up-to-fix-grammer-and-typo Fix sentence and a typo
| * | | | | Fix typo in multi database guide depentent -> dependent [ci skip]Abhay Nikam2019-06-041-2/+2
|/ / / / / | | | | | | | | | | | | | | | Fixed the redundant addition of to [ci skip]
* | | | | Merge pull request #36389 from eileencodes/multi-db-docsEileen M. Uchitelle2019-06-042-0/+274
|\ \ \ \ \ | |_|_|_|/ |/| | | | Document multiple databases in Rails
| * | | | Document multiple databases in Railseileencodes2019-06-042-0/+274
| |/ / / | | | | | | | | | | | | | | | | This file documents how to use multiple databases, what features are supported, what features are coming soon, and caveats.
* | / / Merge pull request #36395 from cpruitt/6-0-stableEileen M. Uchitelle2019-06-041-3/+9
| |/ / |/| | | | | Do not clear deprecated initializer dependencies if using classic autoloader
* | | Merge pull request #36397 from ↵Yuji Yaginuma2019-06-041-0/+2
|\ \ \ | |/ / |/| | | | | | | | y-yagi/only_clear_cache_when_view_paths_are_specified Only clear cache when view paths are specified
| * | Only clear cache when view paths are specifiedyuuji.yaginuma2019-06-041-0/+2
|/ / | | | | | | | | | | | | | | | | | | | | | | Currently, `clear_cache_if_necessary` is executed even if view paths are not set like `rails console`. If the watcher class is `EventedFileUpdateChecker` and the watch directories are empty, the application root directory will watch. This is because listen uses the current directory as the default watch directory. https://github.com/guard/listen/blob/8d85b4cd5788592799adea61af14a29bf2895d87/lib/listen/adapter/config.rb#L13 As a result, `node_modules` also watch. This cause a warning of `listen`. Ref: https://github.com/rails/rails/pull/36377#issuecomment-498399576
* | Merge pull request #36384 from ↵Ryuta Kamizono2019-06-033-2/+28
|\ \ | | | | | | | | | | | | guigs/fix-invalid-schema-when-pk-column-has-comment Fix invalid schema dump when primary key column has a comment
| * | Fix invalid schema dump when primary key column has a commentGuilherme Goettems Schneider2019-06-033-2/+28
| | | | | | | | | | | | | | | | | | | | | | | | Before this fix it would either generate an invalid schema, passing `comment` option twice to `create_table`, or it move the comment from primary key column to the table if table had no comment when the dump was generated. The situation now is that a comment on primary key will be ignored (not present on schema). Fixes #29966
* | | Merge pull request #36379 from abhaynikam/36034-remove-unnecessary-require-pathYuji Yaginuma2019-06-033-3/+0
|\ \ \ | | | | | | | | Remove unnecessary require pathname from actionpack controller specs
| * | | Remove unnecessary require pathname from actionpack controller specsAbhay Nikam2019-06-023-3/+0
| | | |
* | | | Fix `subscribed` with no pattern to subscribe all messagesRyuta Kamizono2019-06-033-11/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a regression for #36184. And also, add new `monotonic` argument to the last of the method signature rather than the first.
* | | | Refactor `create_table`'s options separationRyuta Kamizono2019-06-032-12/+11
|/ / / | | | | | | | | | | | | | | | | | | | | | `create_table` and `t.column` have the same named options (e.g. `:comment`, `:primary_key`), so it should be separated table options from column options. Related #36373.
* | | Merge pull request #36375 from kamipo/fast_saveRyuta Kamizono2019-06-021-16/+26
|\ \ \ | | | | | | | | Avoid making extra 5 arrays in each `save`
| * | | Avoid making extra 5 arrays in each `save`Ryuta Kamizono2019-06-011-16/+26
| | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each `save` calls `all_timestamp_attributes_in_model` to fill timestamp columns. Allthough the `all_timestamp_attributes_in_model` returns the same value every time, the `all_timestamp_attributes_in_model` makes extra 5 arrays every time. This avoids the making extra 5 arrays by memoizing the result, it makes `save` economical and a bit faster. https://gist.github.com/kamipo/1ddad2235073f508637bf9a72d64bb83 Before (2a015f6c0be0593a624b0c800e5335319ac4c660): ``` {["~/rails/activerecord/lib/active_record/timestamp.rb", 76, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 64, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 80, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 68, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 73, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640]} Warming up -------------------------------------- User.create * 10 36.000 i/100ms Calculating ------------------------------------- User.create * 10 353.644 (± 7.4%) i/s - 1.764k in 5.021876s ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/timestamp.rb", 83, :T_ARRAY]=>[1, 0, 1, 1, 1, 40], ["~/rails/activerecord/lib/active_record/timestamp.rb", 87, :T_ARRAY]=>[1, 0, 1, 1, 1, 40], ["~/rails/activerecord/lib/active_record/timestamp.rb", 64, :T_ARRAY]=>[1, 1, 1, 1, 1, 0], ["~/rails/activerecord/lib/active_record/timestamp.rb", 69, :T_ARRAY]=>[1, 1, 1, 1, 1, 0], ["~/rails/activerecord/lib/active_record/timestamp.rb", 74, :T_ARRAY]=>[1, 1, 1, 1, 1, 0]} Warming up -------------------------------------- User.create * 10 37.000 i/100ms Calculating ------------------------------------- User.create * 10 380.063 (± 7.1%) i/s - 1.924k in 5.097917s ```
* | | Merge pull request #36378 from yahonda/test_pluck_columns_with_same_nameRyuta Kamizono2019-06-021-1/+1
|\ \ \ | | | | | | | | Address occasional test_pluck_columns_with_same_name failure
| * | | Address test_pluck_columns_with_same_name failure due to nondeterministic ↵Yasuo Honda2019-06-021-1/+1
|/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sort order ```ruby $ bundle exec rake test_postgresql ... snip ... Failure: CalculationsTest#test_pluck_columns_with_same_name [/home/yahonda/git/rails/activerecord/test/cases/calculations_test.rb:842]: --- expected +++ actual @@ -1 +1 @@ -[["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]] +[["The Third Topic of the day", "The Fourth Topic of the day"], ["The First Topic", "The Second Topic of the day"]] ```
* / / Simplify `ActionDispatch::Response#content_type`yuuji.yaginuma2019-06-021-2/+1
|/ /
* | Merge pull request #36034 from y-yagi/fixes_35709Yuji Yaginuma2019-06-0123-120/+171
|\ \ | | | | | | Change `ActionDispatch::Response#content_type` returning Content-Type header as it is
| * | Change `ActionDispatch::Response#content_type` returning Content-Type header ↵yuuji.yaginuma2019-06-0123-120/+171
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | as it is Since #35709, `Response#conten_type` returns only MIME type correctly. It is a documented behavior that this method only returns MIME type, so this change seems appropriate. https://github.com/rails/rails/blob/39de7fac0507070e3c5f8b33fbad6fced84d97ed/actionpack/lib/action_dispatch/http/response.rb#L245-L249 But unfortunately, some users expect this method to return all Content-Type that does not contain charset. This seems to be breaking changes. We can change this behavior with the deprecate cycle. But, in that case, a method needs that include Content-Type with additional parameters. And that method name is probably the `content_type` seems to properly. So I changed the new behavior to more appropriate `media_type` method. And `Response#content_type` changed (as the method name) to return Content-Type header as it is. Fixes #35709. [Rafael Mendonça França & Yuuji Yaginuma ]
* | Merge pull request #36373 from ↵Ryuta Kamizono2019-06-013-1/+10
|\| | | | | | | | | guigs/fix-table-comment-also-being-applied-to-the-primary-key-column Fix table comment also being applied to the primary key column
| * Fix table comment also being applied to the primary key columnGuilherme Goettems Schneider2019-05-313-1/+10
|/
* Merge pull request #36369 from jhawthorn/parallel_worker_crashJohn Hawthorn2019-05-312-0/+26
|\ | | | | Fail parallel tests if workers exit early
| * Fail parallel tests if workers exit earlyJohn Hawthorn2019-05-302-0/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, if a test worker exited early, the in-flight test it was supposed to run wasn't reported as a failure. If all workers exited immediately, this would be reported as ex. Finished in 1.708349s, 39.2192 runs/s, 79.0237 assertions/s. 67 runs, 135 assertions, 0 failures, 0 errors, 2 skips This commit validates that all workers finish running tests by ensuring that the queue is empty after they exit. This works because we signal the workers to exit by pushing nil onto the queue, so that there should be a number of items left in the queue matching potentially missed tests.