You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by an...@apache.org on 2021/09/01 15:32:17 UTC
[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10778 :
*PolicyEntryHandler, *PolicyParser - sonar findings
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch SLING-10778
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit f244e427bdcae5594ecb5e189db5742038c03a81
Author: angela <an...@adobe.com>
AuthorDate: Wed Sep 1 17:32:03 2021 +0200
SLING-10778 : *PolicyEntryHandler, *PolicyParser - sonar findings
---
.../handlers/AbstractPolicyEntryHandler.java | 9 +++---
.../cpconverter/handlers/AbstractPolicyParser.java | 2 +-
.../handlers/RepPolicyEntryHandler.java | 37 ++++++++++++----------
.../handlers/RepPrincipalPolicyEntryHandler.java | 20 ++++++------
4 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyEntryHandler.java
index e7d4452..952cf1b 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyEntryHandler.java
@@ -20,7 +20,6 @@ import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.vault.fs.io.Archive;
import org.apache.jackrabbit.vault.util.PlatformNameFormat;
import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter;
-import org.apache.sling.feature.cpconverter.ConverterException;
import org.apache.sling.feature.cpconverter.accesscontrol.AclManager;
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
@@ -50,7 +49,7 @@ abstract class AbstractPolicyEntryHandler extends AbstractRegexEntryHandler {
@Override
public void handle(@NotNull String path, @NotNull Archive archive, @NotNull Archive.Entry entry, @NotNull ContentPackage2FeatureModelConverter converter)
- throws IOException, ConverterException {
+ throws IOException {
String resourcePath;
Matcher matcher = getPattern().matcher(path);
// we are pretty sure it matches, here
@@ -75,12 +74,12 @@ abstract class AbstractPolicyEntryHandler extends AbstractRegexEntryHandler {
AbstractPolicyParser policyParser = createPolicyParser(new RepoPath(PlatformNameFormat.getRepositoryPath(resourcePath)),
converter.getAclManager(),
handler);
- boolean hasRejectedAcls;
+ boolean hasRejectedNodes;
try (InputStream input = archive.openInputStream(entry)) {
- hasRejectedAcls = policyParser.parse(input);
+ hasRejectedNodes = policyParser.parse(input);
}
- if (hasRejectedAcls) {
+ if (hasRejectedNodes) {
try (Reader reader = new StringReader(stringWriter.toString());
OutputStreamWriter writer = new OutputStreamWriter(converter.getMainPackageAssembler().createEntry(path))) {
IOUtils.copy(reader, writer);
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
index 9cf4e4f..3395dc5 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractPolicyParser.java
@@ -50,7 +50,7 @@ abstract class AbstractPolicyParser extends AbstractJcrNodeParser<Boolean> {
// ACL processing result
boolean hasRejectedNodes = false;
- public AbstractPolicyParser(@NotNull String primaryType, @NotNull RepoPath repositoryPath, @NotNull AclManager aclManager, @NotNull TransformerHandler handler) {
+ AbstractPolicyParser(@NotNull String primaryType, @NotNull RepoPath repositoryPath, @NotNull AclManager aclManager, @NotNull TransformerHandler handler) {
super(primaryType);
this.handler = handler;
this.repositoryPath = repositoryPath;
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
index c87b65e..69ef44f 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
@@ -20,13 +20,14 @@ import org.apache.sling.feature.cpconverter.accesscontrol.AccessControlEntry;
import org.apache.sling.feature.cpconverter.accesscontrol.AclManager;
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
import org.xml.sax.Attributes;
import org.xml.sax.SAXException;
import javax.xml.transform.sax.TransformerHandler;
import java.util.HashMap;
+import java.util.LinkedList;
import java.util.Map;
-import java.util.Stack;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
@@ -56,10 +57,10 @@ public class RepPolicyEntryHandler extends AbstractPolicyEntryHandler {
operations.put(REP_DENY_ACE, false);
}
- private final Stack<AccessControlEntry> acls = new Stack<>();
+ private final LinkedList<AccessControlEntry> entries = new LinkedList<>();
// just internal pointer for every iteration
- private boolean processCurrentAcl = false;
+ private boolean processCurrentAce = false;
public RepPolicyParser(RepoPath repositoryPath, AclManager aclManager, TransformerHandler handler) {
super(REP_ACL, repositoryPath, aclManager, handler);
@@ -70,42 +71,44 @@ public class RepPolicyEntryHandler extends AbstractPolicyEntryHandler {
throws SAXException {
if (onRepAclNode) {
String primaryType = attributes.getValue(JCR_PRIMARYTYPE);
- if (REP_GRANT_ACE.equals(primaryType) || REP_DENY_ACE.equals(primaryType)) {
+ if (isAccessControlEntry(primaryType)) {
String principalName = attributes.getValue(REP_PRINCIPAL_NAME);
AccessControlEntry ace = createEntry(operations.get(primaryType), attributes);
// handle restrictions added in jr2 format (i.e. not located below rep:restrictions node)
addRestrictions(ace, attributes);
- processCurrentAcl = aclManager.addAcl(principalName, ace);
- if (processCurrentAcl) {
- acls.add(ace);
+ processCurrentAce = aclManager.addAcl(principalName, ace);
+ if (processCurrentAce) {
+ entries.add(ace);
} else {
hasRejectedNodes = true;
}
- } else if (REP_RESTRICTIONS.equals(primaryType) && !acls.isEmpty()) {
- if (processCurrentAcl) {
- AccessControlEntry ace = acls.peek();
- acls.add(ace);
- addRestrictions(ace, attributes);
- }
+ } else if (REP_RESTRICTIONS.equals(primaryType) && !entries.isEmpty() && processCurrentAce) {
+ AccessControlEntry ace = entries.peek();
+ entries.add(ace);
+ addRestrictions(ace, attributes);
}
} else {
super.startElement(uri, localName, qName, attributes);
}
- if (!onRepAclNode || !processCurrentAcl) {
+ if (!onRepAclNode || !processCurrentAce) {
handler.startElement(uri, localName, qName, attributes);
}
}
@Override
public void endElement(String uri, String localName, String qName) throws SAXException {
- if (onRepAclNode && processCurrentAcl && !acls.isEmpty()) {
- acls.pop();
+ if (onRepAclNode && processCurrentAce && !entries.isEmpty()) {
+ entries.pop();
} else {
- processCurrentAcl = false;
+ processCurrentAce = false;
handler.endElement(uri, localName, qName);
}
}
+
+ private static boolean isAccessControlEntry(@Nullable String primaryType) {
+ return REP_GRANT_ACE.equals(primaryType) || REP_DENY_ACE.equals(primaryType);
+ }
}
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
index 0634bbd..3881074 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandler.java
@@ -26,8 +26,8 @@ import org.xml.sax.Attributes;
import org.xml.sax.SAXException;
import javax.xml.transform.sax.TransformerHandler;
+import java.util.LinkedList;
import java.util.List;
-import java.util.Stack;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
@@ -54,9 +54,9 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
private static final String REP_EFFECTIVE_PATH = "rep:effectivePath";
- private final Stack<AccessControlEntry> aces = new Stack<>();
+ private final LinkedList<AccessControlEntry> aces = new LinkedList<>();
- private boolean processCurrentAcl = false;
+ private boolean processCurrentAce = false;
private String principalName = null;
@@ -83,15 +83,15 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
RepoPath effectivePath = new RepoPath(extractEffectivePath(attributes.getValue(REP_EFFECTIVE_PATH)));
AccessControlEntry ace = new AccessControlEntry(true, privileges, effectivePath, true);
- // NOTE: nt-definition doesn't allow for jr2-stype restrictions defined right below the entry.
+ // NOTE: nt-definition doesn't allow for jr2-type restrictions defined right below the entry.
// instead always requires rep:restrictions child node
- processCurrentAcl = aclManager.addAcl(principalName, ace);
- if (processCurrentAcl) {
+ processCurrentAce = aclManager.addAcl(principalName, ace);
+ if (processCurrentAce) {
aces.add(ace);
} else {
hasRejectedNodes = true;
}
- } else if (REP_RESTRICTIONS.equals(primaryType) && !aces.isEmpty() && processCurrentAcl) {
+ } else if (REP_RESTRICTIONS.equals(primaryType) && !aces.isEmpty() && processCurrentAce) {
AccessControlEntry ace = aces.peek();
aces.add(ace);
addRestrictions(ace, attributes);
@@ -100,17 +100,17 @@ public final class RepPrincipalPolicyEntryHandler extends AbstractPolicyEntryHan
super.startElement(uri, localName, qName, attributes);
}
- if (!onRepAclNode || !processCurrentAcl) {
+ if (!onRepAclNode || !processCurrentAce) {
handler.startElement(uri, localName, qName, attributes);
}
}
@Override
public void endElement(String uri, String localName, String qName) throws SAXException {
- if (onRepAclNode && processCurrentAcl && !aces.isEmpty()) {
+ if (onRepAclNode && processCurrentAce && !aces.isEmpty()) {
aces.pop();
} else {
- processCurrentAcl = false;
+ processCurrentAce = false;
principalName = null;
handler.endElement(uri, localName, qName);
}