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 <Fe...@day.com> on 2007/11/29 15:10:51 UTC

Re: svn commit: r599443 - in /incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling: scripting/ servlet/ slingservlets/

Hi Bertrand,

Real Sling also has the session as a - currently undocumented - request
attribute. While you use the Session's class name I use some sling
specific constant. Maybe using the Session's class name is better than
some "obscure" constant like sling.core.session.

While it is probably useful in certain situations to have the session
available, I am not so sure, whether this should be documented and
become part of the API or whether we should just have it. OTOH I am not
a fan of these hidden features, you know are there, but seem to be part
of an expected behaviour.

What do you think ?

Regards
Felix

Am Donnerstag, den 29.11.2007, 13:58 +0000 schrieb
bdelacretaz@apache.org:
> Author: bdelacretaz
> Date: Thu Nov 29 05:58:01 2007
> New Revision: 599443
> 
> URL: http://svn.apache.org/viewvc?rev=599443&view=rev
> Log:
> SLING-109, adaptations for Resource API changes. Also made JCR Session available in request attributes, for easier access
> 
> Modified:
>     incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/scripting/MicroslingScriptResolver.java
>     incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/servlet/MicroslingMainServlet.java
>     incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxGetServlet.java
>     incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxPostServlet.java
> 
> Modified: incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/scripting/MicroslingScriptResolver.java
> URL: http://svn.apache.org/viewvc/incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/scripting/MicroslingScriptResolver.java?rev=599443&r1=599442&r2=599443&view=diff
> ==============================================================================
> --- incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/scripting/MicroslingScriptResolver.java (original)
> +++ incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/scripting/MicroslingScriptResolver.java Thu Nov 29 05:58:01 2007
> @@ -160,19 +160,9 @@
>      public SlingScript resolveScriptInternal(
>              final SlingHttpServletRequest request) throws RepositoryException, SlingException {
>  
> -        final Resource r = request.getResource();
> -
> -        // ensure repository access
> -        if (!(r instanceof NodeProvider)) {
> -            return null;
> -        }
> -
> -        final Session s = ((NodeProvider) r).getNode().getSession();
> +        final Resource r = request.getResource(); 
> +        final Session s = (Session)request.getAttribute(Session.class.getName());
>          MicroslingScript result = null;
> -
> -        if (r == null) {
> -            return null;
> -        }
>  
>          String scriptFilename = scriptFilenameBuilder.buildScriptFilename(
>              request.getMethod(),
> 
> Modified: incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/servlet/MicroslingMainServlet.java
> URL: http://svn.apache.org/viewvc/incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/servlet/MicroslingMainServlet.java?rev=599443&r1=599442&r2=599443&view=diff
> ==============================================================================
> --- incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/servlet/MicroslingMainServlet.java (original)
> +++ incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/servlet/MicroslingMainServlet.java Thu Nov 29 05:58:01 2007
> @@ -167,9 +167,11 @@
>              return;
>          }
>  
> +        // get a Session and make it available as a Request attribute
>          Session session = authenticate(req);
> +        req.setAttribute(Session.class.getName(), session);
> +        
>          try {
> -
>              MicroslingSlingHttpServletRequest request = new MicroslingSlingHttpServletRequest(
>                  hReq, session, serviceLocator);
>              MicroslingSlingHttpServletResponse response = new MicroslingSlingHttpServletResponse(
> 
> Modified: incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxGetServlet.java
> URL: http://svn.apache.org/viewvc/incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxGetServlet.java?rev=599443&r1=599442&r2=599443&view=diff
> ==============================================================================
> --- incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxGetServlet.java (original)
> +++ incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxGetServlet.java Thu Nov 29 05:58:01 2007
> @@ -87,18 +87,11 @@
>      protected Map<String, Object> getSessionInfo(SlingHttpServletRequest request)
>      throws RepositoryException, HttpStatusCodeException, SlingException {
>          final Map<String, Object> result = new HashMap<String, Object>();
> -
> -        // TODO not very convenient way to get a Session...
> -        final Resource root = request.getResourceResolver().getResource("/");
> -        final Node rootNode = ((NodeProvider)root).getNode();
> -        if(rootNode == null) {
> -            throw new HttpStatusCodeException(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,"Root Node not found");
> -        }
> -        final Session s = rootNode.getSession();
> -
> +        
> +        final Session s = (Session)request.getAttribute(Session.class.getName());
>          result.put("workspace",s.getWorkspace().getName());
>          result.put("userID",s.getUserID());
> -
> +        
>          return result;
>      }
>  
> 
> Modified: incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxPostServlet.java
> URL: http://svn.apache.org/viewvc/incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxPostServlet.java?rev=599443&r1=599442&r2=599443&view=diff
> ==============================================================================
> --- incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxPostServlet.java (original)
> +++ incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling/slingservlets/MicrojaxPostServlet.java Thu Nov 29 05:58:01 2007
> @@ -102,13 +102,7 @@
>                  s = currentNode.getSession();
>              } else {
>                  currentPath = SlingRequestPaths.getPathInfo(request);
> -                // TODO not very convenient way to get a Session...
> -                final Resource root = request.getResourceResolver().getResource("/");
> -                final Node rootNode = ((NodeProvider)root).getNode();
> -                if(rootNode == null) {
> -                    throw new HttpStatusCodeException(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,"Root Node not found");
> -                }
> -                s = rootNode.getSession();
> +                s = (Session)request.getAttribute(Session.class.getName());
>              }
>  
>              final String [] pathsToDelete = request.getParameterValues(RP_DELETE_PATH);
> 
> 


Re: svn commit: r599443 - in /incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling: scripting/ servlet/ slingservlets/

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

Am Donnerstag, den 29.11.2007, 22:22 +0100 schrieb Bertrand Delacretaz:
> On Nov 29, 2007 8:31 PM, Felix Meschberger <fm...@gmail.com> wrote:
> 
> > > ...we could either add a
> > > getJcrSession() method to SlingHttpServletRequest, or at least make
> > > the attribute name a constant to expose its use....
> >
> 
> > ...would define a attribute name as a constant in Sling and microsling and
> > document as part of the implementation documentation....
> 
> Sounds good to me, especially using the Session class name as the
> attribute name.

Ok. I will use the Session class name, too.

Regards
Felix


Re: svn commit: r599443 - in /incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling: scripting/ servlet/ slingservlets/

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Nov 29, 2007 8:31 PM, Felix Meschberger <fm...@gmail.com> wrote:

> > ...we could either add a
> > getJcrSession() method to SlingHttpServletRequest, or at least make
> > the attribute name a constant to expose its use....
>

> ...would define a attribute name as a constant in Sling and microsling and
> document as part of the implementation documentation....

Sounds good to me, especially using the Session class name as the
attribute name.

-Bertrand

Re: svn commit: r599443 - in /incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling: scripting/ servlet/ slingservlets/

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

Am Donnerstag, den 29.11.2007, 15:37 +0100 schrieb Bertrand Delacretaz:
> On Nov 29, 2007 3:10 PM, Felix Meschberger <Fe...@day.com> wrote:
> 
> > ...While it is probably useful in certain situations to have the session
> > available, I am not so sure, whether this should be documented and
> > become part of the API or whether we should just have it. ...
> 
> For microsling the JCR Session is required in several places, and
> before this change it was acquired in an ugly way.

We will definitely have the same issue in Sling as soon as we merge the
super-duper default servlet from microsling into Sling.

> If it is needed in Sling as well, we could either add a
> getJcrSession() method to SlingHttpServletRequest, or at least make
> the attribute name a constant to expose its use.

I am somewhat reluctant to creating such a method in the API because
IMHO the main entry point to Resources is the ResourceResolver. Rather I
would define a attribute name as a constant in Sling and microsling and
document as part of the implementation documentation. The attribute
should not be part of the Sling API itself.

Regards
Felix


Re: svn commit: r599443 - in /incubator/sling/trunk/microsling/microsling-core/src/main/java/org/apache/sling/microsling: scripting/ servlet/ slingservlets/

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Nov 29, 2007 3:10 PM, Felix Meschberger <Fe...@day.com> wrote:

> ...While it is probably useful in certain situations to have the session
> available, I am not so sure, whether this should be documented and
> become part of the API or whether we should just have it. ...

For microsling the JCR Session is required in several places, and
before this change it was acquired in an ugly way.

If it is needed in Sling as well, we could either add a
getJcrSession() method to SlingHttpServletRequest, or at least make
the attribute name a constant to expose its use.

>.... OTOH I am not
> a fan of these hidden features, you know are there, but seem to be part
> of an expected behaviour....

Agreed, so documenting the possible Request attributes might be the
best way to solve this.

-Bertrand