stevewithington / MuraFW1

Base Mura CMS plugin using FW/1 as its application framework.
Apache License 2.0
27 stars 23 forks source link

Incorrect Cache Key? #11

Closed cameroncf closed 11 years ago

cameroncf commented 11 years ago

Steve-

I think there may be a big in the Application.cfc file's caching mechanism. Specifically on lines 54-56 where it reads:

// !important ** DO NOT CHANGE **
local.cacheID = UCase(variables.framework.package & '_' & arguments.action);
local.cacheExists = !IsNull(CacheGet(local.cacheID));

Using the arguments.action forces the cache key to sometimes be tied to the wrong thing. IE: If I have a form like so, that does a post:

public any function dspGimmeForm($) {
        return getApplication().doAction('subsystem.app2.formpage')
}

The cache key will be PACKAGE_SUBSYSTEM:APP2.FORMPAGE for the form and also for the result of pasting the form. This will be true for ALL USERS (whoops). Before I go making changes to your code, there is the business of that whole "!important \ DO NOT CHANGE **" comment above the key generation. Does this look like a bug or am I using at the FW/1 plugin wrong?

See also: related forum post

-Cameron

stevewithington commented 11 years ago

Cameron,

This is definitely a bug. I'm tied up on the release of Mura 6 at the moment...but feel free to submit a fix/pull request!

-Steve

stevewithington commented 11 years ago

Caching/persisting the display is completely new in this version by the way.

That said, it would be nice to be able to have a trigger to either save the form view or purge the form view somehow. I suppose it would be easiest to have some type of form key such as 'persistFormView' (boolean) and check for that. This way the default would be to not cache forms...but yet allow for some forms to be cached. Or tie in a session value to the cache key. Just thinking out loud.

cameroncf commented 11 years ago

Or perhaps something like enableCacheForMethods = 'get,post'; With GET alone as the default...

If we wanted to get crazy, doAction() could optionally accept an arg to override on a per-action basis, but I think that's over thinking and overcomplicating it.

I still have the GitHub training wheels on, but I'll see about issuing a pull request shortly for this.

stevewithington commented 11 years ago

If you run into any issues trying to submit a pull, just paste the code here or email me directly. I'd be happy to take a look at what you come up with.

Thanks!

stevewithington commented 11 years ago

@cameroncf when you get a chance, could you try my simple fix for now to see if this solves your issue? thanks!

stevewithington commented 11 years ago

This should be fixed now. If not, please re-open the ticket. Thanks!