This project is archived and is in readonly mode.
[PATCH] Add a NullStore to ActiveSupport::Cache
Reported by David Reese | November 13th, 2008 @ 02:06 PM | in 3.x
With the config action_controller.perform_caching =
false, action controller still tries to use the cache_store
to perform Rails.cache.- style caching. This is surprising
behavior.
Either:
- The config setting should be renamed to be clear about what you are controlling, or preferably,
- Rails.cache.read should return nil when perform_caching is false. cache.read returning nil should already be handled by the application.
There is already a protected "convenience accessor" (Cache#cache) that uses cache_configured? to give this (2) actual behavior, yielding the block when caching is not set up.
Tickets #639, #785, and #1290 all dance around this issue --
#639
is marked 'invalid', and #785 and #1290 are
marked as dupes of #1339. In development, being able to
actually turn off caching while leaving cache_classes =
false would solve many of these issues for common use
scenarios.
Comments and changes to this ticket
-
Doug Cole March 13th, 2009 @ 09:21 PM
I just spend 30 minutes trying to figure this out too, when starting to use Rails.cache. If no one is working on this I'll write a patch - looks pretty simple.
-
CancelProfileIsBroken August 5th, 2009 @ 03:36 PM
- Tag set to “bugmash”
-
John Pignata August 9th, 2009 @ 05:42 AM
Since the Rails.cache functionality exists in ActiveSupport and not ActionController, I propose a new configuration setting of config.active_support.perform_caching= with a default of true.
The patch adds this behavior to Active Support and also modifies the stores to honor the setting. exists? and read will return false and nil respectively when perform caching is set to false. Feedback?
-
John Pignata August 9th, 2009 @ 03:18 PM
- Tag changed from “bugmash” to “bugmash, patch”
- Title changed from “Perform caching=false should not perform caching” to “[PATCH] Perform caching=false should not perform caching”
-
Derander August 10th, 2009 @ 07:37 AM
+1, this seems like an appropriate change. Shouldn't break backwards compatibility.
Applies cleanly to latest master & tests pass.
-
David Reese August 15th, 2009 @ 03:08 PM
- Title changed from “[PATCH] Perform caching=false should not perform caching” to “[PATCH] ActiveSupport should have a "perform_caching=false" setting”
Ok, now I get that when you say "action_controller.perform_caching = false", you're turning off page, action, and fragment caching, not all caching that happens in your controllers. Somehow that was not clear to me.
But, +1 on John's patch -- there should be some way to also turn off ActiveSupport caching, and the patch does that ably.
One thing I would change for cleanliness -- I'd set @@perform_caching=true first, which seems to be common practice in Rails. The section where @@perform_caching is checked for nil seemed a little opaque:
{mkd-extraction-e508fd6e2a88aa82336506488855005a}I'd add this, and then you don't need to check for nil in #cache_configured?, just return perform_caching:
{mkd-extraction-d4304b7970b636c816dabf43df1f88e0}
+ @@perform_caching = trueI tried to make a new patch but it seemed to add my patch onto John's patch. If there's a cleaner way to to that someone should probably go ahead and do it.
-
John Pignata August 15th, 2009 @ 11:33 PM
- Tag changed from “bugmash, patch” to “activesupport, bugmash, cache, patch”
Good feedback - thanks, David. Attached is a patch with your feedback that will apply to master.
-
John Pignata August 15th, 2009 @ 11:33 PM
- no changes were found...
-
John Pignata August 16th, 2009 @ 12:07 AM
For good measure, here's a patch for 2.3 stable if it's desired to get this in prior to 3.0
-
John Pignata September 27th, 2009 @ 03:12 AM
- Title changed from “[PATCH] ActiveSupport should have a "perform_caching=false" setting” to “[PATCH] Add a NullStore to ActiveSupport::Cache”
- Assigned user set to “Michael Koziarski”
Talked to nzkoz who was of the opinion that this patch should instead be a null store that users could implement for edge cases in debugging and testing. This makes sense and is more flexible than a configuration setting. Attached is a patch to add a null store to master.
-
Elad Meidar September 27th, 2009 @ 03:31 AM
+1 verified and nailed :) Looks like it's a well respected behavior, latest patch applies cleanly on latest master.
-
David Reese September 27th, 2009 @ 03:32 AM
What about "BlackholeStore" instead? It's so much more poetic. (There's some reason we're not python developers, right?)
-
CancelProfileIsBroken September 27th, 2009 @ 11:34 AM
- Tag changed from “activesupport, bugmash, cache, patch” to “activesupport, bugmash, bugmash-review, cache, patch”
-
-
Rizwan Reza February 12th, 2010 @ 12:46 PM
- Tag changed from “activesupport, bugmash, bugmash-review, cache, patch” to “activesupport, bugmash-review, cache, patch”
-
-
Rizwan Reza May 15th, 2010 @ 06:45 PM
- Tag changed from “activesupport, bugmash-review, cache, patch” to “activesupport, bugmash, cache, patch”
-
Simon Jefford May 15th, 2010 @ 10:22 PM
- Tag changed from “activesupport, bugmash, cache, patch” to “activesupport, bugmash, bugmash-review, cache, patch”
I've rebased the above patch, cleaned up some whitespace issues and updated it slightly so overrides the correct methods (they seem to have changed slightly from when this patch was submitted).
The original authorship has been retained in the patch.
+1 verified
-
Simon Jefford May 15th, 2010 @ 11:45 PM
Updated patch - more test coverage + remove unnecessary override
-
Rizwan Reza May 16th, 2010 @ 02:25 AM
- Tag changed from “activesupport, bugmash, bugmash-review, cache, patch” to “activesupport, bugmash, cache, patch”
- State changed from “new” to “open”
We still need some +1s here.
-
Peter Marklund June 15th, 2010 @ 03:32 PM
I like this patch as it enabled global disabling of the cache. To me it would make sense to have an enabled attribute on the Rails.cache object, something I achieved with a monkey patch:
http://developer.newsdesk.se/2010/05/04/enablingdisabling-the-rails...
-
Santiago Pastorino February 2nd, 2011 @ 04:37 PM
- Importance changed from “” to “”
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 2nd, 2011 @ 04:37 PM
- State changed from “open” to “stale”
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
David Reese
Doug Cole
Jeremy Kemper
Michael Koziarski
Rizwan Reza