jsr107 / jsr107spec

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

CacheEntryEvent getOldValue() #362

Closed cruftex closed 7 years ago

cruftex commented 8 years ago

The Spec says at isOldValueAvailable()

Whether the old value is available.

Should be precise about the fact, when the cache implementation sends an old value. In case of a remove event this is not totally obvious.

TODO

The Spec needs to be updated at the following places:

Enhancements to the TCK

Enhancements to the RI

cruftex commented 8 years ago

Thought it is an easy clarification, but it isn't.

Let's record my current findings:

Sending on a remove the value as null and oldValue as the removed one, does not work with the TCK. So it is evident that the oldValue is not to be used for the remove event.

The TCK has no tests at all for CacheEntryEvent.getOldValue() and CacheEntryEvent.isOldValueAvailable().

The RI implementation throws UnsuppportedOperationException, which is not in the Spec.

  @Override
  public V getOldValue() throws UnsupportedOperationException {
    if (isOldValueAvailable()) {
      return oldValue;
    } else {
      throw new UnsupportedOperationException("Old value is not available for key");
    }
  }

=> There is some more work to do. Clarifying the Spec should go hand in hand with the proper TCK tests for it.

gregrluck commented 7 years ago

Whether the old value is available is set in the CacheEntryListenerConfiguration interface.

/**

There is a MutableCacheEntryListenerConfiguration class which allows it to be passed in. A user defined CacheEntryListener is allowed to implement as many of the sub-interfaces as it wants.

The usual thing is that the old value is always known to remove, expire and update, but not to created. However the spec does not say instead making it user selectable at time of creating the MutableCacheEntryListenerConfiguration.

Because a user can set this to true for any type of listener an implementation must be able to supply the old value if required. Of course the value will always be null for create.

Added a test to check that the old value is returned when set to true.

The RI always sets the old value for update.

//created dispatcher.addEvent(CacheEntryCreatedListener.class, new RICacheEntryEvent<K, V>(this, key, value, EventType.CREATED));

//updated dispatcher.addEvent(CacheEntryUpdatedListener.class, new RICacheEntryEvent<K, V>(this, key, value, oldValue, EventType.UPDATED));

//expired dispatcher.addEvent(CacheEntryExpiredListener.class, new RICacheEntryEvent<K, V>(this, key, expiredValue, EXPIRED));

//removed dispatcher.addEvent(CacheEntryRemovedListener.class, new RICacheEntryEvent<K, V>(this, key, value, REMOVED));

Updated CacheEntryEvent to implement getValue from Cache.Entry so that behaviour can be specified and added JavaDoc as follows:

/**