aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Bump RuboCop to 0.67.2Koichi ITO2019-04-166-6/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Performance cops will be extracted from RuboCop to RuboCop Performance when next RuboCop 0.68 will be released. https://github.com/rubocop-hq/rubocop/issues/5977 RuboCop 0.67 is its transition period. Since rails/rails repository uses Performance cops, This PR added rubocop-performance gem to Gemfile. And this PR fixes some offenses using the following auto-correct. ```console % bundle exec rubocop -a Offenses: activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26: C: [Corrected] Layout/SpaceAroundOperators: Operator = > should be surrounded by a single space. "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } ^^ activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26: C: [Corrected] Layout/SpaceAroundOperators: Operator => should be surrounded by a single space. "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } ^^ actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true. module ResolverSharedTests ^ actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected] Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment. def with_file(filename, source="File at #{filename}") ^ actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected] Rails/RefuteMethods: Prefer assert_not_same over refute_same. refute_same a, b ^^^^^^^^^^^ 2760 files inspected, 5 offenses detected, 5 offenses corrected ```
* Add CHANGELOG entry for d1107f4d1e2573948d4941ac44511a0af6241f80Ryuta Kamizono2019-04-161-1/+7
| | | | [ci skip]
* Merge pull request #35962 from y-yagi/migrate_config_file_to_new_settingsYuji Yaginuma2019-04-161-6/+4
|\ | | | | Migrate Code Climate config file to new configuration
| * Migrate Code Climate config file to new configurationyuuji.yaginuma2019-04-161-6/+4
|/ | | | | | | I don't know whether have to move to a new configuration, but I think there is no need to keep using the old configuration. Ref: https://docs.codeclimate.com/docs/advanced-configuration#section-analysis-configuration-versions
* Don't refer `@transaction_state` directlyRyuta Kamizono2019-04-161-5/+5
| | | | | | | | | | | | | Since 8180c39, remaining transaction state is cleared in `force_clear_transaction_record_state` to less work `sync_with_transaction_state`. But it caused a race condition that `@transaction_state` would be cleared by other threads if the state is finalized. To work as before, snapshot `@transaction_state` to local variable not to refer `@transaction_state` directly. Fixes #35983.
* Merge pull request #35975 from xithan/masterRafael França2019-04-153-12/+22
|\ | | | | mounted routes with non-word characters
| * mounted routes with non-word charactersxithan2019-04-153-12/+22
| |
* | Merge pull request #35977 from prathamesh-sonpatki/rm-required-in-generatorsRafael França2019-04-152-14/+3
|\ \ | | | | | | Remove `required: true` from the model generator template
| * | Remove `required: true` from the model generator templatePrathamesh Sonpatki2019-04-152-14/+3
| | | | | | | | | | | | | | | | | | | | | | | | `belongs_to` association have `required: true` by default https://github.com/rails/rails/pull/18937 onwards so we don't need it in the generator template. We still need the code for required in the command line generator as it adds `null: false` in the migration.
* | | Merge pull request #35959 from jhawthorn/unbound_templatesRafael França2019-04-157-14/+237
|\ \ \ | | | | | | | | De-duplicate templates, introduce UnboundTemplate
| * | | Add additional test for sharing templatesJohn Hawthorn2019-04-121-1/+18
| | | |
| * | | Avoid duplication using _find_allJohn Hawthorn2019-04-121-11/+7
| | | |
| * | | Support disabling cache for DigestorJohn Hawthorn2019-04-123-22/+51
| | | | | | | | | | | | | | | | | | | | | | | | This adds a bit of complexity, but is necessary for now to avoid holding extra copies of templates which are resolved from ActionView::Digestor after disabling cache on the lookup context.
| * | | De-dup Templates, introduce UnboundTemplateJohn Hawthorn2019-04-124-10/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously it's possible to have multiple copies of the "same" Template. For example, if index.html.erb is found both the :en and :fr locale, it will return a different Template object for each. The same can happen with formats, variants, and handlers. This commit de-duplicates templates, there will now only be one template per file/virtual_path/locals tuple. We need to consider virtual_path because both `render "index"`, and `render "index.html"` can both find the same file but will have different virtual_paths. IMO this is rare and should be deprecated/removed, but it exists now so we need to consider it in order to cache correctly. This commit introduces a new UnboundTemplate class, which represents a template with unknown locals. Template objects can be built from it by using `#with_locals`. Currently, this is just a convenience around caching templates, but I hope it's a helpful concept that could have more utility in the future.
| * | | Add tests against resolverJohn Hawthorn2019-04-113-0/+109
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We didn't previously have many tests directly against the OptimizedFileSystemResolver or FileSystemResolver, though usually failures would be exposed through other tests. It's easier to test some specifics of the behaviour with unit tests. This also lets us test FileSystemResolver (non-optimized) which I don't think previously had much testing (other than from classses inheriting it).
* | | | Merge pull request #35927 from arbox/masterRafael França2019-04-151-0/+5
|\ \ \ \ | | | | | | | | | | Add section on PostgreSQL to the guides index [ci skip]
| * | | | Mark the section on PostgreSQL to be work in progress [ci skip]Andrei Beliankou2019-04-121-0/+1
| | | | |
| * | | | Add section on PostgreSQL to the guides index [ci skip]Andrei Beliankou2019-04-101-0/+4
| | | | |
* | | | | Merge pull request #35906 from yoones/notes-tags-registrationRafael França2019-04-154-2/+72
|\ \ \ \ \ | | | | | | | | | | | | Notes tags registration
| * | | | | Adds `register_tags`Younes SERRAJ2019-04-124-2/+72
| | | | | |
* | | | | | Merge pull request #35899 from ↵Eileen M. Uchitelle2019-04-152-17/+51
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | eileencodes/fix-connection-when-handler-doesnt-exist Ensure a handler is set when using `connected_to`
| * | | | | | Ensure a handler is set when using `connected_to`eileencodes2019-04-082-17/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After looking at #35800 there is definitely an issue in the `connected_to` method although it's generally behaving. Here are the details: 1) I added a default connection role - writing - to the connection handler lookup. I did this because otherwise if you did this: ``` connected_to(databse: :development) ``` And development wasn't a pre-established role it would create a new handler and connect using that. I don't think this is right so I've updated it to pick up the default (:writing) unless otherwise specified. To set a handler when using the database version pass a hash like you would to `connects_to`: ``` connected_to(database: { readonly_slow: :development }) ``` This will connect the `development` database to the `readonly_slow` handler/role. 2) I updated the tests to match this behavior that we expect. 3) I updated the documentation to clarify that using `connected_to` with a `database` key will establish a new connection every time. This is exactly how `establish_connection` behaves and I feel this is correct. If you want to only establish a connection once you should do that in the model with `connects_to` and then swap on the role instead of on the database hash/key. 4) In regards to #35800 this fixes the case where you pass a symbol to the db and not a hash. But it doesn't fix a case where you may pass an unknown handler to an abstract class that's not connected. This is tricky because technical AbstractFoo doesn't have any connections except for through ApplicationRecord, so in the case of the application that was shared we should only be swapping connections on ActiveRecord::Base because there are no other actual connections - AbstractFoo isn't needed since it's not establishing a new connection. If we need AbstractFoo to connect to a new handler we should establish that connection with the handler in AbstractFoo before trying to shard there.
* | | | | | | Merge pull request #35974 from mrhead/remove-unused-modulesGeorge Claghorn2019-04-151-4/+0
|\ \ \ \ \ \ \ | |_|_|_|_|/ / |/| | | | | | Remove unused modules from StrongParameters
| * | | | | | Remove unused modules from StrongParametersPatrik Bóna2019-04-151-4/+0
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unless I'm missing some undocumented use case, these modules aren't needed in `StrongParameters` anymore since 8e221127ab. Also, all actionpack tests are passing without them.
* | | | | | Fix dirty tracking for `touch`Ryuta Kamizono2019-04-155-9/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before this fix, `touch` only clears dirty tracking for touched attributes, doesn't track saved (touched) changes. This fixes that tracks saved changes and carry over remaining changes. Fixes #33429. Closes #34306.
* | | | | | Correctly set application path in ↵yuuji.yaginuma2019-04-151-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `test_app_update_does_not_change_config_target_version` This is the follow up of 10fa3b3792153c2a213f837bcf51bbf6844c1661.
* | | | | | Make test application the same state as `app:update`yuuji.yaginuma2019-04-151-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In `app:update`, it is decided whether to skip depending on whether `Spring` is defined or not. However, `spring` is not currently specified in Gemfile. As a result, `app:update` determines that `Spring` is not used, and diff appears in the result file. If there is a difference, the console for processing the difference is output and the test stops. To avoid this, do not include `Spring` in app. This is a bit strange approach, so I will revisit this later.
* | | | | | Merge pull request #35967 from ↵Yuji Yaginuma2019-04-151-49/+60
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | ryohashimoto/do_not_app_update_in_app_generator_test Do not execute `rails app:update` in railties/test/generators/app_generator_test.rb
| * | | | | | use Rails::Generators::AppGenerator#update_config_files instead of `rails ↵Ryo Hashimoto2019-04-131-49/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | app:update` in app_generator_test.rb
* | | | | | | Merge pull request #35970 from yskkin/reversible_commentRyuta Kamizono2019-04-159-7/+182
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | make change_column_comment and change_table_comment invertible
| * | | | | | | make change_column_comment and change_table_comment invertibleYoshiyuki Kinjo2019-04-159-7/+182
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We can revert migrations using `change_column_comment` or `change_table_comment` at current master. However, results are not what we expect: comments are remained in new status. This change tells previous comment to these methods in a way like `change_column_default`.
* | | | | | | | Fix broken `AppGeneratorTest#test_spring_no_fork`yuuji.yaginuma2019-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The mock is called three times because the `spring_install?` call has been added in 65344f254cde87950c7f176cb7aa09c002a6f882.
* | | | | | | | Refactor `sync_with_transaction_state`Ryuta Kamizono2019-04-151-8/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All transaction states (:committed, :fully_committed, :rolledback, :fully_rolledback) are mutually exclusive. And also, `force_clear_transaction_record_state` should clear `@transaction_state` as well.
* | | | | | | | generate config.cache_classes = false if SpringXavier Noria2019-04-143-5/+21
| | | | | | | |
* | | | | | | | Merge pull request #35951 from dv/fix-redis-fetch-multi-without-namesGeorge Claghorn2019-04-142-0/+7
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | Make `Redis.cache.fetch_multi([])` not throw an error
| * | | | | | | | Redis fetch without names returns {}David Verhasselt2019-04-122-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When trying to call mget in Redis without any parameters, a Redis error is thrown. To avoid this, we circumvent Redis entirely when there are no key names given.
* | | | | | | | | Remove the Amazon SES ingressGeorge Claghorn2019-04-1411-149/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | It's unusable and not ready to ship in Rails 6.0. We'll rewrite it for 6.1.
* | | | | | | | | Merge pull request #35971 from ↵Ryuta Kamizono2019-04-141-46/+52
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | shioyama/avoid_acceptance_validator_module_inclusion_in_tests Ensure acceptance validator is not applied more than once to Person
| * | | | | | | | | Ensure acceptance validator is not applied more than once to PersonChris Salzberg2019-04-141-46/+52
| | | | | | | | | |
* | | | | | | | | | Remove useless `update_attributes_from_transaction_state` and ↵Ryuta Kamizono2019-04-141-14/+3
|/ / / / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | `set_transaction_state`
* | | | | | | | | Merge pull request #35956 from mrhead/fix-per-exception-retry-counterGeorge Claghorn2019-04-143-2/+26
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | Use individual execution counters when calculating retry delay
| * | | | | | | | | Use individual execution counters when calculating retry delayPatrik Bóna2019-04-123-2/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Individual execution counters were introduced in #34352. However `#determine_delay` which is used to calculate retry delay still uses the global counter. This commit fixes it.
* | | | | | | | | | Don't expose `add_to_transaction`Ryuta Kamizono2019-04-144-18/+12
| |_|/ / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `add_to_transaction` was added at da840d1, but it should not be called by except internal, since `remember_transaction_record_state` should be called only once before saving. And also, currently `add_to_transaction` doesn't always add the record to transaction since da8de91, that is the reason hard to use that even in internal. Even if `add_to_transaction` ensure to add the record to transaction, that is an internal concern, people don't need to explicitly call `add_to_transaction`.
* | | | | | | | | deprecates autoloading constants during initialization [closes #35745]Xavier Noria2019-04-143-0/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | See rationale in the warning message included in the patch.
* | | | | | | | | Merge pull request #35968 from ↵Ryuta Kamizono2019-04-141-18/+32
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | shioyama/apply_acceptance_validator_to_subclasses_in_tests Ensure multiple anonymous modules are not included into test classes
| * | | | | | | | | Ensure multiple anonymous modules are not included into Topic in testsChris Salzberg2019-04-141-18/+32
|/ / / / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each acceptance validator applied to a model class includes an instance of a module builder (LazilyDefineAttributes) into that class. In tests, if the original model class is not subclassed, these modules pile up and cannot be removed, potentially leading to flakey specs and false positive/negatives. To avoid this, always use subclasses in tests whose names (constants) can be removed when the test is done.
* | | | | | | | | Merge pull request #35966 from shioyama/define_validator_on_topic_subclassRyuta Kamizono2019-04-141-5/+7
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | Add validation to subclass in tests to avoid polluting parent class
| * | | | | | | | | Add validation to subclass in tests to avoid polluting parent classChris Salzberg2019-04-131-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These two tests currently both define acceptance validators on the same class, Topic. This means that in either one test or the other, there are not one but *two* instances of the LazilyDefineAttributes module builder in the class' ancestors, which can result in unpredictable results. Subclassing Topic in each test avoids conflicts.
* | | | | | | | | | Merge pull request #35958 from yskkin/bulk_change_tableRyuta Kamizono2019-04-144-48/+43
|\ \ \ \ \ \ \ \ \ \ | |_|_|_|/ / / / / / |/| | | | | | | | | use PostgreSQL's bulk_alter_table implementation
| * | | | | | | | | use PostgreSQL's bulk_alter_table implementationYoshiyuki Kinjo2019-04-134-48/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Running this migration on mysql at current master fails because `add_references_for_alter` is missing. ``` change_table :users, bulk: true do |t| t.references :article end ``` This is also true for postgresql adapter, but its `bulk_alter_table` implementation can fallback in such case. postgresql's implementation is desirable to prevent unknown failure like this.
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-27 09:16:58 +0000