Fix UnArchiver#isOverwrite not working as expected

Regression in efd980d changed the way
UnArchiver#isOverwrite flag work.
It is indented to indicate that UnArchiver should
always override the existing entries.
efd980d changed it to indicate whether existing file
should be overridden if the entry is newer,
while in this case the file should be always overridden.

This commit returns the old behavior: if the entry
is newer, override the existing file; if the entry
is older, override the existing file only if isOverwrite is true.

Fixes: #228

Author: plamentotev

src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java

@@ -401,16 +401,18 @@ else if ( isDirectory )
     protected boolean shouldExtractEntry( File targetDirectory, File targetFileName, String entryName, Date entryDate ) throws IOException
     {
         //     entryname  | entrydate | filename   | filedate | behavior
-        // (1) readme.txt | 1970      | readme.txt | 2020     | never overwrite
-        // (2) readme.txt | 2020      | readme.txt | 1970     | only overwrite when isOverWrite()
-        // (3) README.txt | 1970      | readme.txt | 2020     | case-insensitive filesystem: warn + never overwrite
+        // (1) readme.txt | 1970      | -          | -        | always extract if the file does not exist
+        // (2) readme.txt | 1970      | readme.txt | 2020     | do not overwrite unless isOverwrite() is true
+        // (3) readme.txt | 2020      | readme.txt | 1970     | always override when the file is older than the archive entry
+        // (4) README.txt | 1970      | readme.txt | 2020     | case-insensitive filesystem: warn + do not overwrite unless isOverwrite()
         //                                                      case-sensitive filesystem: extract without warning
-        // (4) README.txt | 2020      | readme.txt | 1970     | case-insensitive filesystem: warn + only overwrite when isOverWrite()
+        // (5) README.txt | 2020      | readme.txt | 1970     | case-insensitive filesystem: warn + overwrite because entry is newer
         //                                                      case-sensitive filesystem: extract without warning
 
         // The canonical file name follows the name of the archive entry, but takes into account the case-
         // sensitivity of the filesystem. So on a case-sensitive file system, file.exists() returns false for
-        // scenario (3) and (4).
+        // scenario (4) and (5).
+        // No matter the case sensitivity of the file system, file.exists() returns false when there is no file with the same name (1).
         if ( !targetFileName.exists() )
         {
             return true;
@@ -423,33 +425,20 @@ protected boolean shouldExtractEntry( File targetDirectory, File targetFileName,
                 targetDirectory.getCanonicalPath() + File.separatorChar,
                 "" )
                 + suffix;
-        boolean fileOnDiskIsNewerThanEntry = targetFileName.lastModified() >= entryDate.getTime();
+        boolean fileOnDiskIsOlderThanEntry = targetFileName.lastModified() < entryDate.getTime();
         boolean differentCasing = !entryName.equals( relativeCanonicalDestPath );
 
-        String casingMessage = String.format( Locale.ENGLISH, "Archive entry '%s' and existing file '%s' names differ only by case."
-                + " This may lead to an unexpected outcome on case-insensitive filesystems.", entryName, canonicalDestPath );
-
-        // (1)
-        if ( fileOnDiskIsNewerThanEntry )
-        {
-            // (3)
-            if ( differentCasing )
-            {
-                getLogger().warn( casingMessage );
-                casingMessageEmitted.incrementAndGet();
-            }
-            return false;
-        }
-
-        // (4)
+        // Warn for case (4) and (5) if the file system is case-insensitive
         if ( differentCasing )
         {
+            String casingMessage = String.format( Locale.ENGLISH, "Archive entry '%s' and existing file '%s' names differ only by case."
+                + " This may lead to an unexpected outcome on case-insensitive filesystems.", entryName, canonicalDestPath );
             getLogger().warn( casingMessage );
             casingMessageEmitted.incrementAndGet();
         }
 
-        // (2)
-        return isOverwrite();
+        // Override the existing file if isOverwrite() is true or if the file on disk is older than the one in the archive
+        return isOverwrite() || fileOnDiskIsOlderThanEntry;
     }
 
 }

src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java

@@ -91,11 +91,14 @@ public void shouldExtractWhenFileOnDiskDoesNotExist(  @TempDir File temporaryFol
         Date entryDate = new Date();
 
         // when & then
+        // always extract the file if it does not exist
+        assertTrue( abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
+        abstractUnArchiver.setOverwrite( false );
         assertTrue( abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
     }
 
     @Test
-    public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir File temporaryFolder ) throws IOException
+    public void shouldExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir File temporaryFolder ) throws IOException
     {
         // given
         File file = new File( temporaryFolder, "whatever.txt" );
@@ -105,11 +108,14 @@ public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir Fi
         Date entryDate = new Date( 0 );
 
         // when & then
+        // if the file is newer than archive entry, extract only if overwrite is true (default)
+        assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
+        abstractUnArchiver.setOverwrite( false );
         assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
     }
 
     @Test
-    public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing( @TempDir File temporaryFolder )
+    public void shouldExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing( @TempDir File temporaryFolder )
         throws IOException
     {
         // given
@@ -120,7 +126,7 @@ public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAbout
         Date entryDate = new Date( 0 );
 
         // when & then
-        assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
+        assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
         assertTrue( this.abstractUnArchiver.casingMessageEmitted.get() > 0 );
     }
 
@@ -135,12 +141,10 @@ public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDisk( @TempDir File
         Date entryDate = new Date( System.currentTimeMillis() );
 
         // when & then
-        this.abstractUnArchiver.setOverwrite( true );
+        // always extract the file if the archive entry is newer than the file on disk
         assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
-
-        // when & then
         this.abstractUnArchiver.setOverwrite( false );
-        assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
+        assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
     }
 
     @Test
@@ -158,7 +162,7 @@ public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDiskAndWarnAboutDiff
         this.abstractUnArchiver.setOverwrite( true );
         assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
         this.abstractUnArchiver.setOverwrite( false );
-        assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
+        assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
         assertTrue( this.abstractUnArchiver.casingMessageEmitted.get() > 0 );
     }