Fix #127 - Fix filters applying actions despite criteria not matching

Praqma · Mar 28, 2019 · 3e22a51 · 3e22a51
1 parent a8ee216
commit 3e22a51
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 13 deletions.
diff --git a/.gitignore b/.gitignore
@@ -4,6 +4,7 @@ build/
 gradle-app.setting
 !gradle-wrapper.jar
 /.nb-gradle/
+output/
 
 bin/
 .project

diff --git a/src/main/groovy/togit/ScriptBase.groovy b/src/main/groovy/togit/ScriptBase.groovy
@@ -123,13 +123,12 @@ abstract class ScriptBase extends Script implements Context {
      * Closure containing DSL methods used for the migration
      * @param closure The DSL code
      */
-    static void migrate(boolean dryRun = false, @DslContext(MigrationContext) Closure closure) {
+    static void migrate(boolean skipExecution = false, boolean skipReset = false, @DslContext(MigrationContext) Closure closure) {
         executeInContext(closure, new MigrationContext())
-        MigrationManager.instance.migrate(dryRun)
+        MigrationManager.instance.migrate(skipExecution)
         LOG.info(migrationComplete())
-        if (!dryRun) {
-            MigrationManager.instance.resetMigrationPlan()
-        }
+        if (skipReset) { return }
+        MigrationManager.instance.resetMigrationPlan()
     }
 
     /**

diff --git a/src/main/groovy/togit/migration/MigrationManager.groovy b/src/main/groovy/togit/migration/MigrationManager.groovy
@@ -39,8 +39,9 @@ class MigrationManager {
         plan = new MigrationPlan()
     }
 
-    void migrate(boolean dryRun = false) {
-        if (dryRun) {
+    void migrate(boolean skipExecution = false) {
+        if (skipExecution) {
+            LOG.info('Skipping execution')
             plan.plan()
             return
         }

diff --git a/src/main/groovy/togit/migration/plan/Snapshot.groovy b/src/main/groovy/togit/migration/plan/Snapshot.groovy
@@ -34,11 +34,11 @@ abstract class Snapshot {
      * @return true if the Snapshot matches all Criteria, otherwise false
      */
     boolean matches(List<Criteria> criteria, List<Snapshot> allSnapshots) {
-        for (def crit : criteria) {
+        for (Criteria crit : criteria) {
             if (!crit.appliesTo(this, allSnapshots)) {
-                false
+                return false
             }
         }
-        true
+        return true
     }
 }
diff --git a/src/test/groovy/toGit/context/dummy/ActionsContextTest.groovy b/src/test/groovy/toGit/context/dummy/ActionsContextTest.groovy
@@ -22,7 +22,7 @@ class ActionsContextTest {
         $/
             source('dummy')
             target('dummy')
-            migrate(true) {
+            migrate(true, true) {
                 filters {
                     filter {
                         actions {

diff --git a/src/test/groovy/toGit/context/dummy/CriteriaContextTest.groovy b/src/test/groovy/toGit/context/dummy/CriteriaContextTest.groovy
@@ -17,7 +17,7 @@ class CriteriaContextTest {
         $/
         source('dummy')
         target('dummy')
-        migrate(true) {
+        migrate(true, true) {
             filters {
                 filter {
                     criteria {

diff --git a/src/test/groovy/toGit/context/dummy/ExtractionsContextTest.groovy b/src/test/groovy/toGit/context/dummy/ExtractionsContextTest.groovy
@@ -18,7 +18,7 @@ class ExtractionsContextTest {
         $/
         source('dummy')
         target('dummy')
-        migrate(true) {
+        migrate(true, true) {
             filters {
                 filter {
                     extractions {

diff --git a/src/test/groovy/toGit/context/dummy/GHI127.groovy b/src/test/groovy/toGit/context/dummy/GHI127.groovy
@@ -0,0 +1,42 @@
+package togit.context.dummy
+
+import org.junit.Test
+import togit.Executor
+import togit.TestHelper
+import togit.migration.MigrationManager
+import togit.migration.sources.dummy.DummySource
+import togit.migration.targets.dummy.DummyTarget
+
+/* Filters would apply actions to Snapshots that didnt meet criteria. */
+class GHI127 {
+
+    private static void runTest() {
+        new Executor().execute(TestHelper.tempCommandFile(testScript()).absolutePath)
+    }
+
+    static String testScript() {
+        """\
+        source('dummy')
+        target('dummy')
+        migrate(false, true) {
+            filters {
+                filter {
+                    criteria {
+                        custom { current, all -> current.identifier == "foo" }
+                    }
+                    actions {
+                        custom { println "blip" }
+                    }
+                }
+            }
+        }""".stripIndent()
+    }
+
+    @Test
+    void onlyAppliesToMatchingActions() {
+        runTest()
+        MigrationManager manager = MigrationManager.instance
+        assert manager.plan.steps.size() == 1
+        assert manager.plan.steps.foo.actions.size() == 1
+    }
+}