This is an automated email from the ASF dual-hosted git repository.
dklco pushed a commit to branch forms-enhancements
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git
The following commit(s) were added to refs/heads/forms-enhancements by this
push:
new 3bafb4b Improve the handling of the error page
3bafb4b is described below
commit 3bafb4b162acf28a06759dba27f5d5403aae3e22
Author: Dan Klco <[email protected]>
AuthorDate: Tue Jan 19 17:30:28 2021 -0500
Improve the handling of the error page
---
.../cms/reference/forms/impl/FormHandler.java | 96 ++++++++++++++--------
.../cms/reference/forms/impl/FormRequestImpl.java | 27 ++++--
.../forms/impl/fields/TextfieldHandler.java | 1 -
.../apps/reference/components/forms/form/edit.json | 6 ++
.../cms/reference/forms/impl/FormHandlerTest.java | 12 +--
reference/src/test/resources/form.json | 1 -
6 files changed, 91 insertions(+), 52 deletions(-)
diff --git
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormHandler.java
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormHandler.java
index 0b8de5b..66feb4f 100644
---
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormHandler.java
+++
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormHandler.java
@@ -30,6 +30,7 @@ import org.apache.jackrabbit.JcrConstants;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.api.servlets.SlingAllMethodsServlet;
import org.apache.sling.api.wrappers.SlingHttpServletRequestWrapper;
import org.apache.sling.cms.Page;
@@ -64,41 +65,38 @@ public class FormHandler extends SlingAllMethodsServlet {
@Override
protected void doPost(SlingHttpServletRequest request,
SlingHttpServletResponse response)
throws ServletException, IOException {
+ ValueMap properties = request.getResource().getValueMap();
String pagePath =
Optional.ofNullable(request.getResource().adaptTo(PageManager.class))
.map(PageManager::getPage).map(Page::getPath)
.orElse(StringUtils.substringBefore(request.getResource().getPath(), "/" +
JcrConstants.JCR_CONTENT));
+ String successPage = null;
+ String errorPage = pagePath;
StringSubstitutor sub = null;
try {
- if (request.getResource().getChild("actions") == null) {
- throw new FormException("No actions provided to handle this
form submission");
- }
- List<Resource> actionResources =
ResourceTree.stream(request.getResource().getChild("actions"))
-
.map(ResourceTree::getResource).collect(Collectors.toList());
-
- FormRequest formRequest = getFormRequest(request);
+ log.debug("Extracting form request...");
+ FormRequest formRequest = request.adaptTo(FormRequest.class);
if (formRequest == null) {
log.warn("Unable to create form request");
-
response.sendRedirect(request.getResourceResolver().map(request, pagePath) +
".html?error=fields");
+ response.sendRedirect(this.resolveUrl(request, errorPage,
"error=init"));
return;
}
+
+ log.debug("Loading fields...");
+ boolean fieldsLoadSucceeded = ((FormRequestImpl)
formRequest).initFields();
sub = new StringSubstitutor(formRequest.getFormData());
- request.getSession().setAttribute(formRequest.getSessionId(),
formRequest.getFormData());
- for (Resource actionResource : actionResources) {
- log.debug("Finding action handler for: {}", actionResource);
- FormAction action = formActions.stream().filter(fa ->
fa.handles(actionResource)).findFirst()
- .orElse(null);
- if (action != null) {
- FormActionResult result =
action.handleForm(actionResource, formRequest);
- if (!result.isSucceeded()) {
- throw new FormException(
- "Failed to invoke action: " + action + " with
message: " + result.getMessage());
- } else {
- log.debug("Successfully invoked action: {}",
result.getMessage());
- }
- }
+ successPage = sub.replace(properties.get("successPage", pagePath));
+ errorPage = sub.replace(properties.get("errorPage", pagePath));
+ if (!fieldsLoadSucceeded) {
+ log.warn("Field initialization failed, check logs");
+ response.sendRedirect(this.resolveUrl(request, errorPage,
"error=fields"));
+ return;
}
+ request.getSession().setAttribute(formRequest.getSessionId(),
formRequest.getFormData());
+
+ log.debug("Calling actions...");
+ callActions(request, formRequest);
request.getSession().removeAttribute(formRequest.getSessionId());
} catch (FormException e) {
log.warn("Exception executing actions", e);
@@ -106,31 +104,61 @@ public class FormHandler extends SlingAllMethodsServlet {
return;
}
- String thankYouPage =
sub.replace(request.getResource().getValueMap().get("successPage", pagePath));
- if (StringUtils.isNotBlank(thankYouPage)) {
- if
("forward".equals(request.getResource().getValueMap().get("successAction",
String.class))) {
+ if (StringUtils.isNotBlank(successPage)) {
+ if ("forward".equals(properties.get("successAction",
String.class))) {
SlingHttpServletRequestWrapper requestWrapper = new
SlingHttpServletRequestWrapper(request) {
@Override
public String getMethod() {
return "GET";
}
};
-
-
request.getRequestDispatcher(thankYouPage).forward(requestWrapper, response);
+
request.getRequestDispatcher(successPage).forward(requestWrapper, response);
} else {
- response.sendRedirect(resolveThankYouPage(request,
thankYouPage));
+ response.sendRedirect(resolveUrl(request, successPage,
"message=success"));
}
} else {
- response.sendRedirect(resolveThankYouPage(request, thankYouPage));
+ response.sendRedirect(resolveUrl(request, successPage,
"message=success"));
}
}
- private String resolveThankYouPage(SlingHttpServletRequest request, String
thankYouPage) {
- if (!thankYouPage.contains(".html")) {
- thankYouPage += ".html";
+ private void callActions(SlingHttpServletRequest request, FormRequest
formRequest) throws FormException {
+ Resource actions = request.getResource().getChild("actions");
+ if (actions == null) {
+ throw new FormException("No actions provided to handle this form
submission");
+ }
+ List<Resource> actionResources =
ResourceTree.stream(actions).map(ResourceTree::getResource)
+ .collect(Collectors.toList());
+
+ for (Resource actionResource : actionResources) {
+ log.debug("Finding action handler for: {}", actionResource);
+ FormAction action = formActions.stream().filter(fa ->
fa.handles(actionResource)).findFirst().orElse(null);
+ if (action != null) {
+ FormActionResult result = action.handleForm(actionResource,
formRequest);
+ if (!result.isSucceeded()) {
+ throw new FormException(
+ "Failed to invoke action: " + action + " with
message: " + result.getMessage());
+ } else {
+ log.debug("Successfully invoked action: {}",
result.getMessage());
+ }
+ }
+ }
+ }
+
+ private String resolveUrl(SlingHttpServletRequest request, String url,
String qs) {
+ if (url.contains("?")) {
+ qs = "&" + qs;
+ } else {
+ qs = "?" + qs;
+ }
+ if (url.startsWith("/")) {
+ if (!url.contains(".html")) {
+ url += ".html";
+ }
+ url += qs;
+ return request.getResourceResolver().map(request, url);
+ } else {
+ return url + qs;
}
- thankYouPage += "?message=success";
- return request.getResourceResolver().map(request, thankYouPage);
}
protected FormRequest getFormRequest(SlingHttpServletRequest request)
throws FormException {
diff --git
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormRequestImpl.java
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormRequestImpl.java
index 396d388..2ab4b7d 100644
---
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormRequestImpl.java
+++
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/FormRequestImpl.java
@@ -101,19 +101,32 @@ public class FormRequestImpl implements FormRequest {
return request;
}
- public void initFields() throws FormException {
+ public boolean initFields() {
List<Resource> fields =
ResourceTree.stream(getFormResource().getChild("fields")).map(ResourceTree::getResource)
.collect(Collectors.toList());
+ boolean successful = true;
for (Resource field : fields) {
- log.debug("Looking for handler for: {}", field);
- for (FieldHandler fieldHandler : fieldHandlers) {
- if (fieldHandler.handles(field)) {
- log.debug("Invoking field handler: {}",
fieldHandler.getClass());
- fieldHandler.handleField(request, field, formData);
- break;
+ formData.remove(getErrorKey(field));
+ try {
+ log.debug("Looking for handler for: {}", field);
+ for (FieldHandler fieldHandler : fieldHandlers) {
+ if (fieldHandler.handles(field)) {
+ log.debug("Invoking field handler: {}",
fieldHandler.getClass());
+ fieldHandler.handleField(request, field, formData);
+ break;
+ }
}
+ } catch (FormException fe) {
+ log.warn("Failed to populate field {} due to exception",
field, fe);
+ successful = false;
+ formData.put(getErrorKey(field), fe.getMessage());
}
}
+ return successful;
+ }
+
+ private String getErrorKey(Resource field) {
+ return "fielderror-" + field.getValueMap().get("name", String.class);
}
@Override
diff --git
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/TextfieldHandler.java
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/TextfieldHandler.java
index d3150a6..f498586 100644
---
a/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/TextfieldHandler.java
+++
b/reference/src/main/java/org/apache/sling/cms/reference/forms/impl/fields/TextfieldHandler.java
@@ -110,7 +110,6 @@ public class TextfieldHandler implements FieldHandler {
formData.put(name + ".contentType",
param.getContentType());
}
} else if ("date".equals(saveAs)) {
-
if (!dateFormats.containsKey(type)) {
throw new FormException("Field " + name + " is not a date
type");
}
diff --git
a/reference/src/main/resources/jcr_root/apps/reference/components/forms/form/edit.json
b/reference/src/main/resources/jcr_root/apps/reference/components/forms/form/edit.json
index aa98ccd..422d9d3 100644
---
a/reference/src/main/resources/jcr_root/apps/reference/components/forms/form/edit.json
+++
b/reference/src/main/resources/jcr_root/apps/reference/components/forms/form/edit.json
@@ -19,6 +19,12 @@
"name": "submitText",
"required": true
},
+ "errorPage": {
+ "jcr:primaryType": "nt:unstructured",
+ "sling:resourceType": "sling-cms/components/editor/fields/path",
+ "label": "Error Page",
+ "name": "errorPage"
+ },
"successPage": {
"jcr:primaryType": "nt:unstructured",
"sling:resourceType": "sling-cms/components/editor/fields/path",
diff --git
a/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/FormHandlerTest.java
b/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/FormHandlerTest.java
index 70106fc..ab0e2ab 100644
---
a/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/FormHandlerTest.java
+++
b/reference/src/test/java/org/apache/sling/cms/reference/forms/impl/FormHandlerTest.java
@@ -17,7 +17,6 @@
package org.apache.sling.cms.reference.forms.impl;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.never;
import java.io.IOException;
@@ -100,17 +99,12 @@ public class FormHandlerTest {
}
})));
+ context.registerAdapter(SlingHttpServletRequest.class,
FormRequest.class, formRequest);
mailService = Mockito.mock(MailService.class);
Mockito.when(mailService.getMessageBuilder()).thenReturn(new
MockMessageBuilder());
final SendEmailAction sendEmailAction = new
SendEmailAction(mailService);
- formHandler = new FormHandler(Arrays.asList(sendEmailAction)) {
- private static final long serialVersionUID = 1L;
-
- protected FormRequest getFormRequest(final SlingHttpServletRequest
request) {
- return formRequest;
- }
- };
+ formHandler = new FormHandler(Arrays.asList(sendEmailAction));
}
@Test
@@ -130,7 +124,7 @@ public class FormHandlerTest {
formHandler.service(context.request(), context.response());
- assertTrue(HttpServletResponse.SC_MOVED_TEMPORARILY ==
context.response().getStatus());
+ assertEquals(HttpServletResponse.SC_MOVED_TEMPORARILY,
context.response().getStatus());
assertEquals("/form-no-actions.html?error=actions",
context.response().getHeader("Location"));
Mockito.verify(mailService, never()).sendMessage(Mockito.any());
}
diff --git a/reference/src/test/resources/form.json
b/reference/src/test/resources/form.json
index 1d558c8..2021c1a 100644
--- a/reference/src/test/resources/form.json
+++ b/reference/src/test/resources/form.json
@@ -119,7 +119,6 @@
},
"file": {
"jcr:primaryType": "nt:unstructured",
- "required": true,
"name": "file",
"type": "file",
"label": "File",
