From b615d6c7e2387ec3da5ad9cd7ce85d3c063f2afe Mon Sep 17 00:00:00 2001 From: Nicolas Vannieuwkerke Date: Mon, 21 Oct 2024 16:01:40 +0200 Subject: [PATCH 1/3] fix race condition with samplesheetToList --- .../validation/SamplesheetConverter.groovy | 34 +++++++++---------- .../validation/SchemaValidator.groovy | 5 +-- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/plugins/nf-schema/src/main/nextflow/validation/SamplesheetConverter.groovy b/plugins/nf-schema/src/main/nextflow/validation/SamplesheetConverter.groovy index 7d432b1..8148633 100644 --- a/plugins/nf-schema/src/main/nextflow/validation/SamplesheetConverter.groovy +++ b/plugins/nf-schema/src/main/nextflow/validation/SamplesheetConverter.groovy @@ -19,16 +19,10 @@ import nextflow.Nextflow @CompileStatic class SamplesheetConverter { - private static Path samplesheetFile - private static Path schemaFile private static ValidationConfig config - private static Map options - SamplesheetConverter(Path samplesheetFile, Path schemaFile, ValidationConfig config, Map options) { - this.samplesheetFile = samplesheetFile - this.schemaFile = schemaFile + SamplesheetConverter(ValidationConfig config) { this.config = config - this.options = options } private static List rows = [] @@ -67,34 +61,38 @@ class SamplesheetConverter { /* Convert the samplesheet to a list of entries based on a schema */ - public static List validateAndConvertToList() { + public static List validateAndConvertToList( + Path samplesheetFile, + Path schemaFile, + Map options + ) { def colors = Utils.logColours(config.monochromeLogs) // Some checks before validating - if(!this.schemaFile.exists()) { - def msg = "${colors.red}JSON schema file ${this.schemaFile.toString()} does not exist\n${colors.reset}\n" + if(!schemaFile.exists()) { + def msg = "${colors.red}JSON schema file ${schemaFile.toString()} does not exist\n${colors.reset}\n" throw new SchemaValidationException(msg) } - if(!this.samplesheetFile.exists()) { - def msg = "${colors.red}Samplesheet file ${this.samplesheetFile.toString()} does not exist\n${colors.reset}\n" + if(!samplesheetFile.exists()) { + def msg = "${colors.red}Samplesheet file ${samplesheetFile.toString()} does not exist\n${colors.reset}\n" throw new SchemaValidationException(msg) } // Validate final validator = new JsonSchemaValidator(config) - def JSONArray samplesheet = Utils.fileToJsonArray(this.samplesheetFile, this.schemaFile) - def List validationErrors = validator.validate(samplesheet, this.schemaFile.text) + def JSONArray samplesheet = Utils.fileToJsonArray(samplesheetFile, schemaFile) + def List validationErrors = validator.validate(samplesheet, schemaFile.text) if (validationErrors) { - def msg = "${colors.red}The following errors have been detected in ${this.samplesheetFile.toString()}:\n\n" + validationErrors.join('\n').trim() + "\n${colors.reset}\n" + def msg = "${colors.red}The following errors have been detected in ${samplesheetFile.toString()}:\n\n" + validationErrors.join('\n').trim() + "\n${colors.reset}\n" log.error("Validation of samplesheet failed!") throw new SchemaValidationException(msg, validationErrors) } // Convert - def LinkedHashMap schemaMap = new JsonSlurper().parseText(this.schemaFile.text) as LinkedHashMap - def List samplesheetList = Utils.fileToList(this.samplesheetFile, this.schemaFile) + def LinkedHashMap schemaMap = new JsonSlurper().parseText(schemaFile.text) as LinkedHashMap + def List samplesheetList = Utils.fileToList(samplesheetFile, schemaFile) this.rows = [] @@ -110,7 +108,7 @@ class SamplesheetConverter { } return result } - logUnusedHeadersWarning(this.samplesheetFile.toString()) + logUnusedHeadersWarning(samplesheetFile.toString()) return channelFormat } diff --git a/plugins/nf-schema/src/main/nextflow/validation/SchemaValidator.groovy b/plugins/nf-schema/src/main/nextflow/validation/SchemaValidator.groovy index 61aa1bb..13dd6be 100644 --- a/plugins/nf-schema/src/main/nextflow/validation/SchemaValidator.groovy +++ b/plugins/nf-schema/src/main/nextflow/validation/SchemaValidator.groovy @@ -224,8 +224,9 @@ class SchemaValidator extends PluginExtensionPoint { final Path schema, final Map options = null ) { - def SamplesheetConverter converter = new SamplesheetConverter(samplesheet, schema, config, options) - return converter.validateAndConvertToList() + def SamplesheetConverter converter = new SamplesheetConverter(config) + def List output = converter.validateAndConvertToList(samplesheet, schema, options) + return output } // From 2ff5b1a85f603bfa6fcac9b21f61508f75dae8e4 Mon Sep 17 00:00:00 2001 From: Nicolas Vannieuwkerke Date: Mon, 21 Oct 2024 16:03:06 +0200 Subject: [PATCH 2/3] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8b6ec0..23da8f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ## Bug fixes +1. Fixed a bug in `samplesheetToList` that caused output mixing when the function was used more than once. + ## Improvements # Version 2.1.2 From fb6a5252d4c9c6df7842792de3f17d5377332270 Mon Sep 17 00:00:00 2001 From: Nicolas Vannieuwkerke Date: Mon, 21 Oct 2024 16:18:57 +0200 Subject: [PATCH 3/3] add a test --- CHANGELOG.md | 2 +- .../SamplesheetConverterTest.groovy | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23da8f7..cf8897d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ## Bug fixes -1. Fixed a bug in `samplesheetToList` that caused output mixing when the function was used more than once. +1. Fixed a bug in `samplesheetToList` that caused output mixing when the function was used more than once in channel operators. ## Improvements diff --git a/plugins/nf-schema/src/test/nextflow/validation/SamplesheetConverterTest.groovy b/plugins/nf-schema/src/test/nextflow/validation/SamplesheetConverterTest.groovy index c2ed023..22cba8c 100644 --- a/plugins/nf-schema/src/test/nextflow/validation/SamplesheetConverterTest.groovy +++ b/plugins/nf-schema/src/test/nextflow/validation/SamplesheetConverterTest.groovy @@ -537,4 +537,42 @@ class SamplesheetConverterTest extends Dsl2Spec{ stdout.contains("[[string1:dependentRequired, string2:dependentRequired, integer1:10, integer2:10, boolean1:true, boolean2:true], string1, 25, false, [], [], [], unique2, 1, itDoesExist]") stdout.contains("[[string1:extraField, string2:extraField, integer1:10, integer2:10, boolean1:true, boolean2:true], string1, 25, false, ${getRootString()}/src/testResources/test.txt, ${getRootString()}/src/testResources/testDir, ${getRootString()}/src/testResources/testDir, unique3, 1, itDoesExist]" as String) } + + def 'samplesheetToList - usage in channels' () { + given: + def SCRIPT_TEXT = ''' + include { samplesheetToList } from 'plugin/nf-schema' + + Channel.of("src/testResources/correct.csv") + .flatMap { it -> + samplesheetToList(it, "src/testResources/schema_input.json") + } + .map { it -> println("first: ${it}") } + + Channel.of("src/testResources/correct_arrays.json") + .flatMap { it -> + samplesheetToList(it, "src/testResources/schema_input_with_arrays.json") + } + .map { it -> println("second: ${it}") } + + ''' + + when: + dsl_eval(SCRIPT_TEXT) + def stdout = capture + .toString() + .readLines() + .findResults {it.startsWith('first') || it.startsWith('second') ? it : null } + + then: + noExceptionThrown() + stdout.contains("first: [[string1:fullField, string2:fullField, integer1:10, integer2:10, boolean1:true, boolean2:true], string1, 25.12, false, ${getRootString()}/src/testResources/test.txt, ${getRootString()}/src/testResources/testDir, ${getRootString()}/src/testResources/test.txt, unique1, 1, itDoesExist]" as String) + stdout.contains("first: [[string1:value, string2:value, integer1:0, integer2:0, boolean1:true, boolean2:true], string1, 25.08, false, [], [], [], [], [], itDoesExist]") + stdout.contains("first: [[string1:dependentRequired, string2:dependentRequired, integer1:10, integer2:10, boolean1:true, boolean2:true], string1, 25, false, [], [], [], unique2, 1, itDoesExist]") + stdout.contains("first: [[string1:extraField, string2:extraField, integer1:10, integer2:10, boolean1:true, boolean2:true], string1, 25, false, ${getRootString()}/src/testResources/test.txt, ${getRootString()}/src/testResources/testDir, ${getRootString()}/src/testResources/testDir, unique3, 1, itDoesExist]" as String) + stdout.contains("second: [[array_meta:[]], [${getRootString()}/src/testResources/testDir/testFile.txt, ${getRootString()}/src/testResources/testDir2/testFile2.txt], [${getRootString()}/src/testResources/testDir, ${getRootString()}/src/testResources/testDir2], [${getRootString()}/src/testResources/testDir, ${getRootString()}/src/testResources/testDir2/testFile2.txt], [string1, string2], [25, 26], [25, 26.5], [false, true], [1, 2, 3], [true], [${getRootString()}/src/testResources/testDir/testFile.txt], [[${getRootString()}/src/testResources/testDir/testFile.txt]]]" as String) + stdout.contains("second: [[array_meta:[look, an, array, in, meta]], [], [], [], [string1, string2], [25, 26], [25, 26.5], [], [1, 2, 3], [false, true, false], [${getRootString()}/src/testResources/testDir/testFile.txt], [[${getRootString()}/src/testResources/testDir/testFile.txt]]]" as String) + stdout.contains("second: [[array_meta:[]], [], [], [], [string1, string2], [25, 26], [25, 26.5], [], [1, 2, 3], [false, true, false], [${getRootString()}/src/testResources/testDir/testFile.txt], [[${getRootString()}/src/testResources/testDir/testFile.txt], [${getRootString()}/src/testResources/testDir/testFile.txt, ${getRootString()}/src/testResources/testDir2/testFile2.txt]]]" as String) + + } }