Files stored in files/qr_codes are kept there forever. Implement some feature so that we can expire the cache.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

levelos’s picture

I'm thinking of a user generated "flush", much like imagecache handles it. I don't see too many situations where the generated images would change, unless you change providers, or the provider makes an upgrade to the API, and this could be handled with a manual user flush. Thoughts?

claudiu.cristea’s picture

I'm thinking on:

  1. Placing a "Flush cache" button in "Global Settings" admin/settings/qr_codes - For admin manual handling
  2. Auto-expire: A "Cache Life" select with options (0 => "Never", 1 => "1 day", 10 => "10 days", 30 => "30 days", ... etc) in "Global Settings" admin/settings/qr_codes and hook_cron() implementation for cleaning old items.
  3. Clear all files when admin is changing the QR Engine

It should be enough...

claudiu.cristea’s picture

Status: Active » Needs review
FileSize
4.63 KB

Here's a patch. It seems OK for me but I didn't test the hook_cron() behavior. On my system cron is disabled for some reasons. @loubab, can you test if cron purge the expired files correctly?

claudiu.cristea’s picture

Typo fix...

claudiu.cristea’s picture

Disable the "Cache clear now!" button when cache is empty.

claudiu.cristea’s picture

Typo fix

claudiu.cristea’s picture

Assigned: Unassigned » claudiu.cristea
levelos’s picture

Status: Needs review » Fixed

Committed with a few minor changes. Nice!

claudiu.cristea’s picture

Status: Fixed » Needs work

Looking into the code, at your changes... It seems that the validation callback reject "0" (Never clear) cache lifetime. I used a select to limit choices and prevent errors which seems to me a better approach. I see that other modules use this too.

But, anyway if we want to use a textfield, we must ensure that "0" value is a valid value. Also we can use ctype_digit() instead of is_numeric() to test.

levelos’s picture

Status: Needs work » Needs review

I've tried on both an existing and a fresh install, and the validation seems to work fine with 0. Can you verify the problem you're having? I see your point on ctype_digit, and happy to change that.

In general, I think if we're allowing users to set the cache lifetime, no need to limit them to arbitrary choices via a select when the validation is so simple.

claudiu.cristea’s picture

Status: Needs review » Fixed

Fixed some minor issues:

Committed in #391142.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.