getAllPages() returns all pages after cache ttl has expired #362
Conversation
| * @param listener cache listener to register | ||
| * @return true if the listener is being added and was not already added | ||
| */ | ||
| boolean registerListener( String cacheName, CacheEventListener listener ); |
There was a problem hiding this comment.
Hi @AlbrechtStriffler, thanks for looking into this!
The CachingManager interface shouldn't include any ehcache specific classes, in order to allow other cache-framework implementations. In order to allow the listener registration, would passing an AtomicBoolean (the private volatile booleans from this PR) instead of the CacheEventListener be enough?
Also, as there's the remote possibility of other CachingManager custom implementations, should this method be a default one with an empty implementation? (Not really sure which would be the correct answer, I'd be surprised if indeed there is someone out there with a custom CachingManager, I'll let you decide on this one)
There was a problem hiding this comment.
Ah, your are right, that CacheEventListener is to specific for the CachingManager. But I think a method like updateAllRequestedFlag(AtomicBoolean flag) is also a bit to specific, no? It is a random implementation/optimization detail of the Caching[Attachment]Provider the CachingManager Interface shouldn't have to know about.
How about we add a generic CacheEvent for jspwiki, that is probably a bit simpler than the one from EhCache and when registering to the specific CachingManager, like the EhcacheCachingManager, we can wrap and delegate to that JSPWikiCacheEvent with the Ehcache (or other) specific implementation of the CacheEvent.
Regarding the method attributes in CacheEvent, instead of the Ehcache we could use 'String cacheName' and instead of Element we could use 'Serializable name' and 'T or Object object'
If you are okay with this, I'm happy to adapt the pull request accordingly.
There was a problem hiding this comment.
Hi!
I thought of throwing an event too, but given that one class is a direct dependency of the other, it seemed too much indirection to me. I agree that updateAllRequestedFlag is also too specific. May be something like registerListener( String cacheName, String listener, T... args)? listener could be used to decide which kind of listener build for a given cacheName, with all the required data passed through args. WDYT?
Either going the events way or going through the registerListener method would fine to me, my main concern is that EhCache classes should not appear outside EhcacheCachingManager
cheers,
juan pablo
Using cachingProvider.getAllPages() will be incomplete if the cache is configured so the entries expire over time. Pages in the cache will be evicted, but the allRequested-flag remains set to true.
To solve this, I registered a listener resetting the flag as soon as the first element expires. To be able to register listener, I also had to slightly extend the CachingManager...