Fix issue overriding nested params from cli #1923

Author: pditommaso

modules/nextflow/src/main/groovy/nextflow/config/ConfigParser.groovy

@@ -24,6 +24,7 @@ import com.google.common.hash.Hashing
 import groovy.transform.PackageScope
 import nextflow.ast.NextflowXform
 import nextflow.exception.ConfigParseException
+import nextflow.extension.Bolts
 import nextflow.file.FileHelper
 import nextflow.util.Duration
 import nextflow.util.MemoryUnit
@@ -195,7 +196,9 @@ class ConfigParser {
     }
 
     ConfigParser setParams(Map vars) {
-        this.paramVars = vars
+        // deep clone the map to prevent side-effect
+        // see https://github.com/nextflow-io/nextflow/issues/1923
+        this.paramVars = Bolts.deepClone(vars)
         return this
     }
 

modules/nextflow/src/test/groovy/nextflow/config/ConfigBuilderTest.groovy

@@ -1876,7 +1876,35 @@ class ConfigBuilderTest extends Specification {
         [foo:1, bar:[:]]            | [bar: [x:10, y:20]]           | [foo: 1, bar: [x:10, y:20]]
         [foo:1, bar:[x:1, y:2]]     | [bar: [x:10, y:20]]           | [foo: 1, bar: [x:10, y:20]]
         [foo:1, bar:[x:1, y:2]]     | [foo: 2, bar: [x:10, y:20]]   | [foo: 2, bar: [x:10, y:20]]
+    }
 
+    def 'prevent config side effects' () {
+        given:
+        def folder = Files.createTempDirectory('test')
+        and:
+        def config = folder.resolve('nf.config')
+        config.text = '''\
+        params.test.foo = "foo_def"
+        params.test.bar = "bar_def"        
+        '''.stripIndent()
+
+        when:
+        def cfg1 = new ConfigBuilder()
+                .setOptions( new CliOptions(userConfig: [config.toString()]))
+                .build()
+        then:
+        cfg1.params.test.foo == "foo_def"
+        cfg1.params.test.bar == "bar_def"
+
+        
+        when:
+        def cfg2 = new ConfigBuilder()
+                .setOptions( new CliOptions(userConfig: [config.toString()]))
+                .setCmdRun( new CmdRun(params: ['test.foo': 'CLI_FOO'] ))
+                .build()
+        then:
+        cfg2.params.test.foo == "CLI_FOO"
+        cfg2.params.test.bar == "bar_def"
     }
 
 }

modules/nf-commons/src/main/nextflow/extension/Bolts.groovy

@@ -911,4 +911,26 @@ class Bolts {
         else
             return self.substring(0,Math.min(self.size()-max, max)) + suffix
     }
+
+    static <T extends Serializable> T deepClone(T obj) {
+        final buffer = new ByteArrayOutputStream()
+        final oos = new ObjectOutputStream(buffer)
+        oos.writeObject(obj)
+        oos.flush()
+
+        final inputStream = new ByteArrayInputStream(buffer.toByteArray())
+        return (T) new ObjectInputStream(inputStream).readObject()
+    }
+
+    @CompileDynamic
+    static <T extends Map> T deepClone(T map) {
+        final result = map.clone()
+        for( def key : map.keySet() ) {
+            def value = map.get(key)
+            if( value instanceof Map ) {
+               map.put(key, deepClone(value))
+            }
+        }
+        return (T)result
+    }
 }

modules/nf-commons/src/test/nextflow/extension/BoltsTest.groovy

@@ -333,4 +333,36 @@ class BoltsTest extends Specification {
         Bolts.redact('12345', 3, 'xx') == '12xx'
     }
 
+    def 'should deep clone obj' () {
+        given:
+        Map map = [foo: 1, bar: [x: 2, y: 3]]
+
+        when:
+        def copy = Bolts.deepClone((Serializable)map)
+        then:
+        copy == map
+
+        when:
+        copy.bar.x = 20
+        then:
+        copy.bar.x == 20
+        map.bar.x == 2
+    }
+
+    def 'should deep clone map' () {
+        given:
+        Map map = [foo: 1, bar: [x: 2, y: 3]]
+
+        when:
+        def copy = Bolts.deepClone((Map)map)
+        then:
+        copy == map
+
+        when:
+        copy.bar.x = 20
+        then:
+        copy.bar.x == 20
+        map.bar.x == 2
+    }
+
 }