aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Introduce a file type template, deprecate `Template#refresh`Aaron Patterson2019-02-014-3/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Every template that specifies a "virtual path" loses the template source when the template gets compiled: https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template.rb#L275 The "refresh" method seems to think that the source code for a template can be recovered if there is a virtual path: https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template.rb#L171-L188 Every call site that allocates a template object *and* provides a "virtual path" reads the template contents from the filesystem: https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template/resolver.rb#L229-L231 Templates that are inline or literals don't provide a "virtual path": https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/renderer/template_renderer.rb#L34 This commit introduces a `FileTemplate` type that subclasses `Template`. The `FileTemplate` keeps a reference to the filename, and reads the source from the filesystem. This effectively makes the template source immutable. Other classes depended on the source to be mutated while being compiled, so this commit also introduces a temporary way to pass the mutated source to the ERB (or whatever) compiler. See `LegacyTemplate`. I think we should consider it an error to provide a virtual path on a non file type template an non-file templates can't recover their source. Here is an example: https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/testing/resolvers.rb#L53 This provides a "virtual path" so the source code (a string literal) is thrown away after compilation. Clearly we can't recover that string, so I think this should be an error.
* use the source returned from encode!Aaron Patterson2019-01-311-3/+5
|
* 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.
* | | | Merge pull request #35094 from palkan/feature/action-cable-load-hooksGeorge Claghorn2019-01-294-0/+19
|\ \ \ \ | |_|/ / |/| | | Add ActionCable channel/connection load hooks
| * | | Add ActionCable channel/connection load hooksVladimir Dementyev2019-01-294-0/+19
|/ / /
* | | Merge pull request #35081 from eileencodes/ec-driver-option-updatedEileen M. Uchitelle2019-01-298-17/+132
|\ \ \ | |/ / |/| | [UPDATED] Implement a way to add browser capabilities
| * | Rename methods and update docsEileen Uchitelle2019-01-295-25/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a minor update to the named methods for the following: - s/desired_capabilities/capabilities - s/driver_options/capabilities Since they are all the same thing we should keep the name the same throughout the feature. Updated docs to match / be a little bit clearer Also updated the Gemfile for selenium-webdriver.
| * | driver_option -> driver_optionsEdouard CHIN2019-01-293-8/+9
| | |
| * | Implement a way to add browser capabilities:Edouard CHIN2019-01-298-11/+117
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * There is currently no way to define specific browser capabilities since our SystemTest driver override the `option` key [Ref](https://github.com/rails/rails/blob/a07d0680787ced3c04b362fa7a238c918211ac70/actionpack/lib/action_dispatch/system_testing/driver.rb#L35) This option key is used internally by selenium to add custom capabilities on the browser. Depending on the Browser, some option are allowed to be passed inside a hash, the driver takes care of setting whatever you passed on the driver option. An example [here](https://github.com/rails/rails/blob/a07d0680787ced3c04b362fa7a238c918211ac70/actionpack/lib/action_dispatch/system_testing/driver.rb#L35) where you are allowed to pass args such as `--no-sandbox` etc However this behavior was only meant for backward compatibility and as you can see it's deprecated. The non-deprecated behavior is to create a `<Driver>::Option` object containing all the capabilities we want. This is what we [currently do](https://github.com/rails/rails/blob/a07d0680787ced3c04b362fa7a238c918211ac70/actionpack/lib/action_dispatch/system_testing/browser.rb#L34-L36) when chrome or firefox are in headless mode. This PR allows to pass a block when calling `driven_by`, the block will be pased a `<Driver>::Option` instance. You can modify this object the way you want by adding any capabilities. The option object will be then passed to selenium. ```ruby driven_by :selenium, using: :chrome do |driver_option| driver_option.add_argument('--no-sandbox') driver_option.add_emulation(device: 'iphone 4') end ```
* | | Merge pull request #35083 from alkesh26/activejob-typo-fixesRyuta Kamizono2019-01-302-2/+2
|\ \ \ | | | | | | | | Activejob typo fixes.
| * | | activejob typo fixes.alkesh262019-01-292-2/+2
|/ / /
* | | Merge pull request #35071 from kamipo/text_without_limitRyuta Kamizono2019-01-2911-23/+65
|\ \ \ | | | | | | | | MySQL: Support `:size` option to change text and blob size
| * | | Allow changing text and blob size without giving the `limit` optionRyuta Kamizono2019-01-2911-23/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In MySQL, the text column size is 65,535 bytes by default (1 GiB in PostgreSQL). It is sometimes too short when people want to use a text column, so they sometimes change the text size to mediumtext (16 MiB) or longtext (4 GiB) by giving the `limit` option. Unlike MySQL, PostgreSQL doesn't allow the `limit` option for a text column (raises ERROR: type modifier is not allowed for type "text"). So `limit: 4294967295` (longtext) couldn't be used in Action Text. I've allowed changing text and blob size without giving the `limit` option, it prevents that migration failure on PostgreSQL.
* | | | Merge pull request #35078 from carlosramireziii/patch-1Ryuta Kamizono2019-01-291-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | Fix usage documentation in VideoAnalyzer [ci skip]
| * | | | Fix usage documentation in VideoAnalyzerCarlos Ramirez III2019-01-281-1/+1
|/ / / / | | | | | | | | The code snippet within the usage documentation comment used the wrong object namespace for the ActiveStorage::Analyzer::VideoAnalyzer
* | | | Merge pull request #35067 from vnbrs/patch-1Rafael França2019-01-281-1/+1
|\ \ \ \ | | | | | | | | | | Add line break to Action Text installation outputs
| * | | | Add line break to Action Text installation outputsVinicius Brasil2019-01-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Action Text installations appends `require("trix")` to the application.js file. The problem is that there isn't a line break in the beginning of the installation output, leading to syntax errors, e.g.: ``` import './application.scss'require("trix") ``` This commit moves the line break from the end to the beginning of the output, fixing it to: ``` import './application.scss' require("trix") ```
* | | | | Merge pull request #35076 from rails/more-ro-objectsAaron Patterson2019-01-287-58/+79
|\ \ \ \ \ | |_|/ / / |/| | | | More Read-Only Changes
| * | | | Remove `with_layout_format` delegationAaron Patterson2019-01-281-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | That method doesn't exist on LookupContext, so the delegate doesn't make sense.
| * | | | Cache the digest path on the stack.Aaron Patterson2019-01-281-7/+5
| | | | | | | | | | | | | | | | | | | | We can remove the ivar by caching the digest on the stack
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-02 12:15:19 +0000