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/07/01 07:46:53 UTC

[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10576 : Improvements to VaultPackageAssembler, ContentPackage2FeatureModelConverter

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

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

commit 5fda60f23548026bf1c64fa8c3a669d4ad471949
Author: angela <an...@adobe.com>
AuthorDate: Thu Jul 1 09:46:34 2021 +0200

    SLING-10576 : Improvements to VaultPackageAssembler, ContentPackage2FeatureModelConverter
---
 .../ContentPackage2FeatureModelConverter.java      | 173 +++++++++++---------
 .../cpconverter/vltpkg/VaultPackageAssembler.java  | 180 ++++++++-------------
 .../cpconverter/vltpkg/VaultPackageUtils.java      |  39 +++++
 .../VaultPackageAssemblerUnparameterizedTest.java  |  24 +--
 4 files changed, 209 insertions(+), 207 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java b/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
index 8dd24fc..d6a02e0 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/ContentPackage2FeatureModelConverter.java
@@ -32,6 +32,7 @@ import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -92,36 +93,50 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
 
     private final RecollectorVaultPackageScanner recollectorVaultPackageScanner;
 
-    private List<PackagesEventsEmitter> emitters = new ArrayList<>();
+    private final List<PackagesEventsEmitter> emitters = new ArrayList<>();
+
+    private final List<Runnable> deployTasks = new ArrayList<>();
+
+    private final File tmpDirectory;
 
     private boolean failOnMixedPackages = false;
 
+    private PackagePolicy contentTypePackagePolicy = PackagePolicy.REFERENCE;
+
+    private boolean removeInstallHooks = false;
+
     public enum PackagePolicy {
-        /** References the content package in the feature model and deploys via the {@link ContentPackage2FeatureModelConverter#artifactsDeployer} */
-        REFERENCE, 
-        /** Drops the content package completely (i.e. neither reference it in the feature model nor deploy anywhere)
-          * @deprecated
-          */
+        /**
+         * References the content package in the feature model and deploys via the {@link ContentPackage2FeatureModelConverter#artifactsDeployer}
+         */
+        REFERENCE,
+        /**
+         * Drops the content package completely (i.e. neither reference it in the feature model nor deploy anywhere)
+         *
+         * @deprecated
+         */
         DROP,
-        /** Deploys the content package via the {@link ContentPackage2FeatureModelConverter#unreferencedArtifactsDeployer} */
-        PUT_IN_DEDICATED_FOLDER;
+        /**
+         * Deploys the content package via the {@link ContentPackage2FeatureModelConverter#unreferencedArtifactsDeployer}
+         */
+        PUT_IN_DEDICATED_FOLDER
     }
 
-    private PackagePolicy contentTypePackagePolicy = PackagePolicy.REFERENCE;
-
-    private boolean removeInstallHooks = false;
-    
     public enum SlingInitialContentPolicy {
-        /** Keep in bundle and don't extract */
+        /**
+         * Keep in bundle and don't extract
+         */
         KEEP,
-        /** Extract from bundle into content-packages and feature model */
+        /**
+         * Extract from bundle into content-packages and feature model
+         */
         EXTRACT_AND_REMOVE,
-        /** Extract from bundle into content-packages and feature model but keep in bundle as well */
+        /**
+         * Extract from bundle into content-packages and feature model but keep in bundle as well
+         */
         EXTRACT_AND_KEEP
     }
 
-    private final File tmpDirectory;
-
     public ContentPackage2FeatureModelConverter() {
         this(false);
     }
@@ -131,7 +146,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         this.recollectorVaultPackageScanner = new RecollectorVaultPackageScanner(this, this.packageManager, strictValidation, subContentPackages);
         try {
             this.tmpDirectory = Files.createTempDirectory("cp2fm-converter").toFile();
-        } catch ( final IOException io) {
+        } catch (final IOException io) {
             throw new RuntimeException("Unable to create a temporary directory", io);
         }
     }
@@ -145,10 +160,10 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         return featuresManager;
     }
 
-    public @NotNull ContentPackage2FeatureModelConverter setFeaturesManager(@Nullable FeaturesManager featuresManager) {
+    public @NotNull ContentPackage2FeatureModelConverter setFeaturesManager(@NotNull FeaturesManager featuresManager) {
         this.featuresManager = featuresManager;
-        if ( featuresManager instanceof PackagesEventsEmitter ) {
-            this.emitters.add((PackagesEventsEmitter)featuresManager);
+        if (featuresManager instanceof PackagesEventsEmitter) {
+            this.emitters.add((PackagesEventsEmitter) featuresManager);
         }
         return this;
     }
@@ -162,12 +177,12 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         return artifactsDeployer;
     }
 
-    public @NotNull ContentPackage2FeatureModelConverter setBundlesDeployer(@Nullable ArtifactsDeployer bundlesDeployer) {
+    public @NotNull ContentPackage2FeatureModelConverter setBundlesDeployer(@NotNull ArtifactsDeployer bundlesDeployer) {
         this.artifactsDeployer = bundlesDeployer;
         return this;
     }
 
-    public @NotNull ContentPackage2FeatureModelConverter setUnreferencedArtifactsDeployer(@Nullable ArtifactsDeployer unreferencedArtifactsDeployer) {
+    public @NotNull ContentPackage2FeatureModelConverter setUnreferencedArtifactsDeployer(@NotNull ArtifactsDeployer unreferencedArtifactsDeployer) {
         this.unreferencedArtifactsDeployer = unreferencedArtifactsDeployer;
         return this;
     }
@@ -176,7 +191,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         return aclManager;
     }
 
-    public @NotNull ContentPackage2FeatureModelConverter setAclManager(@Nullable AclManager aclManager) {
+    public @NotNull ContentPackage2FeatureModelConverter setAclManager(@NotNull AclManager aclManager) {
         this.aclManager = aclManager;
         return this;
     }
@@ -185,12 +200,12 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         return mainPackageAssembler;
     }
 
-    public @NotNull ContentPackage2FeatureModelConverter setEmitter(@Nullable PackagesEventsEmitter emitter) {
+    public @NotNull ContentPackage2FeatureModelConverter setEmitter(@NotNull PackagesEventsEmitter emitter) {
         this.emitters.add(emitter);
         return this;
     }
-    
-    public @NotNull ContentPackage2FeatureModelConverter setContentTypePackagePolicy(PackagePolicy contentTypePackagePolicy) {
+
+    public @NotNull ContentPackage2FeatureModelConverter setContentTypePackagePolicy(@NotNull PackagePolicy contentTypePackagePolicy) {
         this.contentTypePackagePolicy = contentTypePackagePolicy;
         return this;
     }
@@ -210,19 +225,19 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
     }
     
     public void cleanup() throws IOException {
-        if ( this.tmpDirectory.exists() ) {
-            logger.info( "Cleaning up tmp directory {}", this.tmpDirectory);
+        if (this.tmpDirectory.exists()) {
+            logger.info("Cleaning up tmp directory {}", this.tmpDirectory);
 
-            FileUtils.deleteDirectory( this.tmpDirectory );
+            FileUtils.deleteDirectory(this.tmpDirectory);
         }
     }
-    
-    public void convert(@NotNull File...contentPackages) throws Exception {
-        requireNonNull(contentPackages , "Null content-package(s) can not be converted.");
+
+    public void convert(@NotNull File... contentPackages) throws Exception {
+        requireNonNull(contentPackages, "Null content-package(s) can not be converted.");
         secondPass(firstPass(contentPackages));
     }
 
-    protected @NotNull Collection<VaultPackage> firstPass(@NotNull File...contentPackages) throws Exception {
+    protected @NotNull Collection<VaultPackage> firstPass(@NotNull File... contentPackages) throws Exception {
         Map<PackageId, VaultPackage> idFileMap = new LinkedHashMap<>();
         Map<PackageId, VaultPackage> idPackageMapping = new ConcurrentHashMap<>();
 
@@ -256,8 +271,8 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         return idFileMap.values();
     }
 
-    protected void secondPass(@NotNull Collection<VaultPackage> orderedContentPackages) throws Exception {
-        emitters.stream().forEach(e -> e.start());
+    private void secondPass(@NotNull Collection<VaultPackage> orderedContentPackages) throws Exception {
+        emitters.stream().forEach(PackagesEventsEmitter::start);
 
         for (VaultPackage vaultPackage : orderedContentPackages) {
             try {
@@ -278,7 +293,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
 
                 // deploy the new zip content-package to the local mvn bundles dir
 
-                processContentPackageArchive(contentPackageArchive, mainPackageAssembler,null);
+                processContentPackageArchive(contentPackageArchive, mainPackageAssembler, null);
 
                 // finally serialize the Feature Model(s) file(s)
 
@@ -287,7 +302,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
                 logger.info("Conversion complete!");
 
                 featuresManager.serialize();
-                emitters.stream().forEach(e -> e.endPackage());
+                emitters.stream().forEach(PackagesEventsEmitter::endPackage);
             } finally {
                 aclManager.reset();
                 assemblers.clear();
@@ -303,13 +318,13 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         deployPackages();
         mutableContentsIds.clear();
 
-        emitters.stream().forEach(e -> e.end());
+        emitters.stream().forEach(PackagesEventsEmitter::end);
     }
 
-    private void orderDependencies(@NotNull Map<PackageId, VaultPackage> idFileMap,
-                                   @NotNull Map<PackageId, VaultPackage> idPackageMapping,
-                                   @NotNull VaultPackage pack,
-                                   @NotNull Set<PackageId> visited) throws CyclicDependencyException {
+    private static void orderDependencies(@NotNull Map<PackageId, VaultPackage> idFileMap,
+                                          @NotNull Map<PackageId, VaultPackage> idPackageMapping,
+                                          @NotNull VaultPackage pack,
+                                          @NotNull Set<PackageId> visited) throws CyclicDependencyException {
         if (!visited.add(pack.getId())) {
             throw new CyclicDependencyException("Cyclic dependency detected, " + pack.getId() + " was previously visited already");
         }
@@ -353,9 +368,9 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         
         //set dependency to parent package if the parent package is an application package & subpackage is embedded
         if (isEmbeddedPackage && !isContainerPackage) {
-            PackageId parentId = new PackageId((String)parentProps.get(PackageProperties.NAME_GROUP), 
-                                                (String)parentProps.get(PackageProperties.NAME_NAME),
-                                                (String)parentProps.get(PackageProperties.NAME_VERSION));
+            PackageId parentId = new PackageId((String) parentProps.get(PackageProperties.NAME_GROUP),
+                    (String) parentProps.get(PackageProperties.NAME_NAME),
+                    (String) parentProps.get(PackageProperties.NAME_VERSION));
             clonedPackage.addDependency(new Dependency(parentId));
         }
 
@@ -367,7 +382,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
         // restore the previous assembler
         mainPackageAssembler = handler;
 
-        emitters.stream().forEach(e -> e.endSubPackage());
+        emitters.stream().forEach(PackagesEventsEmitter::endSubPackage);
     }
 
     public void processContentPackageArchive(@NotNull File contentPackageArchive, @NotNull VaultPackageAssembler assembler,
@@ -378,10 +393,10 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
             // SLING-8608 - Fail the conversion if the resulting attached content-package is MIXED type
             if (PackageType.MIXED == packageType && failOnMixedPackages) {
                 throw new IllegalStateException("Generated content-package '"
-                                    + vaultPackage.getId()
-                                    + "' located in file "
-                                    + contentPackageArchive
-                                    + " is of MIXED type");
+                        + vaultPackage.getId()
+                        + "' located in file "
+                        + contentPackageArchive
+                        + " is of MIXED type");
             }
 
             ArtifactId mvnPackageId = toArtifactId(vaultPackage.getId(), contentPackageArchive);
@@ -391,7 +406,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
                     case DROP:
                         mutableContentsIds.put(vaultPackage.getId(), getDependencies(vaultPackage));
                         logger.info("Dropping package of PackageType.CONTENT {} (content-package id: {})",
-                                    mvnPackageId.getArtifactId(), vaultPackage.getId());
+                                mvnPackageId.getArtifactId(), vaultPackage.getId());
                         break;
                     case PUT_IN_DEDICATED_FOLDER:
                         mutableContentsIds.put(vaultPackage.getId(), getDependencies(vaultPackage));
@@ -401,7 +416,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
                         }
                         unreferencedArtifactsDeployer.deploy(new FileArtifactWriter(contentPackageArchive), mvnPackageId);
                         logger.info("Put converted package of PackageType.CONTENT {} (content-package id: {}) in {} (not referenced in feature model)",
-                                    mvnPackageId.getArtifactId(), vaultPackage.getId(), unreferencedArtifactsDeployer.getBaseDirectory());
+                                mvnPackageId.getArtifactId(), vaultPackage.getId(), unreferencedArtifactsDeployer.getBaseDirectory());
                         break;
                     case REFERENCE:
                         deploy(assembler, mvnPackageId, runMode);
@@ -411,31 +426,31 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
             }
         }
     }
-    public void deployPackages() throws Exception {
+
+    public void deployPackages() {
         try {
             mutableContentsIds.values().forEach(
-                value -> value.removeIf(dep -> mutableContentsIds.keySet().stream().anyMatch(dep::matches)));
+                    value -> value.removeIf(dep -> mutableContentsIds.keySet().stream().anyMatch(dep::matches)));
 
             deployTasks.forEach(Runnable::run);
         } catch (RuntimeException ex) {
             if (ex.getCause() instanceof Exception) {
-                throw (Exception) ex;
+                throw ex;
             }
             throw ex;
         }
         deployTasks.clear();
     }
 
-    private List<Runnable> deployTasks = new ArrayList<>();
-
-    private void deploy(VaultPackageAssembler assembler, ArtifactId mvnPackageId, String runMode) {
-        getFeaturesManager().addArtifact(runMode, mvnPackageId);
+    private void deploy(@NotNull VaultPackageAssembler assembler, @NotNull ArtifactId mvnPackageId, @Nullable String runMode) {
+        Objects.requireNonNull(getFeaturesManager()).addArtifact(runMode, mvnPackageId);
+        ArtifactsDeployer deployer = Objects.requireNonNull(getArtifactsDeployer());
         deployTasks.add(() -> {
             assembler.updateDependencies(mutableContentsIds);
             try {
                 File finalContentPackageArchive = assembler.createPackage();
                 // deploy the new content-package to the local mvn bundles dir
-                getArtifactsDeployer().deploy(new FileArtifactWriter(finalContentPackageArchive), mvnPackageId);
+                deployer.deploy(new FileArtifactWriter(finalContentPackageArchive), mvnPackageId);
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             }
@@ -449,10 +464,10 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
     public boolean process(@NotNull String entryPath, @NotNull Archive archive, @Nullable Entry entry, boolean useMainPackageAssembler) throws Exception {
         if (resourceFilter != null && resourceFilter.isFilteredOut(entryPath)) {
             throw new IllegalArgumentException("Path '"
-                                               + entryPath
-                                               + "' in archive "
-                                               + archive.getMetaInf().getPackageProperties().getId()
-                                               + " not allowed by user configuration, please check configured filtering patterns");
+                    + entryPath
+                    + "' in archive "
+                    + archive.getMetaInf().getPackageProperties().getId()
+                    + " not allowed by user configuration, please check configured filtering patterns");
         }
 
         EntryHandler entryHandler = handlersManager.getEntryHandlerByEntryPath(entryPath);
@@ -471,8 +486,8 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
             }
         }
         entryHandler.handle(entryPath, archive, entry, this);
-        if (useMainPackageAssembler) {
-            mainPackageAssembler.recordEntryPath(entryPath);
+        if (useMainPackageAssembler && !mainPackageAssembler.recordEntryPath(entryPath)) {
+            logger.warn("Duplicate entry path {}", entryPath);
         }
         return true;
     }
@@ -484,20 +499,20 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
 
     public static @NotNull ArtifactId toArtifactId(@NotNull PackageId packageId, @NotNull File file) {
         String groupId = requireNonNull(packageId.getGroup(),
-            PackageProperties.NAME_GROUP
-                + " property not found in content-package "
-                + file
-                + ", please check META-INF/vault/properties.xml").replace('/', '.');
+                PackageProperties.NAME_GROUP
+                        + " property not found in content-package "
+                        + file
+                        + ", please check META-INF/vault/properties.xml").replace('/', '.');
         // Replace any space with an underscore to adhere to Maven Group Id specification
-        groupId = groupId.replaceAll(" ", "_");
+        groupId = groupId.replace(" ", "_");
 
-        String artifactid = requireNonNull(packageId.getName(),
-            PackageProperties.NAME_NAME
-                + " property not found in content-package "
-                + file
-                + ", please check META-INF/vault/properties.xml");
+        String artifactId = requireNonNull(packageId.getName(),
+                PackageProperties.NAME_NAME
+                        + " property not found in content-package "
+                        + file
+                        + ", please check META-INF/vault/properties.xml");
         // Replace any space with an underscore to adhere to Maven Artifact Id specification
-        artifactid = artifactid.replaceAll(" ", "_");
+        artifactId = artifactId.replace(" ", "_");
 
         // package versions may use suffix "-cp2fm-converted" which is redundant as for artifactIds this is set as dedicated classifier
         String version = packageId.getVersionString();
@@ -508,7 +523,7 @@ public class ContentPackage2FeatureModelConverter extends BaseVaultPackageScanne
             version = DEFAULT_VERSION;
         }
 
-        return new ArtifactId(groupId, artifactid, version, PACKAGE_CLASSIFIER, ZIP_TYPE);
+        return new ArtifactId(groupId, artifactId, version, PACKAGE_CLASSIFIER, ZIP_TYPE);
     }
 
     @Override
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
index e8be6e1..3961aff 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
@@ -55,8 +55,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.function.BiConsumer;
 import java.util.function.Predicate;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
@@ -84,37 +82,35 @@ public class VaultPackageAssembler implements EntryHandler {
     
     private final Set<String> convertedCpPaths = new HashSet<>();
     private final Set<String> allPaths = new HashSet<>();
-
-    private final static class RemoveInstallHooksPredicate implements Predicate<Map.Entry<Object, Object>> {
-        @Override
-        public boolean test(java.util.Map.Entry<Object, Object> entry) {
-            String key = (String)entry.getKey();
-            return !key.startsWith(PackageProperties.PREFIX_INSTALL_HOOK);
-        }
-    }
-
+    private final DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
+    private final Set<Dependency> dependencies;
+    private final File storingDirectory;
+    private final Properties properties;
+    private final File tmpDir;
+    private final boolean removeInstallHooks;
+    
     /**
-     * Creates a new package assembler based on an existing package.
-     * Takes over properties and filter rules from existing package.
-     * @param tempDir the temp dir
-     * @param vaultPackage the package to take as blueprint
-     * @param removeInstallHooks whether to remove install hooks or not
-     * @return the package assembler
+     * This class can not be instantiated from outside
      */
-    public static @NotNull VaultPackageAssembler create(@NotNull File tempDir, @NotNull VaultPackage vaultPackage, boolean removeInstallHooks) {
-        return create(tempDir, vaultPackage, Objects.requireNonNull(vaultPackage.getMetaInf().getFilter()), removeInstallHooks);
+    private VaultPackageAssembler(@NotNull File tempDir, @NotNull File storingDirectory, @NotNull Properties properties, 
+                                  @NotNull Set<Dependency> dependencies, boolean removeInstallHooks) {
+        this.storingDirectory = storingDirectory;
+        this.properties = properties;
+        this.dependencies = dependencies;
+        this.tmpDir = tempDir;
+        this.removeInstallHooks = removeInstallHooks;
     }
-
+    
     /**
      * Creates a new package assembler based on an existing package.
-     * Takes over properties from existing package.
+     * Takes over properties and filter rules from existing package.
+     * 
      * @param baseTempDir the temp dir
      * @param vaultPackage the package to take as blueprint
-     * @param filter the filter with which to initialize the new package
      * @param removeInstallHooks whether to remove install hooks or not
      * @return the package assembler
      */
-    private static @NotNull VaultPackageAssembler create(@NotNull File baseTempDir, @NotNull VaultPackage vaultPackage, @NotNull WorkspaceFilter filter, boolean removeInstallHooks) {
+    public static @NotNull VaultPackageAssembler create(@NotNull File baseTempDir, @NotNull VaultPackage vaultPackage, boolean removeInstallHooks) {
         final File tempDir = new File(baseTempDir, "synthetic-content-packages_" + System.currentTimeMillis());
         PackageId packageId = vaultPackage.getId();
         File storingDirectory = initStoringDirectory(packageId, tempDir);
@@ -139,12 +135,13 @@ public class VaultPackageAssembler implements EntryHandler {
         Set<Dependency> dependencies = getDependencies(vaultPackage);
 
         VaultPackageAssembler assembler = new VaultPackageAssembler(tempDir, storingDirectory, properties, dependencies, removeInstallHooks);
-        assembler.mergeFilters(filter);
+        assembler.mergeFilters(Objects.requireNonNull(vaultPackage.getMetaInf().getFilter()));
         return assembler;
     }
 
     /**
      * Creates a new package assembler.
+     * 
      * @param baseTempDir the temp dir
      * @param packageId the package id from which to generate a minimal properties.xml
      * @param description the description which should end up in the package properties
@@ -163,13 +160,13 @@ public class VaultPackageAssembler implements EntryHandler {
         return new VaultPackageAssembler(tempDir, storingDirectory, props, new HashSet<>(), false);
     }
 
-    static @NotNull File initStoringDirectory(PackageId packageId, @NotNull File tempDir) {
+    private static @NotNull File initStoringDirectory(PackageId packageId, @NotNull File tempDir) {
         String fileName = packageId.toString().replace('/', '-').replace(':', '-');
         File storingDirectory = new File(tempDir, fileName + "-deflated");
-        if(storingDirectory.exists()) {
+        if (storingDirectory.exists()) {
             try {
                 FileUtils.deleteDirectory(storingDirectory);
-            } catch(IOException e) {
+            } catch (IOException e) {
                 throw new IllegalStateException("Unable to delete existing deflated folder: '" + storingDirectory + "'", e);
             }
         }
@@ -181,18 +178,6 @@ public class VaultPackageAssembler implements EntryHandler {
         return storingDirectory;
     }
 
-    private final DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
-
-    private final Set<Dependency> dependencies;
-
-    private final File storingDirectory;
-
-    private final Properties properties;
-
-    private final File tmpDir;
-
-    private final boolean removeInstallHooks;
-
     File getTempDir() {
         return this.tmpDir;
     }
@@ -212,17 +197,6 @@ public class VaultPackageAssembler implements EntryHandler {
         }
     }
 
-    /**
-     * This class can not be instantiated from outside
-     */
-    private VaultPackageAssembler(@NotNull File tempDir, @NotNull File storingDirectory, @NotNull Properties properties, @NotNull Set<Dependency> dependencies, boolean removeInstallHooks) {
-        this.storingDirectory = storingDirectory;
-        this.properties = properties;
-        this.dependencies = dependencies;
-        this.tmpDir = tempDir;
-        this.removeInstallHooks = removeInstallHooks;
-    }
-
     public @NotNull Properties getPackageProperties() {
         return this.properties;
     }
@@ -296,12 +270,11 @@ public class VaultPackageAssembler implements EntryHandler {
                 }
             }
         }
-        for(java.util.Map.Entry<Dependency, Set<Dependency>>  match : matches.entrySet()) {
+        for (java.util.Map.Entry<Dependency, Set<Dependency>> match : matches.entrySet()) {
             dependencies.remove(match.getKey());
             dependencies.addAll(match.getValue());
         }
     }
-    
 
     public void addDependency(@NotNull Dependency dependency) {
         dependencies.add(dependency);
@@ -320,13 +293,13 @@ public class VaultPackageAssembler implements EntryHandler {
         }
 
         final PackageType sourcePackageType;
-        final String sourcePackageTypeValue = (String)properties.get(PackageProperties.NAME_PACKAGE_TYPE);
+        final String sourcePackageTypeValue = (String) properties.get(PackageProperties.NAME_PACKAGE_TYPE);
         if (sourcePackageTypeValue != null) {
             sourcePackageType = PackageType.valueOf(sourcePackageTypeValue.toUpperCase());
         } else {
             sourcePackageType = null;
         }
-        PackageType newPackageType = recalculatePackageType(sourcePackageType, storingDirectory);
+        PackageType newPackageType = VaultPackageUtils.recalculatePackageType(sourcePackageType, storingDirectory);
         if (newPackageType != null) {
             properties.setProperty(PackageProperties.NAME_PACKAGE_TYPE, newPackageType.name().toLowerCase());
         }
@@ -352,7 +325,7 @@ public class VaultPackageAssembler implements EntryHandler {
 
         File xmlFilter = new File(metaDir, FILTER_XML);
         try (InputStream input = adjustedFilter.getSource();
-                FileOutputStream output = new FileOutputStream(xmlFilter)) {
+             FileOutputStream output = new FileOutputStream(xmlFilter)) {
             IOUtils.copy(input, output);
         }
 
@@ -361,13 +334,13 @@ public class VaultPackageAssembler implements EntryHandler {
         final File destFile = new File(this.tmpDir, destFileName);
         final File manifestFile = new File(storingDirectory, JarFile.MANIFEST_NAME.replace('/', File.separatorChar));
         Manifest manifest = null;
-        if ( manifestFile.exists() ) {
-            try ( final InputStream r = new FileInputStream(manifestFile)) {
+        if (manifestFile.exists()) {
+            try (final InputStream r = new FileInputStream(manifestFile)) {
                 manifest = new Manifest(r);
             }
         }
-        try ( final JarOutputStream jos = manifest == null ? new JarOutputStream(new FileOutputStream(destFile)) 
-                                                           : new JarOutputStream(new FileOutputStream(destFile), manifest)) {            
+        try (final JarOutputStream jos = manifest == null ? new JarOutputStream(new FileOutputStream(destFile)) 
+                                                          : new JarOutputStream(new FileOutputStream(destFile), manifest)) {            
             jos.setLevel(Deflater.DEFAULT_COMPRESSION);
             addDirectory(jos, storingDirectory, storingDirectory.getAbsolutePath().length() + 1);
         }
@@ -375,7 +348,7 @@ public class VaultPackageAssembler implements EntryHandler {
         return destFile;
     }
 
-    private static Set<String> toRepositoryPaths(@NotNull Set<String> paths) {
+    private static @NotNull Set<String> toRepositoryPaths(@NotNull Set<String> paths) {
         return paths.stream().map(s -> {
             if (s.startsWith("/jcr_root")) {
                 return s.substring("/jcr_root".length());
@@ -384,8 +357,8 @@ public class VaultPackageAssembler implements EntryHandler {
             }
         }).collect(Collectors.toSet());
     }
-    
-    private static WorkspaceFilter createAdjustedFilter(@NotNull WorkspaceFilter base, @NotNull Set<String> filteredPaths, @NotNull Set<String> cpPaths) throws IOException {
+
+    private static @NotNull WorkspaceFilter createAdjustedFilter(@NotNull WorkspaceFilter base, @NotNull Set<String> filteredPaths, @NotNull Set<String> cpPaths) throws IOException {
         try {
             DefaultWorkspaceFilter dwf = new DefaultWorkspaceFilter();
             for (PathFilterSet pfs : base.getPropertyFilterSets()) {
@@ -399,9 +372,9 @@ public class VaultPackageAssembler implements EntryHandler {
             throw new IOException(e);
         }
     }
-    
-    private static void processPathFilterSet(@NotNull PathFilterSet pfs, @NotNull DefaultWorkspaceFilter newFilter, 
-                                             boolean isPropertyFilterSet, @NotNull Set<String> filteredPaths, 
+
+    private static void processPathFilterSet(@NotNull PathFilterSet pfs, @NotNull DefaultWorkspaceFilter newFilter,
+                                             boolean isPropertyFilterSet, @NotNull Set<String> filteredPaths,
                                              @NotNull Set<String> cpPaths) throws ConfigurationException {
         if (cpPaths.stream().noneMatch(pfs::covers)) {
             // the given filterset no longer covers any of the paths included in the converted content package
@@ -435,82 +408,49 @@ public class VaultPackageAssembler implements EntryHandler {
         }
     }
 
-    private void addDirectory(final JarOutputStream jos, final File dir, final int prefixLength) throws IOException {
-        if ( dir.getAbsolutePath().length() > prefixLength && dir.listFiles().length == 0 ) {
+    private static void addDirectory(@NotNull final JarOutputStream jos, @NotNull final File dir, final int prefixLength) throws IOException {
+        if (dir.getAbsolutePath().length() > prefixLength && dir.listFiles().length == 0) {
             final String dirName = dir.getAbsolutePath().substring(prefixLength).replace(File.separatorChar, '/');
             final JarEntry entry = new JarEntry(dirName);
             entry.setTime(dir.lastModified());
             entry.setSize(0);
             jos.putNextEntry(entry);
-            jos.closeEntry();       
+            jos.closeEntry();
         }
-        for(final File f : dir.listFiles()) {
+        for (final File f : dir.listFiles()) {
             final String name = f.getAbsolutePath().substring(prefixLength).replace(File.separatorChar, '/');
-            if ( f.isFile() && !JarFile.MANIFEST_NAME.equals(name) ) {                
+            if (f.isFile() && !JarFile.MANIFEST_NAME.equals(name)) {
                 final JarEntry entry = new JarEntry(name);
                 entry.setTime(f.lastModified());
                 jos.putNextEntry(entry);
 
-                try ( final FileInputStream in = new FileInputStream(f)) {
+                try (final FileInputStream in = new FileInputStream(f)) {
                     IOUtils.copy(in, jos);
-                }  
-                jos.closeEntry();   
-            } else if ( f.isDirectory() ) {
+                }
+                jos.closeEntry();
+            } else if (f.isDirectory()) {
                 addDirectory(jos, f, prefixLength);
             }
         }
     }
-    
-    static @Nullable PackageType recalculatePackageType(PackageType sourcePackageType, @NotNull File outputDirectory) {
-        if (sourcePackageType != null && sourcePackageType != PackageType.MIXED) {
-            return null;
-        }
-        AtomicBoolean foundMutableFiles = new AtomicBoolean();
-        AtomicBoolean foundImmutableFiles  = new AtomicBoolean();
-        forEachDirectoryBelowJcrRoot(outputDirectory, (child, base) -> {
-            if (child.getName().equals("apps") || child.getName().equals("libs")) {
-                foundImmutableFiles.weakCompareAndSet(false, true);
-            } else {
-                foundMutableFiles.weakCompareAndSet(false, true);
-            }
-        });
-        if (foundImmutableFiles.get() && !foundMutableFiles.get()) {
-            return PackageType.APPLICATION;
-        } else if (!foundImmutableFiles.get() && foundMutableFiles.get()) {
-            return PackageType.CONTENT;
-        } else {
-            return PackageType.MIXED;
-        }
-       
-    }
 
     private void computeFilters(@NotNull File outputDirectory) {
-        forEachDirectoryBelowJcrRoot(outputDirectory, (child, base) -> {
-                TreeNode node = lowestCommonAncestor(new TreeNode(child));
-                File lowestCommonAncestor = node != null ? node.val : null;
-                if (lowestCommonAncestor != null) {
-                    String root = "/" + PlatformNameFormat.getRepositoryPath(base.toURI().relativize(lowestCommonAncestor.toURI()).getPath(), true);
-                    filter.add(new PathFilterSet(root));
-                }
-            });
-    }
-
-    private static void forEachDirectoryBelowJcrRoot(File outputDirectory, BiConsumer<File, File> consumer) {
-        File jcrRootDir = new File(outputDirectory, ROOT_DIR);
-        if (jcrRootDir.exists() && jcrRootDir.isDirectory()) {
-            for (File child : jcrRootDir.listFiles((FileFilter)DirectoryFileFilter.INSTANCE)) {
-                // calls consumer with absolute files
-                consumer.accept(child, jcrRootDir);
+        VaultPackageUtils.forEachDirectoryBelowJcrRoot(outputDirectory, (child, base) -> {
+            TreeNode node = lowestCommonAncestor(new TreeNode(child));
+            File lowestCommonAncestor = node != null ? node.val : null;
+            if (lowestCommonAncestor != null) {
+                String root = "/" + PlatformNameFormat.getRepositoryPath(base.toURI().relativize(lowestCommonAncestor.toURI()).getPath(), true);
+                filter.add(new PathFilterSet(root));
             }
-        }
+        });
     }
 
-    private @Nullable TreeNode lowestCommonAncestor(@NotNull TreeNode root) {
+    private static @Nullable TreeNode lowestCommonAncestor(@NotNull TreeNode root) {
         int currMaxDepth = 0;//curr tree's deepest leaf depth
         int countMaxDepth = 0;//num of deepest leaves
         TreeNode node = null;
 
-        for (File child : root.val.listFiles((FileFilter)DirectoryFileFilter.INSTANCE)) {
+        for (File child : root.val.listFiles((FileFilter) DirectoryFileFilter.INSTANCE)) {
             TreeNode temp = lowestCommonAncestor(new TreeNode(child));
 
             if (temp == null) {
@@ -554,4 +494,12 @@ public class VaultPackageAssembler implements EntryHandler {
         }
 
     }
+
+    private static final class RemoveInstallHooksPredicate implements Predicate<Map.Entry<Object, Object>> {
+        @Override
+        public boolean test(java.util.Map.Entry<Object, Object> entry) {
+            String key = (String) entry.getKey();
+            return !key.startsWith(PackageProperties.PREFIX_INSTALL_HOOK);
+        }
+    }
 }
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageUtils.java b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageUtils.java
index fc35748..899183a 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageUtils.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageUtils.java
@@ -16,12 +16,17 @@
  */
 package org.apache.sling.feature.cpconverter.vltpkg;
 
+import java.io.File;
+import java.io.FileFilter;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Properties;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
+import org.apache.commons.io.filefilter.DirectoryFileFilter;
 import org.apache.jackrabbit.vault.fs.api.PathFilterSet;
 import org.apache.jackrabbit.vault.fs.api.WorkspaceFilter;
 import org.apache.jackrabbit.vault.packaging.Dependency;
@@ -31,6 +36,8 @@ import org.apache.jackrabbit.vault.packaging.VaultPackage;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import static org.apache.jackrabbit.vault.util.Constants.ROOT_DIR;
+
 public class VaultPackageUtils {
 
     private static final String DEPENDENCIES_DELIMITER = ",";
@@ -76,6 +83,38 @@ public class VaultPackageUtils {
         }
     }
 
+    static @Nullable PackageType recalculatePackageType(PackageType sourcePackageType, @NotNull File outputDirectory) {
+        if (sourcePackageType != null && sourcePackageType != PackageType.MIXED) {
+            return null;
+        }
+        AtomicBoolean foundMutableFiles = new AtomicBoolean();
+        AtomicBoolean foundImmutableFiles = new AtomicBoolean();
+        forEachDirectoryBelowJcrRoot(outputDirectory, (child, base) -> {
+            if (child.getName().equals("apps") || child.getName().equals("libs")) {
+                foundImmutableFiles.weakCompareAndSet(false, true);
+            } else {
+                foundMutableFiles.weakCompareAndSet(false, true);
+            }
+        });
+        if (foundImmutableFiles.get() && !foundMutableFiles.get()) {
+            return PackageType.APPLICATION;
+        } else if (!foundImmutableFiles.get() && foundMutableFiles.get()) {
+            return PackageType.CONTENT;
+        } else {
+            return PackageType.MIXED;
+        }
+    }
+
+    static void forEachDirectoryBelowJcrRoot(File outputDirectory, BiConsumer<File, File> consumer) {
+        File jcrRootDir = new File(outputDirectory, ROOT_DIR);
+        if (jcrRootDir.exists() && jcrRootDir.isDirectory()) {
+            for (File child : jcrRootDir.listFiles((FileFilter) DirectoryFileFilter.INSTANCE)) {
+                // calls consumer with absolute files
+                consumer.accept(child, jcrRootDir);
+            }
+        }
+    }
+
     public static @NotNull Set<Dependency> getDependencies(@NotNull VaultPackage vaultPackage) {
         Dependency[] originalDepenencies = vaultPackage.getDependencies();
 
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssemblerUnparameterizedTest.java b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssemblerUnparameterizedTest.java
index 268a933..9f6d8d8 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssemblerUnparameterizedTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssemblerUnparameterizedTest.java
@@ -35,19 +35,19 @@ public class VaultPackageAssemblerUnparameterizedTest {
         resource = VaultPackageAssemblerTest.class.getResource("../mixed");
         File mixedInput = FileUtils.toFile(resource);
 
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.APPLICATION, immutableInput));
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.CONTENT, immutableInput));
-        Assert.assertEquals(PackageType.APPLICATION, VaultPackageAssembler.recalculatePackageType(PackageType.MIXED, immutableInput));
-        Assert.assertEquals(PackageType.APPLICATION, VaultPackageAssembler.recalculatePackageType(null, immutableInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.APPLICATION, immutableInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.CONTENT, immutableInput));
+        Assert.assertEquals(PackageType.APPLICATION, VaultPackageUtils.recalculatePackageType(PackageType.MIXED, immutableInput));
+        Assert.assertEquals(PackageType.APPLICATION, VaultPackageUtils.recalculatePackageType(null, immutableInput));
 
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.APPLICATION, mutableInput));
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.CONTENT, mutableInput));
-        Assert.assertEquals(PackageType.CONTENT, VaultPackageAssembler.recalculatePackageType(PackageType.MIXED, mutableInput));
-        Assert.assertEquals(PackageType.CONTENT, VaultPackageAssembler.recalculatePackageType(null, mutableInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.APPLICATION, mutableInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.CONTENT, mutableInput));
+        Assert.assertEquals(PackageType.CONTENT, VaultPackageUtils.recalculatePackageType(PackageType.MIXED, mutableInput));
+        Assert.assertEquals(PackageType.CONTENT, VaultPackageUtils.recalculatePackageType(null, mutableInput));
 
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.APPLICATION, mixedInput));
-        Assert.assertEquals(null, VaultPackageAssembler.recalculatePackageType(PackageType.CONTENT, mixedInput));
-        Assert.assertEquals(PackageType.MIXED, VaultPackageAssembler.recalculatePackageType(PackageType.MIXED, mixedInput));
-        Assert.assertEquals(PackageType.MIXED, VaultPackageAssembler.recalculatePackageType(null, mixedInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.APPLICATION, mixedInput));
+        Assert.assertNull(VaultPackageUtils.recalculatePackageType(PackageType.CONTENT, mixedInput));
+        Assert.assertEquals(PackageType.MIXED, VaultPackageUtils.recalculatePackageType(PackageType.MIXED, mixedInput));
+        Assert.assertEquals(PackageType.MIXED, VaultPackageUtils.recalculatePackageType(null, mixedInput));
     }
 }