You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by si...@apache.org on 2019/05/09 12:48:16 UTC

[sling-org-apache-sling-feature-cpconverter] branch master updated (c9c641f -> 43f883f)

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

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


    from c9c641f  make sure destination dir is created if it does not exist
     new 59f03c6  added check when content-package(s) path(s) point to a wrong location added re-ordering log minor output format
     new 5100843  returning a generic collection to avoid creating a list on a set
     new 43f883f  SLING-8390 - Converter not handling serviceusers and acls spread across multiple packages

The 3 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.


Summary of changes:
 .../apache/sling/feature/cpconverter/acl/Acl.java  |  20 ++--
 .../feature/cpconverter/acl/DefaultAclManager.java | 112 +++++++++++++--------
 ...ntentPackage2FeatureModelConverterLauncher.java |  20 ++--
 .../feature/cpconverter/cli/ShutDownHook.java      |   2 -
 ...tPackage2FeatureModelConverterLauncherTest.java |   9 +-
 5 files changed, 100 insertions(+), 63 deletions(-)


[sling-org-apache-sling-feature-cpconverter] 03/03: SLING-8390 - Converter not handling serviceusers and acls spread across multiple packages

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

simonetripodi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git

commit 43f883fafd699e40e18d2780075480329e148cb8
Author: stripodi <st...@simos-mbp>
AuthorDate: Thu May 9 14:48:07 2019 +0200

    SLING-8390 - Converter not handling serviceusers and acls spread across
    multiple packages
    
    improved paths management
---
 .../apache/sling/feature/cpconverter/acl/Acl.java  |  20 ++--
 .../feature/cpconverter/acl/DefaultAclManager.java | 112 +++++++++++++--------
 2 files changed, 84 insertions(+), 48 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/acl/Acl.java b/src/main/java/org/apache/sling/feature/cpconverter/acl/Acl.java
index 0f483ff..0e21ead 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/acl/Acl.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/acl/Acl.java
@@ -16,10 +16,8 @@
  */
 package org.apache.sling.feature.cpconverter.acl;
 
-import java.util.Formatter;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.stream.Collectors;
 
 /**
  * Simple single ACL statement representation.
@@ -46,14 +44,20 @@ public final class Acl {
         }
     }
 
-    protected void addAclStatement(Formatter formatter) {
-        formatter.format("%s %s on %s", operation, privileges, path);
+    public String getOperation() {
+        return operation;
+    }
 
-        if (!restrictions.isEmpty()) {
-            formatter.format(" restriction(%s)", restrictions.stream().collect(Collectors.joining(",")));
-        }
+    public String getPrivileges() {
+        return privileges;
+    }
+
+    public String getPath() {
+        return path;
+    }
 
-        formatter.format("%n");
+    public List<String> getRestrictions() {
+        return restrictions;
     }
 
 }
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
index d6f50eb..f05164c 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
@@ -20,13 +20,15 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.util.Formatter;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.Map.Entry;
+import java.util.stream.Collectors;
 
 import org.apache.sling.feature.Extension;
 import org.apache.sling.feature.ExtensionType;
@@ -41,9 +43,9 @@ public final class DefaultAclManager implements AclManager {
 
     private final Set<String> preProvidedSystemUsers = new LinkedHashSet<>();
 
-    private final Set<String> systemUsers = new LinkedHashSet<>();
+    private final Set<String> preProvidedPaths = new HashSet<String>();
 
-    private final Set<String> paths = new TreeSet<String>();
+    private final Set<String> systemUsers = new LinkedHashSet<>();
 
     private final Map<String, List<Acl>> acls = new HashMap<>();
 
@@ -55,19 +57,19 @@ public final class DefaultAclManager implements AclManager {
     }
 
     public Acl addAcl(String systemUser, String operation, String privileges, String path) {
-        addPath(path);
-
         Acl acl = new Acl(operation, privileges, path);
         acls.computeIfAbsent(systemUser, k -> new LinkedList<>()).add(acl);
         return acl;
     }
 
-    private void addPath(String path) {
-        paths.add(path);
+    private void addPath(String path, Set<String> paths) {
+        if (preProvidedPaths.add(path)) {
+            paths.add(path);
+        }
 
         int endIndex = path.lastIndexOf('/');
         if (endIndex > 0) {
-            addPath(path.substring(0, endIndex));
+            addPath(path.substring(0, endIndex), paths);
         }
     }
 
@@ -82,35 +84,18 @@ public final class DefaultAclManager implements AclManager {
 
             formatter = new Formatter();
 
-            // make sure all paths are created first
-
-            for (String path : paths) {
-                File currentDir = packageAssembler.getEntry(path);
-                String type = DEFAULT_TYPE;
-
-                if (currentDir.exists()) {
-                    File currentContent = new File(currentDir, CONTENT_XML_FILE_NAME);
-                    if (currentContent.exists()) {
-                        try (FileInputStream input = new FileInputStream(currentContent)) {
-                            type = new PrimaryTypeParser(DEFAULT_TYPE).parse(input);
-                        } catch (Exception e) {
-                            throw new RuntimeException("A fatal error occurred while parsing the '"
-                                + currentContent
-                                + "' file, see nested exceptions: "
-                                + e);
-                        }
-                    }
-                }
+            for (String systemUser : systemUsers) {
+                List<Acl> authorizations = acls.remove(systemUser);
 
-                formatter.format("create path (%s) %s%n", type, path);
-            }
+                // make sure all paths are created first
 
-            // create then the users
+                addPaths(authorizations, packageAssembler, formatter);
+
+                // create then the users
 
-            for (String systemUser : systemUsers) {
                 formatter.format("create service user %s%n", systemUser);
 
-                List<Acl> authorizations = acls.remove(systemUser);
+                // finally add ACLs
 
                 addAclStatement(formatter, systemUser, authorizations);
             }
@@ -122,7 +107,7 @@ public final class DefaultAclManager implements AclManager {
 
                 if (preProvidedSystemUsers.contains(systemUser)) {
                     List<Acl> authorizations = currentAcls.getValue();
-
+                    addPaths(authorizations, packageAssembler, formatter);
                     addAclStatement(formatter, systemUser, authorizations);
                 }
             }
@@ -140,20 +125,67 @@ public final class DefaultAclManager implements AclManager {
 
     public void reset() {
         systemUsers.clear();
-        paths.clear();
         acls.clear();
     }
 
-    private void addAclStatement(Formatter formatter, String systemUser, List<Acl> authorizations) {
-        if (authorizations != null && !authorizations.isEmpty()) {
-            formatter.format("set ACL for %s%n", systemUser);
+    private void addPaths(List<Acl> authorizations, VaultPackageAssembler packageAssembler, Formatter formatter) {
+        if (areEmpty(authorizations)) {
+            return;
+        }
+
+        Set<String> paths = new TreeSet<String>();
+        for (Acl authorization : authorizations) {
+            addPath(authorization.getPath(), paths);
+        }
+
+        for (String path : paths) {
+            File currentDir = packageAssembler.getEntry(path);
+            String type = DEFAULT_TYPE;
+
+            if (currentDir.exists()) {
+                File currentContent = new File(currentDir, CONTENT_XML_FILE_NAME);
+                if (currentContent.exists()) {
+                    try (FileInputStream input = new FileInputStream(currentContent)) {
+                        type = new PrimaryTypeParser(DEFAULT_TYPE).parse(input);
+                    } catch (Exception e) {
+                        throw new RuntimeException("A fatal error occurred while parsing the '"
+                            + currentContent
+                            + "' file, see nested exceptions: "
+                            + e);
+                    }
+                }
+            }
+
+            formatter.format("create path (%s) %s%n", type, path);
+        }
+    }
+
+    private static void addAclStatement(Formatter formatter, String systemUser, List<Acl> authorizations) {
+        if (areEmpty(authorizations)) {
+            return;
+        }
+
+        formatter.format("set ACL for %s%n", systemUser);
 
-            for (Acl authorization : authorizations) {
-                authorization.addAclStatement(formatter);
+        for (Acl authorization : authorizations) {
+            formatter.format("%s %s on %s",
+                             authorization.getOperation(),
+                             authorization.getPrivileges(),
+                             authorization.getPath());
+
+            if (!authorization.getRestrictions().isEmpty()) {
+                formatter.format(" restriction(%s)",
+                                 authorization.getRestrictions().stream().collect(Collectors.joining(",")));
             }
 
-            formatter.format("end%n");
+            formatter.format("%n");
         }
+
+        formatter.format("end%n");
+    }
+
+    private static boolean areEmpty(List<Acl> authorizations) {
+        return authorizations == null || authorizations.isEmpty();
     }
 
 }


[sling-org-apache-sling-feature-cpconverter] 01/03: added check when content-package(s) path(s) point to a wrong location added re-ordering log minor output format

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

simonetripodi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git

commit 59f03c608bdf59848c62e0e7712710f29d68ef62
Author: stripodi <st...@simos-mbp>
AuthorDate: Thu May 9 13:02:23 2019 +0200

    added check when content-package(s) path(s) point to a wrong location
    added re-ordering log
    minor output format
---
 .../ContentPackage2FeatureModelConverterLauncher.java    | 16 +++++++++++-----
 .../sling/feature/cpconverter/cli/ShutDownHook.java      |  2 --
 ...ContentPackage2FeatureModelConverterLauncherTest.java |  8 ++------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
index fd5ef42..ca6d468 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
@@ -139,7 +139,11 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
                 converter.setResourceFilter(filter);
             }
 
-            List<File> orderedContentPackages = order(contentPackages, logger);
+            logger.info("Ordering input content-package(s) {}...", contentPackages);
+
+            List<File> orderedContentPackages = order(contentPackages);
+
+            logger.info("New content-package(s) order: {}", orderedContentPackages);
 
             for (File contentPackage : orderedContentPackages) {
                 converter.convert(contentPackage);
@@ -158,20 +162,22 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
                 logger.error("Unable to convert content-package {}: {}", contentPackages, t.getMessage());
             }
 
-            logger.info( "" );
+            logger.info( "+-----------------------------------------------------+" );
 
             System.exit(1);
         }
-
-        logger.info( "+-----------------------------------------------------+" );
     }
 
-    protected List<File> order(List<File> contentPackages, final Logger logger) throws Exception {
+    protected List<File> order(List<File> contentPackages) throws Exception {
         Map<PackageId, File> idFileMap = new LinkedHashMap<>();
         Map<ZipVaultPackage, File> packageFileMapping = new HashMap<>();
         Map<PackageId, ZipVaultPackage> idPackageMapping = new HashMap<>();
 
         for (File file : contentPackages) {
+            if (!file.exists() || file.isDirectory()) {
+                throw new Exception("File " + file + " does not exist or it is a directory");
+            }
+
             try {
                 ZipVaultPackage pack = new ZipVaultPackage(file, false, true);
                 packageFileMapping.put(pack, file);
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java b/src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
index a1253b0..e56629b 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
@@ -33,8 +33,6 @@ final class ShutDownHook extends Thread {
 
     @Override
     public void run() {
-        logger.info("");
-
         // format the uptime string
         Formatter uptimeFormatter = new Formatter();
         uptimeFormatter.format("Total time:");
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java b/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
index d269c4c..6bfd042 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
@@ -27,8 +27,6 @@ import java.util.List;
 import org.apache.commons.io.FileUtils;
 import org.apache.jackrabbit.vault.packaging.CyclicDependencyException;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public class ContentPackage2FeatureModelConverterLauncherTest {
 
@@ -49,14 +47,13 @@ public class ContentPackage2FeatureModelConverterLauncherTest {
     @Test
     public void testPackageOrdering() throws Exception {
         ContentPackage2FeatureModelConverterLauncher launcher = new ContentPackage2FeatureModelConverterLauncher();
-        Logger logger = LoggerFactory.getLogger("test");
         List<File> contentPackages = new ArrayList<File>();
 
         for (String pkgName : TEST_PACKAGES_INPUT) {
             URL packageUrl = getClass().getResource(pkgName);
             contentPackages.add(FileUtils.toFile(packageUrl));
         }
-        List<File> ordered = launcher.order(contentPackages, logger);
+        List<File> ordered = launcher.order(contentPackages);
         Iterator<File> fileIt = ordered.iterator();
         for (String expected : TEST_PACKAGES_OUTPUT) {
             File next = fileIt.next();
@@ -67,14 +64,13 @@ public class ContentPackage2FeatureModelConverterLauncherTest {
     @Test(expected = CyclicDependencyException.class)
     public void testDependencyCycle() throws Exception {
         ContentPackage2FeatureModelConverterLauncher launcher = new ContentPackage2FeatureModelConverterLauncher();
-        Logger logger = LoggerFactory.getLogger("test");
         List<File> contentPackages = new ArrayList<File>();
 
         for (String pkgName : TEST_PACKAGES_CYCLIC_DEPENDENCY) {
             URL packageUrl = getClass().getResource(pkgName);
             contentPackages.add(FileUtils.toFile(packageUrl));
         }
-        launcher.order(contentPackages, logger);
+        launcher.order(contentPackages);
     }
 
 }


[sling-org-apache-sling-feature-cpconverter] 02/03: returning a generic collection to avoid creating a list on a set

Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

simonetripodi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git

commit 5100843d36c93cd832f4d52c2d2e5f9f83d64d34
Author: stripodi <st...@simos-mbp>
AuthorDate: Thu May 9 14:32:45 2019 +0200

    returning a generic collection to avoid creating a list on a set
---
 .../cli/ContentPackage2FeatureModelConverterLauncher.java         | 8 ++++----
 .../cli/ContentPackage2FeatureModelConverterLauncherTest.java     | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
index ca6d468..71f7755 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
@@ -18,10 +18,10 @@ package org.apache.sling.feature.cpconverter.cli;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -141,7 +141,7 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
 
             logger.info("Ordering input content-package(s) {}...", contentPackages);
 
-            List<File> orderedContentPackages = order(contentPackages);
+            Collection<File> orderedContentPackages = order(contentPackages);
 
             logger.info("New content-package(s) order: {}", orderedContentPackages);
 
@@ -168,7 +168,7 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
         }
     }
 
-    protected List<File> order(List<File> contentPackages) throws Exception {
+    protected Collection<File> order(List<File> contentPackages) throws Exception {
         Map<PackageId, File> idFileMap = new LinkedHashMap<>();
         Map<ZipVaultPackage, File> packageFileMapping = new HashMap<>();
         Map<PackageId, ZipVaultPackage> idPackageMapping = new HashMap<>();
@@ -192,7 +192,7 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
             orderDependencies(idFileMap, packageFileMapping, idPackageMapping, pack, new HashSet<PackageId>());
         }
 
-        return new LinkedList<>(idFileMap.values());
+        return idFileMap.values();
     }
 
     private void orderDependencies(Map<PackageId, File> idFileMap,
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java b/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
index 6bfd042..d65bcbf 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncherTest.java
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals;
 import java.io.File;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 
@@ -53,7 +54,7 @@ public class ContentPackage2FeatureModelConverterLauncherTest {
             URL packageUrl = getClass().getResource(pkgName);
             contentPackages.add(FileUtils.toFile(packageUrl));
         }
-        List<File> ordered = launcher.order(contentPackages);
+        Collection<File> ordered = launcher.order(contentPackages);
         Iterator<File> fileIt = ordered.iterator();
         for (String expected : TEST_PACKAGES_OUTPUT) {
             File next = fileIt.next();