You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2016/11/23 14:20:01 UTC

svn commit: r1770974 - in /sling/trunk/installer/core/src: main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java

Author: bdelacretaz
Date: Wed Nov 23 14:20:01 2016
New Revision: 1770974

URL: http://svn.apache.org/viewvc?rev=1770974&view=rev
Log:
SLING-6319 - fix prepareToRemove method

Modified:
    sling/trunk/installer/core/src/main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java
    sling/trunk/installer/core/src/test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java

Modified: sling/trunk/installer/core/src/main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/core/src/main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java?rev=1770974&r1=1770973&r2=1770974&view=diff
==============================================================================
--- sling/trunk/installer/core/src/main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java (original)
+++ sling/trunk/installer/core/src/main/java/org/apache/sling/installer/core/impl/OsgiInstallerImpl.java Wed Nov 23 14:20:01 2016
@@ -504,17 +504,25 @@ implements OsgiInstaller, ResourceChange
      *  data file, or delete it if it's not needed anymore.
      */
     private void prepareToRemove(InternalResource existing, Collection<InternalResource> incoming) {
-        // check if we got the same resource
-        if ( existing.getPrivateCopyOfFile() != null ) {
-            boolean found = false;
+        if(existing.getPrivateCopyOfFile() != null) {
             for(final InternalResource r : incoming) {
-                if ( r.getURL().equals(existing.getURL()) && r.getPrivateCopyOfFile() == null ) {
-                    found = true;
-                    r.setPrivateCopyOfFile(existing.getPrivateCopyOfFile());
+                if(r.getURL().equals(existing.getURL())) {
+                    // We have a resource r in "incoming" that's the same as "existing"
+                    if(r.getPrivateCopyOfFile() == null) {
+                        // New one has not data file, use the existing one
+                        logger.debug("{} has no private data file, using the one from {}", r.getURL(), existing.getURL());
+                        r.setPrivateCopyOfFile(existing.getPrivateCopyOfFile());
+                        existing.setPrivateCopyOfFile(null);
+                    } else if(r.getPrivateCopyOfFile().equals(existing.getPrivateCopyOfFile())) {
+                        logger.debug("{} has same private data file as existing resource, keeping it", r.getURL());
+                        existing.setPrivateCopyOfFile(null);
+                    }
                     break;
                 }
             }
-            if ( !found ) {
+            
+            if(existing.getPrivateCopyOfFile() != null) {
+                logger.debug("Private data file not needed anymore, deleting it: {}", existing.getURL());
                 existing.getPrivateCopyOfFile().delete();
             }
         }

Modified: sling/trunk/installer/core/src/test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/core/src/test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java?rev=1770974&r1=1770973&r2=1770974&view=diff
==============================================================================
--- sling/trunk/installer/core/src/test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java (original)
+++ sling/trunk/installer/core/src/test/java/org/apache/sling/installer/core/impl/OsgiInstallerImplTest.java Wed Nov 23 14:20:01 2016
@@ -131,7 +131,8 @@ public class OsgiInstallerImplTest {
         assertDataFiles(A, B, C);
         
         installer.registerResources(SCHEME, mockBundles(C, D));
-        // TODO this is wrong, should be C, D
-        assertDataFiles(B, D);
+        // TODO B should be gone...not critical but suboptimal,
+        // we might need to review this private files logic more broadly
+        assertDataFiles(B, C, D);
     }
 }
\ No newline at end of file