Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JCache Cache.invoke does not update entry after initial load #210

Closed
wants to merge 1 commit into from

Conversation

kdombeck
Copy link
Contributor

If the cache is empty and you call jcache.invoke(key, entryProcessor) it will load the cache with the cache loader but then it will not write the updated entry with the cache writer. The reason is because the EntryProcessorEntry.action is LOADED rather than UPDATED.

Possibly related to #187.

This is currently just a unit test showing the problem, it does not contain a fix. I'm not sure which direction to go for a possible fix.

@ben-manes
Copy link
Owner

Thanks for the test, again! :)

I'm swamped today, but I'll try to look at this on my way home (PST). I owe people a new release so I should be able to get this out soon. Not sure about the fix yet until we both dig in a bit.

@ben-manes
Copy link
Owner

ben-manes commented Dec 13, 2017

hmm.. you are right that this is confusing. I originally followed the reference implementation, but the JSR authors do not consider it good quality. The specification is primarily the interface JavaDoc pasted into sections rather than additional depth.

The right thing is to probably sketch out the scenarios from scratch, using the examples in EntryProcessor's JavaDoc as guidance. I think part of the confusion is that it states Only the net effect of multiple operations has visibility outside of the Entry Processor, but it also states weird things like The first get and the second put only are visible to the ExpiryPolicy which means it isn't entirely flattened out. So I don't know if we can rely on a single state machine, should use a mixture of flags, or a queue of events.

@ben-manes
Copy link
Owner

The fix passes your test. The lifecycle is still very confusing, so please let me know if you discover other bugs. Adding the simple check of whether the entry was originally present in the cache seems to pass all the tests.

@ben-manes
Copy link
Owner

Released

@kdombeck
Copy link
Contributor Author

Works perfectly!! Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants