Investigate wrapper exceptions

[cruftex]

The Spec says:

There is also a blocking implementation of CompletionListener, CompletionListenerFuture. It implements both CompletionListener and Future. If the onException(Exception e) method of CompletionListener is called, the exception is wrapped in ExecutionException and rethrown by the Future get() and get(long timeout, TimeUnit unit) methods.

But the code, does not wrap:

  public void onException(Exception e) throws IllegalStateException {
    synchronized (this) {
      if (isCompleted) {
        throw new IllegalStateException("Attempted to use a CompletionListenerFuture instance more than once");
      } else {
        isCompleted = true;
        exception = e;
        notify();
      }
    }
  }

ExecutionException means probably java.util.concurrent.ExecutionException. Should be linked or fully qualified if exceptions are mentioned not in the scope of javax.cache. Shouldn't it be a CacheLoaderException?!

TODO:

• investigate wrapper usage
• some general statement about customizations and wrapper should be in the spec

Releated: https://github.com/jsr107/jsr107tck/issues/99 https://github.com/jsr107/jsr107tck/issues/85

We need probably more specific issues on what actually needs change. Any more findings and suggestions are appreciated.

Relevant tests:

• org.jsr107.tck.integration.CacheLoaderTest
• org.jsr107.tck.integration.CacheWriterTest

[cruftex]

Regarding the loadAll() the remarks from @christian-esken:

org.jsr107.tck.integration.CacheLoaderTest

// Observation: shouldPropagateExceptionUsingLoadAll() requires a CacheLoaderException

// Issue: Javadocs do not mandate this for Cache.loadAll().
//    a) Javadoc states: "If a problem is
//     encountered during the retrieving or loading of the objects,
//     an exception is provided to the {@link CompletionListener}."
//     b) The @throws declaration is "@throws CacheException        thrown if there is a problem performing the load."

// Proposed change: Change CacheLoaderTest to check for CacheException

The relevant code is

    CompletionListenerFuture future = new CompletionListenerFuture();
    cache.loadAll(keys, false, future);

    //wait for the load to complete
    try{
      future.get();
    } catch (ExecutionException e) {
      assertThat(e.getCause(), instanceOf(CacheLoaderException.class));
    }

OTOH CompletionListenerFuture does correctly document that it wraps with ExecutionException on the method get(), opposing my initial statement.

Re-evaluating everything I think the current checks and Spec is consistent and useful as well.

1. The immediate exception is the ExceutionException as documented
2. The checks that the cause is CacheLoaderException which is correct, too, since an exception originated from the loader

Effectively in this case the original cause is double wrapped. Once by the CacheLoaderException, and again by the ExecutionException. That's suprising, but the only way to fulfill the future interface contract.

BTW: There is a mistake in the test not including a fail(). Opening a TCK issue for that.