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:16 UTC

[sling-org-apache-sling-feature-cpconverter] branch SLING-10778 created (now f244e42)

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

angela pushed a change to branch SLING-10778
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git.


      at f244e42  SLING-10778 : *PolicyEntryHandler, *PolicyParser - sonar findings

This branch includes the following new commits:

     new f244e42  SLING-10778 : *PolicyEntryHandler, *PolicyParser - sonar findings

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10778 : *PolicyEntryHandler, *PolicyParser - sonar findings

Posted by an...@apache.org.
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);
             }