jsr107 / jsr107spec

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

clarifications on CacheWriter javadoc/specifcation and what is testable constraint for tck #222

Closed jfialli closed 11 years ago

jfialli commented 11 years ago

The following javadoc comments from CacheWriter support testing atomicity between cache mutation operation and write-through invocations of CacheWriter.






Possibly contradicting weaking javadoc comments for constraint tests are following comments in CacheWriter javadoc.



Based on clarification, will determine if tck test to test

  1. for non-batch operations, if CacheWriter methods throws CacheException, corresponding cache mutation operation will not have occurred.
  2. for batch operations (such as putAll and removeAll) if batch method throws a CacheException, that only CacheWriter operations that succeeded also resulted in corresponding cache mutation.

Yes these look like earlier comments which we left in place after we did define the semantics. I have removed those two weakening in the class JavaDoc.


Javadoc corrections:

File: Cache.java Methods: boolean remove(K key); boolean remove(K key, V oldValue);

Replace:

Fixed. GL


Recommended Javadoc Enhancement:

Not as obvious from the Cache mutation operation javadoc that write-through failures throwing CacheException are one of the problems that can cause a cache mutation operation to fail.

Add an @see CacheWriter#write to Cache methods put, getAndPut, putIfAbsent, overloads of replace. getAndReplace.

Add an @see CacheWriter#delete to Cache method overloads of remove and getAndRemove

Add an @see CacheWriter.writeAll to Cache method putAll.

add an @see CacheWriter.deleteAll to Cache method overloads of deleteAll.

All above fixed. GL


Strongly recommended Javadoc enhancement for CacheWriter javadoc for batch methods writeAll and deleteAll.

Parameter entries is an in-out parameter for CacheWriter.writeAll and CacheWriter.deleteAll. The javadoc does not point this out sufficiently in javadoc for parameter entries. It is mentioned in the base javadoc for the method, but the parameter entries should directly state what it values signifies upon entering and when returning from the method call.

There is a whole paragraph in there right now on this point:

What exactly do you propose?


JSR 107 Specification and JSR 107 API discrepancy on CacheWriter batch methods. Those methods do not use Iterable, they use Set.

See page 16, bullet starting with "Use of Iterable".

Yes, removed the reference to CacheWriter using Iterable. They use Collection though, not Set.


Moved exception issue to new issue #224 so that we can some discussion on it.

gregrluck commented 11 years ago

See inline for corrections in each case. Let me know if anything further needs to be done. Have new #224 on the wrapping of exceptions issue.

jfialli commented 11 years ago

Proposal for @param javadoc for CacheWriter.writeAll and CacheWriter.deleteAll.

For writeAll method param:

@param entries Must be a mutable collection. Upon invocation, it contains the entries to write for write-through. Upon return to the caller, collection entries must only contain entries that were not successfully written. (see partial success above)

For deleteAll method param:

@param keys Must be a mutable collection. Upon invocation, it contains the keys to delete for write-through. Upon return to the caller, collection keys must only contain the keys that were not successfully deleted. (see partial success above)