You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ol...@apache.org on 2020/05/27 01:35:25 UTC

[maven-shared-utils] branch pr-28 updated (759edda -> e571803)

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

olamy pushed a change to branch pr-28
in repository https://gitbox.apache.org/repos/asf/maven-shared-utils.git.


    omit 759edda  [MSHARED-884] - Use TimeUnit rather than constants in millisecond
    omit 1b6fd3c  [MSHARED-884] - Prefer Files.newBufferedReader/Writer methods
    omit 69d33b0  [MSHARED-884] - Consistent explicit use of Charset instances
    omit 603a180  [MSHARED-884] - Avoid conflicts with Java 9 additional Buffer methods
    omit e29f7f3  [MSHARED-884] - Only overwrite existing file if there's a content change
    omit 598c3db  [MSHARED-884] - Refactor to deal with the simpler no-filters case first
    omit 2780d00  [MSHARED-884] - Add tests for existing FileUtils.copyFile() method with filtering.
    omit 1bcc082  [MSHARED-884] - Add tests for existing FileUtils.copyFile() method with no filtering.
     new e571803  [MSHARED-884] - Add tests for existing FileUtils.copyFile() method with no filtering.

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (759edda)
            \
             N -- N -- N   refs/heads/pr-28 (e571803)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


[maven-shared-utils] 01/01: [MSHARED-884] - Add tests for existing FileUtils.copyFile() method with no filtering.

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olamy pushed a commit to branch pr-28
in repository https://gitbox.apache.org/repos/asf/maven-shared-utils.git

commit e57180309a64758e161513d80f1185e13dc84826
Author: Rob Oxspring <ro...@imapmail.org>
AuthorDate: Fri May 1 00:35:56 2020 +0100

    [MSHARED-884] - Add tests for existing FileUtils.copyFile() method with no filtering.
    
    - Add tests for existing FileUtils.copyFile() method with filtering.
    - Refactor to deal with the simpler no-filters case first
    - Only overwrite existing file if there's a content change
    - Avoid conflicts with Java 9 additional Buffer methods
    - Consistent explicit use of Charset instances
    - Prefer Files.newBufferedReader/Writer methods
    - Use TimeUnit rather than constants in millisecond
---
 .../apache/maven/shared/utils/io/FileUtils.java    | 164 ++++++++---
 .../maven/shared/utils/io/FileUtilsTest.java       | 321 ++++++++++++++++++++-
 2 files changed, 426 insertions(+), 59 deletions(-)

diff --git a/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java b/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
index f7de64c..8929c4d 100644
--- a/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
+++ b/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java
@@ -30,17 +30,20 @@ import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.io.OutputStream;
-import java.io.OutputStreamWriter;
+import java.io.RandomAccessFile;
 import java.io.Reader;
 import java.io.Writer;
 import java.net.URL;
+import java.nio.Buffer;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.charset.Charset;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CoderResult;
 import java.nio.file.Files;
 import java.security.SecureRandom;
 import java.text.DecimalFormat;
@@ -110,7 +113,7 @@ public class FileUtils
     /**
      * The file copy buffer size (30 MB)
      */
-    private static final long FILE_COPY_BUFFER_SIZE = ONE_MB * 30;
+    private static final int FILE_COPY_BUFFER_SIZE = ONE_MB * 30;
 
     /**
      * The vm line separator
@@ -271,13 +274,12 @@ public class FileUtils
     @Nonnull public static String fileRead( @Nonnull File file, @Nullable String encoding )
         throws IOException
     {
-        StringBuilder buf = new StringBuilder();   
-        if ( encoding == null ) 
-        {
-            encoding = Charset.defaultCharset().name();
-        }
+        Charset charset = charset( encoding );
+
+        StringBuilder buf = new StringBuilder();
+
 
-        try ( Reader reader = new InputStreamReader( new FileInputStream( file ), encoding ) )
+        try ( Reader reader = Files.newBufferedReader( file.toPath(), charset ) )
         {
             int count;
             char[] b = new char[512];
@@ -329,15 +331,11 @@ public class FileUtils
     public static void fileAppend( @Nonnull String fileName, @Nullable String encoding, @Nonnull String data )
         throws IOException
     {
-        
-        if ( encoding == null ) 
-        {
-            encoding = Charset.defaultCharset().name();
-        }
-        
+        Charset charset = charset( encoding );
+
         try ( OutputStream out = new FileOutputStream( fileName, true ) )
         {
-          out.write( data.getBytes( encoding ) );
+          out.write( data.getBytes( charset ) );
         }
     }
 
@@ -381,13 +379,9 @@ public class FileUtils
     public static void fileWrite( @Nonnull File file, @Nullable String encoding, @Nonnull String data )
         throws IOException
     {
-        
-        if ( encoding == null ) 
-        {
-            encoding = Charset.defaultCharset().name();
-        }
+        Charset charset = charset( encoding );
 
-        try ( Writer writer = new OutputStreamWriter( new FileOutputStream( file ), encoding ) ) 
+        try ( Writer writer = Files.newBufferedWriter( file.toPath(), charset ) )
         {
             writer.write( data );
         }
@@ -418,13 +412,9 @@ public class FileUtils
     public static void fileWriteArray( @Nonnull File file, @Nullable String encoding, @Nullable String... data )
         throws IOException
     {
-        
-        if ( encoding == null ) 
-        {
-            encoding = Charset.defaultCharset().name();
-        }
+        Charset charset = charset( encoding );
 
-        try ( Writer writer = new OutputStreamWriter( new FileOutputStream( file ), encoding ) )
+        try ( Writer writer = Files.newBufferedWriter( file.toPath(), charset ) )
         {
             for ( int i = 0; data != null && i < data.length; i++ )
             {
@@ -1810,34 +1800,97 @@ public class FileUtils
                                  @Nullable FilterWrapper[] wrappers, boolean overwrite )
         throws IOException
     {
-        if ( wrappers != null && wrappers.length > 0 )
+        if ( wrappers == null || wrappers.length == 0 )
         {
-            
-            if ( encoding == null || encoding.isEmpty() ) 
+            if ( overwrite || to.lastModified() < from.lastModified() )
             {
-                encoding = Charset.defaultCharset().name();
+                copyFile( from, to );
             }
-            
+        }
+        else
+        {
+            Charset charset = charset( encoding );
+
             // buffer so it isn't reading a byte at a time!
-            try ( Reader fileReader =
-                new BufferedReader( new InputStreamReader( new FileInputStream( from ), encoding ) );
-                            Writer fileWriter = new OutputStreamWriter( new FileOutputStream( to ), encoding ) )
+            try ( Reader fileReader = Files.newBufferedReader( from.toPath(), charset ) )
             {
-
                 Reader wrapped = fileReader;
                 for ( FilterWrapper wrapper : wrappers )
                 {
                     wrapped = wrapper.getReader( wrapped );
                 }
 
-                IOUtil.copy( wrapped, fileWriter );
-            }
-        }
-        else
-        {
-            if ( to.lastModified() < from.lastModified() || overwrite )
-            {
-                copyFile( from, to );
+                if ( overwrite || !to.exists() )
+                {
+                    try ( Writer fileWriter = Files.newBufferedWriter( to.toPath(), charset ) )
+                    {
+                        IOUtil.copy( wrapped, fileWriter );
+                    }
+                }
+                else
+                {
+                    CharsetEncoder encoder = charset.newEncoder();
+
+                    int totalBufferSize = FILE_COPY_BUFFER_SIZE;
+
+                    int charBufferSize = ( int ) Math.floor( totalBufferSize / ( 2 + 2 * encoder.maxBytesPerChar() ) );
+                    int byteBufferSize = ( int ) Math.ceil( charBufferSize * encoder.maxBytesPerChar() );
+
+                    CharBuffer newChars = CharBuffer.allocate( charBufferSize );
+                    ByteBuffer newBytes = ByteBuffer.allocate( byteBufferSize );
+                    ByteBuffer existingBytes = ByteBuffer.allocate( byteBufferSize );
+
+                    CoderResult coderResult;
+                    int existingRead;
+                    boolean writing = false;
+
+                    try ( final RandomAccessFile existing = new RandomAccessFile( to, "rw" ) )
+                    {
+                        int n;
+                        while ( -1 != ( n = wrapped.read( newChars ) ) )
+                        {
+                            ( ( Buffer ) newChars ).flip();
+
+                            coderResult = encoder.encode( newChars, newBytes, n != 0 );
+                            if ( coderResult.isError() )
+                            {
+                                coderResult.throwException();
+                            }
+
+                            ( ( Buffer ) newBytes ).flip();
+
+                            if ( !writing )
+                            {
+                                existingRead = existing.read( existingBytes.array(), 0, newBytes.remaining() );
+                                ( ( Buffer ) existingBytes ).position( existingRead );
+                                ( ( Buffer ) existingBytes ).flip();
+
+                                if ( newBytes.compareTo( existingBytes ) != 0 )
+                                {
+                                    writing = true;
+                                    if ( existingRead > 0 )
+                                    {
+                                        existing.seek( existing.getFilePointer() - existingRead );
+                                    }
+                                }
+                            }
+
+                            if ( writing )
+                            {
+                                existing.write( newBytes.array(), 0, newBytes.remaining() );
+                            }
+
+                            ( ( Buffer ) newChars ).clear();
+                            ( ( Buffer ) newBytes ).clear();
+                            ( ( Buffer ) existingBytes ).clear();
+                        }
+
+                        if ( existing.length() > existing.getFilePointer() )
+                        {
+                            existing.setLength( existing.getFilePointer() );
+                        }
+                    }
+                }
             }
         }
     }
@@ -1856,7 +1909,7 @@ public class FileUtils
 
         if ( file.exists() )
         {
-            try ( BufferedReader reader = new BufferedReader( new FileReader( file ) ) )
+            try ( BufferedReader reader = Files.newBufferedReader( file.toPath(), Charset.defaultCharset() ) )
             {
                 for ( String line = reader.readLine(); line != null; line = reader.readLine() )
                 {
@@ -1874,6 +1927,23 @@ public class FileUtils
     }
 
     /**
+     * Returns the named charset or the default charset.
+     * @param encoding the name or alias of the charset, null or empty
+     * @return A charset object for the named or default charset.
+     */
+    private static Charset charset( String encoding )
+    {
+        if ( encoding == null || encoding.isEmpty() )
+        {
+            return Charset.defaultCharset();
+        }
+        else
+        {
+            return Charset.forName( encoding );
+        }
+    }
+
+    /**
      * For Windows OS, check if the file name contains any of the following characters:
      * <code>":", "*", "?", "\"", "<", ">", "|"</code>
      *
diff --git a/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java b/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
index 69e72a9..24bf43d 100644
--- a/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
+++ b/src/test/java/org/apache/maven/shared/utils/io/FileUtilsTest.java
@@ -14,19 +14,38 @@ import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeThat;
 import static org.junit.Assume.assumeTrue;
 
+import org.apache.maven.shared.utils.Os;
+import org.apache.maven.shared.utils.testhelpers.FileTestHelper;
+import org.codehaus.plexus.util.InterpolationFilterReader;
+import org.hamcrest.CoreMatchers;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
+
+import javax.annotation.Nonnull;
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.Reader;
+import java.io.Writer;
 import java.net.URL;
 import java.nio.file.Files;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
@@ -47,16 +66,6 @@ import java.util.List;
  * under the License.
  */
 
-import org.apache.maven.shared.utils.Os;
-import org.apache.maven.shared.utils.testhelpers.FileTestHelper;
-import org.hamcrest.CoreMatchers;
-import org.junit.Before;
-import org.junit.Ignore;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.rules.TestName;
-
 /**
  * This is used to test FileUtils for correctness.
  *
@@ -441,10 +450,299 @@ public class FileUtilsTest
             testFile1.lastModified() == destination.lastModified());*/
     }
 
+    /** A time today, rounded down to the previous minute */
+    private static long MODIFIED_TODAY = (System.currentTimeMillis() / TimeUnit.MINUTES.toMillis( 1 )) * TimeUnit.MINUTES.toMillis( 1 );
+
+    /** A time yesterday, rounded down to the previous minute */
+    private static long MODIFIED_YESTERDAY = MODIFIED_TODAY - TimeUnit.DAYS.toMillis( 1 );
+
+    /** A time last week, rounded down to the previous minute */
+    private static long MODIFIED_LAST_WEEK = MODIFIED_TODAY - TimeUnit.DAYS.toMillis( 7 );
+
     @Test
-    public void copyFileThatIsSymlink()
+    public void copyFileWithNoFiltersAndNoDestination()
         throws Exception
     {
+        File from = write(
+            "from.txt",
+            MODIFIED_YESTERDAY,
+            "Hello World!"
+        );
+        File to = new File(
+            tempFolder.getRoot(),
+            "to.txt"
+        );
+
+        FileUtils.copyFile( from, to, null, ( FileUtils.FilterWrapper[] ) null);
+
+        assertTrue(
+            "to.txt did not exist so should have been written",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello World!" );
+    }
+
+    @Test
+    public void copyFileWithNoFiltersAndOutdatedDestination()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_YESTERDAY,
+            "Hello World!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_LAST_WEEK,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null, ( FileUtils.FilterWrapper[] ) null);
+
+        assertTrue(
+            "to.txt was outdated so should have been overwritten",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello World!" );
+    }
+
+    @Test
+    public void copyFileWithNoFiltersAndNewerDestination()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_LAST_WEEK,
+            "Hello World!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_YESTERDAY,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null, ( FileUtils.FilterWrapper[] ) null);
+
+        assertTrue(
+            "to.txt was newer so should have been left alone",
+            to.lastModified() < MODIFIED_TODAY
+        );
+        assertFileContent( to, "Older content" );
+    }
+
+    @Test
+    public void copyFileWithNoFiltersAndNewerDestinationButForcedOverwrite()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_LAST_WEEK,
+            "Hello World!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_YESTERDAY,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null, null, true);
+
+        assertTrue(
+            "to.txt was newer but the overwrite should have been forced",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello World!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringButNoFilters()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_YESTERDAY,
+            "Hello ${name}!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_LAST_WEEK,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null );
+
+        assertTrue(
+            "to.txt was outdated so should have been overwritten",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello ${name}!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringAndNoDestination()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_YESTERDAY,
+            "Hello ${name}!"
+        );
+        File to = new File(
+            tempFolder.getRoot(),
+            "to.txt"
+        );
+
+        FileUtils.copyFile( from, to, null, wrappers( "name", "Bob" ) );
+
+        assertTrue(
+            "to.txt did not exist so should have been written",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello Bob!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringAndOutdatedDestination()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_YESTERDAY,
+            "Hello ${name}!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_LAST_WEEK,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null, wrappers( "name", "Bob" ) );
+
+        assertTrue(
+            "to.txt was outdated so should have been overwritten",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello Bob!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringAndNewerDestinationButForcedOverwrite()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_LAST_WEEK,
+            "Hello ${name}!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_YESTERDAY,
+            "Older content"
+        );
+
+        FileUtils.copyFile( from, to, null, wrappers( "name", "Bob" ), true );
+
+        assertTrue(
+            "to.txt was newer but the overwrite should have been forced",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello Bob!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringAndNewerDestinationButModifiedContent()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_LAST_WEEK,
+            "Hello ${name}!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_YESTERDAY,
+            "Hello Charlie!"
+        );
+
+        FileUtils.copyFile( from, to, null, wrappers( "name", "Bob" ) );
+
+        assertTrue(
+            "to.txt was outdated so should have been overwritten",
+            to.lastModified() >= MODIFIED_TODAY
+        );
+        assertFileContent( to, "Hello Bob!" );
+    }
+
+    @Test
+    public void copyFileWithFilteringAndNewerDestinationAndMatchingContent()
+            throws Exception
+    {
+        File from = write(
+            "from.txt",
+            MODIFIED_LAST_WEEK,
+            "Hello ${name}!"
+        );
+        File to = write(
+            "to.txt",
+            MODIFIED_YESTERDAY,
+            "Hello Bob!"
+        );
+
+        String encoding = null;
+
+        FileUtils.copyFile( from, to, null, wrappers( "name", "Bob" ) );
+
+        assertFileContent( to, "Hello Bob!" );
+        assertTrue(
+            "to.txt content should be unchanged and have been left alone",
+            to.lastModified() < MODIFIED_TODAY
+        );
+    }
+
+    private FileUtils.FilterWrapper[] wrappers( String key, String value )
+    {
+        final Map map = new HashMap();
+        map.put( key, value );
+        return new FileUtils.FilterWrapper[]
+            {
+                new FileUtils.FilterWrapper()
+                {
+                    @Override
+                    public Reader getReader( Reader reader )
+                    {
+                        return new InterpolationFilterReader( reader, map );
+                    }
+                }
+            };
+    }
+
+    private File write( @Nonnull String name, long lastModified, @Nonnull String text) throws IOException
+    {
+        final File file = new File( tempFolder.getRoot(), name );
+        try ( final Writer writer = new FileWriter( file ) ) {
+            writer.write(text);
+        }
+        assertTrue( file.setLastModified( lastModified ) );
+        assertEquals( "Failed to set lastModified date on " + file.getPath(), lastModified, file.lastModified() );
+        return file;
+    }
+
+    private static void assertFileContent( @Nonnull File file, @Nonnull String expected ) throws IOException
+    {
+        try ( Reader in = new FileReader( file ))
+        {
+            assertEquals(
+                "Expected " + file.getPath() + " to contain: " + expected,
+                expected,
+                IOUtil.toString( in )
+            );
+        }
+    }
+
+    @Test
+    public void copyFileThatIsSymlink()
+            throws Exception
+    {
         assumeFalse( Os.isFamily( Os.FAMILY_WINDOWS ) );
 
         File destination = new File( tempFolder.getRoot(), "symCopy.txt" );
@@ -457,7 +755,6 @@ public class FileUtilsTest
     }
 
 
-
     @Test
     public void deleteFile()
         throws Exception