Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate JSON schema's from spec schema to validate values in sidecars. #2020

Merged
merged 19 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
1606d7f
Add jsonschema based type checking for sidecar entry values.
rwblair Jul 12, 2024
27a4c45
fix some ts errors, ignore some to get tests going
rwblair Jul 12, 2024
9231f97
add schemapath to json validation error message
rwblair Jul 12, 2024
90032c4
Update bids-validator/src/schema/applyRules.ts
rwblair Jul 12, 2024
5b00b5f
put all ajv exports in a single line with import type modifier as needed
rwblair Jul 15, 2024
13e666c
Update bids-validator/src/schema/applyRules.ts
rwblair Jul 15, 2024
52e1a74
add logger warning if filekey origin map not populated in context. Ba…
rwblair Jul 15, 2024
313ef04
Merge branch 'schema/json_array_type_checks' of github.com:rwblair/bi…
rwblair Jul 15, 2024
9ed3e6e
bump bids-examples
rwblair Jul 18, 2024
fd1575a
bump examples
rwblair Jul 18, 2024
60bbe35
Merge branch 'master' of github.com:bids-standard/bids-validator into…
rwblair Jul 18, 2024
ca0dec4
Merge branch 'master' of github.com:bids-standard/bids-validator into…
rwblair Jul 29, 2024
aaaff98
Merge branch 'master' into schema/json_array_type_checks
rwblair Jul 29, 2024
6b61737
FIX: Compile correct metadata definition
effigies Jul 29, 2024
672fa43
Merge remote-tracking branch 'upstream/master' into schema/json_array…
effigies Jul 29, 2024
8b5ec12
Remove unused ts-expect-error
effigies Jul 29, 2024
92cbef9
chore(fmt): deno fmt [ignore-rev]
effigies Jul 29, 2024
1945f1e
git log --grep "\[ignore-rev\]" --pretty=format:"# %ad - %ae - %s%n%H…
effigies Jul 29, 2024
180d4fa
Merge remote-tracking branch 'upstream/master' into schema/json_array…
effigies Jul 30, 2024
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
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# Mon Jul 29 19:08:32 2024 -0400 - [email protected] - chore(fmt): deno fmt [ignore-rev]
92cbef9624663b719b7b4c22b6104669eb518f09
# Sun Jul 28 22:01:32 2024 -0400 - [email protected] - chore(fmt): deno fmt [ignore-rev]
1bccd0a2d09f1b7ff6a26d99c7845bf59908c421
1 change: 1 addition & 0 deletions bids-validator/src/deps/ajv.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { Ajv, type JSONSchemaType, type ValidateFunction } from 'https://esm.sh/[email protected]'
4 changes: 4 additions & 0 deletions bids-validator/src/issues/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export const filenameIssues: IssueDefinitionRecord = {
severity: 'warning',
reason: 'A data files JSON sidecar is missing a key listed as recommended.',
},
JSON_SCHEMA_VALIDATION_ERROR: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pull this code from schema.rules.errors (and make all other errors there available. Internally represent with code as the key and rest of object as contents. add addSchemaIssue to src/issues/schemaIssues.ts or something.

severity: 'error',
reason: 'Invalid JSON sidecar file. The sidecar is not formatted according the schema.',
},
TSV_ERROR: {
severity: 'error',
reason: 'generic place holder for errors from tsv files',
Expand Down
53 changes: 51 additions & 2 deletions bids-validator/src/schema/applyRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,14 @@
if (value === 'n/a') {
return true
}

if ('anyOf' in schemaObject) {
return schemaObject.anyOf.some((x) => schemaObjectTypeCheck(x, value, schema))
}
if ('enum' in schemaObject && schemaObject.enum) {
return schemaObject.enum.some((x) => x === value)
}
// @ts-expect-error

const format = schemaObject.format
// @ts-expect-error
? schema.objects.formats[schemaObject.format]
Expand Down Expand Up @@ -415,7 +416,8 @@
for (const [key, requirement] of Object.entries(rule.fields)) {
const severity = getFieldSeverity(requirement, context)
// @ts-expect-error
const keyName = schema.objects.metadata[key].name
const metadataDef = schema.objects.metadata[key]
const keyName: string = metadataDef.name
if (severity && severity !== 'ignore' && !(keyName in context.sidecar)) {
if (requirement.issue?.code && requirement.issue?.message) {
context.issues.add({
Expand All @@ -440,6 +442,53 @@
])
}
}

/* Regardless of if key is required/recommended/optional, we do no
* further valdiation if it is not present in sidecar.
*/
if (!(keyName in context.sidecar)) {
return
}

let originFileKey = ''
if (keyName in context.sidecarKeyOrigin) {
originFileKey = `${context.sidecarKeyOrigin[keyName]}:${keyName}`
} else {
logger.warning(
`sidecarKeyOrigin map failed to initialize for ${context.file.path} on key ${keyName}. Validation caching not active for this key.`,

Check warning on line 458 in bids-validator/src/schema/applyRules.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/applyRules.ts#L457-L458

Added lines #L457 - L458 were not covered by tests
)
}
rwblair marked this conversation as resolved.
Show resolved Hide resolved

if (context.dataset.sidecarKeyValidated.has(originFileKey)) {
return
}

const validate = context.dataset.ajv.compile(metadataDef)
const result = validate(context.sidecar[keyName])
if (result === false) {
const evidenceBase = `Failed for this file.key: ${originFileKey} Schema path: ${schemaPath}`
if (!validate.errors) {
context.issues.addNonSchemaIssue('JSON_SCHEMA_VALIDATION_ERROR', [
{
...context.file,
evidence: evidenceBase,
},
])
} else {

Check warning on line 477 in bids-validator/src/schema/applyRules.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/applyRules.ts#L471-L477

Added lines #L471 - L477 were not covered by tests
for (let error of validate.errors) {
const message = 'message' in error ? `message: ${error['message']}` : ''
context.issues.addNonSchemaIssue('JSON_SCHEMA_VALIDATION_ERROR', [
{
...context.file,
evidence: `${evidenceBase} ${message}`,
},
])
}
}
}
if (originFileKey) {
context.dataset.sidecarKeyValidated.add(originFileKey)
}
}
}

Expand Down
38 changes: 37 additions & 1 deletion bids-validator/src/schema/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
import { buildAssociations } from './associations.ts'
import { ValidatorOptions } from '../setup/options.ts'
import { logger } from '../utils/logger.ts'
import { Ajv, JSONSchemaType, ValidateFunction } from '../deps/ajv.ts'
import { memoize } from '../utils/memoize.ts'
import { Schema } from '../types/schema.ts'

export class BIDSContextDataset implements ContextDataset {
dataset_description: Record<string, unknown>
Expand All @@ -25,13 +28,22 @@
ignored: any[]
modalities: any[]
subjects?: ContextDatasetSubjects
ajv: Ajv
sidecarKeyValidated: Set<string>

constructor(options?: ValidatorOptions, description = {}) {
constructor(options?: ValidatorOptions, schema?: Schema, description = {}) {
this.dataset_description = description
this.files = []
this.tree = {}
this.ignored = []
this.modalities = []
this.ajv = new Ajv({ strictSchema: false })
// @ts-expect-error
this.ajv.compile = memoize(this.ajv.compile)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't been able to figure out how to make these types mesh. Could store it as another entry in the dataset context/.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe AJV itself should be a private field and compile is just a method on the context?

this.sidecarKeyValidated = new Set<string>()
if (schema) {
this.setCustomAjvFormats(schema)
}
if (options) {
this.options = options
}
Expand All @@ -44,6 +56,27 @@
this.dataset_description.DatasetType = 'raw'
}
}

setCustomAjvFormats(schema: Schema): void {
if (typeof schema.objects.formats !== 'object') {
// logger.warning(
console.log(
`schema.objects.formats missing from schema, format validation disabled.`,

Check warning on line 64 in bids-validator/src/schema/context.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/context.ts#L62-L64

Added lines #L62 - L64 were not covered by tests
)
return
}

Check warning on line 67 in bids-validator/src/schema/context.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/context.ts#L66-L67

Added lines #L66 - L67 were not covered by tests
const schemaFormats = schema.objects.formats
for (let key of Object.keys(schemaFormats)) {
if (typeof schemaFormats[key]['pattern'] !== 'string') {
// logger.warning(
console.log(
`schema.objects.formats.${key} pattern missing or invalid. Skipping this format for addition to context json validator`,

Check warning on line 73 in bids-validator/src/schema/context.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/context.ts#L71-L73

Added lines #L71 - L73 were not covered by tests
)
continue
}

Check warning on line 76 in bids-validator/src/schema/context.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/schema/context.ts#L75-L76

Added lines #L75 - L76 were not covered by tests
this.ajv.addFormat(key, schemaFormats[key]['pattern'])
}
}
}

export class BIDSContextDatasetSubjects implements ContextDatasetSubjects {
Expand Down Expand Up @@ -78,6 +111,7 @@
datatype: string
modality: string
sidecar: Record<string, any>
sidecarKeyOrigin: Record<string, string>
json: object
columns: ColumnsMap
associations: ContextAssociations
Expand All @@ -102,6 +136,7 @@
this.datatype = ''
this.modality = ''
this.sidecar = {}
this.sidecarKeyOrigin = {}
this.columns = new ColumnsMap()
this.json = {}
this.associations = {} as ContextAssociations
Expand Down Expand Up @@ -166,6 +201,7 @@
.then((text) => JSON.parse(text))
.catch((error) => {})
this.sidecar = { ...this.sidecar, ...json }
Object.keys(json).map((x) => this.sidecarKeyOrigin[x] = validSidecars[0].path)
}
const nextDir = fileTree.directories.find((directory) => {
return this.file.path.startsWith(`${directory.path}/`)
Expand Down
4 changes: 4 additions & 0 deletions bids-validator/src/types/context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ValidatorOptions } from '../setup/options.ts'
import { Ajv } from '../deps/ajv.ts'

export interface ContextDatasetSubjects {
sub_dirs: string[]
Expand All @@ -13,6 +14,8 @@ export interface ContextDataset {
modalities: any[]
subjects?: ContextDatasetSubjects
options?: ValidatorOptions
ajv: Ajv
sidecarKeyValidated: Set<string>
}
export interface ContextSubjectSessions {
ses_dirs: string[]
Expand Down Expand Up @@ -98,6 +101,7 @@ export interface Context {
extension: string
modality: string
sidecar: Record<string, any>
sidecarKeyOrigin: Record<string, string>
associations: ContextAssociations
columns: object
json: object
Expand Down
5 changes: 5 additions & 0 deletions bids-validator/src/types/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export interface GenericRule {
format?: string
required?: string
index_columns?: string[]
metadata?: Record<string, string>
}

export interface SchemaFields {
Expand All @@ -81,7 +82,11 @@ export interface SchemaFields {

interface SchemaType {
type: string
format?: string
enum?: string[]
items?: SchemaType[]
minItems?: number
maxItems?: number
}

interface AnyOf {
Expand Down
3 changes: 3 additions & 0 deletions bids-validator/src/utils/objectPathHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
export const objectPathHandler = {
get(target: unknown, property: string) {
let res = target
if (typeof property === 'symbol') {
return res
}

Check warning on line 14 in bids-validator/src/utils/objectPathHandler.ts

View check run for this annotation

Codecov / codecov/patch

bids-validator/src/utils/objectPathHandler.ts#L13-L14

Added lines #L13 - L14 were not covered by tests
for (const prop of property.split('.')) {
if (hasProp(res, prop)) {
res = res[prop]
Expand Down
4 changes: 2 additions & 2 deletions bids-validator/src/validators/bids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export async function validate(
if (ddFile) {
const description = await ddFile.text().then((text) => JSON.parse(text))
summary.dataProcessed = description.DatasetType === 'derivative'
dsContext = new BIDSContextDataset(options, description)
dsContext = new BIDSContextDataset(options, schema, description)
} else {
dsContext = new BIDSContextDataset(options)
dsContext = new BIDSContextDataset(options, schema)
issues.addNonSchemaIssue('MISSING_DATASET_DESCRIPTION', [] as IssueFile[])
}

Expand Down
2 changes: 1 addition & 1 deletion bids-validator/tests/data/bids-examples
Submodule bids-examples updated 66 files
+3 −3 qmri_irt1/IRT1.json
+1 −1 qmri_irt1/sub-01/anat/sub-01_inv-01_IRT1.json
+1 −1 qmri_irt1/sub-01/anat/sub-01_inv-02_IRT1.json
+1 −1 qmri_irt1/sub-01/anat/sub-01_inv-03_IRT1.json
+1 −1 qmri_irt1/sub-01/anat/sub-01_inv-04_IRT1.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-01_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-02_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-03_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-04_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-05_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-06_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-07_MEGRE.json
+1 −1 qmri_megre/sub-01/anat/sub-01_echo-08_MEGRE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-01_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-02_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-03_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-04_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-05_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-06_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-07_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-08_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-09_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-10_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-11_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-12_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-13_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-14_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-15_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-16_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-17_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-18_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-19_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-20_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-21_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-22_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-23_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-24_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-25_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-26_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-27_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-28_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-29_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-30_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-31_MESE.json
+1 −1 qmri_mese/sub-01/anat/sub-01_echo-32_MESE.json
+1 −1 qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_M0map.json
+1 −1 qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_MTRmap.json
+1 −1 qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_MTsat.json
+1 −1 qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_T1map.json
+2 −2 qmri_mtsat/sub-01/anat/sub-01_flip-1_mt-off_MTS.json
+2 −2 qmri_mtsat/sub-01/anat/sub-01_flip-1_mt-on_MTS.json
+2 −2 qmri_mtsat/sub-01/anat/sub-01_flip-2_mt-off_MTS.json
+1 −1 qmri_mtsat/sub-01/fmap/sub-01_flip-1_TB1DAM.json
+1 −1 qmri_mtsat/sub-01/fmap/sub-01_flip-2_TB1DAM.json
+1 −1 qmri_qsm/sub-01/anat/sub-01_part-mag_T1w.json
+1 −1 qmri_qsm/sub-01/anat/sub-01_part-phase_T1w.json
+2 −3 qmri_tb1tfl/sub-01/fmap/sub-01_acq-anat_TB1TFL.json
+2 −2 qmri_tb1tfl/sub-01/fmap/sub-01_acq-famp_TB1TFL.json
+1 −1 qmri_vfa/VFA.json
+1 −1 qmri_vfa/derivatives/qMRLab/sub-01/anat/sub-01_M0map.json
+1 −1 qmri_vfa/derivatives/qMRLab/sub-01/anat/sub-01_T1map.json
+4 −4 qmri_vfa/derivatives/qMRLab/sub-01/fmap/sub-01_TB1map.json
+2 −2 qmri_vfa/sub-01/anat/sub-01_flip-1_VFA.json
+2 −2 qmri_vfa/sub-01/anat/sub-01_flip-2_VFA.json
+7 −7 qmri_vfa/sub-01/fmap/sub-01_acq-tr1_TB1AFI.json
+7 −7 qmri_vfa/sub-01/fmap/sub-01_acq-tr2_TB1AFI.json
Loading