bradcornford / Setter

An easy way to integrate Database Settings with Laravel
MIT License
13 stars 4 forks source link

Global cache flush #20

Open YOzaz opened 8 years ago

YOzaz commented 8 years ago

Hi, method cacheClear() clears WHOLE cache. I think you should clear your namespace only. Not sure if that's possible though.

bradcornford commented 8 years ago

Hi, yes this is the case. I don't believe the illuminate/cache/repository has the ability to return all items either for me to loop through and remove the setter tagged items.

YOzaz commented 8 years ago

Pitty (doh). Alright, https://github.com/bradcornford/Setter/issues/19 should solve my problem anyway.

YOzaz commented 8 years ago

... if cache driver is memcached (e.g.), it supports tags natively by the way. You may switch to native support in this case, therefore flush by tags in your method.

https://github.com/laravel/framework/blob/5.2/src/Illuminate/Cache/Repository.php#L353 https://github.com/laravel/framework/blob/5.2/src/Illuminate/Cache/MemcachedStore.php#L202

bradcornford commented 8 years ago

Yeah, i did notice that when i initially developed this package. Laravel version 4 however didn't provide any methods within the cache repository to achieve this. I could implement this for version 5 however.

YOzaz commented 8 years ago

https://laravel.com/docs/4.2/cache#cache-tags :smile:

bradcornford commented 8 years ago

Yes, the methods for tags exists within classes that allow it. However, they don't exist within classes that don't, i.e. the repository and filestore classes. Only classes extending the TaggableStore class have the tagging ability. The only way to check, would be to use method_exists and other checks to ensure the functionality exists. This would increase complexity as in some cases it would clear by tags, in others would flush the whole cache.

YOzaz commented 8 years ago

Well, L5 does method checking as well...: https://github.com/laravel/framework/blob/5.2/src/Illuminate/Cache/Repository.php#L355 And actually, method_exists function is crazy fast. Also, you can have an internal array in a class what keys you've set, and flush them manually with foreach. Anyway, probably this is out of this package scope I suppose.

bradcornford commented 8 years ago

Yeah, i'd be happy to implement this in L5, so ill re-open this issues. As for L4, I did think about having an internal array to store 'tags', however this would also need storing too, so would require more work.

YOzaz commented 8 years ago

... while I'm thinking it further - why did you add this cache at all? If that's because of DB query optimization - you can always use native query cache instead of your own.

bradcornford commented 8 years ago

I added it at first to reduce calls onto the database, and reduce overhead of the package. I didn't use the native query cache as I couldn't seem to find a native way of flushing an item when updating.