On 6/2/10 7:42 AM, Felix Meschberger wrote: > Hi, > > On 01.06.2010 17:13, [email protected] 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
