Commit Graph

69884 Commits

Author SHA1 Message Date
schneems
11bcf95287 [ci skip] Document permitted_scalar_filter 2018-08-30 11:03:22 -05:00
Ryuta Kamizono
5e86ee1195 Refactor attributes_for_{create,update} to avoid an extra allocation
Use `delete_if` instead of `reject` to avoid an extra allocation.
2018-08-31 00:06:12 +09:00
Ryuta Kamizono
56ca81a911 Remove attributes_with_values_for_{create,update} for internal use
`attributes_with_values_for_update` is no longer used since ae2d36c, and
`attributes_with_values_for_create` is internally used only one place.
2018-08-30 23:42:33 +09:00
Ryuta Kamizono
a72ac3cfec Remove extra & self.class.column_names in keys_for_partial_write
It should be done only once in `Persistence` module.

c83e30da27/activerecord/lib/active_record/persistence.rb (L721)
c83e30da27/activerecord/lib/active_record/persistence.rb (L740)
2018-08-30 22:43:26 +09:00
Ryuta Kamizono
9297af5daf
Merge pull request #33757 from bogdanvlviv/follow-up-32937
Follow up #32937 [ci skip]
2018-08-30 20:25:00 +09:00
bogdanvlviv
03ba74c8f0
Add info about purpose in cookies to "Upgrading Ruby on Rails" guide [ci skip]
Context https://github.com/rails/rails/pull/33605#discussion_r210354278

Related to #32937, #33605
2018-08-30 11:50:03 +03:00
bogdanvlviv
35740ab2da
Add info about config.action_dispatch.use_cookies_with_metadata to "Configuring Rails Applications" guide [ci skip]
Related to #32937, #33605.
2018-08-30 11:49:52 +03:00
bogdanvlviv
3175d3d549
Fix actionpack/CHANGELOG.md [ci skip]
Remove the reference to the PR.
Usually, we write reference to solved issues in the changelog files.
Related to #33605.

Add missing dots.

Improve formatting.
2018-08-30 10:04:28 +03:00
yuuji.yaginuma
fa132efe82 Add missing require
Without this, `inverse_associations_test.rb` breaks when running in isolation.
https://travis-ci.org/rails/rails/jobs/422266840#L1894-L1899
2018-08-30 08:14:35 +09:00
Rafael França
ed1eda271c
Merge pull request #33729 from kddeisz/plural-automatic-inverse
Find inverse associations with plural names
2018-08-29 16:00:41 -05:00
Richard Schneeman
3d2caab7dc
Merge pull request #33752 from sgrif/sg-faster-try
20% faster `try`
2018-08-29 12:22:20 -07:00
Rafael França
aab0c25f58
Merge pull request #33737 from bogdanvlviv/add-6_0_release_notes-guide
Add "Ruby on Rails 6.0 Release Notes" guide [ci skip]
2018-08-29 13:51:31 -05:00
Sean Griffin
0f462da85d 20% faster try
Following up on #33747, this takes things a step further by pulling out
the method name from the arguments array, letting us skip an allocation
in the case where there are no arguments -- notably, this also no longer
*requires* the splat to be an array, allowing us to benefit from
optimizations in Jruby (and maybe MRI in the future) of skipping the
array allocation entirely.

Benchmark results:

```
Warming up --------------------------------------
                 old   179.987k i/100ms
                 new   199.201k i/100ms
Calculating -------------------------------------
                 old      3.029M (± 1.6%) i/s -     15.299M in   5.052417s
                 new      3.657M (± 1.2%) i/s -     18.326M in   5.012648s

Comparison:
                 new:  3656620.7 i/s
                 old:  3028848.3 i/s - 1.21x  slower
```
2018-08-29 12:42:01 -06:00
Ryuta Kamizono
cc81cd359c
Merge pull request #33744 from bogdanvlviv/fixes-27852
Prevent leaking of user's DB credentials on `rails db:create` failure
2018-08-30 02:44:58 +09:00
Richard Schneeman
0d63712fdf
Merge pull request #33749 from schneems/schneems/faster-fragment
Fewer allocations in caching/fragments.rb
2018-08-29 10:44:33 -07:00
Richard Schneeman
0d0f48a20a
Merge pull request #33750 from schneems/schneems/time_value
Faster time_value.rb
2018-08-29 10:43:29 -07:00
schneems
e81b0ddd7a Faster time_value.rb
The multiplication of the value takes a long time when we can instead mutate and use the string value directly.

The `microsec` perf increases speed by 27% in the ideal case (which is the most common).

```
original_string = ".443959"

require 'benchmark/ips'

Benchmark.ips do |x|
  x.report("multiply") { 
    string = original_string.dup
    (string.to_r * 1_000_000).to_i 
  }
  x.report("new     ") { 
    string = original_string.dup
    if string && string.start_with?(".".freeze) && string.length == 7
      string[0] = ''.freeze
      string.to_i
    end
  }
  x.compare!
end

# Warming up --------------------------------------
#             multiply   125.783k i/100ms
#             new        146.543k i/100ms
# Calculating -------------------------------------
#             multiply      1.751M (± 3.3%) i/s -      8.805M in   5.033779s
#             new           2.225M (± 2.1%) i/s -     11.137M in   5.007110s

# Comparison:
#             new     :  2225289.7 i/s
#             multiply:  1751254.2 i/s - 1.27x  slower
```
2018-08-29 11:45:47 -05:00
Richard Schneeman
96d7504da9
Merge pull request #33747 from schneems/schneems/faster-try
32% Faster Object#try
2018-08-29 09:10:49 -07:00
Eileen Uchitelle
a64dba1dc5 Remove this conditional
I removed the argument so I should remove the conditional too.
2018-08-29 12:08:23 -04:00
Eileen Uchitelle
c91f1482a1 Remove unused argument
The test that used this was updated and it's no longer needed.
2018-08-29 11:08:10 -04:00
Eileen M. Uchitelle
55133a0e8e
Merge pull request #33748 from eileencodes/fix-erb-loading-issue-with-db-yaml
Drop load_database_yaml and fix test
2018-08-29 11:07:01 -04:00
schneems
ba7d1265e3 32% Faster Object#try
Here’s the micro benchmark:

```ruby
module ActiveSupport
  module NewTryable #:nodoc:
    def try(*a, &b)
      return unless a.empty? || respond_to?(a.first)
      return public_send(*a, &b) unless a.empty?

      return nil unless block_given?
      return instance_eval(&b) if b.arity == 0
      yield self
    end

    def try!(*a, &b)
      return public_send(*a, &b) if !a.empty?
      return nil unless block_given?
      return instance_eval(&b) if b.arity == 0
      yield self
    end
  end
end


module ActiveSupport
  module OldTryable #:nodoc:
    def try(*a, &b)
      try!(*a, &b) if a.empty? || respond_to?(a.first)
    end

    def try!(*a, &b)
      if a.empty? && block_given?
        if b.arity == 0
          instance_eval(&b)
        else
          yield self
        end
      else
        public_send(*a, &b)
      end
    end
  end
end

class FooNew
  include ActiveSupport::NewTryable

  def foo
  end
end

class FooOld
  include ActiveSupport::OldTryable

  def foo
  end
end 


foo_new = FooNew.new
foo_old = FooOld.new

require 'benchmark/ips'

Benchmark.ips do |x|
  x.report("old") { foo_old.try(:foo) }
  x.report("new") { foo_new.try(:foo) }
  x.compare!
end

# Warming up --------------------------------------
#                  old   144.178k i/100ms
#                  new   172.371k i/100ms
# Calculating -------------------------------------
#                  old      2.181M (± 8.0%) i/s -     10.813M in   5.001419s
#                  new      2.889M (± 7.7%) i/s -     14.479M in   5.051760s

# Comparison:
#                  new:  2888691.7 i/s
#                  old:  2180740.7 i/s - 1.32x  slower
```

Also reduces memory. On https://www.codetriage.com i’m seeing 1.5% fewer object allocations per request (in object count).

Before:

Total allocated: 1014475 bytes (8525 objects)

After:

Total allocated: 1015499 bytes (8389 objects)
2018-08-29 10:03:48 -05:00
schneems
e92adb3da6 Fewer allocations in caching/fragments.rb
Instead of using a splat on the head and tail we can mutate the array by flattening 1 level. We get further savings by not allocating another via `compact` but instead by using `compact!`
2018-08-29 09:49:24 -05:00
Eileen Uchitelle
6b5df90fb5 Drop load_database_yaml and fix test
We originally did the whole `load_database_yaml` thing because this test
wasn't cooperating and we needed to finish the namespaced rake tasks for
multiple databases.

However, it turns out that YAML can't eval ERB if you don't tell it it's
ERB so you get Pysch parse errors if you're using multi-line ERB or
ERB with conditionals. It's a hot mess.

After trying a few things and thinking it over we decided that it wasn't
worth bandaiding over, the test needed to be improved. The test was
added in #31135 to test that the env is loaded in these tasks. But it
was blowing up because we were trying to read a database name out of the
configuration - however that's not the purpose of this change. We want
to read environment files in the rake tasks, but not in the config
file.

In this PR we changed the test to test what the PR was actually fixing.
We've also deleted the `load_database_yaml` because it caused more
problems than it was worth. This should fix the issues described in
https://github.com/rails/rails/pull/32274#issuecomment-384161057. We
also had these problems at GitHub.

Co-authored-by: alimi <aibrahim2k2@gmail.com>
2018-08-29 10:26:44 -04:00
bogdanvlviv
9b455fe6f0
Prevent leaking of user's DB credentials on rails db:create failure
Issue #27852 reports that when `rails db:create` fails, it causes
leaking of user's DB credentials to $stderr.
We print a DB's configuration hash in order to help users more quickly
to figure out what could be wrong with his configuration.

This commit changes message from
"Couldn't create database for #{configuration.inspect}" to
"Couldn't create '#{configuration['database']}' database. Please check your configuration.".

There are two PRs that fixing it #27878, #27879, but they need a bit more work.
I decided help to finish this and added Author of those PRs credit in this commit.

Since it is a security issue, I think we should backport it to
`5-2-stable`, and `5-1-stable`.
Guided by https://edgeguides.rubyonrails.org/maintenance_policy.html#security-issues

Fixes #27852
Closes #27879
Related to #27878

[Alexander Marrs & bogdanvlviv]
2018-08-29 12:40:30 +03:00
bogdanvlviv
02fa55ac29
Add "Ruby on Rails 6.0 Release Notes" guide [ci skip]
This commit adds a skeleton of "Ruby on Rails 6.0 Release Notes".
It isn't a good time to add changelogs' entries to this guide since we can
redo/revert some things till the final release 6.0.
It would be better to do it close to the release.
But we already can add mentions about major features
that have been added to 6.0. I added mention about "Parallel Testing".
2018-08-29 12:09:43 +03:00
Matthew Draper
068fe7dc90
Merge pull request #33718 from kddeisz/permit-list
Finish converting whitelist and blacklist references
2018-08-29 14:07:37 +09:30
yuuji.yaginuma
28e5085070 Generate the same value as a label of view in system test template
In the system test template, enter a value based on label.
However, since `label` method does not use `titleize` by default.
If generate a value including underscore, cannot find a label and the test
will fail.

```
$ ./bin/rails g scaffold user name:string phone_number:string
$ ./bin/rails t test/system/users_test.rb

E

Error:
UsersTest#test_creating_a_User:
Capybara::ElementNotFound: Unable to find field "Phone Number"
    test/system/users_test.rb:18:in `block in <class:UsersTest>'
```

This removes unnecessary `titleize` so that the generated file will pass
even if the attribute contains an underscore.
2018-08-29 09:58:21 +09:00
Rafael Mendonça França
a8fff60814
Add test to make sure the custom object key can't be serialized 2018-08-28 16:31:15 -04:00
James Brooks
538f42900f Focus search input after page load on /rails/info/routes (#33683) 2018-08-28 14:42:30 -05:00
Ryuta Kamizono
4f09cff52c
Merge pull request #33739 from tleneveu/edgeguides-https-links
Use the HTTPS protocol for links to Edges Guides [ci skip]
2018-08-28 23:24:29 +09:00
Ryuta Kamizono
188daa9322
Merge pull request #33689 from ypresto/ar-fix-dirty-in-around
Mutation tracker should be cleared before continuing around callbacks
2018-08-28 23:15:22 +09:00
tleneveu
3d22686995 Use the HTTPS protocol for links to Edges Guides [ci skip]
Edges Guides is now available in HTTPS, it would be better to use the HTTPS protocol directly.
2018-08-28 13:49:58 +00:00
Rob Race
018ff582a5 Update the comments for TimeWithZone subtraction (#33721)
* Update the comments for TimeWithZone subtraction

While reading the description/documentation comments for the subtraction method, it was confusing. While in practical usage, the returned values make sense. It seems as though the explanation could be a bit clearer.

* Removed erroneous closing parenthesis

[ci skip]

[Rob Race + Ryuta Kamizono]
2018-08-28 17:32:18 +09:00
Ryuta Kamizono
37075aa7f5
Merge pull request #33654 from kamipo/fix_numericality_validator_2
Fix numericality validator to still use value before type cast except Active Record
2018-08-28 17:23:48 +09:00
Yuya Tanaka
34f075fe56 Mutation tracker should be cleared before continuing around callbacks
`changes_applied` should be called before continuing around callback
chain. Otherwise the mutation tracker returns old value for methods like
`changed`? or `id_in_database` in around callbacks. Also methods depend
on `id_in_database`, like `update_column`, are not working in
`around_create` callbacks.

```
class Foo < ActiveRecord::Base
  around_create :around_create_callback

  def around_create_callback
    ...
    yield
    p id_in_database # => nil
    update_column(:generated_column, generate_value) # silently fails
  end

  ...
end
```
2018-08-28 16:32:23 +09:00
speckins
1353610ff2 Call block to #redirect_to in controller context (#33735)
* Call block to #redirect_to in controller context

The documentation for ActionController::Redirecting states that a Proc
argument "will be executed in the controller's context."  However,
unless #instance_eval is used (removed in 6b3ad0ca), that statement is
false for procs defined outside of the controller instance.

This commit restores the documented behavior.

Fixes #33731.

* Move test proc into a constant in another class

Per @rafaelfranca's suggestion.

[Steven Peckins + Rafael Mendonça França]
2018-08-27 22:36:39 -05:00
Nate Pinsky
626958ce98 Add documentation for :collation column option (#33733)
* Add documentation for `:collation` option

The table definition supports a `:collation` option for string and text columns, but this is not documented anywhere that I could find.

I'm not sure if the "If not specified" part is accurate. From [this PR](1515c4d98d), it looks like it passes `nil` and lets the database handle the collation, but I'm happy to change it if I misread the code.

[ci skip]

* FIX remove whitespace

[Nate Pinsky + Rafael Mendonça França]
2018-08-27 21:13:06 -05:00
Kevin Deisz
1c7275a0ba
Find inverse associations with plural names
Previously ActiveRecord couldn't find inverse associations if they were plural, which is a pretty standard use case. This commit changes the behavior to first attempt to find the singular version, then attempt to find the plural version. That makes it work and find plural associations as in the example below:

```
class Post
  has_many :comments
end

class Comment
  belongs_to :post
end
```

Previously the `:post` association reflection would only attempt to find a `comment` inverse, as opposed to both a `comment` and `comments` inverse.
2018-08-27 15:22:37 -04:00
Rafael França
3218a66281
Merge pull request #31972 from bogdanvlviv/update-form_helpers-guide
Update "Action View Form Helpers" guide
2018-08-27 11:36:13 -05:00
bogdanvlviv
841fc1837b
Update "Action View Form Helpers" guide [ci skip] 2018-08-27 19:24:38 +03:00
Kevin Deisz
7c9751d7fe
Permit list usage cleanup and clearer documentation 2018-08-27 09:51:46 -04:00
Ryuta Kamizono
7e881a18d9
Merge pull request #33727 from bogdanvlviv/follow-up-33523
Follow up #33523 [ci skip]
2018-08-27 21:47:16 +09:00
bogdanvlviv
3a598ae6d1
Add :namespace option to the api docs of form_with [ci skip] 2018-08-27 15:40:09 +03:00
bogdanvlviv
318ccf5b3b
Follow up #33523 [ci skip]
This commit is the next work after #33523.

Also, this commit removes mention about hidden `utf8` input. Since
form helpers don't generate this input by default since #32125.

Note that I also had created PR #31972 with improvements to
"Action View Form Helpers" guide, but I'll rebase it after merging the
current PR.
2018-08-27 15:39:49 +03:00
Ryuta Kamizono
6d5eefd453 Remove unused requires 2018-08-25 23:03:15 +09:00
Matthew Draper
1d9842467e
Merge pull request #33723 from pinewong/patch-3
Clarify example of the test [ci skip]
2018-08-25 23:28:02 +09:30
黄松
f54060916f
Clarify example of the test [ci skip]
ActionDispatch::TestProcess::FixtureFile
2018-08-25 21:44:11 +08:00
Ryuta Kamizono
519d09cb99
Merge pull request #33661 from jychen7/33428-test-has-many-association-enum
Add test case to test enum in has_many
2018-08-25 21:54:22 +09:00
Rich
eed8b23318 Add test case to test enum in has_many
There is test in has_one to test enum, but there is no for has_many.

[Rich Chen]
2018-08-25 19:58:48 +08:00