aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #35130 from rails/move-delay-to-options-argumentEileen M. Uchitelle2019-02-014-11/+61
|\ | | | | Refactor options for database selector middleware
| * Refactor options for middlewareEileen Uchitelle2019-02-014-11/+61
| | | | | | | | | | | | | | | | | | | | Right now we only have one option that's supported, the delay. However I can see us supporting other options in the future. This PR refactors the options to get passed into the resolver so whether you're using middleware or using the config options you can pass options to the resolver. This will also make it easy to add new options in the future.
* | Merge pull request #35082 from Shopify/eagerly-materialize-test-transactionsRafael França2019-02-013-5/+8
|\ \ | | | | | | Eagerly materialize the fixtures transaction
| * | Eagerly materialize the fixtures transactionJean Boussier2019-01-293-5/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The transaction used to restore fixtures is an implementation detail that should be abstracted away. Idealy a test should behave the same wether or not transactional fixtures are enabled. However since transactions have been made lazy, the fixture transaction started leaking into tests case. e.g. consider the following (oversimplified) test: ```ruby class SQLSubscriber attr_accessor :sql def initialize @sql = [] end def call(*, event) sql << event[:sql] end end subscriber = SQLSubscriber.new ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) User.connection.execute('SELECT 1', 'Generic name') assert_equal ['SELECT 1'], subscriber.sql ``` On Rails 6 it starts to break because the `sql` array will be `['BEGIN', 'SELECT 1']`. Several things are wrong here: - That transaction is not generated by the tested code, so it shouldn't be visible. - The transaction is not even closed yet, which again doesn't reflect the reality.
* | | Merge pull request #35131 from alkesh26/activesupport-typo-fixesRyuta Kamizono2019-02-027-7/+7
|\ \ \ | | | | | | | | ActiveSupport typo fixes.
| * | | ActiveSupport typo fixes.alkesh262019-02-017-7/+7
|/ / /
* | | Merge pull request #35043 from simoleone/activestorage/s3/content-typeEileen M. Uchitelle2019-02-012-2/+20
|\ \ \ | |_|/ |/| | include the content type when uploading to S3
| * | include the content type when uploading to S3Simo Leone2019-01-242-2/+20
| | |
* | | Merge pull request #35126 from alkesh26/railities-typo-fixEileen M. Uchitelle2019-02-0111-14/+14
|\ \ \ | | | | | | | | Railties typo fixes.
| * | | Railities typo fixes.alkesh262019-02-0111-14/+14
| | | |
* | | | Fix doc of `ActionDispatch::SystemTestCase` [ci skip]yuuji.yaginuma2019-02-011-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | * Fix broken format. * Need to specify driver to the first argument of `driven_by`. * `add_emulation` doesn't have `device` keyword. Ref: https://github.com/SeleniumHQ/selenium/blob/master/rb/lib/selenium/webdriver/chrome/options.rb#L142-L162
* | | | Enable `Lint/ErbNewArguments` cop to avoid the deprecated arguments warningRyuta Kamizono2019-02-013-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | Related 5754a29a974d31cab2b4392716b9825a3d910a69. And follows Ruby standard library style https://github.com/ruby/ruby/commit/3406c5d.
* | | | Merge pull request #35116 from kamipo/fix_through_association_creationRyuta Kamizono2019-02-014-27/+11
|\ \ \ \ | | | | | | | | | | Fix has_many through association creation
| * | | | Revert "Merge pull request #33729 from kddeisz/plural-automatic-inverse"Ryuta Kamizono2019-02-012-27/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit ed1eda271c7ac82ecb7bd94b6fa1b0093e648a3e, reversing changes made to 3d2caab7dc92a13d4dd369678d5b4ce659df8e52. Reason: 7c3da6e0030aa080fcb89af58b094ed50d861a44
| * | | | Add regression test for has_many through record creationRyuta Kamizono2019-02-012-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #33729 affected the behavior of the has_many through record creation. Since #33729, the intermediate reflection of simple has_many through association has `inverse_of` to the association, it causes extra through record creation, the extra through record required valid before the association record is saved. https://github.com/rails/rails/blob/23125378673bcc606b274027666a126573e136f8/activerecord/lib/active_record/associations/has_many_through_association.rb#L95-L102 I think that #33729 need to more work to care about has_many through association, that PR should be reverted to not break existing apps.
* | | | | Merge pull request #35117 from kamipo/remove_unused_attr_writerRyuta Kamizono2019-02-012-7/+0
|\ \ \ \ \ | | | | | | | | | | | | Remove unused attr_writers `visitor` and `indexes`
| * | | | | Remove unused attr_writers `visitor` and `indexes`Ryuta Kamizono2019-02-012-7/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I deprecated two unused attr_writers `visitor` and `indexes` at 8056fe0 and f4bc364 conservatively, since those are accidentaly exposed in the docs. https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/AbstractAdapter.html https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/TableDefinition.html But I've found that `view_renderer` attr_writer is removed without deprecation at #35093, that is also exposed in the doc. https://api.rubyonrails.org/v5.2/classes/ActionView/Base.html I'd like to also remove the deprecated attr_writers since I think that removing `visitor` and `indexes` attr_writers is as safe as removing `view_renderer` attr_writer.
* | | | | | Merge pull request #31253 from knu/backquote_returns_stringYuji Yaginuma2019-02-013-14/+4
|\ \ \ \ \ \ | | | | | | | | | | | | | | Make the backquote operator always return a string
| * | | | | | Remove the Kernel#` override that turns ENOENT into nilAkinori MUSHA2019-01-313-14/+4
| | |/ / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ActiveSupport overrides `` Kernel#` `` so that it would not raise `Errno::ENOENT` but return `nil` instead (due to the last statement `STDERR.puts` returning nil) if a given command were not found. Because of this, you cannot safely say somthing like `` `command`.chomp `` when ActiveSupport is loaded. It turns out that this is an outdated monkey patch for Windows platforms to emulate Unix behavior on an ancient version of Ruby, and it should be removed by now.
* | | | | | add tests to make sure deprecated API is still supportedAaron Patterson2019-01-312-2/+14
| |/ / / / |/| | | |
* | | | | Merge pull request #35112 from alkesh26/activerecord-typo-fixesVipul A M2019-01-311-1/+1
|\ \ \ \ \ | | | | | | | | | | | | ActiveRecord typo fixes. [ci skip]
| * | | | | ActiveRecord typo fixe.alkesh262019-01-311-1/+1
| |/ / / /
* | | | | Merge pull request #35115 from alkesh26/activestorage-typo-fixRyuta Kamizono2019-02-011-1/+1
|\ \ \ \ \ | |/ / / / |/| / / / | |/ / / | | | | ActiveStorage typo fix. [ci skip]
| * / / ActiveStorage typo fix.alkesh262019-01-311-1/+1
|/ / /
* | | Merge pull request #35109 from ↵Yuji Yaginuma2019-01-311-1/+1
|\ \ \ | | | | | | | | | | | | | | | | abhaynikam/35073-fix-automatic-database-switching-doc-typo Fix typo: inherts -> inherits [ci skip]
| * | | Fixed typo for DatabaseSelector::Resolver documentationAbhay Nikam2019-01-311-1/+1
|/ / /
* | | Remove redundant begin blockRyuta Kamizono2019-01-311-6/+3
| | | | | | | | | | | | | | | We enabled `Style/RedundantBegin` cop at #34764, but it is hard to detect an offence if returning value put after the block.
* | | Fix `ERB.new` argument deprecated warningyuuji.yaginuma2019-01-311-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | This fixes following warning. ``` warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments. ```
* | | Merge pull request #35108 from jhawthorn/db-selection-timestamp-afterEileen M. Uchitelle2019-01-302-2/+43
|\ \ \ | | | | | | | | Write update_last_write_timestamp after request
| * | | Write update_last_write_timestamp after requestJohn Hawthorn2019-01-302-2/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We need to update using the timestamp from the end of the request, not the start. For example, if a request spends 5+ seconds writing, we still want to wait another 5 seconds for replication lag. Since we now run the update after we yield, we need to use ensure to make sure we update the timestamp even if there is an exception.
* | | | Merge pull request #35104 from alkesh26/activemodel-typo-fixesKasper Timm Hansen2019-01-302-2/+2
|\ \ \ \ | | | | | | | | | | ActiveModel typo fixes.
| * | | | activemodel typo fixes.alkesh262019-01-312-2/+2
| | | | |
* | | | | Merge pull request #34980 from y-yagi/fixes_34979Yuji Yaginuma2019-01-312-2/+10
|\ \ \ \ \ | |_|/ / / |/| | | | Don't add `RAILS_ENV` in generate action
| * | | | Don't add `RAILS_ENV` in generate actionyuuji.yaginuma2019-01-192-2/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the case of generator, `RAILS_ENV` is interpreted as an argument as it is. Avoid this because it will result unintended by the user. Fixes #34979.
* | | | | Merge pull request #35093 from rails/av-base-constructorAaron Patterson2019-01-3014-26/+61
|\ \ \ \ \ | | | | | | | | | | | | Tighten up the AV::Base constructor
| * | | | | Tighten up the AV::Base constructorAaron Patterson2019-01-2914-26/+61
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The AV::Base constructor was too complicated, and this commit tightens up the parameters it will take. At runtime, AV::Base is most commonly constructed here: https://github.com/rails/rails/blob/94d54fa4ab641a0ddeb173409cb41cc5becc02a9/actionview/lib/action_view/rendering.rb#L72-L74 This provides an AV::Renderer instance, a hash of assignments, and a controller instance. Since this is the common case for construction, we should remove logic from the constructor that handles other cases. This commit introduces special constructors for those other cases. Interestingly, most code paths that construct AV::Base "strangely" are tests.
* | | | | | Merge pull request #35073 from eileencodes/db-selectionEileen M. Uchitelle2019-01-309-0/+353
|\ \ \ \ \ \ | | | | | | | | | | | | | | Part 8: Multi db improvements, Adds basic automatic database switching to Rails
| * | | | | | Adds basic automatic database switching to RailsEileen Uchitelle2019-01-309-0/+353
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following PR adds behavior to Rails to allow an application to automatically switch it's connection from the primary to the replica. A request will be sent to the replica if: * The request is a read request (`GET` or `HEAD`) * AND It's been 2 seconds since the last write to the database (because we don't want to send a user to a replica if the write hasn't made it to the replica yet) A request will be sent to the primary if: * It's not a GET/HEAD request (ie is a POST, PATCH, etc) * Has been less than 2 seconds since the last write to the database The implementation that decides when to switch reads (the 2 seconds) is "safe" to use in production but not recommended without adequate testing with your infrastructure. At GitHub in addition to the a 5 second delay we have a curcuit breaker that checks the replication delay and will send the query to a replica before the 5 seconds has passed. This is specific to our application and therefore not something Rails should be doing for you. You'll need to test and implement more robust handling of when to switch based on your infrastructure. The auto switcher in Rails is meant to be a basic implementation / API that acts as a guide for how to implement autoswitching. The impementation here is meant to be strict enough that you know how to implement your own resolver and operations classes but flexible enough that we're not telling you how to do it. The middleware is not included automatically and can be installed in your application with the classes you want to use for the resolver and operations passed in. If you don't pass any classes into the middleware the Rails default Resolver and Session classes will be used. The Resolver decides what parameters define when to switch, Operations sets timestamps for the Resolver to read from. For example you may want to use cookies instead of a session so you'd implement a Resolver::Cookies class and pass that into the middleware via configuration options. ``` config.active_record.database_selector = { delay: 2.seconds } config.active_record.database_resolver = MyResolver config.active_record.database_operations = MyResolver::MyCookies ``` Your classes can inherit from the existing classes and reimplment the methods (or implement more methods) that you need to do the switching. You only need to implement methods that you want to change. For example if you wanted to set the session token for the last read from a replica you would reimplement the `read_from_replica` method in your resolver class and implement a method that updates a new timestamp in your operations class.
* | | | | | | Merge pull request #35102 from ↵Eileen M. Uchitelle2019-01-303-5/+20
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | eileencodes/fix-case-when-url-in-url-config-is-nil Fix case when we want a UrlConfig but the URL is nil
| * | | | | | | Fix case when we want a UrlConfig but the URL is nilEileen Uchitelle2019-01-303-5/+20
| | |_|_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously if the `url` key in a config hash was nil we'd ignore the configuration as invalid. This can happen when you're relying on a `DATABASE_URL` in the env and that is not set in the environment. ``` production: <<: *default url: ENV['DATABASE_URL'] ``` This PR fixes that case by checking if there is a `url` key in the config instead of checking if the `url` is not nil in the config. In addition to changing the conditional we then need to build a url hash to merge with the original hash in the `UrlConfig` object. Fixes #35091
* | | | | | | Merge pull request #35101 from bogdanvlviv/remove-unused-codeRyuta Kamizono2019-01-312-9/+0
|\ \ \ \ \ \ \ | |/ / / / / / |/| | | | | | Remove unused code
| * | | | | | Remove unused codebogdanvlviv2019-01-302-9/+0
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Remove `fragment_cache_key` helper declaration. It was removed in e70d3df7c9b05c129b0fdcca57f66eca316c5cfc - Remove `by_private_lifo`. It is unused since a7becf147afc85c354e5cfa519911a948d25fc4d
* | | | | | Merge pull request #35080 from sos4nt/add_hash_assocRyuta Kamizono2019-01-303-0/+28
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | Add HashWithIndifferentAccess#assoc
| * | | | | | Add HashWithIndifferentAccess#assocStefan Schüßler2019-01-303-0/+27
| | | | | | |
* | | | | | | Relax version locking of the selenium-webdriverRyuta Kamizono2019-01-302-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 3.5.2 (September 07, 2017) is a little older.
* | | | | | | Add `require "selenium/webdriver"` to all using `DrivenBySeleniumWith*` classesRyuta Kamizono2019-01-301-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | https://travis-ci.org/rails/rails/jobs/486285170#L1349-L1366 ``` % git grep -n DrivenBySeleniumWith test/abstract_unit.rb:374:class DrivenBySeleniumWithChrome < ActionDispatch::SystemTestCase test/abstract_unit.rb:378:class DrivenBySeleniumWithHeadlessChrome < ActionDispatch::SystemTestCase test/abstract_unit.rb:382:class DrivenBySeleniumWithHeadlessFirefox < ActionDispatch::SystemTestCase test/dispatch/system_testing/screenshot_helper_test.rb:10: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:18: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:28: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:40: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:48: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:62: new_test = DrivenBySeleniumWithChrome.new("x") test/dispatch/system_testing/screenshot_helper_test.rb:76:class SeleniumScreenshotsTest < DrivenBySeleniumWithChrome test/dispatch/system_testing/system_test_case_test.rb:11:class OverrideSeleniumSubclassToRackTestTest < DrivenBySeleniumWithChrome test/dispatch/system_testing/system_test_case_test.rb:19:class SetDriverToSeleniumTest < DrivenBySeleniumWithChrome test/dispatch/system_testing/system_test_case_test.rb:25:class SetDriverToSeleniumHeadlessChromeTest < DrivenBySeleniumWithHeadlessChrome test/dispatch/system_testing/system_test_case_test.rb:31:class SetDriverToSeleniumHeadlessFirefoxTest < DrivenBySeleniumWithHeadlessFirefox test/dispatch/system_testing/system_test_case_test.rb:49:class UndefMethodsTest < DrivenBySeleniumWithChrome ```
* | | | | | | selenium-webdriver is not always required for system testingRyuta Kamizono2019-01-302-4/+3
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | But `NameError: uninitialized constant ActionDispatch::SystemTesting::Browser::Selenium` is pretty confused. I've little improved missing constant error to `NameError: uninitialized constant Selenium`.
* | | | | | Fix system testing failureRyuta Kamizono2019-01-302-8/+13
| | | | | | | | | | | | | | | | | | | | | | | | https://travis-ci.org/rails/rails/jobs/486155626#L1317-L1335
* | | | | | Merge pull request #35092 from utilum/update_sneakers_to_latest_versionRyuta Kamizono2019-01-301-5/+6
|\ \ \ \ \ \ | | | | | | | | | | | | | | Update Sneakers to the latest version
| * | | | | | Update Sneakers to the latest versionutilum2019-01-291-5/+9
| |/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | Sneakers 2.11.0 has a more recent Bunny dependency which squashes some Ruby 2.6 warnings tickled by ActiveJob tests.
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-05 18:21:46 +0000