On 20.2.13 21:36, Jukka Zitting wrote:
The problem is that SessionDelegate.needsRefresh() requires clean up after
the "real" method returns.
We could refactor the code so that such clean up is not needed.
AFAICT the mechanism is currently used to avoid repeated refreshing of
the session from nested operations.
The problem is actually more subtle than that. The mechanism avoids
refresh being called from reentrant calls to any session scoped
operation (that's the reason for using a counter in perform). Otherwise
we risk that a session is refreshed while we are in the middle of a
session scoped operation. This can lead to very unpredictable test
failures and bugs.
An alternative design would be to
turn needsRefresh to a flag in SessionDelegate that could be raised by
observation and other things like that. The enter() method would then
trigger a refresh and clear the flag, which would prevent further
nested operations from repeatedly refreshing until the flag gets
raised again.
This would already work using ObservationManagerImpl.hasEvents(), which
is exactly such a flag. However, it does not prevent us from the
reentrant case I described above.
[...] This will at the same time address the
issue I brought up at our last meeting that implementing JCR method in terms
of each others will result in the preconditions being checked multiple times
instead of just once.
The delegate pattern should allow us to avoid these cases. Instead of
JCR methods calling each other, they could be calling the relevant
delegate methods directly.
Yes and it would also solve the reentrance problem. It would however
mean that we need to strictly disallow for any JCR session scoped method
to call any JCR session scoped method. I'm not sure how to reliable
communicate this since not adhering to it wouldn't immediately fail but
rather show up in some unpredictable behaviour probably much later.
Michael
