jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jukka Zitting <jukka.zitt...@gmail.com>
Subject Re: enter() method for JCR implementation classes
Date Tue, 05 Mar 2013 09:56:18 GMT
Hi,

On Thu, Feb 28, 2013 at 1:13 PM, Michael Dürig <mduerig@apache.org> wrote:
> Just some anecdotal evidence: a single call to Node.addNode() results in a
> call to Node.getPrimaryNodeType() which in turn ends up calling
> Property.getValue() and which eventually results in a call to
> Property.isMultiple(). So there will be quite a bit of stuff to untangle.
> Not sure how to go about this in order to catch them all.

Yeah, I find this pattern somewhat troublesome and much more prevalent
than I'd expected. It would be good to avoid crossing the JCR API
boundary multiple times during the execution of a single operation. To
do this, I'd suggest we adjust the Impl/Delegate boundary as follows:

Responsibilities of JCR Impl classes:
- name/path mapping for both method arguments and return values
  -> NamePathMapper should be in SessionImpl instead of SessionDelegate
- tracking and instantiation of other JCR Impl objects
  -> Delegate classes should refer to neither the JCR API nor the Impl classes
  -> Values should be returned as PropertyState instances that are
mapped to JCR Values by an Impl class

Delegate classes
- access to the Oak API
- the checkStatus() and perform() logic
  -> Something like:
- all the "business logic" associated with complex operations
  -> the complex SessionObject classes from Impl classes should be
pushed down to Delegates
  -> dlg.perform(dlg.getSomeOperation(oakName, ...))

To make it easier to enforce such a stricter design boundary with
tools like JDepend, I'd move the Delegate classes to a separate
package structure under o.a.j.oak.jcr.delegate.

> OTOH as your main concern initially was to reduce boilerplate, we could also
> try the other approach I proposed earlier: enrich SessionOperation with
> preconditions to be checked before the core of the call is actually
> executed.

Agreed. One particular SessionOperation adjustment I'd like to add is
information on whether the operation can change the state of a
session. Since otherwise the session is immutable (see also below), we
could shortcut the status checks to be performed only after such a
state-changing operation (like refresh() and all transient updates)
has been made. Since the majority of sessions are read-only and never
change their states from the initial snapshot, this would allow us to
avoid most status checks entirely.

> performed. Currently there is still the chance, that the conditions become
> invalid right after the check methods returned but before the operation is
> actually performed.

How would that happen? Since we're operating on an immutable snapshot
of the repository, the only way for the state of a session to change
is through a concurrent call from the client, possibly triggered by an
observation thread. An easy way to prevent that from being a problem
is to synchronize the perform() method.

BR,

Jukka Zitting

Mime
View raw message