1. 27 1月, 2020 3 次提交
  2. 24 1月, 2020 1 次提交
    • E
      Move advisory lock to it's own connection · 1ee4a881
      eileencodes 提交于
      This PR moves advisory lock to it's own connection instead of
      `ActiveRecord::Base` to fix #37748. As a note the issue is present on
      both mysql and postgres. We don't see it on sqlite3 because sqlite3
      doesn't support advisory locks.
      
      The underlying problem only appears if:
      
      1) the app is using multiple databases, and therefore establishing a new
      connetion in the abstract models
      2) the app has a migration that loads a model (ex `Post.update_all`)
      which causes that new connection to get established.
      
      This is because when Rails runs migrations the default connections are
      established, the lock is taken out on the `ActiveRecord::Base`
      connection. When the migration that calls a model is loaded, a new
      connection will be established and the lock will automatically be
      released.
      
      When Rails goes to release the lock in the ensure block it will find
      that the connection has been closed. Even if the connection wasn't
      closed the lock would no longer exist on that connection.
      
      We originally considered checking if the connection was active, but
      ultimately that would hide that the advisory locks weren't working
      correctly because there'd be no lock to release.
      
      We also considered making the lock more granular - that it only blocked
      on each migration individually instead of all the migrations for that
      connection. This might be the right move going forward, but right now
      multi-db migrations that load models are very broken in Rails 6.0 and
      master.
      
      John and I don't love this fix, it requires a bit too much knowledge of
      internals and how Rails picks up connections. However, it does fix the
      issue, makes the lock more global, and makes the lock more resilient to
      changing connections.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      1ee4a881
  3. 23 1月, 2020 1 次提交
    • K
      Allow schema cache path to be defined in the config file · 2c2ff822
      Katrina Owen 提交于
      This updates the database tasks for dumping the Active Record schema cache as
      well as clearing the schema cache file, allowing the path to be defined in the
      database configuration YAML file.
      
      As before, the value can also be defined in an ENV variable, though this would
      not work for a multi-db application. If the value is specified neither in the
      DB config, nor in the ENV, then the path will continue to be derived from the
      DB config spec_name.
      
      Note that in order to make this change cleaner I also moved a bit of logic
      out of a rake task and into the DatabaseTasks class, for symmetry.
      
      We have two rake tasks for the schema cache:
      
          $ rake db:schema:cache:dump
          $ rake db:schema:cache:clear
      
      The cache:dump task was implemented in DatabaseTasks, but the
      cache:clear one was not.
      
      I also added some tests for the behavior that I was changing, since some of
      the code paths weren't tested.
      2c2ff822
  4. 22 1月, 2020 1 次提交
    • E
      Deprecate `#remove_connection` in favor of `#remove_connection_pool` · 7315c91d
      eileencodes 提交于
      Calling `#remove_connection` on the handler is deprecated in favor of
      `#remove_connection_pool`. This change was made to support changing the
      return value from a hash to a `DatabaseConfig` object.
      `#remove_connection` will be removed in 6.2.
      
      NOTE: `#remove_connection` on `ActiveRecord::Base` will also now return
      a `DatabaseConfig` object. We didn't use a deprecation here since
      it's not documented that this method used to return a `Hash`.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      7315c91d
  5. 20 1月, 2020 4 次提交
  6. 18 1月, 2020 1 次提交
    • E
      Deprecate and replace `#default_hash` and `#[]` · 2a53fe63
      eileencodes 提交于
      Database configurations are now objects almost everywhere, so we don't
      need to fake access to a hash with `#default_hash` or it's alias `#[]`.
      Applications should `configs_for` and pass `env_name` and `spec_name` to
      get the database config object. If you're looking for the default for
      the test environment you can pass `configs_for(env_name: "test", spec_name:
      "primary")`. Change test to developement to get the dev config, etc.
      
      `#default_hash` and `#[]` will be removed in 6.2.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      2a53fe63
  7. 17 1月, 2020 1 次提交
    • R
      Allow updating the database selector context with the response · 62e05ce9
      Rosa Gutierrez 提交于
      Currently the database selector middleware only passes the request to
      the context.
      
      This is enough for the resolver to decide whether to switch
      to the primary, but for a custom resolver and a custom context class,
      it's not enough to persist any information for subsequent requests. For
      example, say you want to use a cookie to decide whether when to switch.
      It's not possible to set the response cookie from this middleware, since
      neither the context nor the resolver have access to it.
      
      This includes an extra step to update the context after the response has
      been computed.
      62e05ce9
  8. 15 1月, 2020 2 次提交
  9. 14 1月, 2020 2 次提交
  10. 11 1月, 2020 2 次提交
  11. 10 1月, 2020 3 次提交
    • E
      Fix numericality validator when defined on abstract class: · c507e930
      Edouard CHIN 提交于
      - ### Problem
      
        It's no longer possible to define a numeric validation on abstract
        class:
      
        ```ruby
          class AnimalsBase < ApplicationRecord
            self.abstract_class = true
      
            validates :age, numericality: { min: 18 }
          end
      
          class Dog < AnimalsBase
          end
      
          Dog.create!(age: 0) => ActiveRecord::TableNotSpecified: Dog has no table configured. Set one with Dog.table_name=
        ```
      
        ### Solution
      
        Instead of trying to get the type for attribute on the class
        defining the validation, get it from the record being validated.
      c507e930
    • E
      Ensure the reading connection always raises if we try to write · 1c98e6c0
      eileencodes 提交于
      Since test fixtures share connections (due to transactional tests) we
      end up overwriting the reading configuration so Rails doesn't recognize
      it as a replica connection.
      
      This change ensures that if we're using the `reading` role that
      connections will always have prevent writes turned on.
      
      If you need a replica connection that does not block writes, you should
      use a different role name other than `:reading`.
      
      The db selector test and connection handlers test have been updated to
      test for these changes. In the db selector test we don't always have a
      writing handler so I updated test fixtures to return if that's nil.
      
      Lastly one test needed to be updated to use a different handler name due
      to it needing to write to successfully test what it needs to test.
      
      Fixes #37765
      1c98e6c0
    • E
      Fix the reading can write resolver test · d89ee16b
      eileencodes 提交于
      This test wasn't correct. If we're calling `resolver.read` and want to
      actually read from the replicas then the role would be reading not
      writing. This was because the session store needed to be changed so that
      we actually are "reading from the replicas" instead of reading from the
      primary.
      d89ee16b
  12. 09 1月, 2020 1 次提交
    • E
      Deprecate "primary" as a connection_specification_name for ActiveRecord::Base · b74fbe4e
      eileencodes 提交于
      As multiple databases have evolved it's becoming more and more
      confusing that we have a `connection_specification_name` that defaults
      to "primary" and a `spec_name` on the database objects that defaults to
      "primary" (my bad).
      
      Even more confusing is that we use the class name for all
      non-ActiveRecord::Base abstract classes that establish connections. For
      example connections established on `class MyOtherDatabaseModel <
      ApplicationRecord` will use `"MyOtherDatabaseModel"` as it's connection
      specification name while `ActiveRecord::Base` uses `"primary"`.
      
      This PR deprecates the use of the name `"primary"` as the
      `connection_specification_name` for `ActiveRecord::Base` in favor of
      using `"ActiveRecord::Base"`.
      
      In this PR the following is true:
      
      * If `handler.establish_connection(:primary)` is called, `"primary"`
      will not throw a deprecation warning and can still be used for the
      `connection_specification_name`. This also fixes a bug where using this
      method to establish a connection could accidentally overwrite the actual
      `ActiveRecord::Base` connection IF that connection was not using a
      configuration named `:primary`.
      * Calling `handler.retrieve_connection "primary"` when
      `handler.establish_connection :primary` has never been called will
      return the connection for `ActiveRecord::Base` and throw a deprecation
      warning.
      * Calling `handler.remove_connection "primary"` when
      `handler.establish_connection :primary` has never been called will
      remove the connection for `ActiveRecord::Base` and throw a deprecation
      warning.
      
      See #38179 for details on more motivations for this change.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      b74fbe4e
  13. 08 1月, 2020 2 次提交
    • E
      Restore previous behavior of parallel test databases · c0509885
      eileencodes 提交于
      This commit is somewhat of a bandaid fix for a bug that was revealed in #38029
      and then #38151. #38151 can cause problems in certain cases when an app has
      a 3-tier config, with replicas, because it reorders the configuration
      and changes the implict default connection that gets picked up.
      
      If an app calls `establish_connection` with no arguments or doesn't call
      `connects_to` in `ApplicationRecord` AND uses parallel testing
      databases, the application may pick up the wrong configuration.
      
      This is because when the code in #38151 loops through the configurations
      it will update the non-replica configurations and then put them at the
      end of the list. If you don't specify which connection you want, Rails
      will pick up the _first_ connection for that environment. So given the
      following configuration:
      
      ```
      test:
        primary:
          database: my_db
        replica:
          database: my_db
          replica: true
      ```
      
      The database configurations will get reordered to be `replica`,
      `primary` and when Rails calls `establish_connection` with no arguments
      it will pick up `replica` because it's first in the list.
      
      Looking at this bug it shows that calling `establish_connection` with no
      arguments (which will pick up the default env + first configuration in
      the list) OR when `establish_connection` is called with an environment
      like `:test` it will also pick up that env's first configuration. This
      can have surprising behavior in a couple cases:
      
      1) In the parallel testing changes we saw users getting the wrong db
      configuration and hitting an `ActiveRecord::ReadOnlyError`
      2) Writing a configuration that puts `replica` before `primary`, also
      resulting in a `ActiveRecord::ReadOnlyError`
      
      The real fix for this issue is to deprecate calling
      `establish_connection` with an env or nothing and require an explcit
      configuration (like `primary`). This would also involve blessing
      `:primary` as the default connection Rails looks for on boot. In
      addition, this would require us deprecating connection specification
      name "primary" in favor of the class name always since that will get
      mega-confusing (seriously, it's already mega-confusing).
      
      We'll work on fixing these underlying issues, but wanted to get a fix
      out that restores previous behavior.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      c0509885
    • M
      Avoid extraneous preloading when loading across `has_one` associations · 9aa59f9d
      Michael Fowler 提交于
      The Preloader relies on other objects to bind the retrieved records to their
      parents. When executed across a hash, it assumes that the results of
      `preloaded_records` is the appropriate set of records to pass in to the next
      layer.
      
      Filtering based on the reflection properties in `preloaded_records` allows us to
      avoid excessive preloading in the instance where we are loading across a
      `has_one` association distinguished by an order (e.g. "last comment" or
      similar), by dropping these records before they are returned to the
      Preloader. In this situation, we avoid potentially very long key lists in
      generated queries and the consequential AR object instantiations.
      
      This is mostly relevant if the underlying linked set has relatively many
      records, because this is effectively a multiplier on the number of records
      returned on the far side of the preload. Unfortunately, avoiding the
      over-retrieval of the `has_one` association seems to require substantial changes
      to the preloader design, and probably adaptor-specific logic -- it is a
      top-by-group problem.
      9aa59f9d
  14. 07 1月, 2020 3 次提交
  15. 05 1月, 2020 1 次提交
  16. 04 1月, 2020 3 次提交
    • S
      Allow `or` in case of `from` clause with same value · 8d58bdda
      sinsoku 提交于
      8d58bdda
    • S
      Avoid assigning duplicate values in QueryMethods · aca85f62
      sinsoku 提交于
      The ArgumentError occurs even though structures is compatible.
      Because some query methods keep duplicate values.
      
      For example, the behavior of `joins` method is as following:
      
      ```ruby
      relation = Post.joins(:author).joins(:author)
      relation.joins_values
      #=> [:author, :author]
      relation.or(Post.joins(:author))
      #=> ArgumentError: Relation passed to #or must be structurally compatible. Incompatible values: [:joins]
      ```
      
      This commit changes to not keep duplicate values.
      
      Fixes #38052
      aca85f62
    • J
      Pass env_name as a string in test databases · 1f938f52
      John Crepezzi 提交于
      In 154abcab we switched from using `Rails.env` to fetch the `env_name` to
      `ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_sym` which
      changed the type from a `String` to a `Symbol`.
      
      This commit brings things back to the original state, so we can find the
      configurations correctly!
      
      It also modifies the configuration in the configurations array, so that
      future connections can find the database with the updated keyword value.
      1f938f52
  17. 03 1月, 2020 2 次提交
    • R
      14149105
    • A
      Enforce fresh ETag header after collection changes · 58b04096
      Aaron Lipman 提交于
      Add ActiveRecord::Relation#cache_key_with_version. This method will be
      used by ActionController::ConditionalGet to ensure that when collection
      cache versioning is enabled, requests using ConditionalGet don't return
      the same ETag header after a collection is modified.
      
      Prior to the introduction of collection cache versioning in
      4f2ac80d, all collection cache keys
      included a version. However, with cache versioning enabled, collection
      cache keys remain constant. In turn, ETag headers remain constant,
      rendering them ineffective.
      
      This commit takes the cache_key_with_version method used for individual
      Active Record objects (from aa8749eb),
      and adds it to collections.
      58b04096
  18. 01 1月, 2020 1 次提交
    • K
      Allow #nulls_first and #nulls_last in PostgreSQL · 66b19b5d
      Kevin Deisz 提交于
      When using PostgreSQL, it's useful to be able to specify NULLS FIRST and NULLS LAST on ordered columns. With this commit you can do that now, as in:
      
      ```ruby
      User.arel_table[:first_name].desc.nulls_last
      ```
      66b19b5d
  19. 27 12月, 2019 3 次提交
    • Y
      Add test case about assigning nil in enum · f5ec5246
      Yoshiyuki Hirano 提交于
      This commit will be squished into f4fbdb1b after maintainer's review.
      f5ec5246
    • E
      Clear callback triggers when transaction completes · d3060af7
      Eugene Kenny 提交于
      The `_trigger_update_callback` and `_trigger_destroy_callback`
      attributes were added in 9252da96 to
      avoid running transactional callbacks when an attempt to modify a record
      fails inside a transaction due to the record being invalid, for example.
      
      However the values weren't being reset between transactions, which meant
      they leaked from one transaction to another and caused false positives
      where unsuccessful modifications still triggered callbacks. Clearing
      them when a transaction commits or is rolled back fixes the problem.
      d3060af7
    • A
      Avoid unnecessary SQL query when calling cache_key · 76bb9712
      Aaron Lipman 提交于
      With collection_cache_versioning enabled, a collection's volatile info
      (size & max updated_at timestamp) is included in
      ActiveRecord::Relation#cache_version, not #cache_key.
      
      Avoid the SQL query to used determine this volatile info when generating
      an un-versioned cache key. This query does not need to be executed
      unless cache_version is called separately.
      76bb9712
  20. 25 12月, 2019 1 次提交
    • Y
      Allow AR::Enum definitions with boolean values · f4fbdb1b
      Yoshiyuki Hirano 提交于
      If `AR::Enum` is used for boolean field, it would be not expected
      behavior for us.
      
      fixes #38075
      
      Problem:
      
      In case of using boolean for enum, we can set with string (hash key)
      to instance, but we cannot set with actual value (hash value).
      
      ```ruby
      class Post < ActiveRecord::Base
        enum status: { enabled: true, disabled: false }
      end
      
      post.status = 'enabled'
      post.status # 'enabled'
      
      post.status = true
      post.status # 'enabled'
      
      post.status = 'disabled'
      post.status # 'disabled'
      
      post.status = false
      post.status # nil (This is not expected behavior)
      ```
      
      After looking into `AR::Enum::EnumType#cast`, I found that `blank?`
      method converts from false value to nil (it seems it may not intentional behavior).
      
      In this patch, I improved that if it defines enum with boolean,
      it returns reasonable behavior.
      f4fbdb1b
  21. 23 12月, 2019 2 次提交