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: index handling
Date Tue, 15 May 2012 20:51:27 GMT
Hi,

On Tue, May 15, 2012 at 9:32 PM, Julian Reschke <julian.reschke@gmx.de> wrote:
> On 2012-05-15 20:39, Jukka Zitting wrote:
>> Couldn't we just pass them as is down to oak-core, where such names
>> can be rejected as invalid by a commit hook? Or if an exception is
>> required before save(), I'd rather have such checks explicitly in
>> NodeImpl&  friends instead of overloading the path mapper with extra
>> responsibilities.
>
> So you want it either higher in the stack or lower but not here? :-)

:-) "here" is also fine if there's a good case for it.

My main worry is that we end up overloading the path and name mappers
with responsibilities that don't really belong there. See for example
the recent discussion in OAK-89 where I argued that the name mapper
shouldn't be responsible for throwing exceptions that are relevant or
interpreted only higher up the stack. Instead the mapper should only
be responsible for a tightly scoped API that simply returns the mapped
name or null if such a mapping is not possible. The interpretation of
what a missing mapping means should be left to the higher level since
there's at least three different outcomes. Similarly I'm concerned
about special case rules like an index not being allowed on the last
element of a path creeping into the path mapper.

> Yes, we can make this work, but it essentially means that the path is parsed
> twice.

I don't see the duplicate parsing as a problem as the common case can
be heavily optimized (essentially to a no-op) and the second parsing
step is much simpler since it doesn't need to worry about name mapping
anymore. The path resolver is pretty simple, apart from identifier
resolution it only needs a fraction of the amount of code we already
have in JcrPathParser. A quick and dirty version of such a resolver is
included below:

    Tree resolvePath(String path, Tree context, Tree root) {
        if (path.equals("/")) {
            return root;
        } else if (path.startsWith("/")) {
            return resolvePath(path.substring(1), root);
        } else if (path.startsWith("[")) {
            return resolveIdentifier(path, root);
        } else if (!path.isEmpty()) {
            return resolveRelative(path, context);
        } else {
            return null;
        }
    }

    Tree resolveRelative(String path, Tree context) {
        int slash = path.indexOf('/');
        while (slash != -1) {
            context = resolveElement(path.substring(0, slash), context);
            if (context == null) {
                return null;
            }
            path = path.substring(slash + 1);
            slash = path.indexOf('/');
        }
        if (!path.isEmpty()) {
            context = resolveElement(path, context);
        }
        return context;
    }

    Tree resolveElement(String element, Tree context) {
        if (".".equals(element)) {
            return context;
        } else if ("..".equals(element)) {
            return context.getParent();
        } else {
            if (element.endsWith("[1]")) {
                element = element.substring(0, element.length - 3));
            }
            return context.getChild(element);
        }
    }

BR,

Jukka Zitting

Mime
View raw message