Fix plugin crash when duplicate paket.template ids in file tree (#4)

* Fix plugin crash when duplicate paket.template ids in file tree

Each paket.template file in the file tree is spawning a new `PaketPack`
task. This commit handles situation when multiple paket.template files
with the same `id` are location in the file tree.

The plugin will only create one task and skips any other task creation
if a pack task with the same name was already created.

* Sort template file list to make it deterministic

Add better error messages in case duplicate package ids in template
files are found

* Fix initial sort

linux sort is in reverse alphbetical order

src/integrationTest/groovy/wooga/gradle/paket/PaketIntegrationBaseSpec.groovy

@@ -22,7 +22,7 @@ import nebula.test.functional.ExecutionResult
 import spock.lang.Shared
 import spock.lang.Unroll
 
-abstract class PaketIntegrationBaseSpec extends IntegrationSpec{
+abstract class PaketIntegrationBaseSpec extends IntegrationSpec {
 
     @Shared
     def bootstrapTestCases
@@ -76,12 +76,12 @@ abstract class PaketIntegrationBaseSpec extends IntegrationSpec{
         then: "bootstrap task was [UP-TO-DATE]"
         result.wasUpToDate(bootstrapTaskName)
 
-        when:"delete bootstrapper"
+        when: "delete bootstrapper"
         def paketDir = new File(projectDir, '.paket')
         def paketBootstrap = new File(paketDir, bootstrapperFileName)
         paketBootstrap.delete()
 
-        and:"run the task again"
+        and: "run the task again"
         def result2 = runTasksSuccessfully(taskToRun)
 
         then:
@@ -98,4 +98,21 @@ abstract class PaketIntegrationBaseSpec extends IntegrationSpec{
     private boolean containsOutput(String stdout, String taskName, String stateIdentifier) {
         stdout.contains("$taskName $stateIdentifier".toString())
     }
+
+    def projectWithPaketTemplates(ids) {
+        def files = []
+        ids.each { String id ->
+            def subDirectory = new File(projectDir, id)
+            subDirectory.mkdirs()
+            files << projectWithPaketTemplate(subDirectory, id)
+        }
+        files
+    }
+
+    def projectWithPaketTemplate(File directory, String id = "Test.Package") {
+        def templateFile = new File(directory, "paket.template")
+        templateFile.createNewFile()
+        templateFile.append("id $id")
+        templateFile
+    }
 }

src/integrationTest/groovy/wooga/gradle/paket/pack/PaketPackIntegrationSpec.groovy

@@ -17,8 +17,10 @@
 
 package wooga.gradle.paket.pack
 
+import org.gradle.api.file.FileTree
 import spock.lang.Unroll
 import wooga.gradle.paket.PaketIntegrationDependencyFileSpec
+import wooga.gradle.paket.pack.tasks.PaketPack
 
 class PaketPackIntegrationSpec extends PaketIntegrationDependencyFileSpec {
 
@@ -72,7 +74,7 @@ class PaketPackIntegrationSpec extends PaketIntegrationDependencyFileSpec {
     }
 
     @Unroll
-    def "can depend on generated pack tasks #taskToRun"(String taskToRun)  {
+    def "can depend on generated pack tasks #taskToRun"(String taskToRun) {
         given: "the build.gradle with second task that must run before packetPack"
         buildFile << """
             task(preStep) {
@@ -94,4 +96,63 @@ class PaketPackIntegrationSpec extends PaketIntegrationDependencyFileSpec {
         where:
         taskToRun << ["paketPack-WoogaTest", "buildNupkg", "assemble"]
     }
+
+    def "skips pack task creation for duplicate package id"() {
+        given: "some paket template files in the file system with same id"
+        def subDir1 = new File(projectDir, "sub1")
+        def subDir2 = new File(projectDir, "sub2")
+        subDir1.mkdirs()
+        subDir2.mkdirs()
+
+        projectWithPaketTemplate(subDir1, "Test.Package1")
+        projectWithPaketTemplate(subDir2, "Test.Package1")
+
+        when: "applying paket-pack plugin"
+        def result = runTasksSuccessfully(taskToRun)
+
+        then:
+        result.standardOutput.contains("Multiple paket.template files with id Test.Package1")
+        result.standardOutput.contains("Template file with same id already in use")
+        result.standardOutput.contains("Skip template file:")
+
+        where:
+        taskToRun << ["paketPack-TestPackage1", "buildNupkg", "assemble"]
+    }
+
+    @Unroll("verify sort order for template file duplication #subDir1Name|#subDir2Name|#subDir3Name")
+    def "skips pack task creation for duplicate package id and uses first in sorted order"() {
+        given: "some paket template files in the file system with same id"
+        def subDir1 = new File(projectDir, subDir1Name)
+        def subDir2 = new File(projectDir, subDir2Name)
+        def subDir3 = new File(projectDir, subDir3Name)
+
+        subDir1.mkdirs()
+        subDir2.mkdirs()
+        subDir3.mkdirs()
+
+        def templateFiles = [
+                projectWithPaketTemplate(subDir1, "Test.Package1"),
+                projectWithPaketTemplate(subDir2, "Test.Package1"),
+                projectWithPaketTemplate(subDir3, "Test.Package1")
+        ]
+
+        when: "applying paket-pack plugin"
+        def result = runTasksSuccessfully("tasks")
+
+        then:
+        def expectedFileToUse = templateFiles.remove(expectedFileIndex)
+        def unusedOne = templateFiles[0]
+        def unusedTwo = templateFiles[1]
+        result.standardOutput.contains("Multiple paket.template files with id Test.Package1")
+        result.standardOutput.contains("Template file with same id already in use ${expectedFileToUse.path}")
+        result.standardOutput.contains("Skip template file: ${unusedOne.path}")
+        result.standardOutput.contains("Skip template file: ${unusedTwo.path}")
+
+        where:
+        subDir1Name   | subDir2Name   | subDir3Name   | expectedFileIndex
+        "sub1/sub1"   | "sub2"        | "sub1/sub2"   | 1
+        "sub1/subxzy" | "sub2/subklm" | "sub1/subabc" | 2
+        "sub1"        | "sub2"        | "sub3"        | 0
+        "sub1/sub1"   | "sub1"        | "sub1/sub2"   | 1
+    }
 }

src/main/groovy/wooga/gradle/paket/pack/PaketPackPlugin.groovy

@@ -19,6 +19,8 @@ package wooga.gradle.paket.pack
 
 import org.gradle.api.Plugin
 import org.gradle.api.Project
+import org.gradle.api.logging.Logger
+import org.gradle.api.logging.Logging
 import org.gradle.api.plugins.BasePlugin
 import org.gradle.api.tasks.TaskContainer
 import wooga.gradle.paket.base.DefaultPaketPluginExtension
@@ -28,6 +30,8 @@ import wooga.gradle.paket.pack.tasks.PaketPack
 
 class PaketPackPlugin implements Plugin<Project> {
 
+    static Logger logger = Logging.getLogger(PaketPackPlugin)
+
     Project project
     TaskContainer tasks
 
@@ -48,11 +52,40 @@ class PaketPackPlugin implements Plugin<Project> {
 
         def templateFiles = project.fileTree(project.projectDir)
         templateFiles.include PAKET_TEMPLATE_PATTERN
+        templateFiles = templateFiles.sort()
+        templateFiles = templateFiles.sort(true, new Comparator<File>() {
+            @Override
+            int compare(File o1, File o2) {
+                String sep = File.separator
+                if(o1.path.count(sep) > o2.path.count(sep)) {
+                    return 1
+                }
+                else if(o1.path.count(sep) < o2.path.count(sep)) {
+                    return -1
+                }
+                else
+                {
+                    return 0
+                }
+            }
+        })
+
         templateFiles.each { File file ->
             def templateReader = new PaketTemplateReader(file)
             def packageID = templateReader.getPackageId()
             def packageName = packageID.replaceAll(/\./, '')
-            def packTask = tasks.create(TASK_PACK_PREFIX + packageName, PaketPack.class)
+            def taskName = TASK_PACK_PREFIX + packageName
+
+            def packTask = tasks.findByName(taskName)
+            if (packTask && PaketPack.isInstance(packTask)) {
+                File templateFileInUse = ((PaketPack) packTask).templateFile
+                logger.warn("Multiple paket.template files with id ${packageID}.")
+                logger.warn("Template file with same id already in use $templateFileInUse.path")
+                logger.warn("Skip template file: $file.path")
+                return
+            }
+
+            packTask = tasks.create(taskName, PaketPack.class)
 
             packTask.group = BasePlugin.BUILD_GROUP
             packTask.templateFile = file

src/test/groovy/wooga/gradle/paket/pack/PaketPackPluginSpec.groovy

@@ -103,6 +103,19 @@ class PaketPackPluginSpec extends ProjectSpec {
         tasks.every { it.dependsOn.contains(project.tasks[PaketBasePlugin.BOOTSTRAP_TASK_NAME]) }
     }
 
+    def "skips pack task creation for duplicate package id"() {
+        given: "some paket template files in the file system with same id"
+        projectWithPaketTemplate(projectDir,"Test.Package1")
+        projectWithPaketTemplates(["Test.Package1"])
+
+        when: "applying paket-pack plugin"
+        project.pluginManager.apply(PLUGIN_NAME)
+
+        then:
+        def tasks = project.tasks.withType(PaketPack)
+        tasks.size() == 1
+    }
+
     def "adds artifact to configuration [nupkg]"() {
         given: "some paket template files in the file system"
         projectWithPaketTemplates(["Test.Package1", "Test.Package2", "Test.Package3"])