diff --git a/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java b/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java index 818339ed..a996bd46 100644 --- a/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java +++ b/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java @@ -129,58 +129,22 @@ private void removeServices( final MavenProject project, final Clazzpath cp ) neededClasses.removeAll( removable ); try { + // getRuntimeClasspathElements returns a list of + // - the build output directory + // - all the paths to the dependencies' jars + // We thereby need to ignore the build directory because we don't want + // to remove anything from it, as it's the starting point of the + // minification process. for ( final String fileName : project.getRuntimeClasspathElements() ) { - try ( final JarFile jar = new JarFile( fileName ) ) + // Ignore the build directory from this project + if ( fileName.equals( project.getBuild().getOutputDirectory() ) ) { - for ( final Enumeration entries = jar.entries(); entries.hasMoreElements(); ) - { - final JarEntry jarEntry = entries.nextElement(); - if ( jarEntry.isDirectory() || !jarEntry.getName().startsWith( "META-INF/services/" ) ) - { - continue; - } - - final String serviceClassName = - jarEntry.getName().substring( "META-INF/services/".length() ); - final boolean isNeededClass = neededClasses.contains( cp.getClazz( serviceClassName ) ); - if ( !isNeededClass ) - { - continue; - } - - try ( final BufferedReader bufferedReader = - new BufferedReader( new InputStreamReader( jar.getInputStream( jarEntry ), UTF_8 ) ) ) - { - for ( String line = bufferedReader.readLine(); line != null; - line = bufferedReader.readLine() ) - { - final String className = line.split( "#", 2 )[0].trim(); - if ( className.isEmpty() ) - { - continue; - } - - final Clazz clazz = cp.getClazz( className ); - if ( clazz == null || !removable.contains( clazz ) ) - { - continue; - } - - log.debug( className + " was not removed because it is a service" ); - removeClass( clazz ); - repeatScan = true; // check whether the found classes use services in turn - } - } - catch ( final IOException e ) - { - log.warn( e.getMessage() ); - } - } + continue; } - catch ( final IOException e ) + if ( removeServicesFromJar( cp, neededClasses, fileName ) ) { - log.warn( e.getMessage() ); + repeatScan = true; } } } @@ -192,6 +156,69 @@ private void removeServices( final MavenProject project, final Clazzpath cp ) while ( repeatScan ); } + private boolean removeServicesFromJar( Clazzpath cp, Set neededClasses, String fileName ) + { + boolean repeatScan = false; + try ( final JarFile jar = new JarFile( fileName ) ) + { + for ( final Enumeration entries = jar.entries(); entries.hasMoreElements(); ) + { + final JarEntry jarEntry = entries.nextElement(); + if ( jarEntry.isDirectory() || !jarEntry.getName().startsWith( "META-INF/services/" ) ) + { + continue; + } + + final String serviceClassName = jarEntry.getName().substring( "META-INF/services/".length() ); + final boolean isNeededClass = neededClasses.contains( cp.getClazz( serviceClassName ) ); + if ( !isNeededClass ) + { + continue; + } + + try ( final BufferedReader configFileReader = new BufferedReader( + new InputStreamReader( jar.getInputStream( jarEntry ), UTF_8 ) ) ) + { + // check whether the found classes use services in turn + repeatScan = scanServiceProviderConfigFile( cp, configFileReader ); + } + catch ( final IOException e ) + { + log.warn( e.getMessage() ); + } + } + } + catch ( final IOException e ) + { + log.warn( "Not a JAR file candidate. Ignoring classpath element '" + fileName + "' (" + e + ")." ); + } + return repeatScan; + } + + private boolean scanServiceProviderConfigFile( Clazzpath cp, BufferedReader configFileReader ) throws IOException + { + boolean serviceClassFound = false; + for ( String line = configFileReader.readLine(); line != null; line = configFileReader.readLine() ) + { + final String className = line.split( "#", 2 )[0].trim(); + if ( className.isEmpty() ) + { + continue; + } + + final Clazz clazz = cp.getClazz( className ); + if ( clazz == null || !removable.contains( clazz ) ) + { + continue; + } + + log.debug( className + " was not removed because it is a service" ); + removeClass( clazz ); + serviceClassFound = true; + } + return serviceClassFound; + } + private void removeClass( final Clazz clazz ) { removable.remove( clazz ); diff --git a/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java b/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java index bfbaee24..25be4d8a 100644 --- a/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java +++ b/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java @@ -19,7 +19,10 @@ * under the License. */ +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -27,15 +30,23 @@ import static org.mockito.Mockito.when; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Set; import java.util.TreeSet; +import java.util.jar.JarOutputStream; import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.DependencyResolutionRequiredException; +import org.apache.maven.model.Build; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.mockito.ArgumentCaptor; @@ -43,16 +54,25 @@ public class MinijarFilterTest { + @Rule + public TemporaryFolder tempFolder = TemporaryFolder.builder().assureDeletion().build(); + + private File outputDirectory; private File emptyFile; + private File jarFile; + private Log log; + private ArgumentCaptor logCaptor; @Before public void init() throws IOException { - TemporaryFolder tempFolder = new TemporaryFolder(); - tempFolder.create(); + this.outputDirectory = tempFolder.newFolder(); this.emptyFile = tempFolder.newFile(); - + this.jarFile = tempFolder.newFile(); + new JarOutputStream( new FileOutputStream( this.jarFile ) ).close(); + this.log = mock(Log.class); + logCaptor = ArgumentCaptor.forClass(CharSequence.class); } /** @@ -64,11 +84,7 @@ public void testWithMockProject() { assumeFalse( "Expected to run under JDK8+", System.getProperty("java.version").startsWith("1.7") ); - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - - MavenProject mavenProject = mockProject( emptyFile ); - - Log log = mock( Log.class ); + MavenProject mavenProject = mockProject( outputDirectory, emptyFile ); MinijarFilter mf = new MinijarFilter( mavenProject, log ); @@ -84,14 +100,10 @@ public void testWithMockProject() public void testWithPomProject() throws IOException { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - // project with pom packaging and no artifact. - MavenProject mavenProject = mockProject( null ); + MavenProject mavenProject = mockProject( outputDirectory, null ); mavenProject.setPackaging( "pom" ); - Log log = mock( Log.class ); - MinijarFilter mf = new MinijarFilter( mavenProject, log ); mf.finished(); @@ -105,7 +117,7 @@ public void testWithPomProject() } - private MavenProject mockProject( File file ) + private MavenProject mockProject( File outputDirectory, File file, String... classPathElements ) { MavenProject mavenProject = mock( MavenProject.class ); @@ -129,17 +141,29 @@ private MavenProject mockProject( File file ) when( mavenProject.getArtifact().getFile() ).thenReturn( file ); - return mavenProject; + Build build = new Build(); + build.setOutputDirectory( outputDirectory.toString() ); + + List classpath = new ArrayList<>(); + classpath.add( outputDirectory.toString() ); + if ( file != null ) + { + classpath.add(file.toString()); + } + classpath.addAll( Arrays.asList( classPathElements ) ); + when( mavenProject.getBuild() ).thenReturn( build ); + try { + when(mavenProject.getRuntimeClasspathElements()).thenReturn(classpath); + } catch (DependencyResolutionRequiredException e) { + fail("Encountered unexpected exception: " + e.getClass().getSimpleName() + ": " + e.getMessage()); + } + return mavenProject; } @Test public void finsishedShouldProduceMessageForClassesTotalNonZero() { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - - Log log = mock( Log.class ); - MinijarFilter m = new MinijarFilter( 1, 50, log ); m.finished(); @@ -153,10 +177,6 @@ public void finsishedShouldProduceMessageForClassesTotalNonZero() @Test public void finishedShouldProduceMessageForClassesTotalZero() { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - - Log log = mock( Log.class ); - MinijarFilter m = new MinijarFilter( 0, 0, log ); m.finished(); @@ -166,4 +186,24 @@ public void finishedShouldProduceMessageForClassesTotalZero() assertEquals( "Minimized 0 -> 0", logCaptor.getValue() ); } + + /** + * Verify that directories are ignored when scanning the classpath for JARs containing services, + * but warnings are logged instead + * + * @see MSHADE-366 + */ + @Test + public void removeServicesShouldIgnoreDirectories() throws Exception { + String classPathElementToIgnore = tempFolder.newFolder().getAbsolutePath(); + MavenProject mockedProject = mockProject( outputDirectory, jarFile, classPathElementToIgnore ); + + new MinijarFilter(mockedProject, log); + + verify( log, times( 1 ) ).warn( logCaptor.capture() ); + + assertThat( logCaptor.getValue().toString(), startsWith( + "Not a JAR file candidate. Ignoring classpath element '" + classPathElementToIgnore + "' (" ) ); + } + }