<< Back to previous view

[CMEMOIZE-3] snapshot assumes too much about a CacheProtocol implementation Created: 13/Dec/11  Updated: 14/Dec/11  Resolved: 14/Dec/11

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Paul Stadig Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File 3-fix-snapshot-assumptions.diff     File 3-fix-snapshot-assumptions-updated.diff    
Patch: Code and Test
Approval: Accepted

 Description   

I believe this line https://github.com/clojure/core.memoize/blob/9bdf608af8bbf4a753fc19758446dd821cb45979/src/main/clojure/clojure/core/memoize.clj#L72 assumes too much about implementations of CacheProtocol, namely that they have a field named 'cache'.

The first call to .cache makes sense, since PluggableMemoization has a 'cache' field, but the second call to .cache assumes that any implementation of CacheProtocol that PluggableMemoization wraps will have a 'cache' field.

I ran into this issue when I desired to have a cache that ignores nils, so I attempted to wrap the TTLCache with an implementation of CacheProtocol that would not cache nil values. Calls to snapshot failed since it was trying to access a 'cache' field on my NoNilCache.

I suspect that the outer call to .cache could be replaced with a call to seq, since defcache adds an implementation of seq to each cache implementation which should pull out the keys/values from the cache.

I'd be glad to work up a patch given that I'm not off base in my assumptions.

 Comments   
Comment by Fogus [ 13/Dec/11 3:08 PM ]

@Paul

The better assumption is that the underlying caches are associative. Thanks for reporting and a patch would be wonderful.

Comment by Paul Stadig [ 13/Dec/11 4:16 PM ]

Added test and removed one layer of calls to .cache. I tested this against my code base, and it fixes the issue I was seeing.

Comment by Paul Stadig [ 13/Dec/11 4:28 PM ]

Updated patch. The tests worked before by throwing an exception on failure, but I updated it by adding an is assertion.

Generated at Sat May 25 22:31:06 CDT 2013 using JIRA 4.4#649-r158309.