Use of InvocationContext vs. explicit Option in the API

[ben.wang]

I understand that in the new 2.0 API, you will expose InvocationContext for the user as a thread local variable. Right now, I can see that Option can be set there such that all subsquent code will be affected by that Option.

PojoCache 2.0 also has the need to use an Option to set a BR region. I am debating now to either 1) to be consistent with Cache/Node such that I also use a similar InvocationContext to set it explictly, or 2) overload my APIs (3 of them) with an Option parameter, e.g., cache.attach(id, pojo, option).

I am inclining to go with the first one. However, after thinking it over, I am not that comfortable with it yet. Manik, I hope you can convince me. :-)

The good thing about setting Option in a threadlocal variable is that there is not extra APIs so the interface is cleaner. However, IMO, InvocationContext should belong to the SPI per se, not the API. That is, user should not see it. Af far as I can see, currently InvocationContext only carries Option that is interested to a caller.

In addition, a user will need to:

Option option = new Option();
InvocationContext.getCurrent().setOption(option);
cache.put(fqn, key, value);

...

Nothing wrong but I just think it is relatively prone to errors.

[brian.stansberry]

I personally prefer the overloaded API as well (I don't mind lots of methods, as long as they all belong there and it's clear what's meant to be used when.)

Another thing I thought about was having the accessor/mutator for InvocationContext.setOptionOverrides() be static (or adding a static version), to at least get rid of the getCurrent() call. (The static method would do that for you). Similar to Thread.sleep() instead of Thread.currentThread().sleep().

But then I remember I'm always typing Thread.currentThread().sleep() and Thread.currentThread().setContextClassLoader() and having the IDE complain about the former and not the latter. That's always annoyed me.

[manik]

InvocationContext has always been stored in ThreadLocal, except that this was always done by the TreeCache class when you called the overloaded method. This has always had invocation-level scope.

Now you'd do it explicitly and the reasons for this shift are:

1. Every time you use this, this is very explicit. Little chance of using the wrong overloaded method.

2. Cleaner interface API, since this is needed in Cache and not CacheSPI (form Hibernate, for example)

+1 on the accessors/mutators being static and handling the ThreadLocal business internally.