1. 11 1月, 2020 6 次提交
  2. 10 1月, 2020 9 次提交
  3. 09 1月, 2020 5 次提交
    • A
      Reduce number of created objects in Hash#as_json · 9256ae8a
      Alexander Pauly 提交于
      9256ae8a
    • K
      Merge pull request #38195 from Shopify/fix-class-attribute-regression · 3eab31c4
      Kasper Timm Hansen 提交于
      Do not define instance predicate if the instance reader isn't as well
      3eab31c4
    • J
      Do not define instance predicate if the instance reader isn't as well · 0309179f
      Jean Boussier 提交于
      This regressed in 49d1b5a9.
      
      I didn't notice it because the method would be defined but would
      raise a NoMethodError.
      0309179f
    • 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
    • R
      Fix deprecation warnings in Active Support tests · 67e18160
      Ryuta Kamizono 提交于
      67e18160
  4. 08 1月, 2020 20 次提交
    • R
      Merge pull request #38169 from gsamokovarov/rails-middleware-move-before-after · 031763ab
      Rafael França 提交于
      Delayed middleware delete does not allow move operations
      031763ab
    • R
      Merge pull request #38186 from vishaltelangre/deprecate-AS-range-include_time_with_zone-ext · ff341ae0
      Rafael França 提交于
      Deprecate using `Range#include?` to check the inclusion of a value in a date time range
      ff341ae0
    • R
      Merge pull request #38188 from roramirez/remove-test-code-not-used · 039ad441
      Rafael França 提交于
      Remove method encode from url_test in Actionmailer:
      039ad441
    • R
      a901372e
    • V
      Deprecate using `Range#include?` to check the inclusion of a value in a date time range · 740bb178
      Vishal Telangre 提交于
      Use `Range#cover?` instead of `Range#include?` to check the inclusion of a value in a date time range.
      740bb178
    • R
      Remove method encode from url_test in Actionmailer: · db46f448
      Rodrigo Ramírez Norambuena 提交于
      This method is introduced in c064802d but at this moment is not
      use anymore.
      db46f448
    • R
      Merge pull request #38118 from mkrfowler/reduce_preloaded_records · 2c7553cd
      Rafael França 提交于
      Reduce the number of records loaded when preloading across a `has_one`
      2c7553cd
    • R
      Merge pull request #38184 from hegyi/improve-link-generation-performance-when-constraints-are-given · d8a5ea1d
      Rafael França 提交于
      Memoize regex when checking missing route keys
      d8a5ea1d
    • G
      Delayed middleware delete does not allow move operations · 40fc1651
      Genadi Samokovarov 提交于
      While trying to fix #16433, we made the middleware deletions always
      happen at the end. While this works for the case of deleting the
      Rack::Runtime middleware, it makes operations like the following
      misbehave.
      
      ```ruby
      gem "bundler", "< 1.16"
      
      begin
        require "bundler/inline"
      rescue LoadError => e
        $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
        raise e
      end
      
      gemfile(true) do
        source "https://rubygems.org"
      
        git_source(:github) { |repo| "https://github.com/#{repo}.git" }
      
        gem "rails", github: "rails/rails"
      end
      
      require "action_controller/railtie"
      
      class TestApp < Rails::Application
        config.root = __dir__
        secrets.secret_key_base = "secret_key_base"
      
        config.logger = Logger.new($stdout)
        Rails.logger  = config.logger
      
        middleware.insert_after ActionDispatch::Session::CookieStore, ::Rails::Rack::Logger, config.log_tags
        middleware.delete ::Rails::Rack::Logger
      end
      
      require "minitest/autorun"
      require "rack/test"
      
      class BugTest < Minitest::Test
        include Rack::Test::Methods
      
        def test_returns_success
          get "/"
          assert last_response.ok?
        end
      
        private
          def app
            Rails.application
          end
      end
      ```
      
      In the case ️  the ::Rails::Rack::Logger would be deleted instead of
      moved, because the order of middleware stack building execution will be:
      
      ```ruby
      [:insert, ActionDispatch::Session::CookieStore, [::Rails::Rack::Logger]]
      [:delete, ::Rails::Rack::Logger, [config.log_tags]]
      ```
      
      This is pretty surprising and hard to reason about behaviour, unless you
      go spelunking into the Rails configuration code.
      
      I have a few solutions in mind and all of them have their drawbacks.
      
      1. Introduce a `Rails::Configuration::MiddlewareStackProxy#delete!` that
      delays the deleted operations. This will make `#delete` to be executed
      in order. The drawback here is backwards incompatible behavior and a new
      public method.
      
      2. Just revert to the old operations. This won't allow people to delete
      the `Rack::Runtime` middleware.
      
      3. Legitimize the middleware moving with the new `#move_after` and
      `#move_before` methods. This does not breaks any backwards
      compatibility, but includes 2 new methods to the middleware stack.
      
      I have implemented `3.` in this pull request.
      
      Happy holidays! 🎄
      40fc1651
    • A
      Memoize regex when checking missing route keys · e0eff2c3
      Adam Hegyi 提交于
      When the route definition has parameters, we can supply a regex for
      validation purposes:
      
          get "/a/:b" => "test#index", constraints: { b: /abc/ }, as: test
      
      This regex is going to be used to check the supplied values during
      link generation:
      
          test_path("abc") # check "abc" against /abc/ regex
      
      The link generation code checks each parameter. To properly validate the
      parameter, it creates a new regex with start and end of string modifiers:
      
          /\A#{original_regex}\Z/
      
      This means for each link generation the code stringifies the existing
      regex and creates a new one. When a new regex is created, it needs to be
      compiled, for large regexes this can take quite a bit of time.
      
      This change memoizes the generated regex for each route when constrains
      are given. It also removes the RegexCaseComparator class since it is not
      in use anymore.
      e0eff2c3
    • R
      Merge pull request #38183 from Tietew/test_session_dig · b50ce8d6
      Ryuta Kamizono 提交于
      Add AC::TestSession#dig method like AD::Request::Session
      b50ce8d6
    • I
      Add AC::TestSession#dig method like AD::Request::Session · 8e9c48b5
      IWASE 提交于
      8e9c48b5
    • E
      Merge pull request #38179 from eileencodes/fix-resolve-symbol-conn · 09fbee82
      Eileen M. Uchitelle 提交于
      Restore previous behavior of parallel test databases
      09fbee82
    • R
      Merge pull request #38177 from RicardoTrindade/bump_image_processing · 8c65783d
      Ryuta Kamizono 提交于
      Bump image_processing gem version to fixe ruby 2.7 warnings
      8c65783d
    • 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
    • X
      addresses RuboCop feedback · ad04bc0d
      Xavier Noria 提交于
      ad04bc0d
    • R
    • X
      restores the ability to manually eager load applications · c0d91a4f
      Xavier Noria 提交于
      The main interface to eager loading is config.eager_load. The logic that
      implies happens during the boot process.
      
      With the introduction of Zeitwerk, application code is loaded in the
      finisher as everything else, but in previous versions of Rails users
      could eager load the application code regardless of config.eager_load.
      
      Use cases:
      
         * Some gems like indexers need to have everything in memory and would
         be a bad user experience to ask users to conditionally set the eager
         load flag.
      
         * Some tests may need to have everything in memory and would be a bad
         experience to have the flag enabled globally in the test environment.
      
      I personally feel that the contract between this method and the entire
      eager loading process is ill-defined. I believe this method is
      essentially internal. The purpose of this patch is simply to restore this
      functionality emulating what it did before because rethinking the design
      of this interface may need time.
      c0d91a4f
    • M
      Fuse traversals of the record set when preloading · 989dda36
      Michael Fowler 提交于
      Although consuming code will almost certainly retraverse this set, we can avoid
      walking it twice here. As an extra upside, we can avoid the double-use of an
      identity-sensitive hash; this is convenient, because it is another collection we
      actually don't need to build.
      989dda36
    • 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