search

arnowelzel / lightbox-photoswipe

Integration of PhotoSwipe to WordPress
https://arnowelzel.de/wp/en/projects/wordpress/lightbox-with-photoswipe
GNU General Public License v2.0
27 stars 7 forks source link

Update lightbox-photoswipe.php #47

Closed B-e-n-G closed 4 years ago

B-e-n-G commented 4 years ago

Added a new option use_cache in order to use the WordPress caching mechanisms wp_cache_add and wp_cache_get instead of the database table wp_lightbox_photoswipe_img.

This is working well with varios caching plugins like Redis Object Cache.

The description of the option in the backend may need to be improved:

Use wp_cache_add/get instead database

...and there aren't any translations yet.

arnowelzel commented 4 years ago

Interesting idea, looks good. The only thing I am missing yet is to remove the database tables if the cache is used:

When the cache is active, make sure the existing tables are removed similar to https://github.com/arnowelzel/lightbox-photoswipe/blob/98b53bd9fc46a8ca0c8ae65af4ce47f5df660481/uninstall.php#L9

If the cache works well it might also be worth it to omit the database use completely. However it should be possible to clean up the cache manually as well - at least when removing the plugin (see https://github.com/arnowelzel/lightbox-photoswipe/blob/98b53bd9fc46a8ca0c8ae65af4ce47f5df660481/uninstall.php#L23). Without the cache one can just remove and reinstall the plugin to make sure all cached data is gone. Does this also happen when the plugin get's removed after it added some cache entries?

arnowelzel commented 4 years ago

Thank you for the quick fixes! The only little thing left is the cache key - see my comment. Otherwise it looks good.

B-e-n-G commented 4 years ago

Interesting idea, looks good. The only thing I am missing yet is to remove the database tables if the cache is used:

When the cache is active, make sure the existing tables are removed similar to

https://github.com/arnowelzel/lightbox-photoswipe/blob/98b53bd9fc46a8ca0c8ae65af4ce47f5df660481/uninstall.php#L9

If the cache works well it might also be worth it to omit the database use completely. However it should be possible to clean up the cache manually as well - at least when removing the plugin (see

https://github.com/arnowelzel/lightbox-photoswipe/blob/98b53bd9fc46a8ca0c8ae65af4ce47f5df660481/uninstall.php#L23 ). Without the cache one can just remove and reinstall the plugin to make sure all cached data is gone. Does this also happen when the plugin get's removed after it added some cache entries?

Good point... I have added an action update_option_lightbox_photoswipe_use_cache which is calling the new method update_option_use_cache. This method clears the table and the cron job in case of selected option "Use wp_cache_add..." and creates table and cron in case of this option isn't selected.

Clearing the redis cache in case of uninstall this plugin isn't necessary, because wp_cache_add becomes CACHE_EXPIRE_IMG_DETAILS as third parameter. The redis will clear all entries after this time automatically