Updated Branches: refs/heads/master 626e59333 -> 36f458b63
o Made the classpath object truly immutable and threadsafe Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/36f458b6 Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/36f458b6 Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/36f458b6 Branch: refs/heads/master Commit: 36f458b633553c7b4ff755e0aeb3a637e0375493 Parents: 626e593 Author: Kristian Rosenvold <krosenv...@apache.org> Authored: Wed May 29 20:05:31 2013 +0200 Committer: Kristian Rosenvold <krosenv...@apache.org> Committed: Wed May 29 20:05:31 2013 +0200 ---------------------------------------------------------------------- .../plugin/surefire/AbstractSurefireMojo.java | 3 +- ...BooterDeserializerStartupConfigurationTest.java | 2 +- .../booterclient/ForkConfigurationTest.java | 2 +- .../apache/maven/surefire/booter/Classpath.java | 108 +++++++++------ .../surefire/booter/ClasspathConfiguration.java | 5 +- .../maven/surefire/booter/PropertiesWrapper.java | 2 + .../maven/surefire/booter/ClasspathTest.java | 35 ++--- .../surefire/booter/PropertiesWrapperTest.java | 5 +- 8 files changed, 88 insertions(+), 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index 28ae11c..d254d55 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -1177,9 +1177,8 @@ public abstract class AbstractSurefireMojo ClasspathCache.setCachedClasspath( providerName, providerClasspath ); } - Classpath inprocClassPath = new Classpath( providerClasspath ); Artifact surefireArtifact = getCommonArtifact(); - inprocClassPath.addClassPathElementUrl( surefireArtifact.getFile().getAbsolutePath() ); + Classpath inprocClassPath = providerClasspath.addClassPathElementUrl( surefireArtifact.getFile().getAbsolutePath() ); final Classpath testClasspath = generateTestClasspath(); http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java index 5446bd6..bbcf5e1 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java @@ -101,7 +101,7 @@ public class BooterDeserializerStartupConfigurationTest { Classpath testClassPath = new Classpath( Arrays.asList( new String[]{ "CP1", "CP2" } ) ); Classpath providerClasspath = new Classpath( Arrays.asList( new String[]{ "SP1", "SP2" } ) ); - return new ClasspathConfiguration( testClassPath, providerClasspath, new Classpath(), true, true ); + return new ClasspathConfiguration( testClassPath, providerClasspath, Classpath.emptyClasspath(), true, true ); } public static ClassLoaderConfiguration getSystemClassLoaderConfiguration() http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java index c37b9f2..9a310b2 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java @@ -71,7 +71,7 @@ public class ForkConfigurationTest public static ForkConfiguration getForkConfiguration( String argLine, String jvm ) throws IOException { - return new ForkConfiguration( new Classpath(), null, null, jvm, new File( "." ).getCanonicalFile(), argLine, + return new ForkConfiguration( Classpath.emptyClasspath(), null, null, jvm, new File( "." ).getCanonicalFile(), argLine, null, false, 1, false ); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/surefire-booter/src/main/java/org/apache/maven/surefire/booter/Classpath.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/Classpath.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/Classpath.java index a667a9c..2fdb4eb 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/Classpath.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/Classpath.java @@ -21,86 +21,93 @@ package org.apache.maven.surefire.booter; import org.apache.maven.surefire.util.UrlUtils; +import javax.annotation.concurrent.Immutable; +import javax.annotation.concurrent.ThreadSafe; import java.io.File; import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; /** * An ordered list of classpath elements with set behaviour * + * A Classpath is immutable and thread safe. + * + * * @author Kristian Rosenvold */ -public class Classpath +@ThreadSafe +@Immutable +public class Classpath implements Iterable<String> { + private final List<String> unmodifiableElements; + public static Classpath join( Classpath firstClasspath, Classpath secondClasspath ) { - Classpath joinedClasspath = new Classpath(); - joinedClasspath.addElementsOfClasspath( firstClasspath ); - joinedClasspath.addElementsOfClasspath( secondClasspath ); - return joinedClasspath; + LinkedHashSet<String> accumulated = new LinkedHashSet<String>( ); + if (firstClasspath != null) firstClasspath.addTo( accumulated ); + if (secondClasspath != null) secondClasspath.addTo( accumulated ); + return new Classpath( accumulated ); } - private final List<String> elements; - public Classpath() - { - this.elements = new ArrayList<String>(); + private void addTo(Collection<String> c){ + c.addAll( unmodifiableElements ); } - public Classpath( Classpath other ) + private Classpath() { - this.elements = new ArrayList<String>( other.elements ); + this.unmodifiableElements = Collections.emptyList(); } - public Classpath( List<String> elements ) + + public Classpath( Classpath other, String additionalElement ) { - this(); - addElements( elements ); + ArrayList<String> elems = new ArrayList<String>( other.unmodifiableElements ); + elems.add( additionalElement ); + this.unmodifiableElements = Collections.unmodifiableList( elems ); } - public void addClassPathElementUrl( String path ) + public Classpath( Iterable<String> elements ) { - if ( path == null ) - { - throw new IllegalArgumentException( "Null is not a valid class path element url." ); - } - else if ( !elements.contains( path ) ) + List<String> newCp = new ArrayList<String>( ); + for ( String element : elements ) { - elements.add( path ); + newCp.add( element ); } + this.unmodifiableElements = Collections.unmodifiableList( newCp ); } - private void addElements( List<String> additionalElements ) + public static Classpath emptyClasspath() { - for ( Object additionalElement : additionalElements ) - { - String element = (String) additionalElement; - addClassPathElementUrl( element ); - } + return new Classpath(); } - private void addElementsOfClasspath( Classpath otherClasspath ) + public Classpath addClassPathElementUrl( String path ) { - if ( otherClasspath != null ) + if ( path == null ) { - addElements( otherClasspath.elements ); + throw new IllegalArgumentException( "Null is not a valid class path element url." ); } + return !unmodifiableElements.contains( path ) ? new Classpath( this, path ) : this; } public List<String> getClassPath() { - return Collections.unmodifiableList( elements ); + return unmodifiableElements; } public List<URL> getAsUrlList() throws MalformedURLException { List<URL> urls = new ArrayList<URL>(); - for ( String url : elements ) + for ( String url : unmodifiableElements ) { File f = new File( url ); urls.add( UrlUtils.getURL( f ) ); @@ -111,7 +118,7 @@ public class Classpath public void writeToSystemProperty( String propertyName ) { StringBuilder sb = new StringBuilder(); - for ( String element : elements ) + for ( String element : unmodifiableElements ) { sb.append( element ).append( File.pathSeparatorChar ); } @@ -131,7 +138,8 @@ public class Classpath Classpath classpath = (Classpath) o; - return !( elements != null ? !elements.equals( classpath.elements ) : classpath.elements != null ); + return !( unmodifiableElements + != null ? !unmodifiableElements.equals( classpath.unmodifiableElements ) : classpath.unmodifiableElements != null ); } @@ -164,36 +172,50 @@ public class Classpath public int hashCode() { - return elements != null ? elements.hashCode() : 0; + return unmodifiableElements != null ? unmodifiableElements.hashCode() : 0; } public String getLogMessage( String descriptor ) { StringBuilder result = new StringBuilder(); result.append( descriptor ).append( " classpath:" ); - for ( String element : elements ) + for ( String element : unmodifiableElements ) { result.append( " " ).append( element ); } return result.toString(); } + public String getCompactLogMessage( String descriptor ) { StringBuilder result = new StringBuilder(); result.append( descriptor ).append( " classpath:" ); - for ( String element : elements ) + for ( String element : unmodifiableElements ) { result.append( " " ); - if (element != null){ + if ( element != null ) + { int pos = element.lastIndexOf( File.separatorChar ); - if (pos >= 0){ - result.append( element.substring( pos + 1) ); - } else - result.append( element); + if ( pos >= 0 ) + { + result.append( element.substring( pos + 1 ) ); + } + else + { + result.append( element ); + } - } else result.append(element); + } + else + { + result.append( element ); + } } return result.toString(); } + public Iterator<String> iterator() + { + return unmodifiableElements.iterator(); + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ClasspathConfiguration.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ClasspathConfiguration.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ClasspathConfiguration.java index 2df6e46..9b3f9c3 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ClasspathConfiguration.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ClasspathConfiguration.java @@ -57,12 +57,13 @@ public class ClasspathConfiguration public ClasspathConfiguration( boolean enableAssertions, boolean childDelegation ) { - this( new Classpath(), new Classpath(), new Classpath(), enableAssertions, childDelegation ); + this( Classpath.emptyClasspath(), Classpath.emptyClasspath(), Classpath.emptyClasspath(), enableAssertions, childDelegation ); } ClasspathConfiguration( PropertiesWrapper properties ) { - this( properties.getClasspath( CLASSPATH ), properties.getClasspath( SUREFIRE_CLASSPATH ), new Classpath(), + this( properties.getClasspath( CLASSPATH ), properties.getClasspath( SUREFIRE_CLASSPATH ), + Classpath.emptyClasspath(), properties.getBooleanProperty( ENABLE_ASSERTIONS ), properties.getBooleanProperty( CHILD_DELEGATION ) ); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PropertiesWrapper.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PropertiesWrapper.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PropertiesWrapper.java index 28fb1d3..e5e3d8e 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PropertiesWrapper.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PropertiesWrapper.java @@ -28,6 +28,8 @@ import java.util.Properties; import org.apache.maven.surefire.util.internal.StringUtils; /** + * Makes java.util.Properties behave like it's 2013 + * * @author Kristian Rosenvold */ public class PropertiesWrapper http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ClasspathTest.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ClasspathTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ClasspathTest.java index 700a3a5..78a180b 100644 --- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ClasspathTest.java +++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ClasspathTest.java @@ -24,6 +24,8 @@ import java.util.List; import junit.framework.TestCase; +import static org.apache.maven.surefire.booter.Classpath.emptyClasspath; + /** * @author Kristian Rosenvold */ @@ -39,7 +41,7 @@ public class ClasspathTest public void testShouldWriteEmptyPropertyForEmptyClasspath() throws Exception { - Classpath classpath = new Classpath(); + Classpath classpath = Classpath.emptyClasspath(); classpath.writeToSystemProperty( DUMMY_PROPERTY_NAME ); assertEquals( "", System.getProperty( DUMMY_PROPERTY_NAME ) ); } @@ -47,9 +49,7 @@ public class ClasspathTest public void testShouldWriteSeparatedElementsAsSystemProperty() throws Exception { - Classpath classpath = new Classpath(); - classpath.addClassPathElementUrl( DUMMY_URL_1 ); - classpath.addClassPathElementUrl( DUMMY_URL_2 ); + Classpath classpath = Classpath.emptyClasspath().addClassPathElementUrl( DUMMY_URL_1 ).addClassPathElementUrl( DUMMY_URL_2 ); classpath.writeToSystemProperty( DUMMY_PROPERTY_NAME ); assertEquals( DUMMY_URL_1 + File.pathSeparatorChar + DUMMY_URL_2 + File.pathSeparatorChar, System.getProperty( DUMMY_PROPERTY_NAME ) ); @@ -57,9 +57,8 @@ public class ClasspathTest public void testShouldAddNoDuplicateElements() { - Classpath classpath = new Classpath(); - classpath.addClassPathElementUrl( DUMMY_URL_1 ); - classpath.addClassPathElementUrl( DUMMY_URL_1 ); + Classpath classpath = + emptyClasspath().addClassPathElementUrl( DUMMY_URL_1 ).addClassPathElementUrl( DUMMY_URL_1 ); assertClasspathConsistsOfElements( classpath, new String[]{ DUMMY_URL_1 } ); } @@ -81,10 +80,8 @@ public class ClasspathTest public void testShouldHaveAllElementsAfterJoiningTwoDifferentClasspaths() throws Exception { - Classpath firstClasspath = new Classpath(); - firstClasspath.addClassPathElementUrl( DUMMY_URL_1 ); - Classpath secondClasspath = new Classpath(); - secondClasspath.addClassPathElementUrl( DUMMY_URL_2 ); + Classpath firstClasspath = Classpath.emptyClasspath(); + Classpath secondClasspath = firstClasspath.addClassPathElementUrl( DUMMY_URL_1 ).addClassPathElementUrl( DUMMY_URL_2 ); Classpath joinedClasspath = Classpath.join( firstClasspath, secondClasspath ); assertClasspathConsistsOfElements( joinedClasspath, new String[]{ DUMMY_URL_1, DUMMY_URL_2 } ); } @@ -92,10 +89,8 @@ public class ClasspathTest public void testShouldNotHaveDuplicatesAfterJoiningTowClasspathsWithEqualElements() throws Exception { - Classpath firstClasspath = new Classpath(); - firstClasspath.addClassPathElementUrl( DUMMY_URL_1 ); - Classpath secondClasspath = new Classpath(); - secondClasspath.addClassPathElementUrl( DUMMY_URL_1 ); + Classpath firstClasspath = Classpath.emptyClasspath().addClassPathElementUrl( DUMMY_URL_1 ); + Classpath secondClasspath = Classpath.emptyClasspath().addClassPathElementUrl( DUMMY_URL_1 ); Classpath joinedClasspath = Classpath.join( firstClasspath, secondClasspath ); assertClasspathConsistsOfElements( joinedClasspath, new String[]{ DUMMY_URL_1 } ); } @@ -132,16 +127,14 @@ public class ClasspathTest private Classpath createClasspathWithTwoElements() { - Classpath classpath = new Classpath(); - classpath.addClassPathElementUrl( DUMMY_URL_1 ); - classpath.addClassPathElementUrl( DUMMY_URL_2 ); - return classpath; + Classpath classpath = Classpath.emptyClasspath(); + return classpath.addClassPathElementUrl( DUMMY_URL_1 ).addClassPathElementUrl( DUMMY_URL_2 ); } public void testShouldThrowIllegalArgumentExceptionWhenNullIsAddedAsClassPathElementUrl() throws Exception { - Classpath classpath = new Classpath(); + Classpath classpath = Classpath.emptyClasspath(); try { classpath.addClassPathElementUrl( null ); @@ -155,7 +148,7 @@ public class ClasspathTest public void testShouldNotAddNullAsClassPathElementUrl() throws Exception { - Classpath classpath = new Classpath(); + Classpath classpath = Classpath.emptyClasspath(); try { classpath.addClassPathElementUrl( null ); http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/36f458b6/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PropertiesWrapperTest.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PropertiesWrapperTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PropertiesWrapperTest.java index 560b00b..bda705b 100644 --- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PropertiesWrapperTest.java +++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PropertiesWrapperTest.java @@ -99,10 +99,7 @@ public class PropertiesWrapperTest private Classpath createClasspathWithTwoElements() { - Classpath classpath = new Classpath(); - classpath.addClassPathElementUrl( FIRST_ELEMENT ); - classpath.addClassPathElementUrl( SECOND_ELEMENT ); - return classpath; + return Classpath.emptyClasspath().addClassPathElementUrl( FIRST_ELEMENT ).addClassPathElementUrl( SECOND_ELEMENT ); } private Classpath readClasspathFromProperties()
