jsr107 / jsr107spec

JSR107 Cache Specification
Apache License 2.0
414 stars 164 forks source link

EntryProcessor effects clarification and corrections #335

Closed cruftex closed 8 years ago

cruftex commented 8 years ago

The Spec needs some more clarification for the EntryProcessor semantics and has mistakes. Snippet of the example 3 of the EntryProcessor:

 * <h4>Example 3</h4>
 * In this example, an {@link EntryProcessor} calls:
 * <ol>
 * <li>{@link MutableEntry#getValue()}</li>
 * <li>{@link MutableEntry#setValue(Object)}}</li>
 * <li>{@link MutableEntry#getValue()}</li>
 * <li>{@link MutableEntry#setValue(Object)}</li>
 * <li>{@link MutableEntry#remove()}</li>
 * </ol>
 * This will have the following {@link Cache} effects:
 * <br>
 * Final value of the cache: last setValue<br>
 * Statistics: one get and one remove as the second get and the two puts are
 * internal to the EntryProcessor.<br>
 * Listeners: remove if there was initial value in the cache, otherwise no
 * listener invoked.
 * <br> CacheLoader: Invoked by the first get only if a loader was
 * registered.
 * <br> CacheWriter: Invoked by the remove only as the two puts are internal to
 * the Entry Processor.<br>
 * ExpiryPolicy: The first get only is visible to the ExpiryPolicy. There is no
 * remove event in ExpiryPolicy.

Proposed correction for final value

Final value of the cache: last setValue

Should read:

Final value of the cache: neutral effect if there is no existing value or removal otherwise.

Proposed correction for CacheLoader effect

CacheLoader: Invoked by the first get only if a loader was registered.

Should read:

CacheLoader: Invoked by the first get only if the entry is not present, a loader was registered and read through is enabled.

Proposed correction for CacheWriter effect

CacheWriter: Invoked by the remove only as the two puts are internal to the Entry Processor.

This implies that CacheWriter.delete() is always called, regardless there was an entry or not. The TCK asserts that the writer is not called on MutableEntry.remove() if no entry exists before.

TCK challenge: https://github.com/jsr107/jsr107tck/issues/84

Consequent change to align with existing TCK:

CacheWriter: Invokes CacheWriter#delete if there was initial value in the cache, no writer is invoked otherwise.

The texts of the other examples need some corrections also.

An explicit example should be added for MutableEntry.remove() on a non existent entry.

The change/clarification on CacheWriter would align with the TCK and existing implementations do not need changing. The downside: Its against the concept of the EntryProcessor, being able to carry out any cache operation with it.

gregrluck commented 8 years ago

Proposed correction for final value - the entry is removed if it was present Proposed correction for CacheLoader effect - CacheLoader: Invoked by the first get only if the entry is not present, a loader was registered and read through is enabled. Proposed correction for CacheWriter effect - CacheWriter: Invoked by the remove only as the two puts are internal to the Entry Processor, provided that the first #getValue was non-null.

The TCK's behaviour is correct. Will close TCK challenge: jsr107/jsr107tck#84