From 79f109fe317502f857afb7a5978d5dba3fd9cd1f Mon Sep 17 00:00:00 2001 From: Thierry Lacour Date: Thu, 28 Mar 2019 09:47:50 +0100 Subject: [PATCH] Fix #127 - Fix filters applying actions despite criteria not matching --- .gitignore | 1 + src/main/groovy/togit/ScriptBase.groovy | 9 ++-- .../togit/migration/MigrationManager.groovy | 5 ++- .../togit/migration/plan/Snapshot.groovy | 6 +-- .../context/dummy/ActionsContextTest.groovy | 2 +- .../context/dummy/CriteriaContextTest.groovy | 2 +- .../dummy/ExtractionsContextTest.groovy | 2 +- .../groovy/toGit/context/dummy/GHI127.groovy | 41 +++++++++++++++++++ 8 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 src/test/groovy/toGit/context/dummy/GHI127.groovy diff --git a/.gitignore b/.gitignore index a7cc066..3a8fc31 100644 --- 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 index c115868..f08679e 100644 --- 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 index 422faf7..8de4e8a 100644 --- 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 index 8a83876..be77a08 100644 --- 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, List 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 index 2dd58d2..05971f0 100644 --- 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 index caadf03..8434e0c 100644 --- 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 index 93be538..fafa521 100644 --- 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 new file mode 100644 index 0000000..f5bc907 --- /dev/null +++ b/src/test/groovy/toGit/context/dummy/GHI127.groovy @@ -0,0 +1,41 @@ +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 + +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 testCopy() { + runTest() + MigrationManager manager = MigrationManager.instance + assert manager.plan.steps.size() == 1 + assert manager.plan.steps.foo.actions.size() == 1 + } +}