On 2012-04-27 15:17, Jukka Zitting wrote:
Hi,
Instead of discussing this in the abstract, how about we look at a few
concrete cases to see how those would be best handled?
For example here's what the JCR spec says that the Session.save()
method [1] can throw:
AccessDeniedException - if any of the changes to be persisted would violate the
access
privileges of the this Session. Also thrown if any of the changes to be
persisted would
cause the removal of a node that is currently referenced by a REFERENCE property
that this Session does not have read access to.
ItemExistsException - if any of the changes to be persisted would be prevented
by the
presence of an already existing item in the workspace.
ConstraintViolationException - if any of the changes to be persisted would
violate
a node type or restriction. Additionally, a repository may use this exception to
enforce implementation- or configuration-dependent restrictions.
InvalidItemStateException - if any of the changes to be persisted conflicts with
a change already persisted through another session and the implementation is
such that this conflict can only be detected at save-time and therefore was not
detected earlier, at change-time.
ReferentialIntegrityException - if any of the changes to be persisted would
cause
the removal of a node that is currently referenced by a REFERENCE property
that this Session has read access to.
VersionException - if the save would make a result in a change to persistent
storage that would violate the read-only status of a checked-in node.
LockException - if the save would result in a change to persistent storage
that would violate a lock.
NoSuchNodeTypeException - if the save would result in the addition of a node
with an unrecognized node type.
RepositoryException - if another error occurs.
Our current implementation of save only throws generic
RepositoryExceptions and doesn't handle possible RuntimeExceptions
thrown from below:
public void save() throws RepositoryException {
try {
root.commit();
} catch (CommitFailedException e) {
throw new RepositoryException(e);
}
}
Here's how I'd fix this:
public void save() throws RepositoryException {
try {
root.commit();
} catch (CommitFailedException e) {
RepositoryException re =
exceptionMapper.getRepositoryException(e);
if (re == null) {
re = new RepositoryException("Failed to save transient
changes", e);
}
throw re;
} catch (RuntimeException e) {
throw new RepositoryException("Unexpected save() failure", e);
}
}
I wouldn't want to catch RuntimeException. I'd prefer if oak-core would
only throw OakException (extends RuntimeException), as suggested by
Michael Dürig (*).
I also think it would be good if these Oak exceptions could carry
sufficient information to generate a JCR exception.
The mappings that have been suggested by Michael and above would help,
but we'd lose correctness in the call stack, right?
> ...
Best regards, Julian
(*) My *actual* preference would be to throw the proper
RepositoryExceptions right away, as suggested by Angela.
