Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# nextflow-io/nf-schema: Changelog

# Version 3.1.0

## Bug fixes

1. 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

This version contains some breaking changes to the nf-schema API. See the [migration guide](https://nextflow-io.github.io/nf-schema/3.0.0/3_0_0_migration_guide) for more information.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String> cliParams = (session.cliParams?.keySet()?.toList()*.toString() ?: []) as List<String>
generatorOptions.addConverter(Map<String, Object>) { Map<String,Object> 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<String, Object>
}

JSONObject paramsJSON = new JSONObject(generatorOptions.build().toJson(params))
Expand Down
21 changes: 21 additions & 0 deletions src/main/groovy/nextflow/validation/utils/Types.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,25 @@ 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) {
return params.collectEntries { Object name, Object value ->
if (cliParams == null || !cliParams.containsKey(name)) {
return [(name): value]
Comment thread
nvnieuwk marked this conversation as resolved.
}
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
}

}
74 changes: 74 additions & 0 deletions src/test/groovy/nextflow/validation/ValidateCliParamsTest.groovy
Original file line number Diff line number Diff line change
@@ -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
)
}

}
47 changes: 47 additions & 0 deletions src/test/groovy/nextflow/validation/utils/TypesTest.groovy
Original file line number Diff line number Diff line change
@@ -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']]
}

}
Loading