You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2010/06/02 13:42:59 UTC
Re: svn commit: r950106 - in /sling/trunk/bundles: api/src/main/java/org/apache/sling/api/resource/
commons/auth/src/main/java/org/apache/sling/commons/auth/impl/ jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/
jcr/resource/src/test/java...
Hi,
On 01.06.2010 17:13, justin@apache.org wrote:
> Author: justin
> Date: Tue Jun 1 15:13:10 2010
> New Revision: 950106
>
> URL: http://svn.apache.org/viewvc?rev=950106&view=rev
> Log:
> SLING-1541 - adding getUserID() method to ResourceResolver
>
> Modified:
> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java
> sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java
> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java
> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/ResourceProviderEntryTest.java
> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/MockResourceResolver.java
>
> Modified: sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java
> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java?rev=950106&r1=950105&r2=950106&view=diff
> ==============================================================================
> --- sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java (original)
> +++ sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java Tue Jun 1 15:13:10 2010
> @@ -357,4 +357,14 @@ public interface ResourceResolver extend
> * @since 2.1
> */
> void close();
> +
> +
> + /**
> + * Get the user ID, if any, associated with this resource resolver.
> + * The meaning of this identifier is an implementation detail defined
> + * by the underlying repository.
Yes, its an implementation detail. But why repository ? Is this
copy/paste from the JCR spec ? I would say, the userID is derived from
the "credentials" passed into the
ResourceResolverFactory.getResourceResolver method to create the
ResourceResolver.
> This method may return null.
While generally the return value will probably never be null, just
documenting that it may be null, makes (theoretically) live more
complicated.
Why not guarnateeing that the user ID is never null ?
> + *
> + * @return the user ID
> + */
> + String getUserID();
> }
>
> Modified: sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java
> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java?rev=950106&r1=950105&r2=950106&view=diff
> ==============================================================================
> --- sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java (original)
> +++ sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java Tue Jun 1 15:13:10 2010
> @@ -820,14 +820,13 @@ public class SlingAuthenticator implemen
>
> // JCR session for backwards compatibility
> + Session session = resolver.adaptTo(Session.class);
> request.setAttribute(REQUEST_ATTRIBUTE_SESSION, session);
We need this for backwards compatibility, right. But how about setting
this backwards compat stuff in the SlingMainServlet ?
Regards
Felix
Re: svn commit: r950106 - in /sling/trunk/bundles: api/src/main/java/org/apache/sling/api/resource/
commons/auth/src/main/java/org/apache/sling/commons/auth/impl/ jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/
jcr/resource/src/test/java...
Posted by Justin Edelson <ju...@gmail.com>.
On 6/2/10 7:42 AM, Felix Meschberger wrote:
> Hi,
>
> On 01.06.2010 17:13, justin@apache.org wrote:
>> Author: justin
>> Date: Tue Jun 1 15:13:10 2010
>> New Revision: 950106
>>
>> URL: http://svn.apache.org/viewvc?rev=950106&view=rev
>> Log:
>> SLING-1541 - adding getUserID() method to ResourceResolver
>>
>> Modified:
>> sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java
>> sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java
>> sling/trunk/bundles/jcr/resource/src/main/java/org/apache/sling/jcr/resource/internal/JcrResourceResolver.java
>> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/ResourceProviderEntryTest.java
>> sling/trunk/bundles/jcr/resource/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/MockResourceResolver.java
>>
>> Modified: sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java
>> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java?rev=950106&r1=950105&r2=950106&view=diff
>> ==============================================================================
>> --- sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java (original)
>> +++ sling/trunk/bundles/api/src/main/java/org/apache/sling/api/resource/ResourceResolver.java Tue Jun 1 15:13:10 2010
>> @@ -357,4 +357,14 @@ public interface ResourceResolver extend
>> * @since 2.1
>> */
>> void close();
>> +
>> +
>> + /**
>> + * Get the user ID, if any, associated with this resource resolver.
>> + * The meaning of this identifier is an implementation detail defined
>> + * by the underlying repository.
>
> Yes, its an implementation detail. But why repository ? Is this
> copy/paste from the JCR spec ?
Not copy/paste, but obviously inspired by it in as much as JCR doesn't
define what this method should return.
Would it be better to say "...defined by the ResourceResolverFactory
implementation or the underlying repository"? The point I was trying to
express in the JavaDoc is that Sling doesn't define the semantics of
this value.
> I would say, the userID is derived from
> the "credentials" passed into the
> ResourceResolverFactory.getResourceResolver method to create the
> ResourceResolver.
Depends upon what you mean by "derived." ResourceResolverFactory and
repository implementations should be free to use as much or as little of
the authentication info map as necessary. When using something like
OpenID or SAML, the value of ResourceResolver.getUserID() should be very
different than anything in the authentication info map.
>
>> This method may return null.
>
> While generally the return value will probably never be null, just
> documenting that it may be null, makes (theoretically) live more
> complicated.
>
> Why not guarnateeing that the user ID is never null ?
Happy to do so. What do you think JcrResourceResolver.getUserID() should
return when Session.getUserID() returns null?
Justin
>
>> + *
>> + * @return the user ID
>> + */
>> + String getUserID();
>> }
>>
>> Modified: sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java
>> URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java?rev=950106&r1=950105&r2=950106&view=diff
>> ==============================================================================
>> --- sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java (original)
>> +++ sling/trunk/bundles/commons/auth/src/main/java/org/apache/sling/commons/auth/impl/SlingAuthenticator.java Tue Jun 1 15:13:10 2010
>> @@ -820,14 +820,13 @@ public class SlingAuthenticator implemen
>>
>> // JCR session for backwards compatibility
>> + Session session = resolver.adaptTo(Session.class);
>> request.setAttribute(REQUEST_ATTRIBUTE_SESSION, session);
>
> We need this for backwards compatibility, right. But how about setting
> this backwards compat stuff in the SlingMainServlet ?
This should probably be done as part of the merging of commons.auth and
engine. Is that still going to happen?
>
> Regards
> Felix