diff --git a/CHANGELOG.md b/CHANGELOG.md index b302ea55..119d6402 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Bug fixes 1. Fixed an issue where a schema referenced with the `schema` keyword was only looked up relative to the project that is running. It is now looked up next to the schema that references it first, and relative to the project otherwise, so a pipeline schema keeps working when it is read from another directory (e.g. when the pipeline is included in another one). +2. Fixed an issue where the command line value of a nested parameter (e.g. `--group.flag true`) was not cast to the type in the schema, while the value of a top-level parameter was. The nested values of the command line parameters are now followed to find the values to cast. # Version 3.0.0 diff --git a/src/main/groovy/nextflow/validation/parameters/ParameterValidator.groovy b/src/main/groovy/nextflow/validation/parameters/ParameterValidator.groovy index 431c2ae2..3f345be5 100644 --- a/src/main/groovy/nextflow/validation/parameters/ParameterValidator.groovy +++ b/src/main/groovy/nextflow/validation/parameters/ParameterValidator.groovy @@ -5,7 +5,7 @@ import static nextflow.NF.isSyntaxParserV2 import static nextflow.validation.utils.Colors.getLogColors import static nextflow.validation.utils.Common.getBasePath import static nextflow.validation.utils.Common.getValueFromJsonPointer -import static nextflow.validation.utils.Types.parseParamValue +import static nextflow.validation.utils.Types.castCliValues import java.nio.file.Path import groovy.json.JsonGenerator @@ -147,13 +147,7 @@ class ParameterValidator { // in which case we can rely on the static type system to do the casting for us. // This mimics the type casting behaviour of syntax parser V1 so shouldn't introduce any breaking changes. if (castCliParams) { - List cliParams = (session.cliParams?.keySet()?.toList()*.toString() ?: []) as List - generatorOptions.addConverter(Map) { Map map -> - map.collectEntries { k, v -> - // Only cast parameters that were explicitly provided via the CLI - return (cliParams.contains(k) && v in String) ? [k, parseParamValue(v as String)] : [k, v] - } - } + params = castCliValues(params, session.cliParams as Map) as Map } JSONObject paramsJSON = new JSONObject(generatorOptions.build().toJson(params)) diff --git a/src/main/groovy/nextflow/validation/utils/Types.groovy b/src/main/groovy/nextflow/validation/utils/Types.groovy index 1579044a..7c1b63b1 100644 --- a/src/main/groovy/nextflow/validation/utils/Types.groovy +++ b/src/main/groovy/nextflow/validation/utils/Types.groovy @@ -103,4 +103,28 @@ public class Types { return str } + // + // Cast the String values given on the command line to the type they have. The values of a nested + // parameter (`--group.flag true`) are found in the nested maps of the command line parameters. + // Values that were not given on the command line are left as they are. + // + static Map castCliValues(Map params, Map cliParams) { + if (cliParams == null) { + return params + } + return params.collectEntries { Object name, Object value -> + if (!cliParams.containsKey(name)) { + return [(name): value] + } + Object cast = value + if (value in Map) { + Object cliValue = cliParams[name] + cast = castCliValues(value as Map, cliValue in Map ? cliValue as Map : [:]) + } else if (value in String) { + cast = parseParamValue(value as String) + } + return [(name): cast] + } as Map + } + } diff --git a/src/test/groovy/nextflow/validation/ValidateCliParamsTest.groovy b/src/test/groovy/nextflow/validation/ValidateCliParamsTest.groovy new file mode 100644 index 00000000..2ceb554e --- /dev/null +++ b/src/test/groovy/nextflow/validation/ValidateCliParamsTest.groovy @@ -0,0 +1,74 @@ +/* groovylint-disable LineLength, MethodName */ +package nextflow.validation + +import groovy.transform.CompileDynamic +import nextflow.Session +import nextflow.validation.config.ValidationConfig +import nextflow.validation.exceptions.SchemaValidationException +import nextflow.validation.parameters.ParameterValidator +import spock.lang.Specification + +import java.nio.file.Path + +/** + * Validation of the values given on the command line, which are cast to the type they have. + * The session is mocked so that the command line params can be set directly. + */ +@CompileDynamic +class ValidateCliParamsTest extends Specification { + + private static final String SCHEMA = 'src/testResources/nextflow_schema_nested_parameters.json' + + void 'should accept a nested value given on the command line'() { + given: + Session session = mockSession([map: [is: [so: [deep: 'true']]]], [map: [is: [so: [deep: 'true']]]]) + + when: + validate(session) + + then: + noExceptionThrown() + } + + void 'should reject a nested value given on the command line that is not of the type of the schema'() { + given: + Session session = mockSession([map: [is: [so: [deep: 'maybe']]]], [map: [is: [so: [deep: 'maybe']]]]) + + when: + validate(session) + + then: + SchemaValidationException error = thrown(SchemaValidationException) + error.message.contains('--map.is.so.deep (maybe): Value is [string] but should be [boolean]') + } + + void 'should not cast a nested value that was not given on the command line'() { + given: + Session session = mockSession([map: [is: [so: [deep: 'true']]]], [:]) + + when: + validate(session) + + then: + SchemaValidationException error = thrown(SchemaValidationException) + error.message.contains('--map.is.so.deep (true): Value is [string] but should be [boolean]') + } + + private Session mockSession(Map params, Map cliParams) { + Session session = Mock(Session) + session.params >> params + session.cliParams >> cliParams + session.config >> [params: params] + session.baseDir >> Path.of('.').toAbsolutePath() + return session + } + + private void validate(Session session) { + ValidationConfig config = new ValidationConfig([monochromeLogs: true], session) + new ParameterValidator(config).validateParametersMap( + [parameters_schema: Path.of(SCHEMA).toAbsolutePath().toString(), cast_cli_params: true], + session + ) + } + +} diff --git a/src/test/groovy/nextflow/validation/utils/TypesTest.groovy b/src/test/groovy/nextflow/validation/utils/TypesTest.groovy new file mode 100644 index 00000000..8c0f5848 --- /dev/null +++ b/src/test/groovy/nextflow/validation/utils/TypesTest.groovy @@ -0,0 +1,47 @@ +/* groovylint-disable LineLength, MethodName */ +package nextflow.validation.utils + +import static nextflow.validation.utils.Types.castCliValues + +import groovy.transform.CompileDynamic +import spock.lang.Specification + +/** + * Casting of the values given on the command line. + */ +@CompileDynamic +class TypesTest extends Specification { + + void 'should cast the values given on the command line'() { + expect: + castCliValues(params, cliParams) == expected + + where: + params | cliParams | expected + [flag: 'true', count: '3'] | [flag: 'true', count: '3'] | [flag: true, count: 3] + [ratio: '0.5', name: 'abc'] | [ratio: '0.5', name: 'abc'] | [ratio: 0.5, name: 'abc'] + [flag: 'true'] | [:] | [flag: 'true'] + [flag: 'true'] | null | [flag: 'true'] + [flag: 'true', other: 'false'] | [flag: 'true'] | [flag: true, other: 'false'] + [flag: true, count: 3] | [flag: 'true', count: '3'] | [flag: true, count: 3] + } + + void 'should cast the nested values given on the command line'() { + expect: + castCliValues(params, cliParams) == expected + + where: + params | cliParams | expected + [group: [flag: 'true', count: '3']] | [group: [flag: 'true', count: '3']] | [group: [flag: true, count: 3]] + [group: [flag: 'true', other: 'false']] | [group: [flag: 'true']] | [group: [flag: true, other: 'false']] + [group: [is: [so: [deep: 'true']]]] | [group: [is: [so: [deep: 'true']]]] | [group: [is: [so: [deep: true]]]] + [group: [flag: 'true']] | [:] | [group: [flag: 'true']] + [group: [flag: 'true']] | [other: 'true'] | [group: [flag: 'true']] + } + + void 'should not cast a nested value with the name of a top-level command line parameter'() { + expect: + castCliValues([flag: 'true', group: [flag: 'true']], [flag: 'true']) == [flag: true, group: [flag: 'true']] + } + +}