aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib/abstract_controller
Commit message (Collapse)AuthorAgeFilesLines
* Freeze string literals when not mutated.schneems2015-07-192-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I wrote a utility that helps find areas where you could optimize your program using a frozen string instead of a string literal, it's called [let_it_go](https://github.com/schneems/let_it_go). After going through the output and adding `.freeze` I was able to eliminate the creation of 1,114 string objects on EVERY request to [codetriage](codetriage.com). How does this impact execution? To look at memory: ```ruby require 'get_process_mem' mem = GetProcessMem.new GC.start GC.disable 1_114.times { " " } before = mem.mb after = mem.mb GC.enable puts "Diff: #{after - before} mb" ``` Creating 1,114 string objects results in `Diff: 0.03125 mb` of RAM allocated on every request. Or 1mb every 32 requests. To look at raw speed: ```ruby require 'benchmark/ips' number_of_objects_reduced = 1_114 Benchmark.ips do |x| x.report("freeze") { number_of_objects_reduced.times { " ".freeze } } x.report("no-freeze") { number_of_objects_reduced.times { " " } } end ``` We get the results ``` Calculating ------------------------------------- freeze 1.428k i/100ms no-freeze 609.000 i/100ms ------------------------------------------------- freeze 14.363k (± 8.5%) i/s - 71.400k no-freeze 6.084k (± 8.1%) i/s - 30.450k ``` Now we can do some maths: ```ruby ips = 6_226k # iterations / 1 second call_time_before = 1.0 / ips # seconds per iteration ips = 15_254 # iterations / 1 second call_time_after = 1.0 / ips # seconds per iteration diff = call_time_before - call_time_after number_of_objects_reduced * diff * 100 # => 0.4530373333993266 miliseconds saved per request ``` So we're shaving off 1 second of execution time for every 220 requests. Is this going to be an insane speed boost to any Rails app: nope. Should we merge it: yep. p.s. If you know of a method call that doesn't modify a string input such as [String#gsub](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37) please [give me a pull request to the appropriate file](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37), or open an issue in LetItGo so we can track and freeze more strings. Keep those strings Frozen ![](https://www.dropbox.com/s/z4dj9fdsv213r4v/let-it-go.gif?dl=1)
* [ci skip] Don't use TrueClass, FalseClass in docsclaudiob2015-07-021-6/+0
| | | | | | | | | This sort of documentation style comes from 2009, probably due to the merging of merb (see https://github.com/rails/rails/commit/38b608ecab2441cd0c4e75bc08bdf57fcf85dd71#diff-017d9bc9b1d2bdae199b938d72c15488R120). Rails follows Ruby's convention to define which values are "truthy" or "falsey", so there is no need to specify that the returned value must strictly be a TrueClass or FalseClass. /cc @fxn
* Remove mistaken end from controller_path doc [ci skip]Mehmet Emin İNAÇ2015-06-221-1/+1
|
* Raise ArgumentError if an unrecognised callback is skippedIain Beeston2015-04-031-3/+3
| | | | | | | | | | | | | | At present, if you skip a callback that hasn't been defined, activesupport callbacks silently does nothing. However, it's easy to mistype the name of a callback and mistakenly think that it's being skipped, when it is not. This problem even exists in the current test suite. CallbacksTest::SkipCallbacksTest#test_skip_person attempts to skip callbacks that were never set up. This PR changes `skip_callback` to raise an `ArgumentError` if the specified callback cannot be found.
* Removed non-standard and unused require 'active_support/deprecation' from ↵Vipul A M2015-02-271-2/+0
| | | | parts out of active_support.
* fix NameError in `skip_filter`. callback doesn't exist.yuuji.yaginuma2015-02-271-1/+1
|
* Deprecate `AbstractController::Callbacks#skip_action_callback`Iain Beeston2015-02-241-0/+1
| | | | | | | | | | As part of #19029, in future `skip_before_action`, `skip_after_action` and `skip_around_action` will raise an ArgumentError if the specified callback does not exist. `skip_action_callback` calls all three of these methods and will almost certainly result in an ArgumentError. If anyone wants to remove all three callbacks then they can still call the three individual methods. Therefore let's deprecate `skip_action_callback` now and remove it when #19029 is merged.
* Merge pull request #11790 from printercu/patch-3Rafael Mendonça França2015-02-121-7/+8
|\ | | | | | | ActionController#translate supports symbols
| * ActionController#translate also lookups shortcut without action nameMax Melentiev2013-10-221-1/+5
| |
| * ActionController#translate supports symbolsprintercu2013-09-251-6/+3
| | | | | | | | Made it similar to views helper.
* | Use public Module#include, in favor of https://bugs.ruby-lang.org/issues/8846robertomiranda2015-01-311-2/+2
| | | | | | | | ref: https://github.com/rails/rails/pull/18763#issuecomment-72349769
* | fix typo in `_filter` deprecation message. [ci skip]Yves Senn2015-01-301-5/+5
| |
* | Deprecate all *_filter callbacks in favor of *_action callbacksAbdelkader Boudih2015-01-081-5/+27
| |
* | Merge pull request #18404 from claudiob/rebase-14549Rafael Mendonça França2015-01-081-0/+10
|\ \ | | | | | | Add test case and documentation for skip_before_filter.
| * | Add test/doc for :if/:except in skip_before_actionclaudiob2015-01-081-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new test/docs further explain the conflicts that can happen when mixing `:if`/`:unless` options with `:only`/`:except` options in `skip_before_action`. The gist is that "positive" filters always have priority over negative ones. The previous commit already showed that `:only` has priority over `:if`. This commit shows that `:if` has priority over `:except`. For instance, the following snippets are equivalent: ```ruby skip_before_action :some_callback, if: -> { condition }, except: action ``` ```ruby skip_before_action :some_callback, if: -> { condition } ```
| * | Add test case and documentation for skip_before_filter.Lauro Caetano2015-01-081-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | Test case for using skip_before_filter with the options :only and :if both present. In this case, the :if option will be ignored and :only will be executed. Closes #14549 (the commit was cherry-picked from there).
* | | Remove ActionController::HideActions (closes #18336)brainopia2015-01-061-14/+2
|/ /
* | remove unneeded check since /_one_time_conditions/ is removedAditya Kapoor2015-01-051-2/+1
| |
* | Merge pull request #17227 from claudiob/explicitly-abort-callbacksRafael Mendonça França2015-01-031-1/+1
|\ \ | | | | | | | | | | | | | | | | | | Introduce explicit way of halting callback chains by throwing :abort. Deprecate current implicit behavior of halting callback chains by returning `false` in apps ported to Rails 5.0. Completely remove that behavior in brand new Rails 5.0 apps. Conflicts: railties/CHANGELOG.md
| * | Throw :abort halts default CallbackChainsclaudiob2015-01-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes arguments and default value of CallbackChain's :terminator option. After this commit, Chains of callbacks defined **without** an explicit `:terminator` option will be halted as soon as a `before_` callback throws `:abort`. Chains of callbacks defined **with** a `:terminator` option will maintain their existing behavior of halting as soon as a `before_` callback matches the terminator's expectation. For instance, ActiveModel's callbacks will still halt the chain when a `before_` callback returns `false`.
* | | Fix a few typos [ci skip]Robin Dupret2015-01-032-3/+3
| | |
* | | Merge branch 'master' of github.com:rails/docrailsVijay Dev2015-01-033-18/+24
|\ \ \ | |/ / |/| |
| * | Better docs for AbstractControllerclaudiob2014-12-223-18/+24
| | | | | | | | | | | | | | | | | | Fixes internal links, adds examples and set fixed-width fonts. [ci skip]
* | | Merge pull request #8740 from amatsuda/missing_source_fileRafael Mendonça França2015-01-021-1/+1
|\ \ \ | |/ / |/| | | | | | | | | | | | | | replace use of MissingSourceFile with LoadError Conflicts: activesupport/test/core_ext/load_error_test.rb
| * | replace use of MissingSourceFile with LoadErrorAkira Matsuda2013-01-041-1/+1
| | |
* | | Pass symbol as an argument instead of a blockErik Michaels-Ober2014-11-291-1/+1
| | |
* | | give a better error message for misspelled helpersXavier Noria2014-10-251-1/+11
| | | | | | | | | | | | | | | | | | See comment in this patch for the rationale. References #16468
* | | remove deprecated `MissingHelperError` proxy.Yves Senn2014-08-141-3/+0
| | | | | | | | | | | | The error was moved outside of the `ClassMethods` module.
* | | Deprecate `*_path` methods in mailers@schneems and @sgrif2014-07-302-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Email does not support relative links since there is no implicit host. Therefore all links inside of emails must be fully qualified URLs. All path helpers are now deprecated. When removed, the error will give early indication to developers to use `*_url` methods instead. Currently if a developer uses a `*_path` helper, their tests and `mail_view` will not catch the mistake. The only way to see the error is by sending emails in production. Preventing sending out emails with non-working path's is the desired end goal of this PR. Currently path helpers are mixed-in to controllers (the ActionMailer::Base acts as a controller). All `*_url` and `*_path` helpers are made available through the same module. This PR separates this behavior into two modules so we can extend the `*_path` methods to add a Deprecation to them. Once deprecated we can use this same area to raise a NoMethodError and add an informative message directing the developer to use `*_url` instead. The module with warnings is only mixed in when a controller returns false from the newly added `supports_relative_path?`. Paired @sgrif & @schneems
* | | Relpace `=~ Regexp.new str` with `.include? str` in AC::Base#_valid_action_name?Viktar Basharymau2014-06-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because it is more natural way to test substring inclusion. Also, in this particular case it is much faster. In general, using `Regexp.new str` for such kind of things is dangerous. The string must be escaped, unless you know what you're doing. Example: Regexp.new "\\" # HELLO WINDOWS # RegexpError: too short escape sequence: /\/ The right way to do this is escape the string Regexp.new Regexp.escape "\\" # => /\\/ Here is the benchmark showing how faster `include?` call is. ``` require 'benchmark/ips' Benchmark.ips do |x| x.report('include?') { !"index".to_s.include? File::SEPARATOR } x.report(' !~ ') { "index" !~ Regexp.new(File::SEPARATOR) } end __END__ Calculating ------------------------------------- include? 75754 i/100ms !~ 21089 i/100ms ------------------------------------------------- include? 3172882.3 (±4.5%) i/s - 15832586 in 5.000659s !~ 322918.8 (±8.6%) i/s - 1602764 in 4.999509s ``` Extra `.to_s` call is needed to handle the case when `action_name` is `nil`. If it is omitted, some tests fail.
* | | Partially revert deprecation of *_filterRafael Mendonça França2014-06-031-27/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are going to deprecate only on Rails 5 to make easier plugin maintainers support different Rails versions. Right now we are only discouraging their usage. This reverts commit 6c5f43bab8206747a8591435b2aa0ff7051ad3de. Conflicts: actionpack/CHANGELOG.md
* | | Deprecate all *_filter callbacks in favor of *_action callbacksRafael Mendonça França2014-05-271-20/+24
| | | | | | | | | | | | | | | This is the continuation of the work started at 9d62e04838f01f5589fa50b0baa480d60c815e2c
* | | Remove duplicated to_s method call.Guo Xiang Tan2014-05-251-1/+1
| | |
* | | fixed a typo [ci skip]Aida2014-05-171-1/+1
| | |
* | | docs, make `ActionNotFound` public API. [ci skip]Yves Senn2014-05-111-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | This is a follow up to #15058. This exception is regularly raised during development. This means it will enter the user realm. We should provide an API page to show that this exception is public API. /cc @schneems
* | | adding complete message in documentation [ci skip]Gaurav Sharma2014-05-111-3/+4
| | |
* | | Only accept actions without File::SEPARATOR in the name.Rafael Mendonça França2014-05-061-3/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | This will avoid directory traversal in implicit render. Fixes: CVE-2014-0130 Conflicts: actionpack/lib/abstract_controller/base.rb
* | | Get rid of extra local var that does not add to the logicCarlos Antonio da Silva2014-05-051-2/+2
| | | | | | | | | | | | There are too many "action name" variables around the process method.
* | | replace class_eval by define_method in abstract_controller/callbackskirill2014-04-201-35/+29
| | |
* | | Check if the `request` variable isn't nil when calling render_to_stringJoan Karadimov2014-02-201-1/+3
| | | | | | | | | | | | closes #14125
* | | Introduce `render :body` for render raw contentPrem Sichanugrist2014-02-181-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is an option for sending a raw content back to browser. Note that this rendering option will unset the default content type and does not include "Content-Type" header back in the response. You should only use this option if you are expecting the "Content-Type" header to not be set. More information on "Content-Type" header can be found on RFC 2616, section 7.2.1. Please see #12374 for more detail.
* | | Require action_view to fix missing constantPhilipe Fatio2014-02-071-0/+1
| | | | | | | | | | | | | | | Previously, requiring action_view/view_paths did cause an uninitialized constant error for ENCODING_FLAG, which is defined in action_view.
* | | Move the null mime type to request.formatCarlos Antonio da Silva2013-12-231-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | TLDR: always return an object that responds to the query methods from request.format, and do not touch Mime::Type[] lookup to avoid bugs. --- Long version: The initial issue was about being able to do checks like request.format.html? for request with an unknown format, where request.format would be nil. This is where the issue came from at first in #7837 and #8085 (merged in cba05887dc3b56a46a9fe2779b6b228880b49622), but the implementation went down the path of adding this to the mime type lookup logic. This unfortunately introduced subtle bugs, for instance in the merged commit a test related to send_file had to be changed to accomodate the introduction of the NullType. Later another bug was found in #13064, related to the content-type being shown as #<Mime::NullType:...> for templates with localized extensions but no format included. This one was fixed in #13133, merged in 43962d6ec50f918c9970bd3cd4b6ee5c7f7426ed. Besides that, custom handlers were not receiving the proper template formats anymore when passing through the rendering process, because of the NullType addition. That was found while migrating an application from 3.2 to 4.0 that uses the Markerb gem (a custom handler that generates both text and html emails from a markdown template). --- This changes the implementation moving away from returning this null object from the mime lookup, and still fixes the initial issue where request.format.zomg? would raise an exception for unknown formats due to request.format being nil.
* | | Merge branch 'master' of github.com:lifo/docrailsVijay Dev2013-12-201-1/+1
|\ \ \
| * | | Typos. return -> returns. [ci skip]Lauro Caetano2013-12-031-1/+1
| | | |
* | | | Require action_view explicitly in AC::BaseŁukasz Strzałkowski2013-12-081-0/+1
| | | |
* | | | Merge pull request #13189 from strzalek/retain-ap-av-depJeremy Kemper2013-12-051-0/+1
|\ \ \ \ | | | | | | | | | | Retain ActionPack dependency on ActionView. Fixes #12979.
| * | | | Retain ActionPack dependency on ActionViewŁukasz Strzałkowski2013-12-051-0/+1
| | | | |
* | | | | Improve a couple exception messages related to variants and mime typesCarlos Antonio da Silva2013-12-031-8/+10
|/ / / / | | | | | | | | | | | | | | | | Avoid one-liner conditionals when they are too big. Avoid concatenating strings to build error messages. Improve messages a bit.
* / / / Action Pack VariantsŁukasz Strzałkowski2013-12-042-1/+11
|/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | By default, variants in the templates will be picked up if a variant is set and there's a match. The format will be: app/views/projects/show.html.erb app/views/projects/show.html+tablet.erb app/views/projects/show.html+phone.erb If request.variant = :tablet is set, we'll automatically be rendering the html+tablet template. In the controller, we can also tailer to the variants with this syntax: class ProjectsController < ActionController::Base def show respond_to do |format| format.html do |html| @stars = @project.stars html.tablet { @notifications = @project.notifications } html.phone { @chat_heads = @project.chat_heads } end format.js format.atom end end end The variant itself is nil by default, but can be set in before filters, like so: class ApplicationController < ActionController::Base before_action do if request.user_agent =~ /iPad/ request.variant = :tablet end end end This is modeled loosely on custom mime types, but it's specifically not intended to be used together. If you're going to make a custom mime type, you don't need a variant. Variants are for variations on a single mime types.
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-09 07:56:18 +0000