You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/06/10 12:40:32 UTC

[sling-org-apache-sling-feature-cpconverter] branch master updated: SLING-10477 : Review usage of UserEntryHandler in RecollectorVaultPackageScanner (#91)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 84d4de7  SLING-10477 : Review usage of UserEntryHandler in RecollectorVaultPackageScanner (#91)
84d4de7 is described below

commit 84d4de7738883a01a3c4619aac9c36406dcdd736
Author: anchela <an...@adobe.com>
AuthorDate: Thu Jun 10 14:38:50 2021 +0200

    SLING-10477 : Review usage of UserEntryHandler in RecollectorVaultPackageScanner (#91)
    
    
    
    Co-authored-by: Karl Pauls <ka...@gmail.com>
---
 .../handlers/AbstractUserEntryHandler.java         |   4 +-
 .../vltpkg/RecollectorVaultPackageScanner.java     |   2 +
 .../feature/cpconverter/AbstractConverterTest.java | 174 ++++++++++++++++
 .../ContentPackage2FeatureModelConverterTest.java  | 199 +-----------------
 .../ConverterUserAndPermissionTest.java            | 223 +++++++++++++++++++++
 .../apache/sling/feature/cpconverter/demo-cp2.zip  | Bin 0 -> 14913 bytes
 .../apache/sling/feature/cpconverter/demo-cp3.zip  | Bin 0 -> 16253 bytes
 7 files changed, 409 insertions(+), 193 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserEntryHandler.java
index a162bdb..b34871d 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserEntryHandler.java
@@ -54,10 +54,10 @@ abstract class AbstractUserEntryHandler extends AbstractRegexEntryHandler {
                 // write back regular users, groups and their intermediate folders that did not get converted into
                 // repo-init statements to the content package
                 VaultPackageAssembler assembler = converter.getMainPackageAssembler();
-                // FIXME: assembler is null when handler is called from RecollectorVaultPackageScanner (???)
+                // if we don't have an assembler we are in the firstPass and are only collecting
                 if (assembler != null) {
                     try (InputStream input = new ByteArrayInputStream(tmp);
-                         OutputStream output = assembler.createEntry(path)){
+                         OutputStream output = assembler.createEntry(path)) {
                         IOUtils.copy(input, output);
                     }
                 }
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java
index b8eff4d..ac7fe0f 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java
@@ -24,6 +24,7 @@ import org.apache.jackrabbit.vault.packaging.PackageId;
 import org.apache.jackrabbit.vault.packaging.PackageManager;
 import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter;
 import org.apache.sling.feature.cpconverter.handlers.EntryHandler;
+import org.apache.sling.feature.cpconverter.handlers.GroupEntryHandler;
 import org.apache.sling.feature.cpconverter.handlers.UsersEntryHandler;
 import org.apache.sling.feature.cpconverter.handlers.VersionResolverContentPackageEntryHandler;
 import org.jetbrains.annotations.NotNull;
@@ -42,6 +43,7 @@ public final class RecollectorVaultPackageScanner extends BaseVaultPackageScanne
         this.converter = converter;
         handlers = new EntryHandler[] {
                 new UsersEntryHandler(),
+                new GroupEntryHandler(),
                 new VersionResolverContentPackageEntryHandler(this, subContentPackages)
         };
     }
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/AbstractConverterTest.java b/src/test/java/org/apache/sling/feature/cpconverter/AbstractConverterTest.java
new file mode 100644
index 0000000..6b23576
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/cpconverter/AbstractConverterTest.java
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.feature.cpconverter;
+
+import org.apache.jackrabbit.vault.util.Constants;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Artifacts;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.io.json.FeatureJSONReader;
+import org.jetbrains.annotations.NotNull;
+
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+import java.util.StringTokenizer;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public abstract class AbstractConverterTest {
+
+    static void deleteDirTree(File dir) throws IOException {
+        Path tempDir = dir.toPath();
+
+        Files.walk(tempDir)
+                .sorted(Comparator.reverseOrder())
+                .map(Path::toFile)
+                .forEach(File::delete);
+    }
+
+    static Feature getFeature(File outputDirectory, String name) throws Exception {
+        File featureFile = new File(outputDirectory, name);
+        assertTrue(featureFile + " was not correctly created", featureFile.exists());
+
+        try (Reader reader = new FileReader(featureFile)) {
+            return FeatureJSONReader.read(reader, featureFile.getAbsolutePath());
+        }
+    }
+
+    static void verifyFeatureFile(File outputDirectory,
+                                  String name,
+                                  String expectedArtifactId,
+                                  List<String> expectedArtifacts,
+                                  List<String> expectedConfigurations,
+                                  List<String> expectedContentPackagesExtensions) throws Exception {
+        Feature feature = getFeature(outputDirectory, name);
+
+        assertEquals(expectedArtifactId, feature.getId().toMvnId());
+
+        for (String expectedArtifact : expectedArtifacts) {
+            assertTrue(expectedArtifact + " not found in Feature " + expectedArtifactId, feature.getBundles().containsExact(ArtifactId.fromMvnId(expectedArtifact)));
+            verifyInstalledArtifact(outputDirectory, expectedArtifact);
+        }
+
+        for (String expectedConfiguration : expectedConfigurations) {
+            assertNotNull(expectedConfiguration + " not found in Feature " + expectedArtifactId, feature.getConfigurations().getConfiguration(expectedConfiguration));
+        }
+
+        if (expectedContentPackagesExtensions.size() > 0) {
+            Artifacts contentPackages = feature.getExtensions().getByName("content-packages").getArtifacts();
+            assertEquals(expectedContentPackagesExtensions.size(), contentPackages.size());
+
+            for (String expectedContentPackagesExtension : expectedContentPackagesExtensions) {
+                assertTrue(expectedContentPackagesExtension + " not found in Feature " + expectedArtifactId,
+                        contentPackages.containsExact(ArtifactId.fromMvnId(expectedContentPackagesExtension)));
+                verifyInstalledArtifact(outputDirectory, expectedContentPackagesExtension);
+            }
+        }
+    }
+
+    static void verifyContentPackageEntryNames(File contentPackage, String...expectedEntryNames) throws Exception {
+        try (ZipFile zipFile = new ZipFile(contentPackage)) {
+            List<String> expectedEntryNamesList = Arrays.asList(expectedEntryNames);
+            for (Enumeration<? extends ZipEntry> zipEntries = zipFile.entries(); zipEntries.hasMoreElements();) {
+                ZipEntry zipEntry = zipEntries.nextElement();
+                String entryName = zipEntry.getName();
+                if (!entryName.endsWith("/")) {
+                    assertTrue("ZipEntry with name " + entryName + " not expected", expectedEntryNamesList.contains(entryName));
+                }
+            }
+        }
+    }
+
+    static void verifyContentPackage(File contentPackage, String...expectedEntries) throws Exception {
+        verifyContentPackage(contentPackage, Collections.emptySet(), Arrays.asList(expectedEntries));
+    }
+
+    static void verifyContentPackage(File contentPackage, @NotNull Iterable<String> notExpected, @NotNull Iterable<String> expectedEntries) throws Exception {
+        try (ZipFile zipFile = new ZipFile(contentPackage)) {
+            for (String expectedEntry : expectedEntries) {
+                assertNotNull("Expected entry not found: " + expectedEntry + " in file " + contentPackage, zipFile.getEntry(expectedEntry));
+            }
+            for (String notExpectedEntry : notExpected) {
+                assertNull("Not expected entry found: " + notExpectedEntry + " in file " + contentPackage, zipFile.getEntry(notExpectedEntry));
+            }
+        }
+    }
+
+    static void verifyPropertiesXmlEntry(File contentPackage, String... expectedPropertyKeys) throws IOException {
+        try (ZipFile zipFile = new ZipFile(contentPackage)) {
+            ZipEntry zipEntry = zipFile.getEntry(Constants.META_DIR + "/" + Constants.PROPERTIES_XML);
+            assertNotNull("Package didn't contain properties.xml", zipEntry);
+            try (InputStream inputStream = zipFile.getInputStream(zipEntry)) {
+                Properties properties = new Properties();
+                properties.loadFromXML(inputStream);
+                for (String expectedPropertyKey : expectedPropertyKeys) {
+                    if (expectedPropertyKey.startsWith("!")) {
+                        String key = expectedPropertyKey.substring(1);
+                        assertFalse("Properties.xml was not supposed to contain key " +  key + " but it does", properties.containsKey(key));
+                    } else {
+                        assertTrue("Properties.xml was supposed to contain key " +  expectedPropertyKey + " but it does not", properties.containsKey(expectedPropertyKey));
+                    }
+                }
+            }
+        }
+    }
+
+    static void verifyInstalledArtifact(File outputDirectory, String coordinates) {
+        ArtifactId bundleId = ArtifactId.fromMvnId(coordinates);
+
+        StringTokenizer tokenizer = new StringTokenizer(bundleId.getGroupId(), ".");
+        while (tokenizer.hasMoreTokens()) {
+            outputDirectory = new File(outputDirectory, tokenizer.nextToken());
+        }
+
+        outputDirectory = new File(outputDirectory, bundleId.getArtifactId());
+        outputDirectory = new File(outputDirectory, bundleId.getVersion());
+
+        StringBuilder bundleFileName = new StringBuilder()
+                .append(bundleId.getArtifactId())
+                .append('-')
+                .append(bundleId.getVersion());
+        if (bundleId.getClassifier() != null) {
+            bundleFileName.append('-').append(bundleId.getClassifier());
+        }
+        bundleFileName.append('.').append(bundleId.getType());
+
+        File bundleFile = new File(outputDirectory, bundleFileName.toString());
+        assertTrue("Bundle " + bundleFile + " does not exist", bundleFile.exists());
+
+        File pomFile = new File(outputDirectory, String.format("%s-%s.pom", bundleId.getArtifactId(), bundleId.getVersion()));
+        assertTrue("POM file " + pomFile + " does not exist", pomFile.exists());
+    }
+}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java b/src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
index ed67aaa..5bdd248 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverterTest.java
@@ -29,13 +29,13 @@ import static org.mockito.Mockito.verify;
 
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
 import java.io.StringReader;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
@@ -47,6 +47,7 @@ import javax.json.JsonArray;
 import javax.json.JsonObject;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.vault.fs.spi.PrivilegeDefinitions;
 import org.apache.jackrabbit.vault.packaging.CyclicDependencyException;
 import org.apache.jackrabbit.vault.packaging.PackageId;
@@ -73,14 +74,14 @@ import org.apache.sling.feature.cpconverter.shared.ConverterConstants;
 import org.apache.sling.feature.cpconverter.vltpkg.DefaultPackagesEventsEmitter;
 import org.apache.sling.feature.io.json.FeatureJSONReader;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
-@RunWith(Parameterized.class)
-public class ContentPackage2FeatureModelConverterTest {
+public class ContentPackage2FeatureModelConverterTest extends AbstractConverterTest {
 
     /**
      * Test package A-1.0. Depends on B and C-1.X
@@ -98,23 +99,10 @@ public class ContentPackage2FeatureModelConverterTest {
 
     private ContentPackage2FeatureModelConverter converter;
     private EntryHandlersManager handlersManager;
-    
-    private final String systemUserRelPath;
-    
-    @Parameterized.Parameters(name = "name={1}")
-    public static Collection<Object[]> parameters() {
-        return Arrays.asList(
-                new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, "Default system user rel-path"},
-                new Object[] { "system/cq:services", "Modified system user rel-path"});
-    }
-    
-    public ContentPackage2FeatureModelConverterTest(@NotNull String systemUserRelPath, @NotNull String name) {
-        this.systemUserRelPath = systemUserRelPath;
-    }
         
     @Before
     public void setUp() {
-        handlersManager = new DefaultEntryHandlersManager(Collections.emptyMap(), false, ContentPackage2FeatureModelConverter.SlingInitialContentPolicy.KEEP, systemUserRelPath);
+        handlersManager = new DefaultEntryHandlersManager();
         converter = new ContentPackage2FeatureModelConverter()
                     .setEntryHandlersManager(handlersManager)
                     .setAclManager(new DefaultAclManager());
@@ -509,131 +497,6 @@ public class ContentPackage2FeatureModelConverterTest {
         }
     }
 
-    private void deleteDirTree(File dir) throws IOException {
-        Path tempDir = dir.toPath();
-
-        Files.walk(tempDir)
-            .sorted(Comparator.reverseOrder())
-            .map(Path::toFile)
-            .forEach(File::delete);
-    }
-
-    private Feature getFeature(File outputDirectory, String name) throws Exception {
-        File featureFile = new File(outputDirectory, name);
-        assertTrue(featureFile + " was not correctly created", featureFile.exists());
-
-        try (Reader reader = new FileReader(featureFile)) {
-            return FeatureJSONReader.read(reader, featureFile.getAbsolutePath());
-        }
-    }
-
-    private void verifyFeatureFile(File outputDirectory,
-                                   String name,
-                                   String expectedArtifactId,
-                                   List<String> expectedArtifacts,
-                                   List<String> expectedConfigurations,
-                                   List<String> expectedContentPackagesExtensions) throws Exception {
-        Feature feature = getFeature(outputDirectory, name);
-
-        assertEquals(expectedArtifactId, feature.getId().toMvnId());
-
-        for (String expectedArtifact : expectedArtifacts) {
-            assertTrue(expectedArtifact + " not found in Feature " + expectedArtifactId, feature.getBundles().containsExact(ArtifactId.fromMvnId(expectedArtifact)));
-            verifyInstalledArtifact(outputDirectory, expectedArtifact);
-        }
-
-        for (String expectedConfiguration : expectedConfigurations) {
-            assertNotNull(expectedConfiguration + " not found in Feature " + expectedArtifactId, feature.getConfigurations().getConfiguration(expectedConfiguration));
-        }
-
-        if (expectedContentPackagesExtensions.size() > 0) {
-        Artifacts contentPackages = feature.getExtensions().getByName("content-packages").getArtifacts();
-        assertEquals(expectedContentPackagesExtensions.size(), contentPackages.size());
-
-            for (String expectedContentPackagesExtension : expectedContentPackagesExtensions) {
-                assertTrue(expectedContentPackagesExtension + " not found in Feature " + expectedArtifactId,
-                    contentPackages.containsExact(ArtifactId.fromMvnId(expectedContentPackagesExtension)));
-                verifyInstalledArtifact(outputDirectory, expectedContentPackagesExtension);
-            }
-        }
-    }
-
-    private void verifyInstalledArtifact(File outputDirectory, String coordinates) {
-        ArtifactId bundleId = ArtifactId.fromMvnId(coordinates);
-
-        StringTokenizer tokenizer = new StringTokenizer(bundleId.getGroupId(), ".");
-        while (tokenizer.hasMoreTokens()) {
-            outputDirectory = new File(outputDirectory, tokenizer.nextToken());
-        }
-
-        outputDirectory = new File(outputDirectory, bundleId.getArtifactId());
-        outputDirectory = new File(outputDirectory, bundleId.getVersion());
-
-        StringBuilder bundleFileName = new StringBuilder()
-                                       .append(bundleId.getArtifactId())
-                                       .append('-')
-                                       .append(bundleId.getVersion());
-        if (bundleId.getClassifier() != null) {
-            bundleFileName.append('-').append(bundleId.getClassifier());
-        }
-        bundleFileName.append('.').append(bundleId.getType());
-
-        File bundleFile = new File(outputDirectory, bundleFileName.toString());
-        assertTrue("Bundle " + bundleFile + " does not exist", bundleFile.exists());
-
-        File pomFile = new File(outputDirectory, String.format("%s-%s.pom", bundleId.getArtifactId(), bundleId.getVersion()));
-        assertTrue("POM file " + pomFile + " does not exist", pomFile.exists());
-    }
-
-    private void verifyContentPackageEntryNames(File contentPackage, String...expectedEntryNames) throws Exception {
-        try (ZipFile zipFile = new ZipFile(contentPackage)) {
-            
-            List<String> expectedEntryNamesList = Arrays.asList(expectedEntryNames);
-            for (Enumeration<? extends ZipEntry> zipEntries = zipFile.entries(); zipEntries.hasMoreElements();) {
-                ZipEntry zipEntry = zipEntries.nextElement();
-                String entryName = zipEntry.getName();
-                if (!entryName.endsWith("/")) {
-                    assertTrue("ZipEntry with name " + entryName + " not expected", expectedEntryNamesList.contains(entryName));
-                }
-            }
-            
-        }
-    }
-
-    private void verifyContentPackage(File contentPackage, String...expectedEntries) throws Exception {
-        verifyContentPackage(contentPackage, Collections.emptySet(), expectedEntries);
-    }
-    
-    private void verifyContentPackage(File contentPackage, @NotNull Set<String> notExpected, String...expectedEntries) throws Exception {
-        try (ZipFile zipFile = new ZipFile(contentPackage)) {
-            for (String expectedEntry : expectedEntries) {
-                assertNotNull("Expected entry not found: " + expectedEntry + " in file " + contentPackage, zipFile.getEntry(expectedEntry));
-            }
-            for (String notExpectedEntry : notExpected) {
-                assertNull("Not expected entry found: " + notExpectedEntry + " in file " + contentPackage, zipFile.getEntry(notExpectedEntry));
-            }
-        }
-    }
-
-    private void verifyPropertiesXmlEntry(File contentPackage, String... expectedPropertyKeys) throws InvalidPropertiesFormatException, IOException {
-        try (ZipFile zipFile = new ZipFile(contentPackage)) {
-            ZipEntry zipEntry = zipFile.getEntry(Constants.META_DIR + "/" + Constants.PROPERTIES_XML);
-            assertNotNull("Package didn't contain properties.xml", zipEntry);
-            try (InputStream inputStream = zipFile.getInputStream(zipEntry)) {
-                Properties properties = new Properties();
-                properties.loadFromXML(inputStream);
-                for (String expectedPropertyKey : expectedPropertyKeys) {
-                    if (expectedPropertyKey.startsWith("!")) {
-                        String key = expectedPropertyKey.substring(1);
-                        assertFalse("Properties.xml was not supposed to contain key " +  key + " but it does", properties.containsKey(key));
-                    } else {
-                        assertTrue("Properties.xml was supposed to contain key " +  expectedPropertyKey + " but it does not", properties.containsKey(expectedPropertyKey));
-                    }
-                }
-            }
-        }
-    }
-
     @Test(expected = IllegalArgumentException.class)
     public void verifyFilteringOutUndesiredPackages() throws Exception {
         RegexBasedResourceFilter resourceFilter = new RegexBasedResourceFilter();
@@ -707,8 +570,8 @@ public class ContentPackage2FeatureModelConverterTest {
                      .convert(packageFile);
     
             String pid = "this.is.just.a.pid";
-            converter.getFeaturesManager().addConfiguration(runmodeA, pid, "/a", new Hashtable<String, Object>());
-            converter.getFeaturesManager().addConfiguration(runmodeB, pid, "/b", new Hashtable<String, Object>());
+            converter.getFeaturesManager().addConfiguration(runmodeA, pid, "/a", new Hashtable<>());
+            converter.getFeaturesManager().addConfiguration(runmodeB, pid, "/b", new Hashtable<>());
     
         } finally {
             deleteDirTree(outputDirectory);
@@ -786,7 +649,7 @@ public class ContentPackage2FeatureModelConverterTest {
 
     }
 
-    private void assertAPIRegion(File featureFile, String region) throws IOException, FileNotFoundException {
+    private static void assertAPIRegion(File featureFile, String region) throws IOException {
         try (Reader reader = new FileReader(featureFile)) {
             Feature feature = FeatureJSONReader.read(reader, featureFile.getAbsolutePath());
 
@@ -1000,50 +863,4 @@ public class ContentPackage2FeatureModelConverterTest {
 
         return loadedResources;
     }
-
-    @Test
-    public void testConvertPackageWithUsersGroupsAndServiceUsers() throws Exception {
-        URL packageUrl = getClass().getResource("demo-cp.zip");
-        File packageFile = FileUtils.toFile(packageUrl);
-        File outputDirectory = new File(System.getProperty("java.io.tmpdir"), getClass().getName() + '_' + System.currentTimeMillis());
-        try {
-            converter.setFeaturesManager(new DefaultFeaturesManager(true, 5, outputDirectory, null, null, null, new DefaultAclManager()))
-                    .setBundlesDeployer(new LocalMavenRepositoryArtifactsDeployer(outputDirectory))
-                    .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
-                    .convert(packageFile);
-
-            File converted = new File(outputDirectory, "my_packages/demo-cp/0.0.0/demo-cp-0.0.0-cp2fm-converted.zip");
-            Set<String> notExpected = new HashSet<>();
-            notExpected.add("jcr_root/apps/demo-cp/.content.xml");
-            notExpected.add("jcr_root/home/users/demo-cp/_rep_policy.xml");
-            notExpected.add("jcr_root/home/groups/demo-cp/_rep_policy.xml");
-            notExpected.add("jcr_root/home/users/system/.content.xml");
-            notExpected.add("jcr_root/home/users/system/_cq_services/.content.xml");
-            notExpected.add("jcr_root/home/users/system/_cq_services/demo-cp/.content.xml");
-            notExpected.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/.content.xml");
-            notExpected.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/_rep_principalPolicy.xml");
-            verifyContentPackage(converted, 
-                    notExpected,
-                    "META-INF/vault/properties.xml",
-                    "META-INF/vault/config.xml",
-                    "META-INF/vault/filter.xml",
-                    "jcr_root/.content.xml",
-                    "jcr_root/demo-cp/.content.xml",
-                    "jcr_root/demo-cp/_rep_policy.xml",
-                    "jcr_root/apps/.content.xml",
-                    "jcr_root/home/.content.xml",
-                    "jcr_root/home/users/demo-cp/.content.xml",
-                    "jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/.content.xml",
-                    "jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/_rep_policy.xml",
-                    "jcr_root/home/groups/.content.xml",
-                    "jcr_root/home/groups/demo-cp/.content.xml",
-                    "jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/.content.xml",
-                    "jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/_rep_policy.xml"
-                    );
-
-        } finally {
-            deleteDirTree(outputDirectory);
-        }
-    }
-    
 }
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java b/src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
new file mode 100644
index 0000000..8355e13
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.sling.feature.cpconverter;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter.PackagePolicy;
+import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager;
+import org.apache.sling.feature.cpconverter.artifacts.LocalMavenRepositoryArtifactsDeployer;
+import org.apache.sling.feature.cpconverter.features.DefaultFeaturesManager;
+import org.apache.sling.feature.cpconverter.handlers.DefaultEntryHandlersManager;
+import org.apache.sling.feature.cpconverter.handlers.EntryHandlersManager;
+import org.apache.sling.feature.cpconverter.shared.ConverterConstants;
+import org.apache.sling.feature.cpconverter.vltpkg.DefaultPackagesEventsEmitter;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Parameterized.class)
+public class ConverterUserAndPermissionTest  extends AbstractConverterTest {
+
+    private static final Set<String> COMMON_EXPECTED_PATHS = new HashSet<>();
+    static {
+        COMMON_EXPECTED_PATHS.add("META-INF/vault/properties.xml");
+        COMMON_EXPECTED_PATHS.add("META-INF/vault/config.xml");
+        COMMON_EXPECTED_PATHS.add("META-INF/vault/filter.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/demo-cp/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/demo-cp/_rep_policy.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/users/demo-cp/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/_rep_policy.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/groups/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/groups/demo-cp/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/.content.xml");
+        COMMON_EXPECTED_PATHS.add("jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/_rep_policy.xml");
+    }
+
+    private static final Set<String> COMMON_NOT_EXPECTED_PATHS = new HashSet<>();
+    static {
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/demo-cp/_rep_policy.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/groups/demo-cp/_rep_policy.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/.content.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/.content.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/demo-cp/.content.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/.content.xml");
+        COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/_rep_principalPolicy.xml");
+    }
+
+    private ContentPackage2FeatureModelConverter converter;
+    private final String systemUserRelPath;
+
+    @Parameterized.Parameters(name = "name={1}")
+    public static Collection<Object[]> parameters() {
+        return Arrays.asList(
+                new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, "Default system user rel-path"},
+                new Object[] {"system/cq:services", "Modified system user rel-path"});
+    }
+
+    public ConverterUserAndPermissionTest(@NotNull String systemUserRelPath, @NotNull String name) {
+        this.systemUserRelPath = systemUserRelPath;
+    }
+
+    @Before
+    public void setUp() {
+        EntryHandlersManager handlersManager = new DefaultEntryHandlersManager(Collections.emptyMap(), false, ContentPackage2FeatureModelConverter.SlingInitialContentPolicy.KEEP, systemUserRelPath);
+        converter = new ContentPackage2FeatureModelConverter()
+                .setEntryHandlersManager(handlersManager)
+                .setAclManager(new DefaultAclManager());
+    }
+
+    @After
+    public void tearDown() throws IOException {
+        converter.close();
+    }
+
+    @Test
+    public void testConvertPackageWithUsersGroupsAndServiceUsers() throws Exception {
+        URL packageUrl = getClass().getResource("demo-cp.zip");
+        File packageFile = FileUtils.toFile(packageUrl);
+        File outputDirectory = new File(System.getProperty("java.io.tmpdir"), getClass().getName() + '_' + System.currentTimeMillis());
+        try {
+            converter.setFeaturesManager(new DefaultFeaturesManager(true, 5, outputDirectory, null, null, null, new DefaultAclManager()))
+                    .setBundlesDeployer(new LocalMavenRepositoryArtifactsDeployer(outputDirectory))
+                    .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
+                    .convert(packageFile);
+
+            File converted = new File(outputDirectory, "my_packages/demo-cp/0.0.0/demo-cp-0.0.0-cp2fm-converted.zip");
+            Set<String> notExpected = new HashSet<>(COMMON_NOT_EXPECTED_PATHS);
+            notExpected.add("jcr_root/apps/demo-cp/.content.xml");
+
+            Set<String> expected = new HashSet<>(COMMON_EXPECTED_PATHS);
+            expected.add("jcr_root/apps/.content.xml");
+            
+            verifyContentPackage(converted, notExpected, expected);
+            assertExpectedPolicies(converted);
+        } finally {
+            deleteDirTree(outputDirectory);
+        }
+    }
+
+    /**
+     * "demo-cp3.zip" contains the same content as "demo-cp.zip" but with altered order leading to ACE being read before 
+     * the corresponding system-user, whose principal is referenced in the ACE.
+     * 
+     * This test would fail if user/group information was not collected during the first pass (i.e. corresponding 
+     * handlers listed in {@link org.apache.sling.feature.cpconverter.vltpkg.RecollectorVaultPackageScanner}.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testConvertPackageWithUsersGroupsAndServiceUsersRepPolicyFirst() throws Exception {
+        URL packageUrl = getClass().getResource("demo-cp3.zip");
+        File packageFile = FileUtils.toFile(packageUrl);
+        File outputDirectory = new File(System.getProperty("java.io.tmpdir"), getClass().getName() + '_' + System.currentTimeMillis());
+        try {
+            converter.setFeaturesManager(new DefaultFeaturesManager(true, 5, outputDirectory, null, null, null, new DefaultAclManager()))
+                    .setBundlesDeployer(new LocalMavenRepositoryArtifactsDeployer(outputDirectory))
+                    .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
+                    .convert(packageFile);
+
+            File converted = new File(outputDirectory, "my_packages/demo-cp/0.0.0/demo-cp-0.0.0-cp2fm-converted.zip");
+
+            Set<String> notExpected = new HashSet<>(COMMON_NOT_EXPECTED_PATHS);
+            notExpected.add("jcr_root/apps/demo-cp/.content.xml");
+            Set<String> expected = new HashSet<>(COMMON_EXPECTED_PATHS);
+            expected.add("jcr_root/apps/.content.xml");
+
+            verifyContentPackage(converted, notExpected, expected);
+            assertExpectedPolicies(converted);
+        } finally {
+            deleteDirTree(outputDirectory);
+        }
+    }
+
+    /**
+     * "demo-cp2.zip" contains the same content as "demo-cp.zip" except for the application content below /apps.
+     * The content package therefore gets type CONTENT assigned (and not MIXED).
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testConvertCONTENTPackageWithUsersGroupsAndServiceUsers() throws Exception {
+        URL packageUrl = getClass().getResource("demo-cp2.zip");
+        File packageFile = FileUtils.toFile(packageUrl);
+        File outputDirectory = new File(System.getProperty("java.io.tmpdir"), getClass().getName() + '_' + System.currentTimeMillis());
+        File unrefOutputDir = new File(outputDirectory, "unref");
+        try {
+            converter.setFeaturesManager(new DefaultFeaturesManager(true, 5, outputDirectory, null, null, null, new DefaultAclManager()))
+                    .setBundlesDeployer(new LocalMavenRepositoryArtifactsDeployer(outputDirectory))
+                    .setUnreferencedArtifactsDeployer(new LocalMavenRepositoryArtifactsDeployer(unrefOutputDir))
+                    .setContentTypePackagePolicy(PackagePolicy.PUT_IN_DEDICATED_FOLDER)
+                    .setEmitter(DefaultPackagesEventsEmitter.open(outputDirectory))
+                    .convert(packageFile);
+
+            File converted = new File(unrefOutputDir, "my_packages/demo-cp/0.0.0/demo-cp-0.0.0-cp2fm-converted.zip");
+            verifyContentPackage(converted, COMMON_NOT_EXPECTED_PATHS, COMMON_EXPECTED_PATHS);
+            assertExpectedPolicies(converted);
+        } finally {
+            deleteDirTree(outputDirectory);
+        }
+    }
+
+    private static void assertExpectedPolicies(@NotNull File converted ) throws IOException {
+        assertPolicy(converted, "jcr_root/demo-cp/_rep_policy.xml", "cp-serviceuser-1", "cp-user1", "cp-group1");
+        assertPolicy(converted, "jcr_root/home/groups/demo-cp/EsYrXeBdSRkna2kqbxjl/_rep_policy.xml", null, "cp-group1");
+        assertPolicy(converted,  "jcr_root/home/users/demo-cp/XPXhA_RKMFRKNO8ViIhn/_rep_policy.xml", null, "cp-user1");
+    }
+    
+    private static void assertPolicy(@NotNull File contentPackage, @NotNull String path, @Nullable String unExpectedPrincipalName, @NotNull String... expectedPrincipalNames) throws IOException {
+        try (ZipFile zipFile = new ZipFile(contentPackage)) {
+            ZipEntry entry = zipFile.getEntry(path);
+            assertNotNull(entry);
+            assertFalse(entry.isDirectory());
+
+            try (InputStream in = zipFile.getInputStream(entry)) {
+                String policy = IOUtils.toString(in, StandardCharsets.UTF_8);
+                for (String principalName : expectedPrincipalNames) {
+                    assertTrue(policy.contains("rep:principalName=\""+principalName+"\""));
+                }
+                if (unExpectedPrincipalName != null) {
+                    assertFalse(policy.contains("rep:principalName=\""+unExpectedPrincipalName+"\""));
+                }
+            }
+        }
+    }
+}
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp2.zip b/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp2.zip
new file mode 100644
index 0000000..d459a5d
Binary files /dev/null and b/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp2.zip differ
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp3.zip b/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp3.zip
new file mode 100644
index 0000000..6345157
Binary files /dev/null and b/src/test/resources/org/apache/sling/feature/cpconverter/demo-cp3.zip differ