craftcms / element-api

Create a JSON API/Feed for your elements in Craft.
MIT License
498 stars 57 forks source link

Enable caching of element queries #12

Closed AugustMiller closed 7 years ago

AugustMiller commented 8 years ago

I don't really know that this is worth a PR, but I wanted to get it in front of the team, in case there are some really obvious no-nos. I'd noticed a couple of other attempts at this, but for one reason or another, I decided to give it a go, myself…

I'm working on a project that promises to have a certain volume of traffic that could begin to impact other site features without some kind of caching for JSON responses. It seemed like the right time to experiment with modifying/contributing to a Craft project!

What's been implemented:

A note on cache keys: I was experiencing an odd behavior with ElementCriteriaModel pre- and post-execution— the locale attribute was null until any of the fetching methods were called. That meant that building a lookup hash would be different than the one built from an query that had been executed. The locale had to be set prior to fetching to make sure that:

This key-building method was adopted from another plugin (which used just a single attribute), but I'm not convinced that this will comprehensively/uniquely describe all the parameters for an EntryCriteriaModel… Or that the language is correctly set by using craft()->getLanguage(). What is the typical approach in Craft for something like this? Is there any way to invalidate the cache when the underlying data changes?

Now we've got ye' old bag of worms wide open. :smile: Any help is appreciated!

:deciduous_tree:

AugustMiller commented 8 years ago

Updated a moment ago to include paginated queries. Seems to work, but still offered with the same caveats!

AugustMiller commented 8 years ago

I've been using this for about a week now, and ended up learning something about the plugin that may affect how caching should be tackled:

In a Transformer class, you can define additional methods— I wrote one to fetch "one-away" entries (i.e. other related elements to the Entry passing through the Transformer).

However! Only the results from the original criteria attributes are actually cached, meaning that any additional queries are not captured by the cache. How exactly that would be handled, I have no idea— perhaps by more logic that independently caches other element queries? Or maybe this already happens, behind-the-scenes?

AugustMiller commented 8 years ago

Aaaaand at the risk of making this a personal to-do list— I've already encountered the need or desire to change cache settings on a per-endpoint basis. Defaults could be declared globally ('cache' => true), then turned back off in a single endpoint, or have different cacheDurations set for each.

michaelhue commented 8 years ago

This seems very useful August, thanks. Any chance of this PR getting merged?

johndwells commented 8 years ago

+1 for some sort of caching made available to the ElementAPI.

howells commented 8 years ago

+1 would love this feature to be merged

AugustMiller commented 8 years ago

Just a quick update, for those who may be curious— I've had this fork running in a production environment for a couple of months now, handling a number of queries from a widget that requests a particular ElementAPI endpoint immediately after page load.

Can't point to any particular server load decrease, but it hasn't blown up yet. ;)

angrybrad commented 8 years ago

My two main concerns with this are 1) what happens with large amounts of result data (i.e. 5k, 10k+ results and 2) what happens with complex data (an entry model with complex Matrix fields, custom plugin fields, relation fields, etc.).

Ultimately the data gets crammed into Yii's set method, and that data needs to be serializable with PHP's serialize method: https://github.com/yiisoft/yii/blob/1.1.17/framework/caching/CCache.php#L181

And PHP's serialize method has many gotchas when serializing complex objects:

https://secure.php.net/manual/en/language.oop5.serialization.php https://davidwalsh.name/php-serialize-unserialize-issues https://stackoverflow.com/questions/1992906/why-it-is-not-possible-to-serialize-php-built-in-objects

Which is why it's generally recommended to cache primitive data structures (strings, ints, etc.) or very simple classes.

AugustMiller commented 8 years ago

This is great feedback. I had no idea about the limits of Serialization.

I imagine an alternative would be caching the resulting JSON?

angrybrad commented 8 years ago

@AugustMiller Seems like that'd be safer/a bit more scalable.

AugustMiller commented 7 years ago

FWIW: For another project, I'd written some controllers and services that needed their responses cached, and implemented a flat-JSON cache layer pretty easily—just haven't had time to port it over to the Element API. I'll publish another comment once it's ready.