You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2008/02/05 17:43:40 UTC

svn commit: r618697 - in /incubator/sling/trunk: api/src/main/java/org/apache/sling/api/scripting/ launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/ launchpad/launchpad-webapp/src/test/resources/integration-tes...

Author: bdelacretaz
Date: Tue Feb  5 08:43:35 2008
New Revision: 618697

URL: http://svn.apache.org/viewvc?rev=618697&view=rev
Log:
SLING-221 and SLING-222, fix sling.include with forced resource type and extensions in included paths

Added:
    incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-forced.esp
    incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java   (with props)
    incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java   (with props)
Modified:
    incubator/sling/trunk/api/src/main/java/org/apache/sling/api/scripting/SlingScriptHelper.java
    incubator/sling/trunk/launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/IncludeTest.java
    incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-test.esp
    incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/ScriptHelper.java
    incubator/sling/trunk/sling/core/src/main/java/org/apache/sling/core/impl/request/SlingRequestDispatcher.java

Modified: incubator/sling/trunk/api/src/main/java/org/apache/sling/api/scripting/SlingScriptHelper.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/api/src/main/java/org/apache/sling/api/scripting/SlingScriptHelper.java?rev=618697&r1=618696&r2=618697&view=diff
==============================================================================
--- incubator/sling/trunk/api/src/main/java/org/apache/sling/api/scripting/SlingScriptHelper.java (original)
+++ incubator/sling/trunk/api/src/main/java/org/apache/sling/api/scripting/SlingScriptHelper.java Tue Feb  5 08:43:35 2008
@@ -80,6 +80,6 @@
      * @throws SlingServletException Wrapping a <code>ServletException</code>
      *             thrown while handling the include.
      */
-    void include(String path, RequestDispatcherOptions options);
+    void include(String path, String requestDispatcherOptions);
 
 }

Modified: incubator/sling/trunk/launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/IncludeTest.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/IncludeTest.java?rev=618697&r1=618696&r2=618697&view=diff
==============================================================================
--- incubator/sling/trunk/launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/IncludeTest.java (original)
+++ incubator/sling/trunk/launchpad/launchpad-webapp/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/IncludeTest.java Tue Feb  5 08:43:35 2008
@@ -16,15 +16,15 @@
  */
 package org.apache.sling.launchpad.webapp.integrationtest;
 
-import org.apache.commons.httpclient.methods.GetMethod;
-import org.apache.sling.ujax.UjaxPostServlet;
-
 import java.io.IOException;
 import java.net.URL;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
-import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.sling.ujax.UjaxPostServlet;
 
 /** Test the {link ScriptHelper#include) functionality */
  public class IncludeTest extends HttpTestBase {
@@ -34,8 +34,10 @@
     private String nodeUrlB;
     private String testTextB;
     private String nodeUrlC;
+    private String nodeUrlD;
+    private String nodeUrlE;
     private String scriptPath;
-    private String toDelete;
+    private Set<String> toDelete = new HashSet<String>();
     
     @Override
     protected void setUp() throws Exception {
@@ -58,21 +60,39 @@
         props.put("pathToInclude", new URL(nodeUrlA).getPath());
         nodeUrlB = testClient.createNode(url, props);
         
+        // Node E is like B but with an extension on the include path
+        props.put("pathToInclude", new URL(nodeUrlA).getPath() + ".html");
+        nodeUrlE = testClient.createNode(url, props);
+        
         // Node C is used for the infinite loop detection test
         props.remove("pathToInclude");
         props.put("testInfiniteLoop","true");
         nodeUrlC = testClient.createNode(url, props);
 
-        // the rendering script goes under /apps in the repository
+        // Node D is used for the "force resource type" test
+        final String forcedResourceType = getClass().getSimpleName() + "/" + System.currentTimeMillis();
+        props.remove("testInfiniteLoop");
+        props.put("forceResourceType", forcedResourceType);
+        props.put("pathToInclude", new URL(nodeUrlA).getPath());
+        nodeUrlD = testClient.createNode(url, props);
+        
+        // Script for forced resource type
+        scriptPath = "/apps/" + forcedResourceType;
+        testClient.mkdirs(WEBDAV_BASE_URL, scriptPath);
+        toDelete.add(uploadTestScript(scriptPath,"include-forced.esp","html.esp"));
+        
+        // The main rendering script goes under /apps in the repository
         scriptPath = "/apps/nt/unstructured";
         testClient.mkdirs(WEBDAV_BASE_URL, scriptPath);
-        toDelete = uploadTestScript(scriptPath,"include-test.esp","html.esp");
+        toDelete.add(uploadTestScript(scriptPath,"include-test.esp","html.esp"));
     }
     
     @Override
     protected void tearDown() throws Exception {
         super.tearDown();
-        testClient.delete(toDelete);
+        for(String script : toDelete) {
+            testClient.delete(script);
+        }
     }
 
     public void testWithoutInclude() throws IOException {
@@ -87,18 +107,36 @@
         assertTrue("Content includes ESP marker",content.contains("ESP template"));
         assertTrue("Content contains formatted test text",content.contains("<p class=\"main\">" + testTextB + "</p>"));
         assertTrue("Include has been used",content.contains("<p>Including"));
+        assertTrue("Text of node A is included (" + content + ")",content.contains(testTextA));
+    }
+    
+    public void testWithIncludeAndExtension() throws IOException {
+        final String content = getContent(nodeUrlE + ".html", CONTENT_TYPE_HTML);
+        assertTrue("Content includes ESP marker",content.contains("ESP template"));
+        assertTrue("Content contains formatted test text",content.contains("<p class=\"main\">" + testTextB + "</p>"));
+        assertTrue("Include has been used",content.contains("<p>Including"));
+        assertTrue("Text of node A is included (" + content + ")",content.contains(testTextA));
     }
     
     public void testInfiniteLoopDetection() throws IOException {
         // Node C has a property that causes an infinite include loop,
         // Sling must return an error 500 in this case
         final GetMethod get = new GetMethod(nodeUrlC + ".html");
-        final int status = httpClient.executeMethod(get);
+        httpClient.executeMethod(get);
         final String content = get.getResponseBodyAsString();
         assertTrue("Response contains infinite loop error message",
                 content.contains("InfiniteIncludeLoopException"));
         
         // TODO_FAILS_ see SLING-207
         // assertEquals("Status is 500 for infinite loop",HttpServletResponse.SC_INTERNAL_SERVER_ERROR,status);
+    }
+    
+    public void testForcedResourceType() throws IOException {
+        final String content = getContent(nodeUrlD + ".html", CONTENT_TYPE_HTML);
+        assertTrue("Content includes ESP marker",content.contains("ESP template"));
+        assertTrue("Content contains formatted test text",content.contains("<p class=\"main\">" + testTextB + "</p>"));
+        assertTrue("Include has been used",content.contains("<p>Including"));
+        assertTrue("Text of node A is included (" + content + ")",content.contains(testTextA));
+        assertTrue("Resource type has been forced (" + content + ")",content.contains("Forced resource type:" + testTextA));
     }
 }

Added: incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-forced.esp
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-forced.esp?rev=618697&view=auto
==============================================================================
--- incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-forced.esp (added)
+++ incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-forced.esp Tue Feb  5 08:43:35 2008
@@ -0,0 +1,4 @@
+<%-- used by IncludeTest --%>
+<div>
+  Forced resource type:<%= resource.node.text %></p>. 
+</div>
\ No newline at end of file

Modified: incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-test.esp
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-test.esp?rev=618697&r1=618696&r2=618697&view=diff
==============================================================================
--- incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-test.esp (original)
+++ incubator/sling/trunk/launchpad/launchpad-webapp/src/test/resources/integration-test/include-test.esp Tue Feb  5 08:43:35 2008
@@ -4,23 +4,36 @@
 		<h1>ESP template</h1>
 		<p class="main"><%= resource.node.text %></p>
 		
+		<h2>Test 1</h2>
 		<%
 			if(resource.node.pathToInclude) {
 			  %>
 			  <p>pathToInclude = <%= resource.node.pathToInclude %></p>
   		  	  <p>Including <%= resource.node.pathToInclude %></p>
 			  <%
-			  sling.include(resource.node.pathToInclude + ".html");
+			  sling.include(resource.node.pathToInclude);
 			}
 		%>
 		
+		<h2>Test 2</h2>
 		<%
 			if(resource.node.testInfiniteLoop) {
 			  %>
 			  <p>testInfiniteLoop = <%= resource.node.testInfiniteLoop %></p>
 			  <%
 			  // try to include the item itself, to cause an infinite loop
-			  sling.include(resource.getPath() + ".html");
+			  sling.include(resource.getPath());
+			}
+		%>
+		
+		<h2>Test 3</h2>
+		<%
+			if(resource.node.pathToInclude && resource.node.forceResourceType) {
+			  %>
+			  <p>pathToInclude = <%= resource.node.pathToInclude %></p>
+  		  	  <p>Including <%= resource.node.pathToInclude %></p>
+			  <%
+			  sling.include(resource.node.pathToInclude, resource.node.forceResourceType);
 			}
 		%>
 	</body>

Added: incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java?rev=618697&view=auto
==============================================================================
--- incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java (added)
+++ incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java Tue Feb  5 08:43:35 2008
@@ -0,0 +1,52 @@
+/*
+ * 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.scripting.resolver;
+
+import java.util.StringTokenizer;
+
+import org.apache.sling.api.request.RequestDispatcherOptions;
+
+/** Parse RequestDispatcherOptions from a human-readable String */
+class RequestDispatcherOptionsParser {
+    RequestDispatcherOptions parse(String str) {
+        if(str==null || str.length() == 0) {
+            return null;
+        }
+        
+        // parse string in the form name:value, name:value
+        // with a shortcut for the forceResourceType option: if the 
+        // string contains no comma or colon, it's the value of that option
+        // (which is the most common use of those options)
+        RequestDispatcherOptions result = new RequestDispatcherOptions();
+        if(str.indexOf(',') < 0 && str.indexOf(':') < 0) {
+            result.put(RequestDispatcherOptions.OPT_FORCE_RESOURCE_TYPE, str.trim());
+        } else {
+            final StringTokenizer tk = new StringTokenizer(str, ",");
+            while(tk.hasMoreTokens()) {
+                final String [] entry = tk.nextToken().split(":");
+                if(entry.length == 2) {
+                    result.put(entry[0].trim(), entry[1].trim());
+                }
+            }
+        }
+        
+        return result;
+    }
+    
+}

Propchange: incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParser.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/ScriptHelper.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/ScriptHelper.java?rev=618697&r1=618696&r2=618697&view=diff
==============================================================================
--- incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/ScriptHelper.java (original)
+++ incubator/sling/trunk/scripting/resolver/src/main/java/org/apache/sling/scripting/resolver/ScriptHelper.java Tue Feb  5 08:43:35 2008
@@ -32,6 +32,8 @@
 import org.apache.sling.api.scripting.SlingScriptHelper;
 import org.apache.sling.scripting.resolver.impl.helper.OnDemandReaderRequest;
 import org.apache.sling.scripting.resolver.impl.helper.OnDemandWriterResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Simple script helper providing access to the (wrapped) response, the
@@ -42,10 +44,10 @@
 public class ScriptHelper implements SlingScriptHelper {
 
     private final SlingScript script;
-
     private final SlingHttpServletRequest request;
-
     private final SlingHttpServletResponse response;
+    
+    private final Logger log = LoggerFactory.getLogger(ScriptHelper.class);
 
     public ScriptHelper(SlingScript script, SlingHttpServletRequest request,
             SlingHttpServletResponse response) {
@@ -76,15 +78,16 @@
         include(path, null);
     }
 
-    /**
-     * @trows SlingIOException Wrapping a <code>IOException</code> thrown
-     *        while handling the include.
-     * @throws SlingServletException Wrapping a <code>ServletException</code>
-     *             thrown while handling the include.
-     */
-    public void include(String path, RequestDispatcherOptions options) {
-        // TODO: Implement for options !!
-        RequestDispatcher dispatcher = getRequest().getRequestDispatcher(path);
+    /** Include the output of another request, using specified options */
+    public void include(String path, String options) {
+        final RequestDispatcherOptionsParser parser = new RequestDispatcherOptionsParser();
+        final RequestDispatcherOptions opt = parser.parse(options); 
+        final RequestDispatcher dispatcher = getRequest().getRequestDispatcher(path);
+        
+        if (opt != null) {
+            getRequest().setAttribute(RequestDispatcherOptions.class.getName(), opt);
+        }
+        
         if (dispatcher != null) {
             try {
                 dispatcher.include(getRequest(), getResponse());

Added: incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java?rev=618697&view=auto
==============================================================================
--- incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java (added)
+++ incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java Tue Feb  5 08:43:35 2008
@@ -0,0 +1,61 @@
+/*
+ * 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.scripting.resolver;
+
+import junit.framework.TestCase;
+
+import org.apache.sling.api.request.RequestDispatcherOptions;
+
+public class RequestDispatcherOptionsParserTest extends TestCase {
+    final RequestDispatcherOptionsParser parser = new RequestDispatcherOptionsParser();
+    
+    public void testNullString() {
+        final RequestDispatcherOptions result = parser.parse(null);
+        assertNull(result);
+    }
+    
+    public void testEmptyString() {
+        final RequestDispatcherOptions result = parser.parse("");
+        assertNull(result);
+    }
+    
+    public void testSingleOption() {
+        final RequestDispatcherOptions result = parser.parse("forceResourceType: widget");
+        assertNotNull(result);
+        assertEquals("Expected option found (" + result + ")",
+                "widget",result.get(RequestDispatcherOptions.OPT_FORCE_RESOURCE_TYPE));
+    }
+    
+    public void testResourceTypeShortcut() {
+        // a single option with no comma or colon means "forceResourceType"
+        final RequestDispatcherOptions result = parser.parse("\t components/widget  ");
+        assertNotNull(result);
+        assertEquals("Expected option found (" + result + ")",
+                "components/widget",result.get(RequestDispatcherOptions.OPT_FORCE_RESOURCE_TYPE));
+    }
+    
+    public void testTwoOptions() {
+        final RequestDispatcherOptions result = parser.parse("forceResourceType: true, replaceSelectors : xyz  ,");
+        assertNotNull(result);
+        assertEquals("Expected option found (" + result + ")",
+                "true",result.get(RequestDispatcherOptions.OPT_FORCE_RESOURCE_TYPE));
+        assertEquals("Expected option found (" + result + ")",
+                "xyz",result.get(RequestDispatcherOptions.OPT_REPLACE_SELECTORS));
+    }
+}

Propchange: incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: incubator/sling/trunk/scripting/resolver/src/test/java/org/apache/sling/scripting/resolver/RequestDispatcherOptionsParserTest.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: incubator/sling/trunk/sling/core/src/main/java/org/apache/sling/core/impl/request/SlingRequestDispatcher.java
URL: http://svn.apache.org/viewvc/incubator/sling/trunk/sling/core/src/main/java/org/apache/sling/core/impl/request/SlingRequestDispatcher.java?rev=618697&r1=618696&r2=618697&view=diff
==============================================================================
--- incubator/sling/trunk/sling/core/src/main/java/org/apache/sling/core/impl/request/SlingRequestDispatcher.java (original)
+++ incubator/sling/trunk/sling/core/src/main/java/org/apache/sling/core/impl/request/SlingRequestDispatcher.java Tue Feb  5 08:43:35 2008
@@ -24,6 +24,7 @@
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
 
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.request.RequestDispatcherOptions;
@@ -55,29 +56,58 @@
         this.path = (resource == null ? "" : resource.getPath());
     }
 
-    public void include(ServletRequest request, ServletResponse response)
+    public void include(ServletRequest request, ServletResponse sResponse)
             throws ServletException, IOException {
 
+        /** TODO: I have made some quick fixes in this method for SLING-221
+         *  and SLING-222, but haven't had time to do a proper review. This
+         *  method might deserve a more extensive rewrite. 
+         */
+        
+        // get options from request attribute if we don't have them yet
+        if(options == null) {
+            options = (RequestDispatcherOptions)request.getAttribute(RequestDispatcherOptions.class.getName());
+        }
+        
         // this may throw an exception in case loading fails, which is
         // ok here, if no content is available at that path null is
         // return, which results in using the servlet container
         SlingHttpServletRequest cRequest = RequestData.unwrap(request);
         RequestData rd = RequestData.getRequestData(cRequest);
         String absPath = getAbsolutePath(cRequest, path);
+        
+        if( ! (sResponse instanceof HttpServletResponse )) {
+            throw new ServletException("Response is not an HttpServletResponse, cannot continue");
+        }
+        final HttpServletResponse response = (HttpServletResponse)sResponse;
 
         if (resource == null) {
             ResourceResolver rr = cRequest.getResourceResolver();
+            
+            if(absPath.startsWith(cRequest.getContextPath())) {
+                absPath = absPath.substring(cRequest.getContextPath().length());
+            }
+            
+            // remove extension before attempting to resolve, like in a "normal" request
+            // TODO use the same parsing as for normal resources?
+            final int lastSlash = absPath.lastIndexOf('/');
+            final int lastDot = absPath.lastIndexOf('.');
+            if(lastDot > lastSlash && lastDot >= 0) {
+                absPath = absPath.substring(0, lastDot);
+            }
             resource = rr.getResource(absPath);
         }
 
         if (resource == null) {
-
-            rd.getSlingMainServlet().includeServlet(request, response, path);
+            response.sendError(HttpServletResponse.SC_NOT_FOUND, "Resource not found in include: " + absPath);
+            
+            // The code below was previously used but causes SLING-222...not sure what's best
+            // rd.getSlingMainServlet().includeServlet(request, response, path);
 
         } else {
 
             // ensure request path info and optional merges
-            SlingRequestPathInfo info = new SlingRequestPathInfo(resource, path);
+            SlingRequestPathInfo info = new SlingRequestPathInfo(resource, absPath);
             info = info.merge(cRequest.getRequestPathInfo());
 
             if (options != null) {