You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Jukka Zitting <ju...@gmail.com> on 2013/01/16 22:04:58 UTC

Re: svn commit: r1434364 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: NodeImpl.java SessionDelegate.java SessionImpl.java WorkspaceImpl.java query/QueryImpl.java version/VersionManagerImpl.java

Hi,

On Wed, Jan 16, 2013 at 10:23 PM,  <md...@apache.org> wrote:
> -    public String getOakPathOrThrowNotFound(String jcrPath) throws PathNotFoundException {
> +    public String getOakPath(String jcrPath) throws PathNotFoundException {
> [...]
> -    public String getOakPathOrThrow(String jcrPath)
> -            throws RepositoryException {

Does this mean that we no longer have separate utility methods for
throwing a PathNotFoundException vs. a generic RepositoryException?

If yes, please revert, as these methods were intentionally added to
satisfy the need for different exceptions in different cases.

BR,

Jukka Zitting

Re: svn commit: r1434364 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: NodeImpl.java SessionDelegate.java SessionImpl.java WorkspaceImpl.java query/QueryImpl.java version/VersionManagerImpl.java

Posted by Michael Dürig <md...@apache.org>.

On 17.1.13 5:33, Jukka Zitting wrote:
> Hi,
>
> On Wed, Jan 16, 2013 at 11:04 PM, Jukka Zitting <ju...@gmail.com> wrote:
>> If yes, please revert, as these methods were intentionally added to
>> satisfy the need for different exceptions in different cases.
>
> Hmm, on second thought that's a bit harsh and not too constructive,
> sorry. Let me rephrase:

Hehe this is contagious isn't it ;-)

>
> If yes, please make sure that all the name- and path-related
> exceptions that get thrown are still appropriate in their contexts.

Right, I might have been too eager in some cases. Will reopen the issue 
and review.

>
> Even though PathNotFoundException ISA RepositoryException, it has
> fairly specific semantics and is thus not appropriate for all cases

Hat tip to Liskov ;-)

Michael

> where names and paths are used. For example, is the following code
> correct:
>
>      try {
>          if (!session.getNode("/a:b").hasNode(*c:d")) {
>              System.out.println("/a:b/c:d does not exist");
>          }
>      } catch (PathNotFoundException e) {
>          System.out.println("/a:b does not exist");
>      }
>
> Currently it would end up printing "/a:b does not exist" even if the
> node /a:b did exist but the namespace prefix c didn't!
>
> BR,
>
> Jukka Zitting
>


Re: svn commit: r1434364 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: NodeImpl.java SessionDelegate.java SessionImpl.java WorkspaceImpl.java query/QueryImpl.java version/VersionManagerImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Wed, Jan 16, 2013 at 11:04 PM, Jukka Zitting <ju...@gmail.com> wrote:
> If yes, please revert, as these methods were intentionally added to
> satisfy the need for different exceptions in different cases.

Hmm, on second thought that's a bit harsh and not too constructive,
sorry. Let me rephrase:

If yes, please make sure that all the name- and path-related
exceptions that get thrown are still appropriate in their contexts.

Even though PathNotFoundException ISA RepositoryException, it has
fairly specific semantics and is thus not appropriate for all cases
where names and paths are used. For example, is the following code
correct:

    try {
        if (!session.getNode("/a:b").hasNode(*c:d")) {
            System.out.println("/a:b/c:d does not exist");
        }
    } catch (PathNotFoundException e) {
        System.out.println("/a:b does not exist");
    }

Currently it would end up printing "/a:b does not exist" even if the
node /a:b did exist but the namespace prefix c didn't!

BR,

Jukka Zitting