You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by dk...@apache.org on 2021/05/13 02:26:06 UTC
[sling-org-apache-sling-app-cms] branch master updated: Minor -
cleaning up SonarQube code quality issues
This is an automated email from the ASF dual-hosted git repository.
dklco pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-app-cms.git
The following commit(s) were added to refs/heads/master by this push:
new 72c714b Minor - cleaning up SonarQube code quality issues
72c714b is described below
commit 72c714b897fd4f72ac29c3404c29972797893550
Author: Dan Klco <kl...@adobe.com>
AuthorDate: Wed May 12 22:25:52 2021 -0400
Minor - cleaning up SonarQube code quality issues
---
.../org/apache/sling/cms/core/internal/CommonUtils.java | 5 +++++
.../cms/core/internal/models/ReferenceOperation.java | 2 +-
.../core/internal/operations/BulkReplaceOperation.java | 16 +++++++++++-----
.../operations/UpdateReferencesPostOperation.java | 5 ++++-
.../core/internal/servlets/PathSuggestionServlet.java | 15 +++++++++++----
.../core/internal/operations/MembersOperationTest.java | 9 ++++-----
.../internal/operations/MembershipOperationTest.java | 9 ++++-----
.../apache/sling/cms/reference/forms/FormConstants.java | 3 +++
.../org/apache/sling/cms/reference/forms/FormUtils.java | 3 +++
.../org/apache/sling/cms/reference/models/ItemList.java | 2 +-
.../org/apache/sling/cms/reference/models/Search.java | 2 +-
11 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/CommonUtils.java b/core/src/main/java/org/apache/sling/cms/core/internal/CommonUtils.java
index cb5062e..e888bab 100644
--- a/core/src/main/java/org/apache/sling/cms/core/internal/CommonUtils.java
+++ b/core/src/main/java/org/apache/sling/cms/core/internal/CommonUtils.java
@@ -21,6 +21,7 @@ import java.util.Optional;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import org.apache.commons.text.StringEscapeUtils;
import org.apache.jackrabbit.api.JackrabbitSession;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.sling.api.resource.ResourceResolver;
@@ -49,4 +50,8 @@ public class CommonUtils {
return userManager;
}).orElseThrow(() -> new RepositoryException("Failed to get user manager"));
}
+
+ public static final String escapeLogMessage(String message){
+ return StringEscapeUtils.escapeHtml4(message.replaceAll("[\\n\\r]"," "));
+ }
}
diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/models/ReferenceOperation.java b/core/src/main/java/org/apache/sling/cms/core/internal/models/ReferenceOperation.java
index 9c63e3d..eb8218c 100644
--- a/core/src/main/java/org/apache/sling/cms/core/internal/models/ReferenceOperation.java
+++ b/core/src/main/java/org/apache/sling/cms/core/internal/models/ReferenceOperation.java
@@ -42,7 +42,7 @@ public abstract class ReferenceOperation {
private Resource resource = null;
- public ReferenceOperation(Resource resource) {
+ protected ReferenceOperation(Resource resource) {
String path = resource.getPath();
if (CMSConstants.NT_PAGE.equals(resource.getResourceType())) {
regex = Pattern.compile("(^\\Q" + path + "\\E($|\\/)|(\\'|\\\")\\Q" + path + "\\E(\\.html|\\'|\\\"|\\/))");
diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/operations/BulkReplaceOperation.java b/core/src/main/java/org/apache/sling/cms/core/internal/operations/BulkReplaceOperation.java
index ca1f708..eb41723 100644
--- a/core/src/main/java/org/apache/sling/cms/core/internal/operations/BulkReplaceOperation.java
+++ b/core/src/main/java/org/apache/sling/cms/core/internal/operations/BulkReplaceOperation.java
@@ -31,6 +31,7 @@ import org.apache.sling.api.SlingException;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.ModifiableValueMap;
import org.apache.sling.api.resource.Resource;
+import org.apache.sling.cms.core.internal.CommonUtils;
import org.apache.sling.servlets.post.Modification;
import org.apache.sling.servlets.post.PostOperation;
import org.apache.sling.servlets.post.PostResponse;
@@ -73,14 +74,19 @@ public class BulkReplaceOperation implements PostOperation {
Pattern rfind = null;
String find = request.getParameter(PN_FIND);
if (MODE_REGEX.equals(request.getParameter(PN_MODE))) {
- log.debug("Using regular expressions to search for {}", find);
-
+ if (log.isDebugEnabled()) {
+ log.debug("Using regular expressions to search for {}", CommonUtils.escapeLogMessage(find));
+ }
+
rfind = Pattern.compile(find);
- } else {
- log.debug("Searching for {}", find);
+ } else if (log.isDebugEnabled()) {
+ log.debug("Searching for {}", CommonUtils.escapeLogMessage(find));
}
String replace = request.getParameter(PN_REPLACE);
- log.debug("Replacing with {}", replace);
+
+ if (log.isDebugEnabled()) {
+ log.debug("Replacing with {}", CommonUtils.escapeLogMessage(replace));
+ }
final List<Modification> changes = new ArrayList<>();
updateProperties(request.getResource(), updateProperties, rfind, find, replace, response, changes);
diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/operations/UpdateReferencesPostOperation.java b/core/src/main/java/org/apache/sling/cms/core/internal/operations/UpdateReferencesPostOperation.java
index fd330a8..a54b242 100644
--- a/core/src/main/java/org/apache/sling/cms/core/internal/operations/UpdateReferencesPostOperation.java
+++ b/core/src/main/java/org/apache/sling/cms/core/internal/operations/UpdateReferencesPostOperation.java
@@ -22,6 +22,7 @@ import java.util.List;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.ModifiableValueMap;
import org.apache.sling.api.resource.Resource;
+import org.apache.sling.cms.core.internal.CommonUtils;
import org.apache.sling.cms.core.internal.models.ReferenceOperation;
import org.apache.sling.servlets.post.Modification;
import org.apache.sling.servlets.post.SlingPostConstants;
@@ -55,7 +56,9 @@ public class UpdateReferencesPostOperation implements SlingPostProcessor {
private void updateReferences(SlingHttpServletRequest request, final List<Modification> changes) {
final String find = request.getResource().getPath();
final String destination = request.getParameter(SlingPostConstants.RP_DEST);
- log.debug("Using destination: {}", destination);
+ if (log.isDebugEnabled()) {
+ log.debug("Using destination: {}", CommonUtils.escapeLogMessage(destination));
+ }
ReferenceOperation ro = new ReferenceOperation(request.getResource()) {
@Override
public void doProcess(Resource resource, String matchingKey) {
diff --git a/core/src/main/java/org/apache/sling/cms/core/internal/servlets/PathSuggestionServlet.java b/core/src/main/java/org/apache/sling/cms/core/internal/servlets/PathSuggestionServlet.java
index aa32d19..8b94b2f 100644
--- a/core/src/main/java/org/apache/sling/cms/core/internal/servlets/PathSuggestionServlet.java
+++ b/core/src/main/java/org/apache/sling/cms/core/internal/servlets/PathSuggestionServlet.java
@@ -33,6 +33,7 @@ import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.servlets.HttpConstants;
import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
+import org.apache.sling.cms.core.internal.CommonUtils;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.metatype.annotations.Designate;
@@ -72,13 +73,17 @@ public class PathSuggestionServlet extends SlingSafeMethodsServlet {
if (StringUtils.isEmpty(path)) {
path = "/";
}
- log.debug("Finding valid paths under {}", path);
-
+ if (log.isDebugEnabled()) {
+ log.debug("Finding valid paths under {}", CommonUtils.escapeLogMessage(path));
+ }
+
String type = request.getParameter("type");
if (!typeFilters.containsKey(type)) {
type = "all";
}
- log.debug("Filtering by type: {}", type);
+ if (log.isDebugEnabled()) {
+ log.debug("Filtering by type: {}", CommonUtils.escapeLogMessage(type));
+ }
JsonArrayBuilder arrBuilder = Json.createArrayBuilder();
Resource parent = request.getResourceResolver().getResource(path);
@@ -89,7 +94,9 @@ public class PathSuggestionServlet extends SlingSafeMethodsServlet {
path = "/";
}
- log.debug("Using stemmed path {}", path);
+ if (log.isDebugEnabled()) {
+ log.debug("Using stemmed path {}", CommonUtils.escapeLogMessage(path));
+ }
parent = request.getResourceResolver().getResource(path);
}
if (parent != null) {
diff --git a/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembersOperationTest.java b/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembersOperationTest.java
index 518d525..eff9e87 100644
--- a/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembersOperationTest.java
+++ b/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembersOperationTest.java
@@ -19,7 +19,6 @@ package org.apache.sling.cms.core.internal.operations;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
@@ -27,6 +26,8 @@ import javax.jcr.AccessDeniedException;
import javax.jcr.RepositoryException;
import javax.jcr.UnsupportedRepositoryOperationException;
+import com.google.common.collect.ImmutableMap;
+
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.sling.cms.core.helpers.SlingCMSTestHelper;
@@ -38,8 +39,6 @@ import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mockito;
-import com.google.common.collect.ImmutableMap;
-
public class MembersOperationTest {
@Rule
@@ -82,10 +81,10 @@ public class MembersOperationTest {
assertEquals("/home/groups/sling-cms/authors", response.getPath());
- assertTrue(added.size() == 1);
+ assertEquals(1, added.size());
assertEquals("/home/users/test2", added.get(0));
- assertTrue(removed.size() == 1);
+ assertEquals(1, removed.size());
assertEquals("/home/users/test", removed.get(0));
}
diff --git a/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembershipOperationTest.java b/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembershipOperationTest.java
index a48188f..60ad713 100644
--- a/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembershipOperationTest.java
+++ b/core/src/test/java/org/apache/sling/cms/core/internal/operations/MembershipOperationTest.java
@@ -19,7 +19,6 @@ package org.apache.sling.cms.core.internal.operations;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
@@ -27,6 +26,8 @@ import javax.jcr.AccessDeniedException;
import javax.jcr.RepositoryException;
import javax.jcr.UnsupportedRepositoryOperationException;
+import com.google.common.collect.ImmutableMap;
+
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.sling.cms.core.helpers.SlingCMSTestHelper;
@@ -39,8 +40,6 @@ import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mockito;
-import com.google.common.collect.ImmutableMap;
-
public class MembershipOperationTest {
@Rule
@@ -84,10 +83,10 @@ public class MembershipOperationTest {
assertEquals("/home/users/test2", response.getPath());
- assertTrue(added.size() == 1);
+ assertEquals(1, added.size());
assertEquals("/home/users/test2", added.get(0));
- assertTrue(removed.size() == 0);
+ assertEquals(0, removed.size());
}
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/forms/FormConstants.java b/reference/src/main/java/org/apache/sling/cms/reference/forms/FormConstants.java
index ea556ae..2d9def6 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/forms/FormConstants.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/forms/FormConstants.java
@@ -18,6 +18,9 @@ package org.apache.sling.cms.reference.forms;
public class FormConstants {
+ private FormConstants() {
+ }
+
public static final String PATH_PROFILE = "profile";
public static final String PN_EMAIL = "email";
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/forms/FormUtils.java b/reference/src/main/java/org/apache/sling/cms/reference/forms/FormUtils.java
index 3e77430..72acb60 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/forms/FormUtils.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/forms/FormUtils.java
@@ -22,6 +22,9 @@ import org.apache.sling.api.resource.Resource;
public class FormUtils {
+ private FormUtils() {
+ }
+
public static final boolean handles(String[] supportedTypes, Resource resource) {
return Stream.of(supportedTypes).anyMatch(t -> t.equals(resource.getResourceType()));
}
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/models/ItemList.java b/reference/src/main/java/org/apache/sling/cms/reference/models/ItemList.java
index 26c8fb5..4ac7c34 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/models/ItemList.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/models/ItemList.java
@@ -85,7 +85,7 @@ public class ItemList {
log.debug("Using page {}", page);
} else {
page = 0;
- log.debug("Page {} not specified or not valid", request.getParameter("page"));
+ log.debug("Page not specified or not valid");
}
int l = Integer.parseInt(limit, 10);
diff --git a/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java b/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
index 6f053bb..b43e0e5 100644
--- a/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
+++ b/reference/src/main/java/org/apache/sling/cms/reference/models/Search.java
@@ -96,7 +96,7 @@ public class Search {
log.debug("Using page {}", page);
} else {
page = 0;
- log.debug("Page {} not specified or not valid", request.getParameter("page"));
+ log.debug("Page not specified or not valid");
}
if (page * limit >= count) {