You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@sling.apache.org by Justin Edelson <ju...@gmail.com> on 2010/02/11 17:06:02 UTC

Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

This, combined with the use of the JCR 2 bundle, is causing some test
failures. It seems to be because AccessControlUtil.getAccessManager()
attempts to call the getAccessManager() method specified by the
javax.jcr.Session interface which isn't implemented by SessionImpl.

Possible fixes are:
* Upgrade jackrabbit.server to JR 2.
* Remove NamespaceMapper modifications to jcr.base and roll back to old
jcr.api bundle in launchpad.

WDYT?

Justin


On 2/11/10 8:17 AM, cziegeler@apache.org wrote:
> Author: cziegeler
> Date: Thu Feb 11 13:17:47 2010
> New Revision: 908956
> 
> URL: http://svn.apache.org/viewvc?rev=908956&view=rev
> Log:
> SLING-1366 : Use dynamic proxy to handle session#impersonate call.
> 
> Added:
>     sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java   (with props)
> Modified:
>     sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository.java
> 
> Modified: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository.java
> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository.java?rev=908956&r1=908955&r2=908956&view=diff
> ==============================================================================
> --- sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository.java (original)
> +++ sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepository.java Thu Feb 11 13:17:47 2010
> @@ -118,7 +118,11 @@
>  
>      private char[] adminPass;
>  
> -    private Loader loader;
> +    /** Namespace handler. */
> +    private Loader namespaceHandler;
> +
> +    /** Session proxy handler. */
> +    private SessionProxyHandler sessionProxyHandler;
>  
>      // the poll interval used while the repository is not active
>      private long pollTimeInActiveSeconds;
> @@ -231,6 +235,11 @@
>  
>              defineNamespacePrefixes(session);
>  
> +            // to support namespace prefixes if session.impersonate is called
> +            // we have to use a proxy
> +            if ( this.sessionProxyHandler != null ) {
> +                return this.sessionProxyHandler.createProxy(session);
> +            }
>              return session;
>  
>          } catch (NoSuchWorkspaceException nswe) {
> @@ -390,7 +399,8 @@
>       */
>      protected void setupRepository(Repository repository) {
>          BundleContext bundleContext = componentContext.getBundleContext();
> -        this.loader = new Loader(this, bundleContext.getBundles());
> +        this.namespaceHandler = new Loader(this, bundleContext.getBundles());
> +        this.sessionProxyHandler = new SessionProxyHandler(this);
>      }
>  
>      /**
> @@ -496,10 +506,11 @@
>       * @param repository
>       */
>      protected void tearDown(Repository repository) {
> -        if (this.loader != null) {
> -            this.loader.dispose();
> -            this.loader = null;
> +        if (this.namespaceHandler != null) {
> +            this.namespaceHandler.dispose();
> +            this.namespaceHandler = null;
>          }
> +        this.sessionProxyHandler = null;
>      }
>  
>      /**
> @@ -525,7 +536,7 @@
>       */
>      public void bundleChanged(BundleEvent event) {
>          // Take care: This is synchronous - take care to not block the system !!
> -        Loader theLoader = this.loader;
> +        Loader theLoader = this.namespaceHandler;
>          if (theLoader != null) {
>              switch (event.getType()) {
>                  case BundleEvent.INSTALLED:
> @@ -693,10 +704,10 @@
>          return false;
>      }
>  
> -    private void defineNamespacePrefixes(final Session session) throws RepositoryException {
> -        if (this.loader != null) {
> +    void defineNamespacePrefixes(final Session session) throws RepositoryException {
> +        if (this.namespaceHandler != null) {
>              // apply namespace mapping
> -            this.loader.defineNamespacePrefixes(session);
> +            this.namespaceHandler.defineNamespacePrefixes(session);
>          }
>  
>          // call post processors
> 
> Added: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java
> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java?rev=908956&view=auto
> ==============================================================================
> --- sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java (added)
> +++ sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java Thu Feb 11 13:17:47 2010
> @@ -0,0 +1,142 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.apache.sling.jcr.base;
> +
> +import java.lang.reflect.InvocationHandler;
> +import java.lang.reflect.InvocationTargetException;
> +import java.lang.reflect.Method;
> +import java.lang.reflect.Proxy;
> +import java.util.HashSet;
> +import java.util.Set;
> +
> +import javax.jcr.Credentials;
> +import javax.jcr.Session;
> +
> +/**
> + * The session proxy handler creates session proxies to handle
> + * the namespace mapping support if impersonate is called on
> + * the session.
> + */
> +public class SessionProxyHandler  {
> +
> +    /** The array of proxied interfaces. */
> +    private Class<?>[] interfaces;
> +
> +    /** The repository */
> +    private final AbstractSlingRepository repository;
> +
> +    public SessionProxyHandler(final AbstractSlingRepository repo) {
> +        this.repository = repo;
> +    }
> +
> +    /** Calculate the interfaces.
> +     * This is done only once - we simply assume that the same repository is
> +     * emitting session from the same class.
> +     */
> +    private Class<?>[] getInterfaces(final Class<?> sessionClass) {
> +        if ( interfaces == null ) {
> +            synchronized ( SessionProxyHandler.class ) {
> +                if ( interfaces == null ) {
> +                    final HashSet<Class<?>> workInterfaces = new HashSet<Class<?>>();
> +
> +                    // Get *all* interfaces
> +                    guessWorkInterfaces( sessionClass, workInterfaces );
> +
> +                    this.interfaces = workInterfaces.toArray( new Class[workInterfaces.size()] );
> +
> +                }
> +            }
> +        }
> +        return interfaces;
> +    }
> +
> +    /**
> +     * Create a proxy for the session.
> +     */
> +    public Session createProxy(final Session session) {
> +        final Class<?> sessionClass = session.getClass();
> +        return (Session)Proxy.newProxyInstance(sessionClass.getClassLoader(),
> +                getInterfaces(sessionClass),
> +                new SessionProxy(session, this.repository));
> +
> +    }
> +
> +
> +    public static final class SessionProxy implements InvocationHandler {
> +        private final Session delegatee;
> +        private final AbstractSlingRepository repository;
> +
> +        public SessionProxy(final Session delegatee, final AbstractSlingRepository repo) {
> +            this.delegatee = delegatee;
> +            this.repository = repo;
> +        }
> +
> +        /**
> +         * @see java.lang.reflect.InvocationHandler#invoke(java.lang.Object, java.lang.reflect.Method, java.lang.Object[])
> +         */
> +        public Object invoke(Object proxy, Method method, Object[] args)
> +        throws Throwable {
> +            if ( method.getName().equals("impersonate") && args != null && args.length == 1) {
> +                final Session session = this.delegatee.impersonate((Credentials)args[0]);
> +                this.repository.defineNamespacePrefixes(session);
> +                return new SessionProxy(session, this.repository);
> +            }
> +            try {
> +                return method.invoke(this.delegatee, args);
> +            } catch (InvocationTargetException ite) {
> +                throw ite.getTargetException();
> +            }
> +        }
> +    }
> +
> +    /**
> +     * Get a list of interfaces to proxy by scanning through
> +     * all interfaces a class implements.
> +     *
> +     * @param clazz           the class
> +     * @param workInterfaces  the set of current work interfaces
> +     */
> +    private void guessWorkInterfaces( final Class<?> clazz,
> +                                      final Set<Class<?>> workInterfaces ) {
> +        if ( null != clazz ) {
> +            addInterfaces( clazz.getInterfaces(), workInterfaces );
> +
> +            guessWorkInterfaces( clazz.getSuperclass(), workInterfaces );
> +        }
> +    }
> +
> +    /**
> +     * Get a list of interfaces to proxy by scanning through
> +     * all interfaces a class implements.
> +     *
> +     * @param classInterfaces the array of interfaces
> +     * @param workInterfaces  the set of current work interfaces
> +     */
> +    private void addInterfaces( final Class<?>[] classInterfaces,
> +                                final Set<Class<?>> workInterfaces ) {
> +        for ( int i = 0; i < classInterfaces.length; i++ ) {
> +            // to avoid problems we simply ignore all pre jsr 283 interfaces - once we
> +            // moved to jcr 2.0 completly we can remove this check
> +            if ( !classInterfaces[i].getName().startsWith("org.apache.jackrabbit.api.jsr283")) {
> +                workInterfaces.add( classInterfaces[i] );
> +            }
> +            addInterfaces(classInterfaces[i].getInterfaces(), workInterfaces);
> +        }
> +    }
> +}
> 
> Propchange: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
> 
> Propchange: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java
> ------------------------------------------------------------------------------
>     svn:keywords = author date id revision rev url
> 
> Propchange: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/SessionProxyHandler.java
> ------------------------------------------------------------------------------
>     svn:mime-type = text/plain
> 
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 13:42, Justin Edelson wrote:

> On 2/12/10 8:22 AM, Ian Boston wrote:
>> 
>> On 12 Feb 2010, at 13:06, Carsten Ziegeler wrote:
>> 
>>> Ian Boston wrote:
>>>> On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:
>>>> 
>>>>>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
>>>>> I think this one should go into the release as this fixes a broken
>>>>> implementation. There is currently a line in there which excludes the
>>>>> pre jsr 283 interfaces. So we should remove this line.
>>>> 
>>>> Done this, you might want to have a quick look at the patch [1] that makes this change, however, the Ace integration tests are failing, same as trunk head at 908994.
>>>> 
>>> Looks good to me; let's give it a go and then see what we need to fix
>>> for the tests.
>> 
>> I am getting failures on 
>> Failed tests: 
>>  testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>>  testModifyAceForGroup(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>>  testRemoveAce(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)
>>  testRemoveAces(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)
>> 
>> 
>> eg 
>> testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)  Time elapsed: 0.151 sec  <<< FAILURE!
>> junit.framework.AssertionFailedError: expected:<200> but was:<500>
>> 
>> I cant find any matching exception in the error log at the moment.
> 
> This sounds to me like you still have the jcr 2 bundle and a newer
> sling.jcr.api bundle. But looking at the patch set, this isn't the case
> so I'm confused.
> 
> If these are the only test failures, you might as well commit your
> changes as these same tests are failing now. I can take a closer look in
> 30-45 mins.
> 
> If it's the same problem I saw yesterday (with the wrong
> getAccessManager() method being called), you won't see an exception in
> the logs because it is logged at the debug level (by AccessControlUtil
> IIRC).


Yes, got a debugger attached the testing launchpad sever, its in there with a MethodNotFound on the session proxy.
Ian

> 
> Justin
> 
>> 
>> Ian
>> 
>> 
>>> 
>>> Carsten
>>> 
>>>> 
>>>> 1. http://codereview.appspot.com/206082/diff2/1:28/1005
>>>> 
>>>>> Regards
>>>>> Carsten
>>>>> 
>>>>> -- 
>>>>> Carsten Ziegeler
>>>>> cziegeler@apache.org
>>>> 
>>>> 
>>> 
>>> 
>>> -- 
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>> 
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On 2/12/10 8:22 AM, Ian Boston wrote:
> 
> On 12 Feb 2010, at 13:06, Carsten Ziegeler wrote:
> 
>> Ian Boston wrote:
>>> On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:
>>>
>>>>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
>>>> I think this one should go into the release as this fixes a broken
>>>> implementation. There is currently a line in there which excludes the
>>>> pre jsr 283 interfaces. So we should remove this line.
>>>
>>> Done this, you might want to have a quick look at the patch [1] that makes this change, however, the Ace integration tests are failing, same as trunk head at 908994.
>>>
>> Looks good to me; let's give it a go and then see what we need to fix
>> for the tests.
> 
> I am getting failures on 
> Failed tests: 
>   testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>   testModifyAceForGroup(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>   testRemoveAce(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)
>   testRemoveAces(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)
> 
> 
> eg 
> testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)  Time elapsed: 0.151 sec  <<< FAILURE!
> junit.framework.AssertionFailedError: expected:<200> but was:<500>
> 
> I cant find any matching exception in the error log at the moment.

This sounds to me like you still have the jcr 2 bundle and a newer
sling.jcr.api bundle. But looking at the patch set, this isn't the case
so I'm confused.

If these are the only test failures, you might as well commit your
changes as these same tests are failing now. I can take a closer look in
30-45 mins.

If it's the same problem I saw yesterday (with the wrong
getAccessManager() method being called), you won't see an exception in
the logs because it is logged at the debug level (by AccessControlUtil
IIRC).

Justin

> 
> Ian
> 
> 
>>
>> Carsten
>>
>>>
>>> 1. http://codereview.appspot.com/206082/diff2/1:28/1005
>>>
>>>> Regards
>>>> Carsten
>>>>
>>>> -- 
>>>> Carsten Ziegeler
>>>> cziegeler@apache.org
>>>
>>>
>>
>>
>> -- 
>> Carsten Ziegeler
>> cziegeler@apache.org
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On 2/12/10 10:33 AM, Ian Boston wrote:
> Sorry for the delay, everything ok here, good to go.
> Ian

I'm having some trouble getting an integration test which validates that
Sling-Namespaces works with impersonation, so please hold on the release
for a bit.

Thanks,
Justin

> 
> On 12 Feb 2010, at 15:02, Ian Boston wrote:
> 
>> I am just running through a second set of external integration tests, should be done in a few minutes.
>> Ian
>>
>> On 12 Feb 2010, at 14:59, Carsten Ziegeler wrote:
>>
>>> With latest trunk I don't get any integration test errors anymore.
>>>
>>> Carsten
>>> -- 
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>>
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

Sorry for the delay, everything ok here, good to go.
Ian

On 12 Feb 2010, at 15:02, Ian Boston wrote:

> I am just running through a second set of external integration tests, should be done in a few minutes.
> Ian
> 
> On 12 Feb 2010, at 14:59, Carsten Ziegeler wrote:
> 
>> With latest trunk I don't get any integration test errors anymore.
>> 
>> Carsten
>> -- 
>> Carsten Ziegeler
>> cziegeler@apache.org
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

I am just running through a second set of external integration tests, should be done in a few minutes.
Ian

On 12 Feb 2010, at 14:59, Carsten Ziegeler wrote:

> With latest trunk I don't get any integration test errors anymore.
> 
> Carsten
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

With latest trunk I don't get any integration test errors anymore.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On 2/12/10 9:52 AM, Ian Boston wrote:
> 
> On 12 Feb 2010, at 14:47, Justin Edelson wrote:
> 
>> On 2/12/10 9:43 AM, Ian Boston wrote:
>>>> Yeah, but that should be a proxied interface. I'll watch for your
>>>> commit.
>>>
>>> Just pushed a commit, but I notice from your comments on code review that there may be further problems with the reverts I did to jcr.api
>>>
>>
>> It's not a big problem, just an unnecessary removal. Just to be clear,
>> jcr.api does NOT need to be released.
>>
>> I'm going to reapply the integration test for the Sling-Namespaces
>> header. This is good to have to ensure that SLING-1366 stays fixed.
> 
> Ok,
> I was just intending to revert 909431 when this was all done which would put head back into position ? Can do either, upto you.

I'd like to have the NamespaceMapper in jcr.api in trunk so I can use
it. I'll apply the rest of the (re-)revert to my JR2 branch and I can
apply it from there once the releases are done.

Justin

> Ian
> 
> 
>>
>> Justin
>>
>>> Ian
>>>
>>>
>>>>
>>>> Justin
>>>>>
>>>>> Ian
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Carsten
>>>>>> --
>>>>>> Carsten Ziegeler
>>>>>> cziegeler@apache.org
>>>>>
>>>
>>
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 14:47, Justin Edelson wrote:

> On 2/12/10 9:43 AM, Ian Boston wrote:
>>> Yeah, but that should be a proxied interface. I'll watch for your
>>> commit.
>> 
>> Just pushed a commit, but I notice from your comments on code review that there may be further problems with the reverts I did to jcr.api
>> 
> 
> It's not a big problem, just an unnecessary removal. Just to be clear,
> jcr.api does NOT need to be released.
> 
> I'm going to reapply the integration test for the Sling-Namespaces
> header. This is good to have to ensure that SLING-1366 stays fixed.

Ok,
I was just intending to revert 909431 when this was all done which would put head back into position ? Can do either, upto you.
Ian


> 
> Justin
> 
>> Ian
>> 
>> 
>>> 
>>> Justin
>>>> 
>>>> Ian
>>>> 
>>>>> 
>>>>> Regards
>>>>> Carsten
>>>>> --
>>>>> Carsten Ziegeler
>>>>> cziegeler@apache.org
>>>> 
>> 
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On 2/12/10 9:43 AM, Ian Boston wrote:
>> Yeah, but that should be a proxied interface. I'll watch for your
>> commit.
> 
> Just pushed a commit, but I notice from your comments on code review that there may be further problems with the reverts I did to jcr.api
> 

It's not a big problem, just an unnecessary removal. Just to be clear,
jcr.api does NOT need to be released.

I'm going to reapply the integration test for the Sling-Namespaces
header. This is good to have to ensure that SLING-1366 stays fixed.

Justin

> Ian
> 
> 
>>
>> Justin
>>>
>>> Ian
>>>
>>>>
>>>> Regards
>>>> Carsten
>>>> --
>>>> Carsten Ziegeler
>>>> cziegeler@apache.org
>>>
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 14:15, Justin Edelson wrote:

> On Feb 12, 2010, at 8:51 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> 
>> 
>> On 12 Feb 2010, at 13:37, Carsten Ziegeler wrote:
>> 
>>> Ian Boston wrote:
>>>> eg
>>>> testModifyAceForUser
>>>> (org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>>>>  Time elapsed: 0.151 sec  <<< FAILURE!
>>>> junit.framework.AssertionFailedError: expected:<200> but was:<500>
>>>> 
>>>> I cant find any matching exception in the error log at the moment.
>>> Yepp, I couldn't find it either :(
>>> 
>>> Can you please try to remove the if statement around the
>>> workInterfaces.add(...) inside the addInterfaces method in the
>>> SessionProxyHandler?
>> 
>> 
>> Will do,
>> Jut put it through the debuger and the reflection in
>> AccessControlUtil is finding that there is no method
>> getAccessControlManager on the session, because thats part of
>> JackrabbitSession not Session (I guess).
> 
> Yeah, but that should be a proxied interface. I'll watch for your
> commit.

Just pushed a commit, but I notice from your comments on code review that there may be further problems with the reverts I did to jcr.api

Ian


> 
> Justin
>> 
>> Ian
>> 
>>> 
>>> Regards
>>> Carsten
>>> --
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On Feb 12, 2010, at 8:51 AM, Ian Boston <ie...@tfd.co.uk> wrote:

>
> On 12 Feb 2010, at 13:37, Carsten Ziegeler wrote:
>
>> Ian Boston wrote:
>>> eg
>>> testModifyAceForUser
>>> (org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
>>>   Time elapsed: 0.151 sec  <<< FAILURE!
>>> junit.framework.AssertionFailedError: expected:<200> but was:<500>
>>>
>>> I cant find any matching exception in the error log at the moment.
>> Yepp, I couldn't find it either :(
>>
>> Can you please try to remove the if statement around the
>> workInterfaces.add(...) inside the addInterfaces method in the
>> SessionProxyHandler?
>
>
> Will do,
> Jut put it through the debuger and the reflection in
> AccessControlUtil is finding that there is no method
> getAccessControlManager on the session, because thats part of
> JackrabbitSession not Session (I guess).

Yeah, but that should be a proxied interface. I'll watch for your
commit.

Justin
>
> Ian
>
>>
>> Regards
>> Carsten
>> --
>> Carsten Ziegeler
>> cziegeler@apache.org
>

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 13:37, Carsten Ziegeler wrote:

> Ian Boston wrote:
>> eg 
>> testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)  Time elapsed: 0.151 sec  <<< FAILURE!
>> junit.framework.AssertionFailedError: expected:<200> but was:<500>
>> 
>> I cant find any matching exception in the error log at the moment.
> Yepp, I couldn't find it either :(
> 
> Can you please try to remove the if statement around the
> workInterfaces.add(...) inside the addInterfaces method in the
> SessionProxyHandler?


Will do,
Jut put it through the debuger and the reflection in AccessControlUtil is finding that there is no method getAccessControlManager on the session, because thats part of JackrabbitSession not Session (I guess).

Ian

> 
> Regards
> Carsten
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

Ian Boston wrote:
> eg 
> testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)  Time elapsed: 0.151 sec  <<< FAILURE!
> junit.framework.AssertionFailedError: expected:<200> but was:<500>
> 
> I cant find any matching exception in the error log at the moment.
Yepp, I couldn't find it either :(

Can you please try to remove the if statement around the
workInterfaces.add(...) inside the addInterfaces method in the
SessionProxyHandler?

Regards
Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 13:06, Carsten Ziegeler wrote:

> Ian Boston wrote:
>> On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:
>> 
>>>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
>>> I think this one should go into the release as this fixes a broken
>>> implementation. There is currently a line in there which excludes the
>>> pre jsr 283 interfaces. So we should remove this line.
>> 
>> Done this, you might want to have a quick look at the patch [1] that makes this change, however, the Ace integration tests are failing, same as trunk head at 908994.
>> 
> Looks good to me; let's give it a go and then see what we need to fix
> for the tests.

I am getting failures on 
Failed tests: 
  testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
  testModifyAceForGroup(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)
  testRemoveAce(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)
  testRemoveAces(org.apache.sling.launchpad.webapp.integrationtest.accessManager.RemoveAcesTest)


eg 
testModifyAceForUser(org.apache.sling.launchpad.webapp.integrationtest.accessManager.ModifyAceTest)  Time elapsed: 0.151 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<200> but was:<500>

I cant find any matching exception in the error log at the moment.

Ian


> 
> Carsten
> 
>> 
>> 1. http://codereview.appspot.com/206082/diff2/1:28/1005
>> 
>>> Regards
>>> Carsten
>>> 
>>> -- 
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>> 
>> 
> 
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

Ian Boston wrote:
> On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:
> 
>>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
>> I think this one should go into the release as this fixes a broken
>> implementation. There is currently a line in there which excludes the
>> pre jsr 283 interfaces. So we should remove this line.
> 
> Done this, you might want to have a quick look at the patch [1] that makes this change, however, the Ace integration tests are failing, same as trunk head at 908994.
> 
Looks good to me; let's give it a go and then see what we need to fix
for the tests.

Carsten

> 
> 1. http://codereview.appspot.com/206082/diff2/1:28/1005
> 
>> Regards
>> Carsten
>>
>> -- 
>> Carsten Ziegeler
>> cziegeler@apache.org
> 
> 


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:

>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
> 
> I think this one should go into the release as this fixes a broken
> implementation. There is currently a line in there which excludes the
> pre jsr 283 interfaces. So we should remove this line.

Done this, you might want to have a quick look at the patch [1] that makes this change, however, the Ace integration tests are failing, same as trunk head at 908994.


1. http://codereview.appspot.com/206082/diff2/1:28/1005

> 
> Regards
> Carsten
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 12:03, Carsten Ziegeler wrote:

> Ian Boston wrote
>> 
>> I have done a patch[1] that reverts the changes and makes a release of all of the above possible. It builds, release:prepare works (but needs base to be released first so that server can bind to 2.0.6) and I have tested it in our custom environment with all our Ruby based integration tests. I might have missed something but there were no errors.
>> 
>> I would propose that we apply the patch (might be best if I do this from git->svn so deletes/moves/renames behave), 
>> release the items (Carsten?, just because your keys are better signed than mine)
>> and if it looks like the vote is going to pass (ie 3 committers have done validation) 
>> then revert the patch (I can do this git->svn) to allow JR2 move to happen.
>> 
> Sounds good to me with some additional comments :)
> 
> - I can do the releases
> - We have to properly reschedule the jira issues (volunteer?)

I can do that, I dont think there are too many version changes.

> 
> And
>> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
> 
> I think this one should go into the release as this fixes a broken
> implementation. There is currently a line in there which excludes the
> pre jsr 283 interfaces. So we should remove this line.

I'll have a look in a moment and update my patch.

Ian



> 
> Regards
> Carsten
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

Ian Boston wrote
> 
> I have done a patch[1] that reverts the changes and makes a release of all of the above possible. It builds, release:prepare works (but needs base to be released first so that server can bind to 2.0.6) and I have tested it in our custom environment with all our Ruby based integration tests. I might have missed something but there were no errors.
> 
> I would propose that we apply the patch (might be best if I do this from git->svn so deletes/moves/renames behave), 
> release the items (Carsten?, just because your keys are better signed than mine)
> and if it looks like the vote is going to pass (ie 3 committers have done validation) 
> then revert the patch (I can do this git->svn) to allow JR2 move to happen.
> 
Sounds good to me with some additional comments :)

- I can do the releases
- We have to properly reschedule the jira issues (volunteer?)

And
> 908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.

I think this one should go into the release as this fixes a broken
implementation. There is currently a line in there which excludes the
pre jsr 283 interfaces. So we should remove this line.

Regards
Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

Many thanks Ian!

As soon as I have a confirmation of the fix of the impersonate bug, I'll
create the releases.

Regards
Carsten

Ian Boston wrote:
> On 12 Feb 2010, at 15:38, Justin Edelson wrote:
> 
>> On 2/12/10 10:35 AM, Carsten Ziegeler wrote:
>>> What about the user manager? Should we release it as well?
>>>
>>> Regards
>>> Carsten
>>>
>> Probably. I see I missed that from the list I posted earlier in this thread.
>>
>> Justin
> 
> I've been through the releases for
> 
> jcr WebDav 2.0.8 [1]
> jcr contentloader 2.0.6 [2]
> jcr usermanager 2.0.4 [3]
> jcr server 2.0.6 [4]
> jcr accessmanager 2.0.4 [5]
> jcr base 2.0.6 [6]
> 
> AFAICT, there are no unresolved issues and no issues in there that are not in the release.
> 
> Ian
> 
> 
> 
> 1. https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314433
> 2 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314111
> 3 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314021
> 4 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314067
> 5 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314114
> 6 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314521


-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 15:38, Justin Edelson wrote:

> On 2/12/10 10:35 AM, Carsten Ziegeler wrote:
>> What about the user manager? Should we release it as well?
>> 
>> Regards
>> Carsten
>> 
> Probably. I see I missed that from the list I posted earlier in this thread.
> 
> Justin

I've been through the releases for

jcr WebDav 2.0.8 [1]
jcr contentloader 2.0.6 [2]
jcr usermanager 2.0.4 [3]
jcr server 2.0.6 [4]
jcr accessmanager 2.0.4 [5]
jcr base 2.0.6 [6]

AFAICT, there are no unresolved issues and no issues in there that are not in the release.

Ian



1. https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314433
2 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314111
3 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314021
4 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314067
5 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314114
6 https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&pid=12310710&fixfor=12314521

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Justin Edelson <ju...@gmail.com>.

On 2/12/10 10:35 AM, Carsten Ziegeler wrote:
> What about the user manager? Should we release it as well?
> 
> Regards
> Carsten
> 
Probably. I see I missed that from the list I posted earlier in this thread.

Justin

Re: Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Carsten Ziegeler <cz...@apache.org>.

What about the user manager? Should we release it as well?

Regards
Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Proposal to release jcr bundles before moving to JR2, was Re: svn commit: r908956 - in /sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base: AbstractSlingRepository.java SessionProxyHandler.java

Posted by Ian Boston <ie...@tfd.co.uk>.

On 12 Feb 2010, at 09:02, Ian Boston wrote:

> 
> On 11 Feb 2010, at 20:49, Justin Edelson wrote:
> 
>> On 2/11/10 3:12 PM, Ian Boston wrote:
>>>> I think we should just update to jcr 2.
>>> 
>>> Ok, I have two questions.
>>> How much work is this inside Sling (I am assuming the branch for 2 is there already) and ?
>> 
>> Felix created a branch in SVN a while back. I have a Jackrabbit 2 branch
>> at github (http://github.com/justinedelson/sling/tree/jr2) which is
>> pretty up-to-date. Latest code review in
>> http://codereview.appspot.com/207067
>> 
>>> What will it impact in terms of bundles, just the server bundle or are there changes all over the code base ?
>> 
>> Looks like:
>> jcr.base
>> jcr.contentloader
>> jcr.jackrabbit-accessmanager
>> jcr.jackrabbit-server
>> jcr.webdav
> 
> Assuming that the concepts in ACL dont change, then being totally selfish for a moment, 
> I will have to port local modifications to jackrabbit-server and contentloader. 
> This isnt a big problem, and certainly should not stop this happening, but it does raise another issue.
> 
> AFAICT, there are a number of bundles that haven't been released, and perhaps before moving to JR2 we should consider doing releases of the above set so those who cant upgrade for whatever reason has a release they can bind do.
> 
> Last releases
> jcr.base 2.0.4 14 May 2009
> 2.1.0 has one open issue (SLING-1366), in reality this should be 2.0.5 is 1366 is reverted.
> depends on 
>      jcr.api 2.0.7-SNAPSHOT, would need SLING-1363 reverting to release as 2.0.8
> 
> 
> jcr.contentloader 2.0.4 14 May 2009
> 2.0.6 has no open issues
> 2.0.8 no issues (at all)
>   no SNAPSHOT dependencies
> 
> jcr.accessmanager 2.0.2 14 May 2009 
> next release 2.0.4
> no open issues 
>    no SNAPSHOT dependencies
> 
> jcr.server 14 May 2009  
> next release 2.0.6 
> 2 open issues (look like they can be closed)
>    depends on jcr.base 2.0.5-SNAPSHOT (which would be released)
>    no other SNAPSHOT dependencies
> 
> jcr.webdav 2.0.6 14 May 2009   
> next release 2.0.8 
> no open issues
>   no SNAPSHOT dependencies
> 
> 
> ---------------------------------
> If we reverted jcr.base back to the 2.0.6 target (temporarily reverting SLING-1363 and related), adjusted server, then we could release all of the above, giving the community a release to bind to before this change. I don't think its a massive amount of work and I would be willing to do it, or if someone else has a process setup they might be quicker.
> 
> WDYT?

I have done a patch[1] that reverts the changes and makes a release of all of the above possible. It builds, release:prepare works (but needs base to be released first so that server can bind to 2.0.6) and I have tested it in our custom environment with all our Ruby based integration tests. I might have missed something but there were no errors.

I would propose that we apply the patch (might be best if I do this from git->svn so deletes/moves/renames behave), 
release the items (Carsten?, just because your keys are better signed than mine)
and if it looks like the vote is going to pass (ie 3 committers have done validation) 
then revert the patch (I can do this git->svn) to allow JR2 move to happen.

?

Before any of that, someone else might like to check that the patch is good and I haven't missed anything. I reverted the following commits 
908956 SLING-1366 : Use dynamic proxy to handle session#impersonate call.
908888 SLING-1363 - We added a new interface so we should increase the minor versio ....
908799 correcting jcr.api package version
908798 SLING-1363 - removing SessionConfigurer interface and moving NamespaceMapper from base to api. Cr...
908780 fixing SLING-1367
908531 SLING-1366 : Readd call to NamespaceMapper
908440 SLING-1363 Must start the session configurer tracker before starting the repository because starting ....
908232 SLING-1363 - adding SessionConfigurer interface

in that sequence.

Ian

1. http://codereview.appspot.com/206082

> 
> The problem with using x.x.x-SNAPSHOTS is that we are giving the community an indication of intent to release x.x.x before going to x.x.x+1
> 
> Ian
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>> 
>> I think the contentloader change is optional.
>> 
>> Justin
>> 
>>> 
>>> Ian
>> 
>> 
>> 
>>> 
>>>> 
>>>> Carsten
>>>> 
>>>> -- 
>>>> Carsten Ziegeler
>>>> cziegeler@apache.org
>>> 
>> 
>