From c3181959dc2339ff454b2fa2d0a52e2c1fcd0ecb Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 29 May 2020 06:37:50 -0400 Subject: [PATCH 1/4] deprecate PropertyUtils constructor and clean up docs --- .../maven/shared/utils/PropertyUtils.java | 44 ++++++++----------- .../maven/shared/utils/PropertyUtilsTest.java | 39 +++++----------- 2 files changed, 29 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java index ca6b6935..487a9a6d 100644 --- a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java +++ b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java @@ -29,26 +29,27 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.apache.maven.shared.utils.io.IOUtil; - /** - * + * Static utility methods for loading properties. */ public class PropertyUtils { /** * The constructor. + * + * @deprecated This is a utility class with only static methods. Don't create instances of it. */ + @Deprecated public PropertyUtils() { // should throw new IllegalAccessError( "Utility class" ); } /** - * @param url The URL which should be used to load the properties. - * @return The loaded properties. - * @deprecated As of 3.1.0, please use method {@link #loadOptionalProperties(java.net.URL)}. This method should not + * @param url the URL which should be used to load the properties + * @return the loaded properties + * @deprecated use {@link #loadOptionalProperties(java.net.URL)} instead. This method should not * be used as it suppresses exceptions silently when loading properties fails and returns {@code null} * instead of an empty {@code Properties} instance when the given {@code URL} is {@code null}. */ @@ -67,9 +68,9 @@ public static java.util.Properties loadProperties( @Nonnull URL url ) } /** - * @param file The file from which the properties will be loaded. - * @return The loaded properties. - * @deprecated As of 3.1.0, please use method {@link #loadOptionalProperties(java.io.File)}. This method should not + * @param file the file from which the properties will be loaded + * @return the loaded properties + * @deprecated use {@link #loadOptionalProperties(java.io.File)} instead. This method should not * be used as it suppresses exceptions silently when loading properties fails and returns {@code null} * instead of an empty {@code Properties} instance when the given {@code File} is {@code null}. */ @@ -89,8 +90,8 @@ public static Properties loadProperties( @Nonnull File file ) /** * @param is {@link InputStream} - * @return The loaded properties. - * @deprecated As of 3.1.0, please use method {@link #loadOptionalProperties(java.io.InputStream)}. This method + * @return the loaded properties + * @deprecated use {@link #loadOptionalProperties(java.io.InputStream)} instead. This method * should not be used as it suppresses exceptions silently when loading properties fails. */ @Deprecated @@ -98,13 +99,12 @@ public static Properties loadProperties( @Nullable InputStream is ) { try { - // to make this the same behaviour as the others we should really return null on any error Properties result = new Properties(); if ( is != null ) { - try + try ( InputStream in = is ) { - result.load( is ); + result.load( in ); } catch ( IOException e ) { @@ -117,10 +117,6 @@ public static Properties loadProperties( @Nullable InputStream is ) { // ignore } - finally - { - IOUtil.close( is ); - } return null; } @@ -154,7 +150,7 @@ public static Properties loadOptionalProperties( final @Nullable URL url ) } /** - * Loads {@code Properties} from a given {@code File}. + * Loads {@code Properties} from a {@code File}. *

* If the given {@code File} is {@code null} or the properties file can't be read, an empty properties object is * returned. @@ -185,7 +181,7 @@ public static Properties loadOptionalProperties( final @Nullable File file ) } /** - * Loads {@code Properties} from a given {@code InputStream}. + * Loads {@code Properties} from an {@code InputStream}. *

* If the given {@code InputStream} is {@code null} or the properties can't be read, an empty properties object is * returned. @@ -203,18 +199,14 @@ public static Properties loadOptionalProperties( final @Nullable InputStream inp if ( inputStream != null ) { - try + try ( InputStream in = inputStream ) // reassign inputStream to autoclose { - properties.load( inputStream ); + properties.load( in ); } catch ( IllegalArgumentException | IOException ex ) { // ignore and return empty properties } - finally - { - IOUtil.close( inputStream ); - } } return properties; diff --git a/src/test/java/org/apache/maven/shared/utils/PropertyUtilsTest.java b/src/test/java/org/apache/maven/shared/utils/PropertyUtilsTest.java index 41067a40..0c696415 100644 --- a/src/test/java/org/apache/maven/shared/utils/PropertyUtilsTest.java +++ b/src/test/java/org/apache/maven/shared/utils/PropertyUtilsTest.java @@ -33,7 +33,6 @@ import java.net.URL; import java.util.Properties; -import org.apache.maven.shared.utils.io.IOUtil; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -166,23 +165,15 @@ public void loadValidInputStream() throws UnsupportedEncodingException @SuppressWarnings( "deprecation" ) public void loadValidFile() throws IOException { - OutputStream out = null; - try + File valid = tempFolder.newFile( "valid" ); + Properties value = new Properties(); + value.setProperty( "a", "b" ); + try ( OutputStream out = new FileOutputStream( valid ) ) { - File valid = tempFolder.newFile( "valid" ); - Properties value = new Properties(); - value.setProperty( "a", "b" ); - out = new FileOutputStream( valid ); value.store( out, "a test" ); - out.close(); - out = null; assertThat( PropertyUtils.loadProperties( valid ), is( value ) ); assertThat( PropertyUtils.loadOptionalProperties( valid ), is( value ) ); } - finally - { - IOUtil.close( out ); - } } @Test @@ -190,22 +181,14 @@ public void loadValidFile() throws IOException @SuppressWarnings( "deprecation" ) public void loadValidURL() throws IOException { - OutputStream out = null; - try - { - File valid = tempFolder.newFile( "valid" ); - Properties value = new Properties(); - value.setProperty( "a", "b" ); - out = new FileOutputStream( valid ); - value.store( out, "a test" ); - out.close(); - out = null; - assertThat( PropertyUtils.loadProperties( valid.toURI().toURL() ), is( value ) ); - assertThat( PropertyUtils.loadOptionalProperties( valid.toURI().toURL() ), is( value ) ); - } - finally + File valid = tempFolder.newFile( "valid" ); + Properties value = new Properties(); + value.setProperty( "a", "b" ); + try ( OutputStream out = new FileOutputStream( valid ) ) { - IOUtil.close( out ); + value.store( out, "a test" ); + assertThat( PropertyUtils.loadProperties( valid.toURI().toURL() ), is( value ) ); + assertThat( PropertyUtils.loadOptionalProperties( valid.toURI().toURL() ), is( value ) ); } } From 6a8154517a76c80d3daa7cb16a069b722ba231f1 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 29 May 2020 06:44:59 -0400 Subject: [PATCH 2/4] deprecate PathUtils constructor --- .../java/org/apache/maven/shared/utils/PathTool.java | 11 +++++++++++ .../org/apache/maven/shared/utils/PropertyUtils.java | 1 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/shared/utils/PathTool.java b/src/main/java/org/apache/maven/shared/utils/PathTool.java index b6162110..28bc3083 100644 --- a/src/main/java/org/apache/maven/shared/utils/PathTool.java +++ b/src/main/java/org/apache/maven/shared/utils/PathTool.java @@ -35,6 +35,17 @@ */ public class PathTool { + + /** + * The constructor. + * + * @deprecated This is a utility class with only static methods. Don't create instances of it. + */ + @Deprecated + public PathTool() + { + } + /** * Determines the relative path of a filename from a base directory. * This method is useful in building relative links within pages of diff --git a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java index 487a9a6d..8145a63d 100644 --- a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java +++ b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java @@ -43,7 +43,6 @@ public class PropertyUtils @Deprecated public PropertyUtils() { - // should throw new IllegalAccessError( "Utility class" ); } /** From 87a1903bb34c2d3e5bcd05bb6c725e6372b9f07f Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 29 May 2020 07:57:54 -0400 Subject: [PATCH 3/4] warn about closing streams --- .../org/apache/maven/shared/utils/PropertyUtils.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java index 8145a63d..e1bf73b6 100644 --- a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java +++ b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java @@ -88,6 +88,10 @@ public static Properties loadProperties( @Nonnull File file ) } /** + * Loads {@code Properties} from an {@code InputStream} and closes the stream. + * In a future release, this will no longer close the stream, so callers + * should close the stream themselves. + * * @param is {@link InputStream} * @return the loaded properties * @deprecated use {@link #loadOptionalProperties(java.io.InputStream)} instead. This method @@ -180,11 +184,10 @@ public static Properties loadOptionalProperties( final @Nullable File file ) } /** - * Loads {@code Properties} from an {@code InputStream}. - *

+ * Loads {@code Properties} from an {@code InputStream} and closes the stream. * If the given {@code InputStream} is {@code null} or the properties can't be read, an empty properties object is - * returned. - *

+ * returned. In a future release, this will no longer close the stream, so callers + * should close the stream themselves. * * @param inputStream the properties resource to load or {@code null} * @return the loaded properties or an empty {@code Properties} instance if properties fail to load From 5b6bd1f8fc04a7d77d0d063dbd0b963b0426e1ea Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 29 May 2020 08:04:04 -0400 Subject: [PATCH 4/4] close streams we create --- .../java/org/apache/maven/shared/utils/PropertyUtils.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java index e1bf73b6..70e21ed2 100644 --- a/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java +++ b/src/main/java/org/apache/maven/shared/utils/PropertyUtils.java @@ -55,9 +55,9 @@ public PropertyUtils() @Deprecated public static java.util.Properties loadProperties( @Nonnull URL url ) { - try + try ( InputStream in = url.openStream() ) { - return loadProperties( url.openStream() ); + return loadProperties( in ); } catch ( Exception e ) { @@ -76,9 +76,9 @@ public static java.util.Properties loadProperties( @Nonnull URL url ) @Deprecated public static Properties loadProperties( @Nonnull File file ) { - try + try ( InputStream in = new FileInputStream( file ) ) { - return loadProperties( new FileInputStream( file ) ); + return loadProperties( in ); } catch ( Exception e ) {