On 4/14/10 8:47 AM, Carsten Ziegeler wrote: > Hi Justin, > > thanks for the changes as we have discussed them roughly. I'm now more > happy with the way of handling this :) Well, that's a start...
> > It seems that these changes broke the tests for the jcr resource bundle > - at least on my machine it is failing as the used second workspace is > not available. Argh. I see the problem now. I had setup code to create the ws2 workspace in the Listener test, but not in the Resolver test. Fixed in r933956. > > I've just committed a change in revision 933928. While I tested your > changes I noticed a drop in performance of nearly 100% with every > request (in our environment which does literally a lot during a > request). Now, it seems that the check for the new path syntax > [workspace:path] causes trouble in our environment as we have paths with > a colon somewhere in the middle. > So I went ahead and changed the check from ":" to ":/" and changed also > the regex based split to a string substring operation. > This brought us nearly back to the performace we had before - I > currently note a drop of roughly 10% though I'm not 100% sure that these > are caused by these changes. I'll further investigate. Makes sense. This code is not optimized, but I wanted to get something committed. > > Anyway, to cut a long story short :) it seems that my initial idea of > just using the ":" as a separator might cause trouble. One solution is > to go like I did with the quick changes and require a path to be > absolute if the workspace prefixed notation is used. > Or we are searching for a different character to separate the two parts? Ideally, there's a character which is: 1) illegal in a JCR node name 2) legal in a URL path I just don't know off the top of my head what that would be. Barring this, I agree that requiring an absolute path is the right solution. If you need to pass a relative path to getResource(), you could always specify a search path entry like "ws2:/" in the JcrResourceResolverFactoryImpl config. Or use the two-argument form of getResource(). > > In addition, I'm wondering if we could make this feature completly > optional (maybe by a resource resolver wrapper etc.)? (But I think this > is something we can look into later on) I think this should just be a config option on JcrResourceResolverFactoryImpl which bypasses the new code blocks entirely. The only thing we need to watch out for here is that if you enable non-default workspace observation, non-default workspace resolution must be enabled, but this is reasonably simple to deal with. Justin > > Regards > Carsten > > justin wrote >> Author: justin >> Date: Tue Apr 13 19:13:31 2010 >> New Revision: 933747 >> >> URL: http://svn.apache.org/viewvc?rev=933747&view=rev >> Log: >> SLING-1447 - committing first pass at workspace names in resource paths >> >> Added: >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/WorkspaceDecoratedResource.java >> >> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrResourceListenerTest.java >> >> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/SynchronousJcrResourceListener.java >> Modified: >> sling/trunk/bundles/jcr/resource/pom.xml >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/JcrResourceConstants.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceListener.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolverFactoryImpl.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/ResourceDecoratorTracker.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/ResourceIteratorDecorator.java >> >> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/helper/ResourceProviderEntry.java >> >> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/JcrResourceResolverTest.java >
