You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2022/01/12 14:56:47 UTC

[jackrabbit-filevault] branch master updated: JCRVLT-590 remove finalizer and properly close ZipVaultPackage (#197)

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

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git


The following commit(s) were added to refs/heads/master by this push:
     new bd04fa9  JCRVLT-590 remove finalizer and properly close ZipVaultPackage (#197)
bd04fa9 is described below

commit bd04fa9ef28fbcaa169f85bed5ad654d1a69ff8d
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Jan 12 15:56:40 2022 +0100

    JCRVLT-590 remove finalizer and properly close ZipVaultPackage (#197)
    
    clarify when calling close is strictly necessary
---
 .../org/apache/jackrabbit/vault/fs/io/Archive.java | 18 +++--
 .../jackrabbit/vault/packaging/JcrPackage.java     |  6 +-
 .../jackrabbit/vault/packaging/VaultPackage.java   |  3 +-
 .../vault/packaging/impl/JcrPackageImpl.java       |  9 +--
 .../packaging/impl/JcrPackageManagerImpl.java      | 87 +++++++++++-----------
 .../vault/packaging/impl/PackageManagerImpl.java   | 14 ++--
 .../vault/packaging/impl/ZipVaultPackage.java      |  9 ---
 .../registry/impl/JcrPackageRegistry.java          | 74 ++++++++----------
 8 files changed, 99 insertions(+), 121 deletions(-)

diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Archive.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Archive.java
index 1e666e5..d53d40a 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Archive.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Archive.java
@@ -41,7 +41,8 @@ public interface Archive extends Closeable {
     void open(boolean strict) throws IOException;
 
     /**
-     * Opens an input stream for the given entry
+     * Opens an input stream for the given entry.
+     * Requires a previous call to {@link #open(boolean)}.
      * @param entry the entry
      * @return the input stream or {@code null} if the entry can't be read
      * @throws IOException if an error occurs
@@ -50,7 +51,8 @@ public interface Archive extends Closeable {
     InputStream openInputStream(@Nullable Entry entry) throws IOException;
 
     /**
-     * Returns an input source for the given entry
+     * Returns an input source for the given entry.
+     * Requires a previous call to {@link #open(boolean)}.
      * @param entry the entry
      * @return the input source or {@code null} if the entry can't be read
      * @throws IOException if an error occurs
@@ -61,6 +63,7 @@ public interface Archive extends Closeable {
     /**
      * Returns the entry that specifies the "jcr_root". if no such
      * entry exists, {@code null} is returned.
+     * Requires a previous call to {@link #open(boolean)}.
      * @return the jcr_root entry or {@code null}
      * @throws IOException if an error occurs
      */
@@ -69,6 +72,7 @@ public interface Archive extends Closeable {
 
     /**
      * Returns the root entry.
+     * Requires a previous call to {@link #open(boolean)}.
      * @return the root entry.
      * @throws IOException if an error occurs
      */
@@ -78,6 +82,7 @@ public interface Archive extends Closeable {
     /**
      * Returns the meta inf. If the archive provides no specific meta data,
      * a default, empty meta inf is returned.
+     * Requires a previous call to {@link #open(boolean)}.
      *
      * @return the meta inf.
      */
@@ -86,6 +91,7 @@ public interface Archive extends Closeable {
 
     /**
      * Returns the entry specified by path.
+     * Requires a previous call to {@link #open(boolean)}.
      * @param path the path
      * @return the entry or {@code null} if not found.
      * @throws IOException if an error occurs
@@ -95,9 +101,9 @@ public interface Archive extends Closeable {
     
     /**
      * Returns a sub archive that is rooted at the given path.
-     * Note that sub archives currently can't have they own meta inf and are
-     * closed automatically if they base is closed.
-     * 
+     * Note that sub archives currently can't have their own meta inf and are
+     * closed automatically if their container archive is closed.
+     * Requires a previous call to {@link #open(boolean)}. 
      * @param root root path
      * @param asJcrRoot if {@code true} the given root is the jcr_root
      * @return the archive or {@code null} if entry specified by root
@@ -108,7 +114,7 @@ public interface Archive extends Closeable {
     Archive getSubArchive(@NotNull String root, boolean asJcrRoot) throws IOException;
 
     /**
-     * closes the archive
+     * Closes the archive. Only necessary to call if the archive has been opened.
      */
     void close();
 
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/JcrPackage.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/JcrPackage.java
index db7d620..93d4a9b 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/JcrPackage.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/JcrPackage.java
@@ -29,7 +29,8 @@ import org.jetbrains.annotations.Nullable;
 import org.osgi.annotation.versioning.ProviderType;
 
 /**
- * Specifies the interface of a vault package stored in the repository.
+ * A Vault package stored in the repository.
+ * Needs to be closed in case {@link #getPackage()}, {@link  #extract(ImportOptions)} or {@link #install(ImportOptions)} has been called.
  */
 @ProviderType
 public interface JcrPackage extends Comparable<JcrPackage>, AutoCloseable {
@@ -85,7 +86,7 @@ public interface JcrPackage extends Comparable<JcrPackage>, AutoCloseable {
 
     /**
      * Returns the vault package stored in the data of this package
-     * @return the package
+     * @return the package, this is closed when {@link #close} is called on this package
      * @throws RepositoryException if an error occurs
      * @throws IOException if an I/O error occurs
      */
@@ -245,6 +246,7 @@ public interface JcrPackage extends Comparable<JcrPackage>, AutoCloseable {
 
     /**
      * Closes this package and destroys all temporary data.
+     * Only necessary to call when {@link #getPackage()}, {@link  #extract(ImportOptions)} or {@link #install(ImportOptions)} has been called.
      */
     void close();
 
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/VaultPackage.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/VaultPackage.java
index 8f1f0d5..9be04f6 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/VaultPackage.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/VaultPackage.java
@@ -95,11 +95,12 @@ public interface VaultPackage extends PackageProperties, AutoCloseable {
     /**
      * Closes this package and releases underlying data. 
      * This will also close the underlying {@link Archive} if it has been opened.
+     * Only necessary to call if package has been opened via {@link #getArchive()} or {@link #getMetaInf()}.
      */
     void close();
 
     /**
-     * Returns the underlying package archive.
+     * Returns the underlying package archive. Opens the archive when called for the first time.
      * This does not need to be closed explicitly but rather is implicitly closed via
      * a call to {@link #close()}.
      * @return the archive or {@code null} if already closed
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageImpl.java
index 4d5a8b0..aecdaac 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageImpl.java
@@ -402,9 +402,9 @@ public class JcrPackageImpl implements JcrPackage {
 
         // process sub packages
         Session s = node.getSession();
-        List<JcrPackageImpl> subPacks = new LinkedList<JcrPackageImpl>();
+        List<JcrPackageImpl> subPacks = new LinkedList<>();
         // contains a value only if a more recent version of the package with the given id (from the key) is already installed
-        Map<PackageId, PackageId> newerPackageIdPerSubPackage = new HashMap<PackageId, PackageId>();
+        Map<PackageId, PackageId> newerPackageIdPerSubPackage = new HashMap<>();
         for (String path: subPackages) {
             if (s.nodeExists(path)) {
                 JcrPackageImpl p = new JcrPackageImpl(mgr, s.getNode(path));
@@ -979,12 +979,9 @@ public class JcrPackageImpl implements JcrPackage {
      * {@inheritDoc}
      */
     public void uninstall(ImportOptions opts) throws RepositoryException, PackageException, IOException {
-        uninstall(new HashSet<PackageId>(), opts);
+        uninstall(new HashSet<>(), opts);
     }
 
-    /**
-     * {@inheritDoc}
-     */
     private void uninstall(Set<PackageId> processed, ImportOptions opts) throws RepositoryException, PackageException, IOException {
         getDefinition();
         if (def != null) {
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageManagerImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageManagerImpl.java
index 30e137a..4e5d477 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageManagerImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageManagerImpl.java
@@ -205,30 +205,27 @@ public class JcrPackageManagerImpl extends PackageManagerImpl implements JcrPack
         } else {
             archive = new ArchiveWrapper(archive);
         }
-        ZipVaultPackage pkg = new ZipVaultPackage(archive, true);
-
-        PackageId pid = pkg.getId();
-        JcrPackage jcrPack = registry.upload(pkg, replace);
-        jcrPack = new JcrPackageImpl(registry, jcrPack.getNode(), pkg);
-        jcrPack.extract(options);
-
         Set<PackageId> ids = new HashSet<>();
-        ids.add(pid);
-
-        if (spfArchive != null) {
-            for (Archive.Entry e: spfArchive.getSubPackageEntries()) {
-                InputStream in = spfArchive.openInputStream(e);
-                if (in != null) {
-                    try (Archive subArchive = new ZipStreamArchive(in)) {
-                        PackageId[] subIds = extract(subArchive, options, replace);
-                        ids.addAll(Arrays.asList(subIds));
+        try (ZipVaultPackage pkg = new ZipVaultPackage(archive, true)) {
+            PackageId pid = pkg.getId();
+            // the returned package is invalid as it does not contain any content
+            try (JcrPackage invalidJcrPack = registry.upload(pkg, replace)) {
+                // create valid package based on invalidJcrPack and the underlying archive
+                try (JcrPackage jcrPack = new JcrPackageImpl(registry, invalidJcrPack.getNode(), pkg)) {
+                    jcrPack.extract(options);
+                    ids.add(pid);
+                    if (spfArchive != null) {
+                        for (Archive.Entry e: spfArchive.getSubPackageEntries()) {
+                            try (InputStream in = spfArchive.openInputStream(e);
+                                    Archive subArchive = new ZipStreamArchive(in)) {
+                                PackageId[] subIds = extract(subArchive, options, replace);
+                                ids.addAll(Arrays.asList(subIds));
+                            }
+                        }
                     }
                 }
             }
         }
-
-        pkg.close();
-        jcrPack.close();
         return ids.toArray(new PackageId[ids.size()]);
     }
 
@@ -248,8 +245,7 @@ public class JcrPackageManagerImpl extends PackageManagerImpl implements JcrPack
     @Override
     public JcrPackage upload(File file, boolean isTmpFile, boolean replace, String nameHint, boolean strict)
             throws RepositoryException, IOException {
-        ZipVaultPackage pack = new ZipVaultPackage(file, isTmpFile, strict);
-        try {
+        try (ZipVaultPackage pack = new ZipVaultPackage(file, isTmpFile, strict)) {
             return registry.upload(pack, replace);
         } catch (PackageExistsException e) {
             throw new ItemExistsException(e.getMessage(), e);
@@ -386,13 +382,19 @@ public class JcrPackageManagerImpl extends PackageManagerImpl implements JcrPack
             throws RepositoryException, PackageException {
         List<JcrPackage> subs = listPackages(def.getMetaInf().getFilter());
         PackageId id = def.getId();
-        for (JcrPackage p: subs) {
-            // check if not include itself
-            if (p.getDefinition().getId().equals(id)) {
-                throw new PackageException("A package cannot include itself. Check filter definition.");
+        try {
+            for (JcrPackage p : subs) {
+                // check if not include itself
+                if (p.getDefinition().getId().equals(id)) {
+                    throw new PackageException("A package cannot include itself. Check filter definition.");
+                }
+                if (!p.isSealed()) {
+                    throw new PackageException("Only sealed (built) sub packages allowed: " + p.getDefinition().getId());
+                }
             }
-            if (!p.isSealed()) {
-                throw new PackageException("Only sealed (built) sub packages allowed: " + p.getDefinition().getId());
+        } finally {
+            for (JcrPackage p : subs) {
+                p.close();
             }
         }
     }
@@ -435,24 +437,21 @@ public class JcrPackageManagerImpl extends PackageManagerImpl implements JcrPack
         opts.setListener(listener);
         opts.setPostProcessor(def.getInjectProcessor());
 
-        VaultPackage dst = rewrap(opts, src, (File) null);
-
-        // update this content
-        Node packNode = pack.getNode();
-        Node contentNode = packNode.getNode(JcrConstants.JCR_CONTENT);
-        InputStream in;
-        try {
-            in = FileUtils.openInputStream(dst.getFile());
-        } catch (IOException e) {
-            throw new PackageException(e);
+        try (VaultPackage dst = rewrap(opts, src, (File) null)) {
+            // update this content
+            Node packNode = pack.getNode();
+            Node contentNode = packNode.getNode(JcrConstants.JCR_CONTENT);
+            try (InputStream in = FileUtils.openInputStream(dst.getFile())) {
+                // stay jcr 1.0 compatible
+                //noinspection deprecation
+                contentNode.setProperty(JcrConstants.JCR_DATA, in);
+                contentNode.setProperty(JcrConstants.JCR_LASTMODIFIED, now);
+                contentNode.setProperty(JcrConstants.JCR_MIMETYPE, JcrPackage.MIME_TYPE);
+                packNode.getSession().save();
+            } catch (IOException e) {
+                throw new PackageException(e);
+            }
         }
-        // stay jcr 1.0 compatible
-        //noinspection deprecation
-        contentNode.setProperty(JcrConstants.JCR_DATA, in);
-        contentNode.setProperty(JcrConstants.JCR_LASTMODIFIED, now);
-        contentNode.setProperty(JcrConstants.JCR_MIMETYPE, JcrPackage.MIME_TYPE);
-        packNode.getSession().save();
-        dst.close();
     }
 
 
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackageManagerImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackageManagerImpl.java
index 71a6758..140a7c9 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackageManagerImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/PackageManagerImpl.java
@@ -173,23 +173,19 @@ public class PackageManagerImpl implements PackageManager {
     @Override
     public VaultPackage rewrap(ExportOptions opts, VaultPackage src, File file)
             throws IOException, RepositoryException {
-        OutputStream out = null;
         boolean isTmp = false;
         boolean success = false;
-        try {
-            if (file == null) {
-                file = File.createTempFile("filevault", ".zip");
-                isTmp = true;
-            }
-            out = FileUtils.openOutputStream(file);
+        if (file == null) {
+            file = File.createTempFile("filevault", ".zip");
+            isTmp = true;
+        }
+        try (OutputStream out = FileUtils.openOutputStream(file);){
             rewrap(opts, src, out);
-            IOUtils.closeQuietly(out);
             success = true;
             VaultPackage pack =  new ZipVaultPackage(file, isTmp);
             dispatch(PackageEvent.Type.REWRAPP, pack.getId(), null);
             return pack;
         } finally {
-            IOUtils.closeQuietly(out);
             if (isTmp && !success) {
                 FileUtils.deleteQuietly(file);
             }
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
index 67fd9c7..a608625 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/ZipVaultPackage.java
@@ -318,13 +318,4 @@ public class ZipVaultPackage extends PackagePropertiesImpl implements VaultPacka
         return getMetaInf().getProperties();
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        try {
-            close();
-        } catch (Throwable e) {
-            // ignore
-        }
-        super.finalize();
-    }
 }
\ No newline at end of file
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/JcrPackageRegistry.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/JcrPackageRegistry.java
index 897348e..1a2a080 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/JcrPackageRegistry.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/JcrPackageRegistry.java
@@ -16,8 +16,8 @@
  */
 package org.apache.jackrabbit.vault.packaging.registry.impl;
 
-import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -34,10 +34,11 @@ import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.nodetype.NodeType;
 
-import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.input.NullInputStream;
 import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.fs.config.MetaInf;
+import org.apache.jackrabbit.vault.fs.io.Archive;
 import org.apache.jackrabbit.vault.fs.io.ImportOptions;
 import org.apache.jackrabbit.vault.fs.io.MemoryArchive;
 import org.apache.jackrabbit.vault.fs.spi.CNDReader;
@@ -62,7 +63,6 @@ import org.apache.jackrabbit.vault.packaging.registry.PackageRegistry;
 import org.apache.jackrabbit.vault.packaging.registry.RegisteredPackage;
 import org.apache.jackrabbit.vault.util.InputStreamPump;
 import org.apache.jackrabbit.vault.util.JcrConstants;
-import org.apache.jackrabbit.util.Text;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
@@ -290,9 +290,6 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         return null;
     }
 
-    /**
-     * {@inheritDoc}
-     */
     public JcrPackage open(Node node, boolean allowInvalid) throws RepositoryException {
         JcrPackage pack = new JcrPackageImpl(this, node);
         try {
@@ -312,9 +309,6 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public PackageId resolve(Dependency dependency, boolean onlyInstalled) throws IOException {
         try {
@@ -447,7 +441,6 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
     public PackageId register(@NotNull File file, boolean replace) throws IOException, PackageExistsException {
         ZipVaultPackage pack = new ZipVaultPackage(file, false, true);
         try (JcrPackage pkg = upload(pack, replace)) {
-            //noinspection resource
             return pkg.getPackage().getId();
         } catch (RepositoryException e) {
             throw new IOException(e);
@@ -460,7 +453,16 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         throw new UnsupportedOperationException("linking files to repository persistence is not supported.");
     }
 
-    public JcrPackage upload(ZipVaultPackage pkg, boolean replace) throws RepositoryException, IOException, PackageExistsException {
+    /**
+     * Uploads the given package to the JCR package manager,
+     * @param pkg the package
+     * @param replace
+     * @return the uploaded package
+     * @throws RepositoryException
+     * @throws IOException
+     * @throws PackageExistsException
+     */
+    public JcrPackage upload(@NotNull ZipVaultPackage pkg, boolean replace) throws RepositoryException, IOException, PackageExistsException {
 
         // open zip packages
         if (pkg.getArchive().getJcrRoot() == null) {
@@ -558,9 +560,6 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         return node;
     }
 
-    /**
-     * {@inheritDoc}
-     */
     public JcrPackage create(String group, String name, String version)
             throws RepositoryException, IOException {
         // sanitize name
@@ -592,36 +591,26 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
     @NotNull
     public JcrPackage createNew(@NotNull Node parent, @NotNull PackageId pid, @Nullable VaultPackage pack, boolean autoSave)
             throws RepositoryException, IOException {
-        Node node = parent.addNode(Text.getName(getInstallationPath(pid) + ".zip"), JcrConstants.NT_FILE);
-        Node content = node.addNode(JcrConstants.JCR_CONTENT, JcrConstants.NT_RESOURCE);
-        content.addMixin(JcrPackage.NT_VLT_PACKAGE);
-        Node defNode = content.addNode(JcrPackage.NN_VLT_DEFINITION);
-        JcrPackageDefinition def = new JcrPackageDefinitionImpl(defNode);
-        def.set(JcrPackageDefinition.PN_NAME, pid.getName(), false);
-        def.set(JcrPackageDefinition.PN_GROUP, pid.getGroup(), false);
-        def.set(JcrPackageDefinition.PN_VERSION, pid.getVersionString(), false);
-        def.touch(null, false);
-        content.setProperty(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
-        content.setProperty(JcrConstants.JCR_MIMETYPE, JcrPackage.MIME_TYPE);
-        InputStream in = new ByteArrayInputStream(new byte[0]);
-        try {
-            if (pack != null && pack.getFile() != null) {
-                in = FileUtils.openInputStream(pack.getFile());
+        final Binary binary;
+        File file = (pack!=null)?pack.getFile():null;
+        if (file != null) {
+            try (InputStream input = new FileInputStream(file)) {
+                binary = parent.getSession().getValueFactory().createBinary(input);
             }
-            // stay jcr 1.0 compatible
-            //noinspection deprecation
-            content.setProperty(JcrConstants.JCR_DATA, in);
-            if (pack != null) {
-                def.unwrap(pack, true, false);
+        } else {
+            try (InputStream input = new NullInputStream(0)) {
+                binary = parent.getSession().getValueFactory().createBinary(input);
             }
+        }
+        try {
+            JcrPackage jcrPack = createNew(parent, pid, binary, pack != null ? pack.getArchive() : null);
             if (autoSave) {
                 parent.getSession().save();
             }
+            return jcrPack;
         } finally {
-            IOUtils.closeQuietly(in);
+            binary.dispose();
         }
-        dispatch(PackageEvent.Type.CREATE, pid, null);
-        return new JcrPackageImpl(this, node, (ZipVaultPackage) pack);
     }
 
     /**
@@ -630,7 +619,7 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
      * @param parent the parent node
      * @param pid the package id of the new package.
      * @param bin the binary containing the zip
-     * @param archive the archive with the meta data
+     * @param archive the archive with the meta data (may be {@code null})
      * @return the created jcr vault package.
      * @throws RepositoryException if an repository error occurs
      * @throws IOException if an I/O error occurs
@@ -638,7 +627,7 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
      * @since 3.1
      */
     @NotNull
-    private JcrPackage createNew(@NotNull Node parent, @NotNull PackageId pid, @NotNull Binary bin, @NotNull MemoryArchive archive)
+    private JcrPackage createNew(@NotNull Node parent, @NotNull PackageId pid, @NotNull Binary binary, @Nullable Archive archive)
             throws RepositoryException, IOException {
         Node node = parent.addNode(Text.getName(getInstallationPath(pid) + ".zip"), JcrConstants.NT_FILE);
         Node content = node.addNode(JcrConstants.JCR_CONTENT, JcrConstants.NT_RESOURCE);
@@ -651,7 +640,7 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         def.touch(null, false);
         content.setProperty(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
         content.setProperty(JcrConstants.JCR_MIMETYPE, JcrPackage.MIME_TYPE);
-        content.setProperty(JcrConstants.JCR_DATA, bin);
+        content.setProperty(JcrConstants.JCR_DATA, binary);
         def.unwrap(archive, false);
         dispatch(PackageEvent.Type.CREATE, pid, null);
         return new JcrPackageImpl(this, node);
@@ -678,9 +667,6 @@ public class JcrPackageRegistry extends AbstractPackageRegistry {
         dispatch(PackageEvent.Type.REMOVE, id, null);
     }
 
-    /**
-     * {@inheritDoc}
-     */
     public JcrPackage rename(JcrPackage pack, String group, String name, String version)
             throws PackageException, RepositoryException {
         if (!pack.isValid()) {