Return-Path: Delivered-To: apmail-incubator-sling-commits-archive@locus.apache.org Received: (qmail 39177 invoked from network); 5 Feb 2008 16:44:09 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 5 Feb 2008 16:44:09 -0000 Received: (qmail 56691 invoked by uid 500); 5 Feb 2008 16:44:01 -0000 Delivered-To: apmail-incubator-sling-commits-archive@incubator.apache.org Received: (qmail 56666 invoked by uid 500); 5 Feb 2008 16:44:01 -0000 Mailing-List: contact sling-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: sling-dev@incubator.apache.org Delivered-To: mailing list sling-commits@incubator.apache.org Received: (qmail 56657 invoked by uid 99); 5 Feb 2008 16:44:01 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Feb 2008 08:44:01 -0800 X-ASF-Spam-Status: No, hits=-100.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Feb 2008 16:43:40 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 1401C1A9832; Tue, 5 Feb 2008 08:43:47 -0800 (PST) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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... Date: Tue, 05 Feb 2008 16:43:40 -0000 To: sling-commits@incubator.apache.org From: bdelacretaz@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20080205164348.1401C1A9832@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 ServletException * 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 toDelete = new HashSet(); @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("

" + testTextB + "

")); assertTrue("Include has been used",content.contains("

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("

" + testTextB + "

")); + assertTrue("Include has been used",content.contains("

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("

" + testTextB + "

")); + assertTrue("Include has been used",content.contains("

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 --%> +

+ Forced resource type:<%= resource.node.text %>

. +
\ 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 @@

ESP template

<%= resource.node.text %>

+

Test 1

<% if(resource.node.pathToInclude) { %>

pathToInclude = <%= resource.node.pathToInclude %>

Including <%= resource.node.pathToInclude %>

<% - sling.include(resource.node.pathToInclude + ".html"); + sling.include(resource.node.pathToInclude); } %> +

Test 2

<% if(resource.node.testInfiniteLoop) { %>

testInfiniteLoop = <%= resource.node.testInfiniteLoop %>

<% // try to include the item itself, to cause an infinite loop - sling.include(resource.getPath() + ".html"); + sling.include(resource.getPath()); + } + %> + +

Test 3

+ <% + if(resource.node.pathToInclude && resource.node.forceResourceType) { + %> +

pathToInclude = <%= resource.node.pathToInclude %>

+

Including <%= resource.node.pathToInclude %>

+ <% + sling.include(resource.node.pathToInclude, resource.node.forceResourceType); } %> 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 IOException thrown - * while handling the include. - * @throws SlingServletException Wrapping a ServletException - * 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) {