Fix template compilation concurrency issue, and template caching concurrency #6405

Closed
wants to merge 538 commits into
from

Projects

None yet
@pinetops
Contributor

This rolls in josevalim's request for a comment for the fix for #6400.

Also two new commits fixing another concurrency issue #6404. First a simplistic implementation, then a more optimal one - but that probably needs further discussion.

josevalim and others added some commits Jan 27, 2012
@josevalim josevalim Merge check box fixes from remote-tracking branch 'cantonio/checkbox-…
…hidden-backport' into 3-2-stable
2e5ec3b
@dhh dhh Revert "Fix expanding cache key for single element arrays"
This reverts commit abe915f.

This broke all existing keys and it's wrong anyway. The array is just there as a convenience option for building the string. It's intentional that [ "stuff"] and "stuff" generates the same key.
66b445c
@dhh dhh Inline the prefix assignment so it doesnt look so daft f78cb55
@vijaydev vijaydev fix typo [ci skip] 766e12a
@fxn fxn query cache instrumentation should included the bindings in the paylo…
…ad [closes #4750]
4137905
@bsodmike bsodmike clarification to prevent confusing newbies; Passenger/Unicorn are app…
… servers, Apache/Nginx are the web servers along with thin a la Heroku etc.
93306be
@jonleighton jonleighton Merge pull request #4763 from kennyj/fix_4754
[MySQL] Fix GH #4754. Remove double-quote characters around PK when using sql_mode=ANSI_QUOTES
c472843
@jonleighton jonleighton Add workaround and deprecation if the inherited hook is not executed.…
… Closes #4757.
0bfc504
@jonleighton jonleighton Allow writing unknown attributes, but with a deprecation warning. Clo…
…ses #4583.
b2955ed
@lest lest fix assets test cfea673
@josevalim josevalim Merge pull request #4786 from lest/patch-2
fix assets test in 3-2-stable
175cdd1
@kennyj kennyj Fix GH #4760. A Block was not evaluated. 91700bf
@marten marten Fix use of Deprecation without requiring active_support/deprecation i…
…n ActiveSupport::Concern
cb46423
@marten marten Fix use of Deprecation without requiring active_support/deprecation i…
…n ActiveSupport::Concern in Base64
563be40
@marten marten Fix use of Deprecation without requiring active_support/deprecation i…
…n message verifier
f3e1b21
@marten marten Fix use of Deprecation without requiring active_support/deprecation i…
…n whiny nil
2380a2d
@josevalim josevalim Merge pull request #4790 from marten/fix-require-deprecation-in-activ…
…esupport

Fix Deprecation usage in ActiveSupport when requiring only parts of AS
7beb5a7
@jonleighton jonleighton Merge pull request #4783 from gregolsen/ids_reader_fix
ids_reader method fixed, test added to has_many association (for PostgreSQL)
85c724d
@josevalim josevalim Merge pull request #4793 from kennyj/fix_4760
[3-2-stable] Fix GH #4760. A Block was not evaluated.
bd93ba5
@tenderlove tenderlove Merge pull request #4779 from bsodmike/bsodmike-3-2-stable
Update comment in Gemfile re Unicorn (minor clarification)
60cb7d6
@tenderlove tenderlove Merge pull request #4735 from arton/master
Re-launch assets:precompile task using original $0 if $0 is batch file so it works on Windows
81f14a5
@jonleighton jonleighton Improve deprecation message e1de540
@rafaelfranca rafaelfranca Fix broken tests added by 85c724d f532cd1
@spastorino spastorino Merge pull request #4811 from rafaelfranca/3-2-stable-fix
Fix broken build in 3-2-stable branch
1444ec6
@kennyj kennyj Fix GH #4749. Remove branch options, because edge is not 3-2-stable. c8c8439
@carlosantoniodasilva carlosantoniodasilva Generate strict validation error messages with attribute name
Backported from master.
91f8cf2
@spastorino spastorino Merge pull request #4815 from kennyj/fix_4749
Fix GH #4749. Remove branch options, because edge is not 3-2-stable.
21735d2
@josevalim josevalim Merge pull request #4822 from carlosantoniodasilva/strict-validation-3-2
Generate strict validation error messages with attribute name (3-2-stable)
0696a51
@sikachu sikachu Fix plugin_new test failure from c8c8439
`plugin_new` generator doesn't generate `sass-rails` in the Gemfile, so
you can't check for that.
b673ca6
@josevalim josevalim Merge pull request #4827 from sikachu/3-2-stable-fix-plugin-new
Fix plugin_new test failure from c8c8439
780bf52
@jonleighton jonleighton Merge pull request #4543 from jdelStrother/find_or_init
Don't instantiate two objects in collection proxy / find_or_instantiate_by
42dab64
@tenderlove tenderlove Merge pull request #4809 from cfeist/feist-sqlite-binary-corruption
Fix for SQLite binary data corrupter (to master branch)
4ca633e
@densya203 densya203 Fix Issue #4819
'uninitialized constant ActiveRecord::Deprecation in Rails3.2.1'

Just a typo of 'ActiveSupport::...'
daa2665
@tenderlove tenderlove Merge pull request #4868 from skult/3-2-stable
Fix Issue #4819
9aa4c6d
@sikachu sikachu Fix override API response bug in respond_with
Default responder was only using the given respond block when user
requested for HTML format, or JSON/XML format with valid resource. This
fix the responder so that it will use the given block regardless of the
validity of the resource. Note that in this case you'll have to check
for object's validity by yourself in the controller.

Fixes #4796
567ac65
@fxn fxn let automatic EXPLAIN ignore CACHE notifications ee6e3c1
@josevalim @josevalim josevalim Merge pull request #4879 from kennyj/fix_4873
Fix GH #4873. Allow swapping same class middleware.
9cb0e12
@josevalim josevalim Merge pull request #4870 from sikachu/3-2-stable-responder-fix
Fix override API response bug in respond_with
44b9992
@josevalim josevalim Clean up a bit default_response handling and cache format negotiation. 2bf2055
@kennyj kennyj GH #4883. Optional start_day argument for Time#all_week b037401
@spastorino spastorino Merge pull request #4890 from kennyj/improvement_4883
GH #4883. Optional start_day argument for Time#all_week
93f5361
@josevalim @josevalim josevalim Merge pull request #4908 from kennyj/fix_3864
Fix url_for method's behavior. GH #3684.
ab44418
@guilleiguaran guilleiguaran Add branch to sass-rails and coffee-rails for apps generated with --d…
…ev or --edge options (3.2.x)
995c076
@tenderlove tenderlove Merge pull request #4912 from guilleiguaran/fix-edge-gemfile
Add branch to sass-rails and coffee-rails for apps generated with --dev or --edge (3-2-stable)
dd54137
@rmm5t rmm5t Fixed force_ssl redirects to include original query params
`ActionController.force_ssl` redirects http URLs to their https equivalent;
however, when a URL contains a query string, the resulting redirect lacked the
original query string.
391e6a4
@guilleiguaran guilleiguaran --edge option should generate app with rails 3-2-stable 5bfee55
@josevalim josevalim Merge pull request #4920 from guilleiguaran/revert-gemfile-edge
In Rails 3.2.x --edge generate apps with rails 3-2-stable
bc85fcb
@iblue @fxn iblue Fixed the documenation for 'to_xml' 8c7378a
@rmm5t rmm5t Added unit test to cover changes to RouteSet.url_for
ActionDispatch::Routing::RouteSet.url_for now handles passing params through to
ActionDispatch::Http::Url.url_for
0e482b3
@josevalim josevalim Merge pull request #4916 from rmm5t/fix_force_ssl_redirect_with_params
Fixed force_ssl redirects to include original query params
f3595b5
@fxn fxn no need to check for this constant dafc3c7
@tenderlove tenderlove always flush all logs. fixes #4277 b332877
@jonleighton jonleighton Fix attribute_before_type_cast for serialized attributes. Fixes #4837.
Conflicts:

	activerecord/lib/active_record/core.rb
77b4edc
@paul @josevalim paul Handle nil in add_index :length option in MySQL
Our schema.rb is being generated with an `add_index` line similar to this:

    add_index "foo", ["foo", "bar"], :name => "xxx", :length => {"foo"=>8, "bar=>nil}

This is the same as it was on Rails 3.1.3, however, now when that
schema.rb is evaluated, its generating bad SQL in MySQL:

    Mysql::Error: You have an error in your SQL syntax; check the manual
    that corresponds to your MySQL server version for the right syntax
    to use near '))' at line 1: CREATE UNIQUE INDEX
    `xxx` ON `foo` (`foo`(8), `bar`())

This commit adds a check for nil on the length attribute to prevent the
empty parens from being output.

Conflicts:

	activerecord/test/cases/migration/index_test.rb

Signed-off-by: José Valim <jose.valim@gmail.com>
0a75336
@josevalim josevalim Push proper test changes for previous commit conflicts. d3d807a
@rafaelfranca rafaelfranca Use real table and columns for index test 7c167dd
@josevalim josevalim Merge pull request #4941 from rafaelfranca/fix-build
Use real table and columns for index test
6d17b36
@tenderlove tenderlove Merge pull request #4988 from kennyj/fix_4720-3
Fix GH #4720. Routing problem with nested namespace and already camelized controller option.
73fcbaa
@jeremy @tenderlove jeremy Fix that failed tests should exit with a nonzero error code.
Partially reverts 14c89e7.

Hat tip to @tenderlove for paring down the TestTask!
92acd5d
@josevalim josevalim Fix deprecation warning in AS::Concern. 305d5d5
@josevalim @josevalim josevalim Merge pull request #5038 from carlosantoniodasilva/fix-db-migrate-redo
Always reenable _dump task in AR databases rake. Closes #5030
5f9a5a5
@spastorino spastorino Rack body respond to each and not to join
This fixes undef `to_str' for Rack::Chunked::Body when using
caches_action + streaming on an action

Closes #5027
7c79996
@tenderlove tenderlove adding tests to document behavior for #4817 201e67e
@josevalim @josevalim josevalim Merge pull request #5049 from fabioyamate/master
Fix sanitize_for_mass_assigment when role is nil
a1b9acb
@arunagw arunagw Fixes failing test with ruby 1.8.7-p358
Same as 91a9b24
c17608f
Aditya Sanghi backporting #4918 to 3.2 stable; adding extra test for accept header …
…given by googlebot
dffd85a
@josevalim josevalim Merge pull request #5073 from asanghi/4918_backport
Backporting #4918 with one added test for googlebot accept header as I saw it
45503ec
@spastorino spastorino Merge pull request #5071 from arunagw/fix_failing_test_ruby187_p358
Fix failing test ruby187 p358
1a0bc2a
@pixeltrix pixeltrix Fix ActionDispatch::Static to serve files with unencoded PCHAR
RFC 3986[1] allows sub-delim characters in path segments unencoded,
however Rack::File requires them to be encoded so we use URI's
unescape method to leave them alone and then escape them again.

Also since the path gets passed to Dir[] we need to escape any glob
characters in the path.

[1]: http://www.ietf.org/rfc/rfc3986.txt
86d3bc3
@pixeltrix pixeltrix Simplify regexp 1b4e6ca
@arunagw arunagw fix test with ruby 187-p358 742b615
@fxn fxn Merge pull request #5081 from arunagw/fix_ar_test
Fix ar test
3b61efc
@lest @vijaydev lest fix spacer template example ed3a15d
@oestrich @vijaydev oestrich Update Time#change docs to reflect the options it uses
[ci skip]
ea54d2c
@avakhov @vijaydev avakhov Fix actionpack readme weblog example 168e653
@ffmike @vijaydev ffmike Documenting the :inverse_of option for associations 44849b1
@vijaydev vijaydev fix a typo [ci skip] b2e9bbd
@pwnall pwnall Removed (outdated?) config.action_view.logger from configuration guide. 64fa7f7
@pixeltrix pixeltrix Remove fixture files with Windows incompatible filenames
Windows doesn't allow `\ / : * ? " < > |` in filenames so create
the fixture files at runtime and ignore the incompatible ones when
running on Windows.
41c182c
@funny-falcon @tenderlove funny-falcon sync __run_callbacks with ruby-trunk
https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/34580
In this revision behavior of respond_to? had changed: now to ask about
protected method one should pass second argument `true`
0c0f278
@tenderlove tenderlove search private / protected methods in trunk ruby 3b824d6
@ckdake ckdake Reset memoized hash keys when new entry added 68d3a46
@josevalim josevalim Merge pull request #5101 from ckdake/ckdake_actionview_handler_reset
Reset memoized hash keys when new ActionView::Template handler is registered
8c870f1
@spastorino spastorino Use the right format when a partial is missing.
Closes #5025
8b86259
@spastorino spastorino Fix a failing test b80d8f7
@lest @tenderlove lest fix output safety issue with select options 7b73913
@amatsuda @tenderlove amatsuda add AS::SafeBuffer#clone_empty 621d219
@amatsuda @tenderlove amatsuda use AS::SafeBuffer#clone_empty for flushing the output_buffer 42fabd2
@amatsuda @tenderlove amatsuda delete vulnerable AS::SafeBuffer#[] dfa33fa
@kennyj kennyj Fix some warnings on 3-2-stable 39d4617
@spastorino spastorino Merge pull request #5106 from kennyj/fix_warnings_20120210
[3-2-stable] Fix some warnings
c426591
@spastorino spastorino Add CHANGELOG entry b122968
@spastorino spastorino Restore lookup formats to the previous value after searching for the …
…failing view
4eff6bc
@tenderlove tenderlove Merge pull request #5096 from lawso017/master
Restoring ability to derive id/sequence from tables with nonstandard sequences for primary keys
d70ed10
@spastorino spastorino Don't wrap the raise with ensure f92c812
@vijaydev vijaydev fix bad docs from f373f29 [ci skip] cc848d6
@tenderlove tenderlove ruby 2.0 makes protected methods return false for respond_to, so pass…
… true as the second param
0052d90
@tenderlove tenderlove more ruby 2.0 respond_to? changes c73f883
@tenderlove tenderlove tag bind params with a bind param object a566ee5
@tenderlove tenderlove prepared statements can be disabled 83e42d5
@brianmario
Contributor

👍

curious if this will help any regarding brianmario/mysql2#66, brianmario/mysql2#209 or brianmario/mysql2#213

Owner

@brianmario I don't think so... I've seen those errors before, but I can't remember exactly the problem. IIRC, it happens if you have a low traffic app where the connection timeout is not long enough, but I can't remember.

@jeremy do you recall issues like these? I seem to remember there was a fix in rails.

Owner
fxn replied Mar 9, 2012

I know of an application that reports lost connections often. It is a busy application (about 40K rpm). A priori seems strange because the connection pool does a mysql_ping on checkout, so you got a successful ping and just milliseconds later the connection is lost (or the server gone, also happens). The MySQL server on the other hand seems to be doing fine.

Not saying it is related to this patch, just a followup.

tenderlove and others added some commits Mar 9, 2012
@jrochkind
Contributor

Awesome. Am hoping this helps solve a related problem I am trying to investigate in my app, causing it to require more connections in the pool than I think it ought to, since rails 3.0 even.

However, you now have two similar but different methods with similar but different names: release(conn) and release_connection(conn). Can you maybe put some inline comments in the new release explaining how it's different than release_connection

Alternately, do they need to be two different methods? The new release seems to possibly do a superset of what the old release_connection did -- should they just be combined?

Owner

release_connection actually takes the thread id associated with the connection. I think it's meant to be called with no arguments. release is meant to be called with a connection, and it's private. We could probably combine release_connection and release, but I wanted to make the smallest change possible. Fix bug in one commit, refactor later. :-)

Contributor
parndt and others added some commits Mar 10, 2012
@parndt parndt Fixes issue #5193 using the instructions provided in the issue. f665e20
@parndt parndt Fixed problem when fixture_path is not always defined (incidentally, …
…only when ActiveRecord is according to test_help.rb).
5ef8069
@denisj @arunagw denisj fix activerecord query_method regression with offset into Fixnum
add test to show offset query_methods on mysql & mysql2

change test to cover public API
f67944e
@josevalim josevalim Merge pull request #5400 from arunagw/issue_4409
Issue 4409
a9320f7
@josevalim josevalim Merge pull request #5398 from parndt/fix_issue_5193
Fix issue 5193
d2ac18a
@rafaelfranca rafaelfranca Do not use the attributes hash in the scaffold functional tests 5bed0e5
@rafaelfranca rafaelfranca Use the attributes hash explicitly ac8469d
@rafaelfranca rafaelfranca Use Ruby 1.8 hash syntax c1610eb
@josevalim josevalim Merge pull request #5410 from rafaelfranca/fix-scaffold-3-2
[3-2-stable] Do not use the attributes hash in the scaffold functional tests
dfbbf31
@kennyj kennyj [3-2-stable] Fix GH #5399. connection_pools's keys are ActiveRecord::…
…Base::ConnectionSpecification objects.
21d9c0f
@avakhov @vijaydev avakhov Fix layout method doc formatting 5f28145
@britto @vijaydev britto Close string quotes a782fea
@tenderlove tenderlove Merge pull request #5417 from kennyj/fix_5399-32
[3-2-stable] Fix GH #5399. connection_pools's keys are ActiveRecord::Base::ConnectionSpecification objects.
596ecf7
@jrochkind jrochkind ConnectionPool.checkout takes account of ruby using 'non-blocking con…
…dition variables' in mutex ConditionVariables
41563b4
@tenderlove tenderlove Merge pull request #5423 from jrochkind/checkout_account_for_monitor_…
…model

ConnectionPool.checkout needs to be restructured to take account of ruby's "non-blocking" strategy for mutex ConditionVariables
f2aea24
@tenderlove
Owner

Is there a ticket associated with this commit? Seems it breaks existing behavior as noted in #5337

Owner

Tickets #790 and #5058.

Owner

Also this is why we have to do this: rack/rack#265

Owner

@pixeltrix makes sense. Thanks! ❤️

dhh and others added some commits Mar 14, 2012
@dhh dhh Do not include the authenticity token in forms where remote: true as …
…ajax forms use the meta-tag value
16ee611
@dhh dhh Allow you to force the authenticity_token to be rendered even on remo…
…te forms if you pass true
e50abba
@drogus drogus Remove ActionController::TestCase#rescue_action_in_public!
This method has no effect since exception handling was
moved to middlewares and ActionController tests do not
use any middlewares.
ccf4ff0
@drogus drogus Check for existence of exactly the called `fixture_path=` method 9dfb41f
@tenderlove tenderlove Merge pull request #5338 from mreinsch/3-2-static_invalid_byte_sequence
3 2 static invalid byte sequence
f918137
@tenderlove tenderlove Merge pull request #5437 from kennyj/fix_5430
Fix GH #5430. A Payload name for schema_search_path should be SCHEMA.
bd3e1ed
@tenderlove tenderlove Merge pull request #5456 from brianmario/redirect-sanitization
Strip null bytes from Location header
f52ad6c
@tenderlove tenderlove Merge pull request #5457 from brianmario/typo-fix
Fix typo in redirect test
e135ff1
@tenderlove
Owner

Y U NO ADD TEST? 😳

Member

A test was pushed later iirc.

Owner

❤️

drogus and others added some commits Mar 15, 2012
@drogus drogus Fix #5440 - multiple render_to_string breaks partials formats
This fixes situation where rendering template to string
sets `rendered_format` to the format rendered there.
This is ok to have consistent formats rendered in partials,
but it breaks on next renders if format is explicitly set
or on last render where default format does not necessarily
need to be the format of first rendered template.
1eb6189
@drogus drogus Add missing test for #5308 7130f91
@josevalim josevalim Merge pull request #5480 from drogus/rendering-issues
Fix for #5440
e5b46cf
@josevalim @spastorino josevalim Ensure load hooks can be called more than once with different contexts. c0a5b85
@drogus drogus Rubyracer does not work on ruby, so add it to Gemfile with :ruby plat…
…form only
41815f5
@kennyj kennyj Fix GH #5435. db:structure:dump should be re-enable. f4f9ec1
@drogus drogus Merge pull request #5493 from kennyj/fix_5435-32
[3-2-stable] Fix GH #5435. db:structure:dump should be re-enable.
d9355be
@arunagw arunagw Build fix for app_generator_test.rb 9451201
@josevalim josevalim Merge pull request #5498 from arunagw/build_fix_app_generator_test_3-…
…2-stable

Build fix app generator test 3 2 stable
c8fbd48
@mikel mikel Increase minimum version of mail.
  Second security vulnerability found in mail file delivery method
  patched in version 2.4.4.
74b7829
@arunagw arunagw Build fix for ruby1.8.7-358 fcc8743
@josevalim josevalim Merge pull request #5505 from arunagw/build_fix_1.8.7-3-2-stable
Build fix 1.8.7 3 2 stable
358333f
@khustochka
Contributor

I wonder what happens in terms of graceful degradation then. With JavaScript off will the form remain protected from forgery?

Member

In one of the next commits there is an option added to pass authenticity_token: true in order to keep that behavior.

Contributor

Thanks, drogus. My fault, I must have looked at the next commit.

Contributor

Not trolling here, but wouldn't this change (and the one in the next commit), break a lot of code where people do The Right Thing, and make their forms unobtrusive with graceful degradation?

I am not protesting the commit, just the fact that it was included in a patch version update (3.2.3) and it has the potential to catch a lot of people unawares...

Owner

👍 to preserve the original behavior for 3-2-stable. We could use a config option to disable 'em instead.

Member

@christos fixed here: d646d9d, thanks for reporting!

dhh and others added some commits Mar 20, 2012
@dhh dhh We dont need to merge in the parameters as thats all being reset by t…
…he rack headers (and its causing problems for Strong Parameters attempt of wrapping request.parameters because it will change in testing)
275ee0d
@mhfs mhfs [3-2-stable] Remove blank line from generated migration 57c6b4c
@kennyj kennyj migrate(:down) method with table_name_prefix 565bfb9
@josevalim josevalim Merge pull request #5533 from mhfs/migration_blank_line_3_2
[3-2-stable] Remove blank line from generated migration
f829515
@mhfs mhfs [3-2-stable] Port of #5522 'Fix adding/removing field's index when ge…
…nerating migration'
35bf748
@drogus drogus Merge pull request #5542 from mhfs/port_5522_to_32stable
Port of #5522 'Fix adding/removing field's index when generating migration'
89f8866
@kennyj kennyj Fix GH #5411. When precompiling, params method is undefined. 8c262f7
@josevalim josevalim Merge pull request #5525 from kennyj/fix_5411
Fix GH #5411. When precompiling, params method is undefined.
b714140
@carlosantoniodasilva carlosantoniodasilva Add order to tests that rely on db ordering, to fix failing tests on pg
Also skip persistente tests related to UPDATE + ORDER BY for postgresql

PostgreSQL does not support updates with order by, and these tests are
failing randomly depending on the fixture loading order now.
b332891
@tenderlove tenderlove Merge pull request #5557 from carlosantoniodasilva/fix-build-3-2
Fix build for branch 3-2-stable
ea4e021
@carlosantoniodasilva carlosantoniodasilva Fix identity map tests 0879ebd
@drogus drogus Merge pull request #5558 from carlosantoniodasilva/fix-build-3-2
Fix build for branch 3-2-stable - Part 2
ef48cea
@tenderlove tenderlove Merge pull request #5537 from kennyj/fix_4399-32
[3-2-stable] migrate(:down) method with table_name_prefix
0382e44
@tenderlove tenderlove chdir before globbing so that we don't need to escape directory names.
fixes #5521
eb0d8ee
@carlosantoniodasilva carlosantoniodasilva Return the same session data object when setting session id
Make sure to return the same hash object instead of returning a new one.
Returning a new one causes failures on cookie store tests, where it
tests for the 'Set-Cookie' header with the session signature.

This is due to the hash ordering changes on Ruby 1.8.7-p358.
907bcce
@abevoelker @vijaydev abevoelker Fix 'Security#Mass Assignment' URL typo ed7567c
@lest lest apply form_for namespace option to date_select bd8a970
@fesplugas

s/@glob/glob/

Member

actually the glob argument could be removed from both methods, couldn't it? /cc @tenderlove

Owner

Thanks @fesplugas. @drogus actually I wanted to pass the glob in as a parameter in order to keep the method isolated from the class. I hope to eventually remove these methods.

Edited for clarity.

drogus and others added some commits Mar 26, 2012
@drogus drogus Merge pull request #5596 from lest/patch-3
apply form_for namespace option to date_select
3f1b8c6
@josevalim josevalim Merge pull request #5597 from carlosantoniodasilva/fix-build-3-2
Fix build for branch 3-2-stable - return the same session hash object
0e916ae
@tenderlove tenderlove Merge pull request #2621 from icco/master
Issue with schema dump
3eb5be6
@drogus drogus If partial is rendered in controller, grab format from template
Previously `rendered_format` was set only based on mime types
passed in Accept header, which was wrong if first type from
Accept was different than rendered partial. The fix is to simply
move setting rendered_format to the place where template
is available and grab format from the template. If it fails
we can fallback to formats passed by Accept header.
449a4fc
@josevalim josevalim Merge pull request #5603 from drogus/fix-rendered-format-for-render-p…
…artial

Fix rendered format for render partial
e31ec47
@spastorino spastorino Bumping to 3.2.3.rc1 5f37260
@spastorino spastorino Merge pull request #5619 from jcoleman/textarea-newline-fix-breaks-haml
Textarea newline fix breaks haml (3-2-stable)
4f66586
@spastorino spastorino Merge pull request #5633 from drogus/embed-auth-token-in-remote-forms
Embed auth token in remote forms
84ca8c8
@exviva

What's the sense of this code? I know that's how it used to be, I'm just curious, maybe it didn't make sense before. It seems to me that it's deleting a value from a hash and immediately assigning it back under the same key.

On a side note, what's the rationale behind not including authenticity_token for AJAX forms? If it doesn't hurt, maybe it doesn't make sense to have this conditional, and an extra config option, and another option for form_for.

Member

On a side note, what's the rationale behind not including authenticity_token for AJAX forms?
If it doesn't hurt, maybe it doesn't make sense to have this conditional, and an extra config
option, and another option for form_for.

It can hurt if you cache the form. If you use ajax only you will get auth token from meta tags anyway, so it matters only if you want to also handle the form without javascript. also, see this: d646d9d

Contributor

Don't the UJS drivers override the authenticity_token hidden input's value with the meta tag's value anyway?

I was referring to your commit when mentioning the extra config option. All in all, I'm still trying to fully understand this change - thanks for the information.

Member

Don't the UJS drivers override the authenticity_token hidden input's value with the meta tag's value anyway?

Frankly, I'm not sure :)

Member

But even if yes, you don't necessarily need to use UJS drivers. We can't rely on it.

@hkarthik

+1 to this. This is also problematic on old tables that were designed for HABTM relationships.

@tiegz

@tenderlove this is prob a dumb question, but just wanted to make sure that shift is intended for binds and not *binds? I noticed just now that this to_sql method will alter the binds param for anything that uses binds after this is called.

Owner

yes, the shift is indented for binds. Binds is a list of tuples, so the * is for expanding the tuple for the quote method.

pinetops added some commits May 19, 2012
@pinetops pinetops Prevent concurrent compilation of templates - closes #6400 7133649
@pinetops pinetops Prevent concurrent access to template cache - closes #6404 d3028e3
@pinetops pinetops Avoid locks in template caching - performance improvement for #6404
This change avoids obtaining a lock for templates that are already compiled by
using a thread local copy of any templates already used by the thread.

There are a few issues with this however:
- It breaks ActionView::Template::Resolver#clear_cache, as there's no good way of
  clearing all the Thread local copies. Within Rails it's only used in one test
  (which could be implemented another way). So the question is possibly if this is
  intented to be part of the public API and if it can be removed.
- If there's any scenario in which Resolvers can be re-created there is a potential
  memory leak as there's no attempt to clear up the thread local storage
- I don't have any performance data to justify the need to avoid locks
69f8bb1
Member
arunagw commented May 20, 2012

Please us the proper branch for this PR. Seems you are doing wrong here.

Contributor

Incorrect pull range

@pinetops pinetops closed this May 20, 2012
@adsummos

this is triggering a deprecation warning for associations when using the assignment operator (record.association_name = [records])

itkin replied Jul 6, 2013

+1

+2

+3 should we ignore these warning on associations or handle them somehow differently?

+5

@giner
giner commented on 4ca633e Dec 6, 2012

You are crazy guys! You haven't even mentioned such an important change on the changelog!
I have spent three days to solve a problem with Redmine upgrade!
Have a look: http://www.redmine.org/issues/12501

@giner Please feel free to send a pull request with a changelog entry if that's missing, we're trying to ensure nothing goes without a changelog now. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment