[MSHADE-432] Duplicate services entries can be generated (#159)

Author: gnodet

src/main/java/org/apache/maven/plugins/shade/resource/ServicesResourceTransformer.java

@@ -22,11 +22,12 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Scanner;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 
@@ -45,7 +46,7 @@ public class ServicesResourceTransformer
 {
     private static final String SERVICES_PATH = "META-INF/services";
 
-    private final Map<String, ArrayList<String>> serviceEntries = new HashMap<>();
+    private final Map<String, Set<String>> serviceEntries = new HashMap<>();
 
     private long time = Long.MIN_VALUE;
 
@@ -68,7 +69,7 @@ public void processResource( String resource, InputStream is, final List<Relocat
         }
         resource = SERVICES_PATH + '/' + resource;
 
-        ArrayList<String> out = serviceEntries.computeIfAbsent( resource, k -> new ArrayList<>() );
+        Set<String> out = serviceEntries.computeIfAbsent( resource, k -> new LinkedHashSet<>() );
 
         Scanner scanner = new Scanner( is, StandardCharsets.UTF_8.name() );
         while ( scanner.hasNextLine() )
@@ -98,10 +99,10 @@ public boolean hasTransformedResource()
     public void modifyOutputStream( JarOutputStream jos )
             throws IOException
     {
-        for ( Map.Entry<String, ArrayList<String>> entry : serviceEntries.entrySet() )
+        for ( Map.Entry<String, Set<String>> entry : serviceEntries.entrySet() )
         {
             String key = entry.getKey();
-            ArrayList<String> data = entry.getValue();
+            Set<String> data = entry.getValue();
 
             JarEntry jarEntry = new JarEntry( key );
             jarEntry.setTime( time );

src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java

@@ -19,11 +19,13 @@
  * under the License.
  */
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.lang.reflect.Field;
 import java.net.URL;
 import java.net.URLClassLoader;
@@ -40,6 +42,7 @@
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
+import java.util.stream.Collectors;
 import java.util.zip.CRC32;
 import java.util.zip.ZipEntry;
 
@@ -50,6 +53,7 @@
 import org.apache.maven.plugins.shade.resource.AppendingTransformer;
 import org.apache.maven.plugins.shade.resource.ComponentsXmlResourceTransformer;
 import org.apache.maven.plugins.shade.resource.ResourceTransformer;
+import org.apache.maven.plugins.shade.resource.ServicesResourceTransformer;
 import org.codehaus.plexus.util.IOUtil;
 import org.codehaus.plexus.util.Os;
 import org.junit.Assert;
@@ -83,6 +87,8 @@ public class DefaultShaderTest
     private static final String[] EXCLUDES = new String[] { "org/codehaus/plexus/util/xml/Xpp3Dom",
         "org/codehaus/plexus/util/xml/pull.*" };
 
+    private final String NEWLINE = "\n";
+
     @Test
     public void testNoopWhenNotRelocated() throws IOException, MojoExecutionException {
         final File plexusJar = new File("src/test/jars/plexus-utils-1.4.1.jar" );
@@ -423,6 +429,54 @@ public void testShaderNoOverwrite() throws Exception
         temporaryFolder.delete();
     }
 
+    @Test
+    public void testShaderWithDuplicateService() throws Exception
+    {
+        TemporaryFolder temporaryFolder = new TemporaryFolder();
+        temporaryFolder.create();
+
+        String serviceEntryName = "META-INF/services/my.foo.Service";
+        String serviceEntryValue = "my.foo.impl.Service1";
+
+        File innerJar1 = temporaryFolder.newFile( "inner1.jar" );
+        try (JarOutputStream jos = new JarOutputStream(Files.newOutputStream( innerJar1.toPath() ) ) )
+        {
+            jos.putNextEntry( new JarEntry(serviceEntryName) );
+            jos.write( ( serviceEntryValue + NEWLINE ).getBytes( StandardCharsets.UTF_8 ) );
+            jos.closeEntry();
+        }
+
+        File innerJar2 = temporaryFolder.newFile( "inner2.jar" );
+        try ( JarOutputStream jos = new JarOutputStream( Files.newOutputStream( innerJar2.toPath() ) ) )
+        {
+            jos.putNextEntry( new JarEntry(serviceEntryName) );
+            jos.write( ( serviceEntryValue + NEWLINE ).getBytes( StandardCharsets.UTF_8 ) );
+            jos.closeEntry();
+        }
+
+        ShadeRequest shadeRequest = new ShadeRequest();
+        shadeRequest.setJars( new LinkedHashSet<>( Arrays.asList( innerJar1, innerJar2 ) ) );
+        shadeRequest.setFilters( Collections.emptyList() );
+        shadeRequest.setRelocators( Collections.emptyList() );
+        shadeRequest.setResourceTransformers( Collections.singletonList( new ServicesResourceTransformer() ) );
+        File shadedFile = temporaryFolder.newFile( "shaded.jar" );
+        shadeRequest.setUberJar( shadedFile );
+
+        DefaultShader shader = newShader();
+        shader.shade( shadeRequest );
+
+        JarFile shadedJarFile = new JarFile( shadedFile );
+        JarEntry entry = shadedJarFile.getJarEntry(serviceEntryName);
+
+        List<String> lines = new BufferedReader( new InputStreamReader( shadedJarFile.getInputStream( entry ), StandardCharsets.UTF_8 ) )
+                .lines().collect( Collectors.toList() );
+
+        //After shading, there should be a single input
+        Assert.assertEquals( Collections.singletonList( serviceEntryValue ), lines );
+
+        temporaryFolder.delete();
+    }
+
     private void writeEntryWithoutCompression( String entryName, byte[] entryBytes, JarOutputStream jos ) throws IOException
     {
         final JarEntry entry = new JarEntry( entryName );

src/test/java/org/apache/maven/plugins/shade/resource/ServiceResourceTransformerTest.java

@@ -121,7 +121,7 @@ public void mergeRelocatedFiles() throws Exception {
             assertNotNull( jarEntry );
             try ( InputStream entryStream = jarFile.getInputStream( jarEntry ) ) {
                 String xformedContent = IOUtils.toString( entryStream, StandardCharsets.UTF_8);
-                assertEquals( contentShaded + contentShaded, xformedContent );
+                assertEquals( contentShaded, xformedContent );
             } finally {
                 jarFile.close();
             }