Commit Graph

96 Commits

Author SHA1 Message Date
Jean Boussier
6842e766a1 Fix Action View collection caching to store fragments as bare strings
Ref: https://github.com/rails/rails/issues/48611

Individual fragments have been cached as bare string since forever,
but somehow fragment cached via collection caching were stored
as `ActionView::OutputBuffer` instances.

This is both bad for performance, but also can cause issues on
Rails upgrades if the internal representation of `AV::OutputBuffer`
changes.
2023-07-04 16:37:22 +02:00
Jean Boussier
f56b4187be Fix Active Record :db_runtime metric
In https://github.com/rails/rails/pull/45796 I overlooked that
`ActiveRecord::LogSubscriber#sql` wasn't only logging the SQL query
but was also responsible for collecting the `:db_runtime` metric.

Ultimately I think it is cleaner to move this concern to `RuntimeRegistry`.
2022-09-15 10:43:33 +02:00
Jean Boussier
8994f09dd2
Merge pull request #44616 from ghiculescu/preload-all-if-proc
Always preload if using proc with multifetch cache
2022-08-09 09:01:30 +02:00
Jean Boussier
4b67dffc8d Refactor Action View tests to stop re-assigning @output_buffer
Followup on https://github.com/rails/rails/pull/45745

`@rendered` is a much better variable for this kind of intermediate
result as that's what is used by DOM assertions.
2022-08-03 15:00:58 +02:00
Alex Ghiculescu
3f1f4c7d0c Always preload if using proc with multifetch cache
https://github.com/rails/rails/pull/31250 optimised rendering a collection with `cached: true`, but also with `cached: proc {}`. The problem is the proc may reference associations that should be preloaded to avoid n+1s in generating the cache key.

For example:

```ruby
@posts = Post.preload(:author)

@view.render partial: "test/partial", collection: @post, cached: proc { |post| [post, post.author] }
```

This will n+1 on `post.author`.

To fix this, this PR will always preload the collection if there's a proc given for `cached`. `cached: true` will still not preload unless it renders, as it did in https://github.com/rails/rails/pull/31250.
2022-05-07 22:19:39 +10:00
Clayton Smith
6e56e18523 Fix flaky Action View tests 2022-02-23 09:39:57 -05:00
Jean Boussier
e26372b713 Implicitly assert no exception is raised in assert_queries & al
Fix: https://github.com/rails/rails/pull/44397
Ref: https://github.com/rails/rails/pull/37313
Ref: https://github.com/rails/rails/pull/42459

This avoid mistakes such as:

```ruby
assert_raise Something do
  assert_queries(1) do
    raise Something
  end
end
```

Co-Authored-By: Alex Coomans <alexc@squareup.com>
2022-02-19 09:11:14 +01:00
Ryan Baumann
168170ff14 Add autocomplete="off" to all generated hidden fields (fixes #42610) 2021-09-21 15:57:56 -04:00
Rafael França
58ed26f616
Revert "Extract methods assert_queries and assert_no_queries" 2021-06-17 13:35:25 -04:00
Ricardo Díaz
f644f7d35b Extract methods assert_queries and assert_no_queries
Both methods are defined in multiple parts of the framework. It would
be useful to put them in a proper place, so that repetition is
avoided.

I chose the implementation from `ActiveRecord` because it's a bit more
complete with the `SQLCounter` class, and also because other parts
depend on it.
2021-06-14 09:55:13 -05:00
Gannon McGibbon
c4c21a9f8d
Prevent string polymorphic route arguments
url_for supports building polymorphic URLs via an array
of arguments (usually symbols and records). If an array is passed,
strings can result in unwanted route helper calls.

CVE-2021-22885
2021-05-04 13:56:37 -07:00
John Hawthorn
fb6b0a51b3 Remove FallbackFileSystemResolver
This removes FallbackFileSystemResolver, LookupContext#with_fallbacks,
and LookupContext.fallbacks.

These used to exist to support `render file:`, and rendering templates
outside of the defined view paths. `render file:` has since been changed
to not use template resolution, and to only render absolute paths.

These were no longer callable through any public API.
2021-04-06 14:20:48 -07:00
Marek Kasztelnik
8e53f9178c Revert "Remove redundant @virtual_path variable"
This reverts commit dd7a673782f102d2b58194007bf18a8e496818e3.
2020-12-30 14:07:09 +01:00
aaron
432698ef2b
Fix SELECT COUNT queries when rendering ActiveRecord collections (#40870)
* Fix `SELECT COUNT` queries when rendering ActiveRecord collections

Fixes #40837

When rendering collections, calling `size` when the collection is an
ActiveRecord relation causes unwanted `SELECT COUNT(*)` queries. This
change ensures the collection is an array before getting the size, and
also loads the relation for any further array inspections.

* Test queries when rendering relation collections

* Add `length` support to partial collection iterator

Allows getting the size of a relation without duplicating records, but
still loads the relation. The length method existence needs to be
checked because you can pass in an `Enumerator`, which does not respond
to `length`.

* Ensure unsubscribed from notifications after tests

[Rafael Mendonça França + aar0nr]
2020-12-18 15:58:11 -05:00
Ryuta Kamizono
528b62e386 Address to false negative for Performance/DeletePrefix,DeleteSuffix
Follow up to c07dff72278fb7f2a3c4c71212a0773a2b25c790.

Actually it is not the cop's fault, but we mistakenly use `^`, `$`, and
`\Z` in much places, the cop doesn't correct those conservatively.

I've checked all those usage and replaced all safe ones.
2020-06-14 13:04:47 +09:00
Aaron Lipman
dd7a673782
Remove redundant @virtual_path variable
Following the introduction of the @current_template variable in 1581cab,
the @virtual_path variable is now redundant, as the value of the virtual
path may be accessed via @current_template.virtual_path. This commit
removes @virtual_path and replaces any references to @virtual_path with
@current_template.virtual_path.
2020-05-26 15:53:11 -04:00
Aaron Patterson
49adb7f4c6
pull preloading behavior in to the collection renderer 2020-02-28 09:23:22 -08:00
Aaron Patterson
0c1fbc7e04
collection rendering has its own class 2020-02-26 14:51:31 -08:00
Santiago Bartesaghi
8872ec431a
perform_caching config affects collection caching 2019-12-05 21:46:02 +01:00
st0012
e289c8d775 Assert query counts in cache relation test
This is to guard the change in #35982
2019-07-28 14:24:47 +08:00
Ryuta Kamizono
c81af6ae72 Enable Layout/EmptyLinesAroundAccessModifier cop
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).

Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).

That cop and enforced style will reduce the our code review cost.
2019-06-13 12:00:45 +09:00
Aaron Patterson
1581cab9ff
Pass the template format to the digestor
This commit passes the template format to the digestor in order to come
up with a key.  Before this commit, the digestor would depend on the
side effect of the template renderer setting the rendered_format on the
lookup context.  I would like to remove that mutation, so I've changed
this to pass the template format in to the digestor.

I've introduced a new instance variable that will be alive during a
template render.  When the template is being rendered, it pushes the
current template on to a stack, setting `@current_template` to the
template currently being rendered.  When the cache helper asks the
digestor for a key, it uses the format of the template currently on the
stack.
2019-02-15 17:27:33 -08:00
Aaron Patterson
f9bea6304d
Move templates to an anonymous subclass of AV::Base
Now we can throw away the subclass and the generated methods will get
GC'd too
2019-02-06 16:52:15 -08:00
Aaron Patterson
e17fe52e0e
Tighten up the AV::Base constructor
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:

  94d54fa4ab/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.
2019-01-29 15:49:40 -08:00
Eileen Uchitelle
e8c1be4ae7 Add allocations to template renderer subscription
This PR adds the allocations to the instrumentation for template and
partial rendering.

Before:

```
  Rendering posts/new.html.erb within layouts/application
  Rendered posts/_form.html.erb (9.7ms)
  Rendered posts/new.html.erb within layouts/application (10.9ms)
Completed 200 OK in 902ms (Views: 890.8ms | ActiveRecord: 0.8ms)
```

After:

```
  Rendering posts/new.html.erb within layouts/application
  Rendered posts/_form.html.erb (Duration: 7.1ms | Allocations: 6004)
  Rendered posts/new.html.erb within layouts/application (Duration: 8.3ms | Allocations: 6654)
Completed 200 OK in 858ms (Views: 848.4ms | ActiveRecord: 0.4ms | Allocations: 1539564)
```
2018-10-10 08:07:12 -04:00
Rafael Mendonça França
49f9dff9b6
Fix more offences 2018-09-25 13:21:40 -04:00
Rafael Mendonça França
f679933daa
Change the empty block style to have space inside of the block 2018-09-25 13:19:35 -04:00
Aaron Patterson
6d698fdb07
Merge pull request #33973 from rails/remove-catch-all
Remove deprecated catch-all route in the AV tests
2018-09-25 09:27:37 -07:00
Aaron Patterson
3301804ff6
make bot happy 2018-09-24 16:01:34 -07:00
Aaron Patterson
4f96e739c2
Remove deprecated catch-all route in the AV tests
This commit removes a deprecated catch-all route in the AV tests.  It
defines and includes the necessary routes for each test such that we
don't need the catch-all anymore.

This also helps push us toward #33970
2018-09-24 15:39:15 -07:00
yuuji.yaginuma
1b86d90136 Enable Performance/UnfreezeString cop
In Ruby 2.3 or later, `String#+@` is available and `+@` is faster than `dup`.

```ruby
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "benchmark-ips"
end

Benchmark.ips do |x|
  x.report('+@') { +"" }
  x.report('dup') { "".dup }
  x.compare!
end
```

```
$ ruby -v benchmark.rb
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
Warming up --------------------------------------
                  +@   282.289k i/100ms
                 dup   187.638k i/100ms
Calculating -------------------------------------
                  +@      6.775M (± 3.6%) i/s -     33.875M in   5.006253s
                 dup      3.320M (± 2.2%) i/s -     16.700M in   5.032125s

Comparison:
                  +@:  6775299.3 i/s
                 dup:  3320400.7 i/s - 2.04x  slower

```
2018-09-23 08:56:55 +09:00
schneems
1bd578ffe6 Don’t allocate array on no args
When no dependencies are present to be digested there is no reason to build an array just to turn around and turn it back into a string.

The dependencies array is not mutated in this method so we can use the same empty array across all invocations.

Total allocated: 791402 bytes (7294 objects)
Total allocated: 777442 bytes (7132 objects)

(791402 - 777442) / 791402.0 # => 1.76 % speed improvement
2018-09-07 17:33:10 -05:00
Lachlan Sylvester
d04a32fe67 Only preload misses on multifetch cache 2018-03-06 14:10:45 +11:00
yuuji.yaginuma
35dc49e517 Remove unused methods from RenderPartialWithRecordIdentificationController
These methods no longer used since a3da293.
2017-09-30 17:38:27 +09:00
Kir Shatrov
424117281e Use frozen string literal in actionview/ 2017-07-24 11:53:43 +03:00
Matthew Draper
87b3e226d6 Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
2017-07-02 02:15:17 +09:30
Matthew Draper
3420a14590 Merge pull request #29540 from kirs/rubocop-frozen-string
Enforce frozen string in Rubocop
2017-07-02 01:11:50 +09:30
Kir Shatrov
cfade1ec7e Enforce frozen string in Rubocop 2017-07-01 02:11:03 +03:00
Pat Allan
3d453b409d Make ActionView frozen string literal friendly.
Plus a couple of related ActionPack patches.
2017-06-20 22:20:04 +10:00
Kasper Timm Hansen
8240636bed Merge pull request https://github.com/rails/rails/pull/28637 from st0012/fix-partial-cache-logging 2017-06-08 21:42:46 +02:00
Stan Lo
2abf6ca0c8 Use a hash to record every partial's cache hit status instead of sharing a boolean. 2017-06-08 21:42:46 +02:00
David Heinemeier Hansson
75fa8dd309 Use recyclable cache keys (#29092) 2017-05-18 18:12:32 +02:00
Ryuta Kamizono
b201474756 Should escape meta characters in regexp 2017-05-07 04:10:00 +09:00
Andrew White
fd16e1a92f Always use original url_for when generating direct routes
Action View overrides `url_for` in the view context to render paths by
default when using `url_for` and this means that direct route helpers
don't get the full url when called with the url suffix. To fix this
always call the original `url_for`.
2017-03-17 21:30:29 +00:00
bogdanvlviv
f1a097733a Add assertion to polymorphic_routes_test.rb
The assertion will ensure that the behavior doesn't regress.
  assert_equal "/projects", polymorphic_path("projects")

Remove FIXME related to polymorphic_url behavior.
polymorphic_url with Symbol or String works equally.
Example:

  default_url_options[:host] = "example.com"

  polymorphic_url(:projects)  # => "http://example.com/projects"
  polymorphic_url("projects") # => "http://example.com/projects"

Related to 37d4415a7b433fcb987b1c6a5b51bf2d8efc5d5e
2017-02-20 01:27:32 +02:00
Akira Matsuda
6c6668510b module Blog; class Post appears twice in AV tests
This causes TypeError when loaded separately
2017-02-02 18:18:08 +09:00
Akira Matsuda
9360b6be63 class Foo < Struct.new(:x) creates an extra unneeded anonymous class
because Struct.new returns a Class, we just can give it a name and use it directly without inheriting from it
2017-01-13 15:13:47 +09:00
Akira Matsuda
b70fc698e1 Reduce string objects by using \ instead of + or << for concatenating strings
(I personally prefer writing one string in one line no matter how long it is, though)
2017-01-12 17:45:37 +09:00
Akira Matsuda
5473e390d3 self. is not needed when calling its own instance method
Actually, private methods cannot be called with `self.`, so it's not just redundant, it's a bad habit in Ruby
2017-01-05 19:58:52 +09:00
Akira Matsuda
1f5bed9855 Privatize unneededly protected methods in Action View tests 2016-12-23 23:49:02 +09:00