JCRVLT-114 Subpackage extraction that contains vlt:definitiion might already have installed state

git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/commons/filevault/trunk@1738155 13f79535-47bb-0310-9956-ffa450edef68
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 d31771f..f41d41b 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
@@ -443,7 +443,11 @@
                     }
                 }
                 if (p.isValid()) {
-                    PackageId pId = p.getPackage().getId();
+                    // ensure that sub package is marked as not-installed. it might contain wrong data in vlt:definition (JCRVLT-114)
+                    JcrPackageDefinitionImpl def = (JcrPackageDefinitionImpl) p.getDefinition();
+                    def.clearLastUnpacked(false);
+
+                    PackageId pId = def.getId();
                     String pName = pId.getName();
                     Version pVersion = pId.getVersion();
 
@@ -452,24 +456,25 @@
                     List<JcrPackage> listPackages = pkgMgr.listPackages(pId.getGroup(), true);
 
                     // keep some status variable if a more recent is found in the next loop
-                    boolean foundMoreRecent = false;
-                    JcrPackage foundPackage = null;
+                    PackageId newerPackageId = null;
 
                     // loop in the list of packages returned previously by package manager
                     for (JcrPackage listedPackage: listPackages) {
                         PackageId listedPackageId = listedPackage.getPackage().getId();
+                        if (listedPackageId.equals(pId)) {
+                            continue;
+                        }
                         // check that the listed package is actually from same name (so normally only version would differ)
                         // if that package is valid, installed, and the version is more recent than the one in our sub package
                         // then we can stop the loop here
-                        if (listedPackageId.getName().equals(pName) && listedPackage.isValid() && listedPackage.isInstalled() && listedPackageId.getVersion().compareTo(pVersion) == 1) {
-                            foundMoreRecent = true; 
-                            foundPackage = listedPackage;
+                        if (pName.equals(listedPackageId.getName()) && listedPackage.isValid() && listedPackage.isInstalled() && listedPackageId.getVersion().compareTo(pVersion) > 0) {
+                            newerPackageId = listedPackageId;
                             break;
                         }
                     }
                     // if a more recent version of that subpackage was found we don't need to add it to the list of sub packages to eventually extract later on.
-                    if (foundMoreRecent) {
-                        log.info("Skipping "+path+" installation due to newer version present and installed at "+foundPackage.getNode().getPath());
+                    if (newerPackageId != null) {
+                        log.info("Skipping installation if subpackage '{}' due to newer installed version: '{}'", pId, newerPackageId);
                     } else {
                         subPacks.add(p);
                     }
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/TestSubPackages.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/TestSubPackages.java
index 4655d04..57bb4d0 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/TestSubPackages.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/TestSubPackages.java
@@ -210,6 +210,25 @@
     }
 
     /**
+     * Tests if non-recursive extraction clears the installed state (JCRVLT-114)
+     */
+    @Test
+    public void testNonRecursiveClearsInstalledState() throws RepositoryException, IOException, PackageException {
+        JcrPackage packNewer = packMgr.upload(getStream("testpackages/subtest_extract_contains_newer_version.zip"), false);
+        assertNotNull(packNewer);
+
+        // extract the sub packages, but don't install them.
+        ImportOptions opts = getDefaultOptions();
+        opts.setNonRecursive(true);
+        packNewer.install(opts);
+
+        // check for sub packages version 1.0.1 exists but not installed
+        assertNodeMissing("/tmp/a");
+        assertNodeExists("/etc/packages/my_packages/subtest_test_version-1.0.1.zip");
+        assertFalse(packMgr.open(admin.getNode("/etc/packages/my_packages/subtest_test_version-1.0.1.zip")).isInstalled());
+    }
+
+    /**
      * Uninstalls a package that contains sub packages where a snapshot of a sub package was deleted
      */
     @Test
@@ -257,6 +276,8 @@
         assertTrue(packMgr.open(admin.getNode("/etc/packages/my_packages/subtest_test_version-1.0.1.zip")).isInstalled());
         assertNodeExists("/tmp/b");
 
+        opts = getDefaultOptions();
+        opts.setNonRecursive(false);
         JcrPackage packOlder = packMgr.upload(getStream("testpackages/subtest_extract_contains_older_version.zip"), false);
         packOlder.install(opts);
         assertNodeExists("/etc/packages/my_packages/subtest_test_version-1.0.zip");
@@ -265,4 +286,32 @@
 
     }
 
+    /**
+     * Tests if skipping sub packages only works for installed packages
+     */
+    @Test
+    public void testNotSkipOlderVersionInstallation() throws RepositoryException, IOException, PackageException {
+        JcrPackage packNewer = packMgr.upload(getStream("testpackages/subtest_extract_contains_newer_version.zip"), false);
+        assertNotNull(packNewer);
+
+        // extract the sub packages, but don't install them.
+        ImportOptions opts = getDefaultOptions();
+        opts.setNonRecursive(true);
+        packNewer.install(opts);
+
+        // check for sub packages version 1.0.1 exists but not installed
+        assertNodeMissing("/tmp/a");
+        assertNodeExists("/etc/packages/my_packages/subtest_test_version-1.0.1.zip");
+        assertFalse(packMgr.open(admin.getNode("/etc/packages/my_packages/subtest_test_version-1.0.1.zip")).isInstalled());
+
+        opts = getDefaultOptions();
+        opts.setNonRecursive(false);
+        JcrPackage packOlder = packMgr.upload(getStream("testpackages/subtest_extract_contains_older_version.zip"), false);
+        packOlder.install(opts);
+        assertNodeExists("/etc/packages/my_packages/subtest_test_version-1.0.zip");
+        assertTrue(packMgr.open(admin.getNode("/etc/packages/my_packages/subtest_test_version-1.0.zip")).isInstalled());
+        assertNodeExists("/tmp/a");
+
+    }
+
 }
\ No newline at end of file