Commit 1aaf79ca authored by Mathijs den Burger's avatar Mathijs den Burger

HSTTWO-4242 Improve error logging of manageContent tag

- only log warnings (errors should only be used when thing are badly
  broken and require sysadmin intervention)

- log the render path of the current template in the warning, so a
  developer can easier locate the manageContent tag in error.

- consistently use the phrase "manageContent tag"
parent b44ffd8f
......@@ -79,17 +79,17 @@ public class HstManageContentTag extends TagSupport {
final HstRequestContext requestContext = RequestContextProvider.get();
if (requestContext == null) {
log.warn("Cannot create a manage content button outside the hst request.");
log.warn("Cannot create a manageContent button outside the hst request.");
return EVAL_PAGE;
}
if (!requestContext.isCmsRequest()) {
log.debug("Skipping manage content tag because not in cms preview.");
log.debug("Skipping manageContent tag because not in cms preview.");
return EVAL_PAGE;
}
if (templateQuery == null && hippoBean == null && parameterName == null) {
log.debug("Skipping manage content tag because neither 'templateQuery', 'hippoBean' or 'parameterName' attribute specified.");
log.debug("Skipping manageContent tag because neither 'templateQuery', 'hippoBean' or 'parameterName' attribute specified.");
return EVAL_PAGE;
}
......@@ -99,7 +99,7 @@ public class HstManageContentTag extends TagSupport {
try {
final Node editNode = documentNode.getCanonicalNode();
if (editNode == null) {
log.debug("Cannot create a manage content tag, cannot find canonical node of '{}'",
log.debug("Cannot create a manageContent tag, cannot find canonical node of '{}'",
documentNode.getPath());
return EVAL_PAGE;
}
......@@ -110,10 +110,10 @@ public class HstManageContentTag extends TagSupport {
return EVAL_PAGE;
}
log.debug("The node path for the manage content tag is '{}'", handleNode.getPath());
log.debug("The node path for the manageContent tag is '{}'", handleNode.getPath());
documentId = handleNode.getIdentifier();
} catch (RepositoryException e) {
log.warn("Error while retrieving the handle of '{}', skipping manage content tag",
log.warn("Error while retrieving the handle of '{}', skipping manageContent tag",
JcrUtils.getNodePathQuietly(hippoBean.getNode()), e);
return EVAL_PAGE;
}
......@@ -122,12 +122,13 @@ public class HstManageContentTag extends TagSupport {
final JcrPath jcrPath = getJcrPath();
final boolean isRelativePathParameter = jcrPath != null && jcrPath.isRelative();
if (isRelativePathParameter && StringUtils.startsWith(rootPath, "/")) {
log.warn("Ignoring manage content tag for component parameter '{}': the @{} annotation of the parameter"
+ " makes it store a relative path to the content root of the channel while the 'rootPath'"
+ " attribute of the manage content tag points to the absolute path '{}'."
log.warn("Ignoring manageContent tag in template '{}' for component parameter '{}':"
+ " the @{} annotation of the parameter makes it store a relative path to the"
+ " content root of the channel while the 'rootPath' attribute of the manageContent"
+ " tag points to the absolute path '{}'."
+ " Either make the root path relative to the channel content root,"
+ " or make the component parameter store an absolute path.",
parameterName, JcrPath.class.getSimpleName(), rootPath);
getComponentRenderPath(), parameterName, JcrPath.class.getSimpleName(), rootPath);
return EVAL_PAGE;
}
......@@ -135,15 +136,17 @@ public class HstManageContentTag extends TagSupport {
try {
absoluteRootPath = checkRootPath(requestContext);
} catch (final RepositoryException e) {
log.error("Exception while checking rootPath parameter.", e);
log.warn("Exception while checking rootPath parameter for manageContent tag in template '{}'.",
getComponentRenderPath(), e);
return EVAL_PAGE;
}
final String componentValue = getComponentValue(requestContext, isRelativePathParameter);
final String componentValue = getComponentValue(isRelativePathParameter);
try {
write(documentId, componentValue, jcrPath, isRelativePathParameter, absoluteRootPath);
} catch (final IOException ignore) {
throw new JspException("Manage content tag exception: cannot write to the output writer.");
throw new JspException("manageContent tag exception in template '" + getComponentRenderPath()
+ "': cannot write to the output writer.");
}
return EVAL_PAGE;
} finally {
......@@ -166,13 +169,15 @@ public class HstManageContentTag extends TagSupport {
try {
final Node rootPathNode = requestContext.getSession().getNode(absoluteRootPath);
if (!rootPathNode.isNodeType(HippoStdNodeType.NT_FOLDER) && !rootPathNode.isNodeType(HippoStdNodeType.NT_DIRECTORY)) {
log.error("Rootpath '{}' is not a folder node. Parameters rootPath and defaultPath are ignored.", rootPath);
log.warn("Rootpath '{}' is not a folder node. Parameters rootPath and defaultPath of manageContent tag"
+ " in template '{}' are ignored.", rootPath, getComponentRenderPath());
rootPath = null;
defaultPath = null;
absoluteRootPath = null;
}
} catch (final PathNotFoundException e) {
log.error("Rootpath '{}' does not exist. Parameters rootPath and defaultPath are ignored.", rootPath, e);
log.warn("Rootpath '{}' does not exist. Parameters rootPath and defaultPath of manageContent tag"
+ " in template '{}' are ignored.", rootPath, getComponentRenderPath());
rootPath = null;
defaultPath = null;
absoluteRootPath = null;
......@@ -180,6 +185,22 @@ public class HstManageContentTag extends TagSupport {
return absoluteRootPath;
}
private String getComponentRenderPath() {
final HstComponentWindow window = getCurrentComponentWindow();
if (window == null) {
return "";
}
final HstComponent component = window.getComponent();
if (component == null) {
return "";
}
final ComponentConfiguration componentConfiguration = component.getComponentConfiguration();
if (componentConfiguration == null) {
return "";
}
return componentConfiguration.getRenderPath();
}
private String getAbsoluteRootPath(final HstRequestContext requestContext) {
if (StringUtils.startsWith(rootPath, "/")) {
return rootPath;
......@@ -188,14 +209,12 @@ public class HstManageContentTag extends TagSupport {
}
}
private String getComponentValue(final HstRequestContext requestContext, final boolean isRelativePathParameter) {
private String getComponentValue(final boolean isRelativePathParameter) {
if (parameterName == null) {
return null;
}
final ServletRequest request = pageContext.getRequest();
final HstComponentWindow window = (HstComponentWindow) request.getAttribute(HST_COMPONENT_WINDOW);
final HstComponentWindow window = getCurrentComponentWindow();
final String prefixedParameterName = getPrefixedParameterName(window, parameterName);
final String componentValue = window.getParameter(prefixedParameterName);
......@@ -205,6 +224,11 @@ public class HstManageContentTag extends TagSupport {
return componentValue;
}
private HstComponentWindow getCurrentComponentWindow() {
final ServletRequest request = pageContext.getRequest();
return (HstComponentWindow) request.getAttribute(HST_COMPONENT_WINDOW);
}
private String getPrefixedParameterName(final HstComponentWindow window, final String parameterName) {
final Object parameterPrefix = window.getAttribute(RENDER_VARIANT);
......@@ -308,8 +332,8 @@ public class HstManageContentTag extends TagSupport {
public void setParameterName(final String parameterName) {
if (StringUtils.isBlank(parameterName)) {
log.warn("The parameterName attribute of a manageContent tag is set to '{}'."
+ " Expected the name of an HST component parameter instead.", parameterName);
log.warn("The parameterName attribute of a manageContent tag in template '{}' is set to '{}'."
+ " Expected the name of an HST component parameter instead.", getComponentRenderPath(), parameterName);
}
this.parameterName = parameterName;
}
......@@ -328,12 +352,11 @@ public class HstManageContentTag extends TagSupport {
public void setTemplateQuery(final String templateQuery) {
if (StringUtils.isBlank(templateQuery)) {
log.warn("The templateQuery attribute of a manageContent tag is set to '{}'."
+ " Expected the name of a template query instead.", templateQuery);
log.warn("The templateQuery attribute of a manageContent tag in template '{}' is set to '{}'."
+ " Expected the name of a template query instead.", getComponentRenderPath(), templateQuery);
}
this.templateQuery = templateQuery;
}
}
......
......@@ -17,6 +17,8 @@ package org.hippoecm.hst.tag;
import java.io.IOException;
import java.io.Writer;
import java.util.List;
import java.util.stream.Collectors;
import javax.jcr.Node;
import javax.jcr.PathNotFoundException;
......@@ -36,6 +38,7 @@ import org.hippoecm.hst.core.parameters.Parameter;
import org.hippoecm.hst.core.parameters.ParametersInfo;
import org.hippoecm.hst.core.request.ResolvedMount;
import org.hippoecm.hst.mock.core.container.MockHstComponentWindow;
import org.hippoecm.hst.mock.core.request.MockComponentConfiguration;
import org.hippoecm.hst.mock.core.request.MockHstRequestContext;
import org.hippoecm.repository.HippoStdNodeType;
import org.hippoecm.repository.api.HippoNode;
......@@ -56,6 +59,7 @@ import static javax.servlet.jsp.tagext.Tag.EVAL_PAGE;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hippoecm.hst.core.container.ContainerConstants.RENDER_VARIANT;
import static org.junit.Assert.assertThat;
......@@ -78,7 +82,12 @@ public class HstManageContentTagTest {
final MockHttpServletRequest request = new MockHttpServletRequest();
window = new MockHstComponentWindow();
window.setComponent(new TestComponent());
final TestComponent component = new TestComponent();
final MockComponentConfiguration componentConfig = new MockComponentConfiguration();
componentConfig.setRenderPath("webfile:/freemarker/test.ftl");
component.init(servletContext, componentConfig);
window.setComponent(component);
request.setAttribute(ContainerConstants.HST_COMPONENT_WINDOW, window);
response = new MockHttpServletResponse();
......@@ -106,7 +115,7 @@ public class HstManageContentTagTest {
try (Log4jInterceptor listener = Log4jInterceptor.onWarn().trap(HstManageContentTag.class).build()) {
assertThat(tag.doEndTag(), is(EVAL_PAGE));
assertThat(response.getContentAsString(), is(""));
assertLogged(listener, "Cannot create a manage content button outside the hst request.");
assertLogged(listener, "Cannot create a manageContent button outside the hst request.");
}
}
......@@ -117,7 +126,7 @@ public class HstManageContentTagTest {
try (Log4jInterceptor listener = Log4jInterceptor.onDebug().trap(HstManageContentTag.class).build()) {
assertThat(tag.doEndTag(), is(EVAL_PAGE));
assertThat(response.getContentAsString(), is(""));
assertLogged(listener, "Skipping manage content tag because not in cms preview.");
assertLogged(listener, "Skipping manageContent tag because not in cms preview.");
}
}
......@@ -302,9 +311,10 @@ public class HstManageContentTagTest {
assertThat(tag.doEndTag(), is(EVAL_PAGE));
assertThat(response.getContentAsString(), is(""));
assertLogged(listener, "Ignoring manage content tag for component parameter 'relPath': the @JcrPath annotation of the parameter"
assertLogged(listener, "Ignoring manageContent tag in template 'webfile:/freemarker/test.ftl'"
+ " for component parameter 'relPath': the @JcrPath annotation of the parameter"
+ " makes it store a relative path to the content root of the channel while the 'rootPath'"
+ " attribute of the manage content tag points to the absolute path '/some/absolute/path'."
+ " attribute of the manageContent tag points to the absolute path '/some/absolute/path'."
+ " Either make the root path relative to the channel content root,"
+ " or make the component parameter store an absolute path.");
}
......@@ -538,8 +548,9 @@ public class HstManageContentTagTest {
tag.setParameterName(null);
tag.doEndTag();
assertLogged(listener, "The parameterName attribute of a manageContent tag is set to 'null'." +
" Expected the name of an HST component parameter instead.");
assertLogged(listener, "The parameterName attribute of a manageContent tag"
+ " in template 'webfile:/freemarker/test.ftl' is set to 'null'."
+ " Expected the name of an HST component parameter instead.");
}
}
......@@ -549,8 +560,9 @@ public class HstManageContentTagTest {
tag.setParameterName("");
tag.doEndTag();
assertLogged(listener, "The parameterName attribute of a manageContent tag is set to ''." +
" Expected the name of an HST component parameter instead.");
assertLogged(listener, "The parameterName attribute of a manageContent tag"
+ " in template 'webfile:/freemarker/test.ftl' is set to ''."
+ " Expected the name of an HST component parameter instead.");
}
}
......@@ -560,8 +572,9 @@ public class HstManageContentTagTest {
tag.setParameterName(" ");
tag.doEndTag();
assertLogged(listener, "The parameterName attribute of a manageContent tag is set to ' '." +
" Expected the name of an HST component parameter instead.");
assertLogged(listener, "The parameterName attribute of a manageContent tag in template"
+ " 'webfile:/freemarker/test.ftl' is set to ' '."
+ " Expected the name of an HST component parameter instead.");
}
}
......@@ -571,7 +584,8 @@ public class HstManageContentTagTest {
tag.setTemplateQuery(null);
tag.doEndTag();
assertLogged(listener, "The templateQuery attribute of a manageContent tag is set to 'null'." +
assertLogged(listener, "The templateQuery attribute of a manageContent tag"
+ " in template 'webfile:/freemarker/test.ftl' is set to 'null'." +
" Expected the name of a template query instead.");
}
}
......@@ -582,8 +596,8 @@ public class HstManageContentTagTest {
tag.setTemplateQuery("");
tag.doEndTag();
assertLogged(listener, "The templateQuery attribute of a manageContent tag is set to ''." +
" Expected the name of a template query instead.");
assertLogged(listener, "The templateQuery attribute of a manageContent tag in template"
+ " 'webfile:/freemarker/test.ftl' is set to ''. Expected the name of a template query instead.");
}
}
......@@ -593,8 +607,8 @@ public class HstManageContentTagTest {
tag.setTemplateQuery(" ");
tag.doEndTag();
assertLogged(listener, "The templateQuery attribute of a manageContent tag is set to ' '." +
" Expected the name of a template query instead.");
assertLogged(listener, "The templateQuery attribute of a manageContent tag in template"
+ " 'webfile:/freemarker/test.ftl' is set to ' '. Expected the name of a template query instead.");
}
}
......@@ -701,8 +715,9 @@ public class HstManageContentTagTest {
}
private static void assertLogged(final Log4jInterceptor listener, final String expectedMessage) {
assertThat("expected log message '" + expectedMessage + "'", listener.messages().anyMatch((msg) -> msg.equals(expectedMessage)), is(true));
assertThat(listener.getEvents().size(), is(1));
final List<String> messages = listener.messages().collect(Collectors.toList());
assertThat(messages.size(), is(1));
assertThat(messages.get(0), equalTo(expectedMessage));
}
private static class BrokenPageContext extends MockPageContext {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment