From d43e55c402dad9afe0a44e5e8f43bbac33eab694 Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Thu, 29 Aug 2024 17:34:28 -0500 Subject: [PATCH 1/8] add ds level check to validate citation.cff using json schema from citation.cff project. --- bids-validator/deno.json | 18 +++++---- bids-validator/src/setup/loadSchema.test.ts | 2 +- bids-validator/src/validators/bids.ts | 2 + .../src/validators/citation.test.ts | 25 ++++++++++++ bids-validator/src/validators/citation.ts | 38 +++++++++++++++++++ bids-validator/tests/data/citation/bad.cff | 15 ++++++++ bids-validator/tests/data/citation/good.cff | 20 ++++++++++ 7 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 bids-validator/src/validators/citation.test.ts create mode 100644 bids-validator/src/validators/citation.ts create mode 100644 bids-validator/tests/data/citation/bad.cff create mode 100644 bids-validator/tests/data/citation/good.cff diff --git a/bids-validator/deno.json b/bids-validator/deno.json index a5c96b9f1..48f631dc3 100644 --- a/bids-validator/deno.json +++ b/bids-validator/deno.json @@ -27,20 +27,22 @@ ] }, "imports": { + "@ajv": "npm:ajv@8.16.0", + "@ajv-formats": "npm:ajv-formats@3.0.1", + "@bids/schema": "jsr:@bids/schema@0.11.2-dev.2+7f1f6737", + "@cliffy/command": "jsr:@cliffy/command@1.0.0-rc.5", + "@cliffy/table": "jsr:@cliffy/table@1.0.0-rc.5", + "@hed/validator": "npm:hed-validator@3.15.4", + "@ignore": "npm:ignore@5.3.2", + "@libs/xml": "jsr:@libs/xml@5.4.13", + "@mango/nifti": "npm:nifti-reader-js@0.6.8", "@std/assert": "jsr:@std/assert@1.0.2", "@std/fmt": "jsr:@std/fmt@1.0.0", "@std/fs": "jsr:@std/fs@1.0.1", "@std/io": "jsr:@std/io@0.224.4", "@std/log": "jsr:@std/log@0.224.5", "@std/path": "jsr:@std/path@1.0.2", - "@ajv": "npm:ajv@8.16.0", - "@bids/schema": "jsr:@bids/schema@0.11.1-dev.4+a73c1b06", - "@cliffy/table": "jsr:@cliffy/table@1.0.0-rc.5", - "@cliffy/command": "jsr:@cliffy/command@1.0.0-rc.5", - "@hed/validator": "npm:hed-validator@3.15.4", - "@ignore": "npm:ignore@5.3.2", - "@mango/nifti": "npm:nifti-reader-js@0.6.8", - "@libs/xml": "jsr:@libs/xml@5.4.13" + "@std/yaml": "jsr:@std/yaml@^1.0.4" }, "tasks": { "test": "deno test -A src/tests/" diff --git a/bids-validator/src/setup/loadSchema.test.ts b/bids-validator/src/setup/loadSchema.test.ts index 896040a75..1dc9b2e63 100644 --- a/bids-validator/src/setup/loadSchema.test.ts +++ b/bids-validator/src/setup/loadSchema.test.ts @@ -1,7 +1,7 @@ import { assert, assertObjectMatch } from '@std/assert' import { loadSchema } from './loadSchema.ts' -Deno.test('schema yaml loader', async (t) => { +Deno.test('schema loader', async (t) => { await t.step('reads in top level files document', async () => { const schemaDefs = await loadSchema() // Look for some stable fields in top level files diff --git a/bids-validator/src/validators/bids.ts b/bids-validator/src/validators/bids.ts index 408943e63..0f5e187aa 100644 --- a/bids-validator/src/validators/bids.ts +++ b/bids-validator/src/validators/bids.ts @@ -17,6 +17,7 @@ import { sidecarWithoutDatafile, unusedStimulus } from './internal/unusedFile.ts import { type BIDSContext, BIDSContextDataset } from '../schema/context.ts' import type { parseOptions } from '../setup/options.ts' import { hedValidate } from './hed.ts' +import { citationValidate } from './citation.ts' /** * Ordering of checks to apply @@ -32,6 +33,7 @@ const perContextChecks: ContextCheckFunction[] = [ const perDSChecks: DSCheckFunction[] = [ unusedStimulus, sidecarWithoutDatafile, + citationValdiate, ] /** diff --git a/bids-validator/src/validators/citation.test.ts b/bids-validator/src/validators/citation.test.ts new file mode 100644 index 000000000..eea82b51d --- /dev/null +++ b/bids-validator/src/validators/citation.test.ts @@ -0,0 +1,25 @@ +import { assert } from '@std/assert' +import { pathsToTree } from '../files/filetree.ts' +import { BIDSFileDeno } from '../files/deno.ts' +import { citationValidate } from './citation.ts' +import { BIDSContextDataset } from '../schema/context.ts' +import { GenericSchema } from '../types/schema.ts' + +Deno.test('citation validation', async (t) => { + await t.step('no errors on the good citation.cff', async () => { + const tree = pathsToTree(['CITATION.cff']) + const dsContext = new BIDSContextDataset({ tree }) + const file = new BIDSFileDeno('tests/data/citation', 'good.cff') + tree.files[0].text = () => file.text() + await citationValidate({} as GenericSchema, dsContext) + assert(dsContext.issues.size === 0) + }) + await t.step('An error on the bad citation.cff', async () => { + const tree = pathsToTree(['CITATION.cff']) + const dsContext = new BIDSContextDataset({ tree }) + const file = new BIDSFileDeno('tests/data/citation', 'bad.cff') + tree.files[0].text = () => file.text() + await citationValidate({} as GenericSchema, dsContext) + assert(dsContext.issues.size === 1) + }) +}) diff --git a/bids-validator/src/validators/citation.ts b/bids-validator/src/validators/citation.ts new file mode 100644 index 000000000..44c3719c3 --- /dev/null +++ b/bids-validator/src/validators/citation.ts @@ -0,0 +1,38 @@ +import type { GenericSchema } from '../types/schema.ts' +import type { BIDSFile, FileTree } from '../types/filetree.ts' +import type { BIDSContextDataset } from '../schema/context.ts' +import { schema as citationSchema } from '@bids/schema/citation' +import { Ajv, type DefinedError } from '@ajv' +import _addFormats from '@ajv-formats' +import { parse } from '@std/yaml' + +// https://github.com/ajv-validator/ajv-formats/issues/85 +const addFormats = _addFormats as unknown as typeof _addFormats.default +const citationFilename = 'CITATION.cff' + +export async function citationValidate( + schema: GenericSchema, + dsContext: BIDSContextDataset, +) { + const citationFile = dsContext.tree.get(citationFilename) + if (!citationFile || 'directories' in citationFile) return + let citation: unknown = {} + try { + citation = parse(await citationFile.text()) + } catch (error) { + throw error + return + } + const ajv = new Ajv() + addFormats(ajv) + const citationValidate = ajv.compile(citationSchema) + if (!citationValidate(citation)) { + for (const err of citationValidate.errors as DefinedError[]) { + dsContext.issues.add({ + code: 'JSON_SCHEMA_VALIDATION_ERROR', + issueMessage: err['message'], + location: citationFilename, + }) + } + } +} diff --git a/bids-validator/tests/data/citation/bad.cff b/bids-validator/tests/data/citation/bad.cff new file mode 100644 index 000000000..71e85ecfc --- /dev/null +++ b/bids-validator/tests/data/citation/bad.cff @@ -0,0 +1,15 @@ +cff-version: 1.2.0 +message: If you use this software, please cite it using these metadata. +title: My Research Software +abstract: This is my awesome research software. It does many things. +version: 0.11.2 +date-released: "2021-07-18" +identifiers: + - description: This is the collection of archived snapshots of all versions of My Research Software + type: doi + value: "10.5281/zenodo.123456" + - description: This is the archived snapshot of version 0.11.2 of My Research Software + type: doi + value: "10.5281/zenodo.123457" +license: Apache-2.0 +repository-code: "https://github.com/citation-file-format/my-research-software" diff --git a/bids-validator/tests/data/citation/good.cff b/bids-validator/tests/data/citation/good.cff new file mode 100644 index 000000000..f935b0fef --- /dev/null +++ b/bids-validator/tests/data/citation/good.cff @@ -0,0 +1,20 @@ +cff-version: 1.2.0 +message: If you use this software, please cite it using these metadata. +title: My Research Software +abstract: This is my awesome research software. It does many things. +authors: + - family-names: Druskat + given-names: Stephan + orcid: "https://orcid.org/1234-5678-9101-1121" + - name: "The Research Software project" +version: 0.11.2 +date-released: "2021-07-18" +identifiers: + - description: This is the collection of archived snapshots of all versions of My Research Software + type: doi + value: "10.5281/zenodo.123456" + - description: This is the archived snapshot of version 0.11.2 of My Research Software + type: doi + value: "10.5281/zenodo.123457" +license: Apache-2.0 +repository-code: "https://github.com/citation-file-format/my-research-software" From 5a04a2bcef008eda19f3ddf76b854fb87f35a643 Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Thu, 29 Aug 2024 17:41:48 -0500 Subject: [PATCH 2/8] spelling mistake, but this causes some more errors... --- bids-validator/src/validators/bids.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bids-validator/src/validators/bids.ts b/bids-validator/src/validators/bids.ts index 0f5e187aa..5b0d96510 100644 --- a/bids-validator/src/validators/bids.ts +++ b/bids-validator/src/validators/bids.ts @@ -33,7 +33,7 @@ const perContextChecks: ContextCheckFunction[] = [ const perDSChecks: DSCheckFunction[] = [ unusedStimulus, sidecarWithoutDatafile, - citationValdiate, + citationValidate, ] /** From fbc9ad39b637cc47dd894a412c79585d9e3095d1 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 30 Aug 2024 08:16:35 -0400 Subject: [PATCH 3/8] chore(build): Mark ajv as external --- bids-validator/build.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/bids-validator/build.ts b/bids-validator/build.ts index 26fc6b7ea..3deec4234 100755 --- a/bids-validator/build.ts +++ b/bids-validator/build.ts @@ -27,7 +27,7 @@ const flags = parse(Deno.args, { const version = await getVersion() -let versionPlugin = { +const versionPlugin = { name: 'version', setup(build: esbuild.PluginBuild) { build.onResolve({ filter: /\.git-meta\.json/ }, (args) => ({ @@ -42,15 +42,25 @@ let versionPlugin = { }, } +const custom_external = { + name: 'custom-external', + setup({ onResolve }) { + onResolve({ filter: /^ajv/ }, (args) => { + return { path: args.path, external: true } + }) + }, +} + const result = await esbuild.build({ format: 'esm', entryPoints: [MAIN_ENTRY, CLI_ENTRY], bundle: true, - outdir: path.join('dist','validator'), + outdir: path.join('dist', 'validator'), minify: flags.minify, target: ['chrome109', 'firefox109', 'safari16'], plugins: [ versionPlugin, + custom_external, ...denoPlugins({ configPath: path.join(dir, 'deno.json'), }), From e76b97e07a7bf7eb2587f02e2f04dcaf0efd5b53 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 30 Aug 2024 08:29:01 -0400 Subject: [PATCH 4/8] fix: Our formats are extensive enough --- bids-validator/build.ts | 10 ---------- bids-validator/deno.json | 1 - bids-validator/src/validators/citation.ts | 10 +++------- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/bids-validator/build.ts b/bids-validator/build.ts index 3deec4234..92cacfd11 100755 --- a/bids-validator/build.ts +++ b/bids-validator/build.ts @@ -42,15 +42,6 @@ const versionPlugin = { }, } -const custom_external = { - name: 'custom-external', - setup({ onResolve }) { - onResolve({ filter: /^ajv/ }, (args) => { - return { path: args.path, external: true } - }) - }, -} - const result = await esbuild.build({ format: 'esm', entryPoints: [MAIN_ENTRY, CLI_ENTRY], @@ -60,7 +51,6 @@ const result = await esbuild.build({ target: ['chrome109', 'firefox109', 'safari16'], plugins: [ versionPlugin, - custom_external, ...denoPlugins({ configPath: path.join(dir, 'deno.json'), }), diff --git a/bids-validator/deno.json b/bids-validator/deno.json index 48f631dc3..32abd0666 100644 --- a/bids-validator/deno.json +++ b/bids-validator/deno.json @@ -28,7 +28,6 @@ }, "imports": { "@ajv": "npm:ajv@8.16.0", - "@ajv-formats": "npm:ajv-formats@3.0.1", "@bids/schema": "jsr:@bids/schema@0.11.2-dev.2+7f1f6737", "@cliffy/command": "jsr:@cliffy/command@1.0.0-rc.5", "@cliffy/table": "jsr:@cliffy/table@1.0.0-rc.5", diff --git a/bids-validator/src/validators/citation.ts b/bids-validator/src/validators/citation.ts index 44c3719c3..218c27ac4 100644 --- a/bids-validator/src/validators/citation.ts +++ b/bids-validator/src/validators/citation.ts @@ -2,12 +2,10 @@ import type { GenericSchema } from '../types/schema.ts' import type { BIDSFile, FileTree } from '../types/filetree.ts' import type { BIDSContextDataset } from '../schema/context.ts' import { schema as citationSchema } from '@bids/schema/citation' -import { Ajv, type DefinedError } from '@ajv' -import _addFormats from '@ajv-formats' +import { compile } from './json.ts' +import type { DefinedError } from '@ajv' import { parse } from '@std/yaml' -// https://github.com/ajv-validator/ajv-formats/issues/85 -const addFormats = _addFormats as unknown as typeof _addFormats.default const citationFilename = 'CITATION.cff' export async function citationValidate( @@ -23,9 +21,7 @@ export async function citationValidate( throw error return } - const ajv = new Ajv() - addFormats(ajv) - const citationValidate = ajv.compile(citationSchema) + const citationValidate = compile(citationSchema) if (!citationValidate(citation)) { for (const err of citationValidate.errors as DefinedError[]) { dsContext.issues.add({ From 3278e414204f1cad03faa5b195ab486a3659f0a9 Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Fri, 30 Aug 2024 08:47:16 -0500 Subject: [PATCH 5/8] Update bids-validator/src/validators/citation.ts Co-authored-by: Chris Markiewicz --- bids-validator/src/validators/citation.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bids-validator/src/validators/citation.ts b/bids-validator/src/validators/citation.ts index 218c27ac4..17c00c70c 100644 --- a/bids-validator/src/validators/citation.ts +++ b/bids-validator/src/validators/citation.ts @@ -21,9 +21,9 @@ export async function citationValidate( throw error return } - const citationValidate = compile(citationSchema) - if (!citationValidate(citation)) { - for (const err of citationValidate.errors as DefinedError[]) { + const validate = compile(citationSchema) + if (!validate(citation)) { + for (const err of validate.errors as DefinedError[]) { dsContext.issues.add({ code: 'JSON_SCHEMA_VALIDATION_ERROR', issueMessage: err['message'], From 3c0b39feac0ba72c9f0d009c7222aff06336b271 Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Fri, 30 Aug 2024 08:54:44 -0500 Subject: [PATCH 6/8] Update bids-validator/src/validators/citation.ts Co-authored-by: Chris Markiewicz --- bids-validator/src/validators/citation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bids-validator/src/validators/citation.ts b/bids-validator/src/validators/citation.ts index 17c00c70c..94c891e35 100644 --- a/bids-validator/src/validators/citation.ts +++ b/bids-validator/src/validators/citation.ts @@ -25,7 +25,8 @@ export async function citationValidate( if (!validate(citation)) { for (const err of validate.errors as DefinedError[]) { dsContext.issues.add({ - code: 'JSON_SCHEMA_VALIDATION_ERROR', + code: 'CITATION_CFF_VALIDATION_ERROR', + subCode: err['instancePath'], issueMessage: err['message'], location: citationFilename, }) From 0ed465bb129ab8f55e0433d0601ffa2a9763ad80 Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Fri, 30 Aug 2024 09:02:12 -0500 Subject: [PATCH 7/8] add file read issues, have citation.ts report it when applicable. --- bids-validator/src/issues/list.ts | 18 ++++++++++++++---- bids-validator/src/validators/citation.test.ts | 2 +- bids-validator/src/validators/citation.ts | 6 +++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/bids-validator/src/issues/list.ts b/bids-validator/src/issues/list.ts index 0f3336094..779a7cdef 100644 --- a/bids-validator/src/issues/list.ts +++ b/bids-validator/src/issues/list.ts @@ -162,6 +162,20 @@ export const bidsIssues: IssueDefinitionRecord = { severity: 'error', reason: 'A json sidecar file was found without a corresponding data file', }, + BLACKLISTED_MODALITY: { + severity: 'error', + reason: 'The modality in this file is blacklisted through validator configuration.', + }, + CITATION_CFF_VALIDATION_ERROR: { + severity: 'error', + reason: + "The file does not pass validation using the citation.cff standard's schema." + + 'https://github.com/citation-file-format/citation-file-format/blob/main/schema-guide.md' + }, + FILE_READ: { + severity: 'error', + reason: 'We were unable to read this file.' + } } const hedIssues: IssueDefinitionRecord = { @@ -191,10 +205,6 @@ const hedIssues: IssueDefinitionRecord = { reason: "You should define 'HEDVersion' for this file. If you don't provide this information, the HED validation will use the latest version available.", }, - BLACKLISTED_MODALITY: { - severity: 'error', - reason: 'The modality in this file is blacklisted through validator configuration.', - }, } export const hedOldToNewLookup: Record> = { diff --git a/bids-validator/src/validators/citation.test.ts b/bids-validator/src/validators/citation.test.ts index eea82b51d..7ec959e50 100644 --- a/bids-validator/src/validators/citation.test.ts +++ b/bids-validator/src/validators/citation.test.ts @@ -20,6 +20,6 @@ Deno.test('citation validation', async (t) => { const file = new BIDSFileDeno('tests/data/citation', 'bad.cff') tree.files[0].text = () => file.text() await citationValidate({} as GenericSchema, dsContext) - assert(dsContext.issues.size === 1) + assert(dsContext.issues.get({ code: 'CITATION_CFF_VALIDATION_ERROR' }).length === 1) }) }) diff --git a/bids-validator/src/validators/citation.ts b/bids-validator/src/validators/citation.ts index 94c891e35..905ef80ef 100644 --- a/bids-validator/src/validators/citation.ts +++ b/bids-validator/src/validators/citation.ts @@ -18,7 +18,11 @@ export async function citationValidate( try { citation = parse(await citationFile.text()) } catch (error) { - throw error + dsContext.issues.add({ + code: 'FILE_READ', + issueMessage: `Error from attempted read of file:\n${error}`, + location: citationFilename, + }) return } const validate = compile(citationSchema) From 0c1b20aa43ed61c92aea708a83e26f6b5f2911ba Mon Sep 17 00:00:00 2001 From: Ross Blair Date: Fri, 30 Aug 2024 09:15:48 -0500 Subject: [PATCH 8/8] loadSchema must be called in order to populate ajv with our formats. --- bids-validator/src/validators/citation.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bids-validator/src/validators/citation.test.ts b/bids-validator/src/validators/citation.test.ts index 7ec959e50..16bcfbcba 100644 --- a/bids-validator/src/validators/citation.test.ts +++ b/bids-validator/src/validators/citation.test.ts @@ -4,8 +4,10 @@ import { BIDSFileDeno } from '../files/deno.ts' import { citationValidate } from './citation.ts' import { BIDSContextDataset } from '../schema/context.ts' import { GenericSchema } from '../types/schema.ts' +import { loadSchema } from '../setup/loadSchema.ts' Deno.test('citation validation', async (t) => { + const schema = await loadSchema() await t.step('no errors on the good citation.cff', async () => { const tree = pathsToTree(['CITATION.cff']) const dsContext = new BIDSContextDataset({ tree })