You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2021/01/12 19:35:08 UTC

[sling-org-apache-sling-servlets-post] branch master updated: SLING-10006 Parameter ":sendError" presence should not replace the PostResponse object (#8)

This is an automated email from the ASF dual-hosted git repository.

enorman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-post.git


The following commit(s) were added to refs/heads/master by this push:
     new c820ce9  SLING-10006 Parameter ":sendError" presence should not replace the PostResponse object (#8)
c820ce9 is described below

commit c820ce9c746f7cd15b2a418255d1face91b60975
Author: Eric Norman <en...@apache.org>
AuthorDate: Tue Jan 12 11:35:01 2021 -0800

    SLING-10006 Parameter ":sendError" presence should not replace the PostResponse object (#8)
    
    SLING-10006 Parameter ":sendError" presence should not replace the
    PostResponse object
---
 pom.xml                                            |   2 +-
 .../servlets/post/AbstractPostResponseWrapper.java | 157 +++++++++++++++++++++
 .../impl/ErrorHandlingPostResponseWrapper.java     |  78 ++++++++++
 .../post/impl/PostResponseWithErrorHandling.java   |  10 +-
 .../sling/servlets/post/impl/SlingPostServlet.java |  41 ++++--
 .../apache/sling/servlets/post/package-info.java   |   2 +-
 .../servlets/post/impl/SlingPostServletTest.java   |  78 +++++++++-
 7 files changed, 350 insertions(+), 18 deletions(-)

diff --git a/pom.xml b/pom.xml
index ed237fa..e5085bd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -28,7 +28,7 @@
 
     <artifactId>org.apache.sling.servlets.post</artifactId>
     <packaging>jar</packaging>
-    <version>2.3.37-SNAPSHOT</version>
+    <version>2.4.0-SNAPSHOT</version>
 
     <name>Apache Sling Default POST Servlets</name>
     <description>
diff --git a/src/main/java/org/apache/sling/servlets/post/AbstractPostResponseWrapper.java b/src/main/java/org/apache/sling/servlets/post/AbstractPostResponseWrapper.java
new file mode 100644
index 0000000..d6fc5a8
--- /dev/null
+++ b/src/main/java/org/apache/sling/servlets/post/AbstractPostResponseWrapper.java
@@ -0,0 +1,157 @@
+/*
+ * 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.servlets.post;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * Provides a simple implementation of PostResponse that can be subclassed by developers wishing to provide specialized behavior 
+ * to an existing PostResponse instance. The default implementation of all methods is to call through to the wrapped 
+ * PostResponse instance.
+ */
+public abstract class AbstractPostResponseWrapper implements PostResponse {
+
+    /**
+     * Use this method to return an instance of the class being wrapped.
+     * 
+     * @return the wrapped PostResponse instance
+     */
+    public abstract PostResponse getWrapped();
+
+    @Override
+    public void setReferer(String referer) {
+        getWrapped().setReferer(referer);
+    }
+
+    @Override
+    public String getReferer() {
+        return getWrapped().getReferer();
+    }
+
+    @Override
+    public void setPath(String path) {
+        getWrapped().setPath(path);
+    }
+
+    @Override
+    public String getPath() {
+        return getWrapped().getPath();
+    }
+
+    @Override
+    public void setCreateRequest(boolean isCreateRequest) {
+        getWrapped().setCreateRequest(isCreateRequest);
+    }
+
+    @Override
+    public boolean isCreateRequest() {
+        return getWrapped().isCreateRequest();
+    }
+
+    @Override
+    public void setLocation(String location) {
+        getWrapped().setLocation(location);
+    }
+
+    @Override
+    public String getLocation() {
+        return getWrapped().getLocation();
+    }
+
+    @Override
+    public void setParentLocation(String parentLocation) {
+        getWrapped().setParentLocation(parentLocation);
+    }
+
+    @Override
+    public String getParentLocation() {
+        return getWrapped().getParentLocation();
+    }
+
+    @Override
+    public void setTitle(String title) {
+        getWrapped().setTitle(title);
+    }
+
+    @Override
+    public void setStatus(int code, String message) {
+        getWrapped().setStatus(code, message);
+    }
+
+    @Override
+    public int getStatusCode() {
+        return getWrapped().getStatusCode();
+    }
+
+    @Override
+    public String getStatusMessage() {
+        return getWrapped().getStatusMessage();
+    }
+
+    @Override
+    public void setError(Throwable error) {
+        getWrapped().setError(error);
+    }
+
+    @Override
+    public Throwable getError() {
+        return getWrapped().getError();
+    }
+
+    @Override
+    public boolean isSuccessful() {
+        return getWrapped().isSuccessful();
+    }
+
+    @Override
+    public void onCreated(String path) {
+        getWrapped().onCreated(path);
+    }
+
+    @Override
+    public void onModified(String path) {
+        getWrapped().onModified(path);
+    }
+
+    @Override
+    public void onDeleted(String path) {
+        getWrapped().onDeleted(path);
+    }
+
+    @Override
+    public void onMoved(String srcPath, String dstPath) {
+        getWrapped().onMoved(srcPath, dstPath);
+    }
+
+    @Override
+    public void onCopied(String srcPath, String dstPath) {
+        getWrapped().onCopied(srcPath, dstPath);
+    }
+
+    @Override
+    public void onChange(String type, String... arguments) {
+        getWrapped().onChange(type, arguments);
+    }
+
+    @Override
+    public void send(HttpServletResponse response, boolean setStatus) throws IOException {
+        getWrapped().send(response, setStatus);
+    }
+
+}
diff --git a/src/main/java/org/apache/sling/servlets/post/impl/ErrorHandlingPostResponseWrapper.java b/src/main/java/org/apache/sling/servlets/post/impl/ErrorHandlingPostResponseWrapper.java
new file mode 100644
index 0000000..972f822
--- /dev/null
+++ b/src/main/java/org/apache/sling/servlets/post/impl/ErrorHandlingPostResponseWrapper.java
@@ -0,0 +1,78 @@
+/*
+ * 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.servlets.post.impl;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.sling.servlets.post.AbstractPostResponseWrapper;
+import org.apache.sling.servlets.post.PostResponse;
+
+/**
+ * SLING-10006 Wrap another PostResponse impl to change the error handling behavior
+ */
+public class ErrorHandlingPostResponseWrapper extends AbstractPostResponseWrapper {
+
+    private final PostResponse wrapped;
+
+    public ErrorHandlingPostResponseWrapper(PostResponse wrapped) {
+        this.wrapped = wrapped;
+    }
+
+    @Override
+    public PostResponse getWrapped() {
+        return this.wrapped;
+    }
+
+    @Override
+    public void send(HttpServletResponse response, boolean setStatus) throws IOException {
+        if (!isSuccessful()) {
+            prepare(response, setStatus);
+
+            // delegate the error rendering
+            String statusMsg = getStatusMessage();
+            if (statusMsg == null) {
+                // fallback to the exception string
+                Throwable error = getError();
+                if (error != null) {
+                    statusMsg = error.toString();
+                }
+            }
+            if (statusMsg == null) {
+                response.sendError(getStatusCode());
+            } else {
+                response.sendError(getStatusCode(), statusMsg);
+            }
+        } else {
+            super.send(response, setStatus);
+        }
+    }
+
+    /**
+     * prepares the response properties
+     */
+    private void prepare(final HttpServletResponse response, final boolean setStatus) {
+        if (setStatus) {
+            // for backward compatibility, set the response status
+            // in case the error rendering script doesn't do the same
+            int statusCode = getStatusCode();
+            response.setStatus(statusCode);
+        }
+    }
+
+}
diff --git a/src/main/java/org/apache/sling/servlets/post/impl/PostResponseWithErrorHandling.java b/src/main/java/org/apache/sling/servlets/post/impl/PostResponseWithErrorHandling.java
index a9af086..5aad39d 100644
--- a/src/main/java/org/apache/sling/servlets/post/impl/PostResponseWithErrorHandling.java
+++ b/src/main/java/org/apache/sling/servlets/post/impl/PostResponseWithErrorHandling.java
@@ -27,12 +27,12 @@ import org.apache.sling.servlets.post.HtmlResponse;
 import org.apache.sling.servlets.post.PostResponse;
 import org.apache.sling.servlets.post.PostResponseCreator;
 import org.apache.sling.servlets.post.SlingPostConstants;
-import org.osgi.service.component.annotations.Component;
 
-@Component(service = PostResponseCreator.class,
-    property = {
-            "service.vendor=The Apache Software Foundation"
-    })
+/**
+ * @deprecated SLING-10006 - this component is now deprecated no longer registered as an
+ * OSGi component as the error handling is now done in a different way in {@link SlingPostServlet#createPostResponse(SlingHttpServletRequest)}.
+ */
+@Deprecated
 public class PostResponseWithErrorHandling implements PostResponseCreator {
 
 	@Override
diff --git a/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java b/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
index 081deb2..4f378a0 100644
--- a/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
+++ b/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
@@ -323,20 +323,45 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
      * or a {@link org.apache.sling.servlets.post.PostResponse} otherwise
      */
     PostResponse createPostResponse(final SlingHttpServletRequest req) {
+        PostResponse response = null;
         for (final PostResponseCreator creator : cachedPostResponseCreators) {
-            final PostResponse response = creator.createPostResponse(req);
+            response = creator.createPostResponse(req);
             if (response != null) {
-                return response;
+                break;
             }
         }
 
-        // Fall through to default behavior
-        final MediaRangeList mediaRangeList = new MediaRangeList(req);
-        if (JSONResponse.RESPONSE_CONTENT_TYPE.equals(mediaRangeList.prefer("text/html", JSONResponse.RESPONSE_CONTENT_TYPE))) {
-            return new JSONResponse();
-        } else {
-            return new HtmlResponse();
+        if (response == null) {
+            // Fall through to default behavior
+            final MediaRangeList mediaRangeList = new MediaRangeList(req);
+            if (JSONResponse.RESPONSE_CONTENT_TYPE.equals(mediaRangeList.prefer("text/html", JSONResponse.RESPONSE_CONTENT_TYPE))) {
+                response = new JSONResponse();
+            } else {
+                response = new HtmlResponse();
+            }
+        }
+
+        if (isSendError(req)) {
+            // SLING-10006 wrap the PostResponse to handle the errors differently
+            response = new ErrorHandlingPostResponseWrapper(response);
+        }
+        return response;
+    }
+
+    /**
+     * Checks whether the normal error handling using Sling's error handlers will be used instead
+     * of the error response based on a template.
+     * 
+     * @param request the request to check
+     * @return true or false
+     */
+    private boolean isSendError(SlingHttpServletRequest request){
+        boolean sendError = false;
+        String sendErrorParam = request.getParameter(SlingPostConstants.RP_SEND_ERROR);
+        if (sendErrorParam != null && "true".equalsIgnoreCase(sendErrorParam)) {
+            sendError = true;
         }
+        return sendError;
     }
 
     private PostOperation getSlingPostOperation(
diff --git a/src/main/java/org/apache/sling/servlets/post/package-info.java b/src/main/java/org/apache/sling/servlets/post/package-info.java
index d6fab44..86fc3d0 100644
--- a/src/main/java/org/apache/sling/servlets/post/package-info.java
+++ b/src/main/java/org/apache/sling/servlets/post/package-info.java
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-@Version("2.3.1")
+@Version("2.4.0")
 package org.apache.sling.servlets.post;
 
 import org.osgi.annotation.versioning.Version;
diff --git a/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java b/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
index 46a201d..2b9d4ed 100644
--- a/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
+++ b/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
@@ -23,8 +23,6 @@ import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.StringTokenizer;
 
-import junit.framework.TestCase;
-
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.request.header.MediaRangeList;
 import org.apache.sling.commons.testing.sling.MockSlingHttpServletRequest;
@@ -35,6 +33,8 @@ import org.apache.sling.servlets.post.SlingPostConstants;
 import org.apache.sling.servlets.post.impl.helper.MockSlingHttpServlet3Request;
 import org.apache.sling.servlets.post.impl.helper.MockSlingHttpServlet3Response;
 
+import junit.framework.TestCase;
+
 public class SlingPostServletTest extends TestCase {
     
     private SlingPostServlet servlet;
@@ -84,9 +84,52 @@ public class SlingPostServletTest extends TestCase {
             }
         };
         PostResponse result = servlet.createPostResponse(req);
+        assertFalse("Did not expect ErrorHandlingPostResponseWrapper PostResonse", result instanceof ErrorHandlingPostResponseWrapper);
         assertTrue(result instanceof JSONResponse);
     }
-    
+
+    public void testGetHtmlResponse() {
+        MockSlingHttpServletRequest req = new MockSlingHttpServlet3Request(null, null, null, null, null);
+        PostResponse result = servlet.createPostResponse(req);
+        assertFalse("Did not expect ErrorHandlingPostResponseWrapper PostResonse", result instanceof ErrorHandlingPostResponseWrapper);
+        assertTrue(result instanceof HtmlResponse);
+    }
+
+    /**
+     * SLING-10006 - verify we get the error handling wrapped PostResponse
+     */
+    public void testGetJsonResponseWithSendError() {
+    	SendErrorParamSlingHttpServletRequest req = new SendErrorParamSlingHttpServletRequest() {
+            @Override
+            public String getHeader(String name) {
+                return name.equals(MediaRangeList.HEADER_ACCEPT) ? "application/json" : super.getHeader(name);
+            }
+
+            public <AdapterType> AdapterType adaptTo(Class<AdapterType> type) {
+                return null;
+            }
+        };
+        req.setSendError("true");
+        
+        PostResponse result = servlet.createPostResponse(req);
+        assertTrue("Expected ErrorHandlingPostResponseWrapper PostResonse", result instanceof ErrorHandlingPostResponseWrapper);
+       	result = ((ErrorHandlingPostResponseWrapper)result).getWrapped();
+        assertTrue(result instanceof JSONResponse);
+    }
+
+    /**
+     * SLING-10006 - verify we get the error handling wrapped PostResponse
+     */
+    public void testGetHtmlResponseWithSendError() {
+    	SendErrorParamSlingHttpServletRequest req = new SendErrorParamSlingHttpServletRequest();
+        req.setSendError("true");
+        
+        PostResponse result = servlet.createPostResponse(req);
+        assertTrue(result instanceof ErrorHandlingPostResponseWrapper);
+       	result = ((ErrorHandlingPostResponseWrapper)result).getWrapped();
+        assertTrue(result instanceof HtmlResponse);
+    }
+
     public void testRedirection() throws Exception {
         String utf8Path = "\u0414\u0440\u0443\u0433\u0430";
         String encodedUtf8 = "%D0%94%D1%80%D1%83%D0%B3%D0%B0";
@@ -193,4 +236,33 @@ public class SlingPostServletTest extends TestCase {
             return null;
         }
     }
+
+    private static class SendErrorParamSlingHttpServletRequest extends
+	    MockSlingHttpServlet3Request {
+		
+		private String sendError;
+		
+		public SendErrorParamSlingHttpServletRequest() {
+		    // nothing to setup, we don't care
+		    super(null, null, null, null, null);
+		}
+		
+		@Override
+		public String getParameter(String name) {
+		    if (SlingPostConstants.RP_SEND_ERROR.equals(name)) {
+		        return sendError;
+		    }
+		
+		    return super.getParameter(name);
+		}
+		
+		void setSendError(String sendErrorParam) {
+		    this.sendError = sendErrorParam;
+		}
+		
+		public <AdapterType> AdapterType adaptTo(Class<AdapterType> type) {
+		    return null;
+		}
+	}
+
 }