Change hook for clearing cache

[GaryJones]

Currently, we hook to post_updated for clearing caches. I've been having trouble with this working (not fully investigated yet), but wondered if we could use save_post hook instead. Presumably, this fires once the Update button is pushed, not after checking to see if some title, content, custom field etc. fields have actually changed? (If not save_post, then some similar pre_* hook instead?)

[bradyvercher]

The save_post hook occurs after post_updated, and the only reason they wouldn't be fired is if there was an error saving data earlier in wp_insert_post() or if the title and content are both empty, but that shouldn't be the case if there's a shortcode in the content. The post_updated hook is wrapped in a conditional, but the only requirement is that the post being saved already have an ID, which it always should when using the editor.

The oEmbed cache uses pre_post_update for invaliding post meta caches, but it doesn't offer any advantages. post_updated was chosen because it passes the post before save and the post after save so shortcodes in both can be invalidated. Invalidating caches for shortcodes in the content after save is kind of an edge case if a Gist has been embedded in multiple posts, but it'd be confusing if it did crop up.

My guess would be something might be missing during the invalidation routine rather than the hook it's attached to?

[remkus]

I can confirm that the cache is indeed not flushed when updating a post.

[bradyvercher]

Do you have any additional insight? Did you make changes to a Gist that aren't being pulled in? I've tested on a couple of installations and haven't noticed anything.

If you install the Debug Bar Transients plugin, you should see a couple of transients for each gist. One starts with _gist_raw_ and the other starts with gist_html_. That plugin will show the expiration timestamp, so after a post update the timestamp should update if the cache is being cleared properly.

There is also a post meta fallback if the GitHub API can't be reached, which would prevent any changes to the gist from being reflected. That should only occur if GitHub is down or if you've hit the API rate limit, which is 60 per hour for unauthenticated requests.

[bradyvercher]

Y'all weren't using the oEmbed method by any chance? Looks like there was a bug preventing those caches from being cleared. See #34.

[GaryJones]

I think I probably was at least for some. I know others (who I now know were using the shortcode, and they) were having no problems with clearing the cache. It didn't even occur to me that oEmbed vs shortcode might be the cause.

[remkus]

I was using the oEmbed too, yes. Haven't touched the post a few days and now it's updated to reflect the changes in the Gist

[GaryJones]

While #35 attempts to solve #34, I'm still a little confused why the deletion is a little convoluted.

The current process seems to be:

• Post is updated and saved
• During the post update hook, inside delete_gist_transients(), we call do_shortcode() / apply the content filters on the content, so that shortcode is parsed again.
• This re-fires the shortcode() method, but with the delete flag set, resulting in the transients being deleted, before aborting.

This apparently leaves no transient gists in place before the next front-end call, so the next visitor still has to wait an extra second or two for the gist to be added to the transients, just the same as if we'd had no visitors for a while and the transient had naturally expired.

The convoluted part seems to be the fact of setting a flag, and running shortcode() again. I think the top three assignments within shortcode() could be moved into a new method, and just call that directly from both shortcode() and delete_gist_transients(), along with a delete_transient() call in the latter. No flag or trying to re-parse the shortcode / oembed needed.

If that logic makes sense, let me know and I'll do a PR.

(Edit: I guess the use of parsing the content as a shortcode is so that we can easily grab the raw shortcode attributes?)

[bradyvercher]

I was in the same boat and didn't even think about oEmbed in regards to this issue.

The cache busting algorithm is a bit convoluted, but we need to generate and ensure the shortcode hashes are the same every time so that we're working with the same transient keys. Piggybacking on the built-in parser is the easiest route, but if you have another idea, I'm all ears.

[GaryJones]

Using the built-in parser to grab the attributes does make total sense now I think about it (as per my above edit).

[bradyvercher]

Fix provided in #35. Also see #34 for more background.