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 4 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
4 changes: 4 additions & 0 deletions bids-validator/src/deps/ajv.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {Ajv} from 'https://esm.sh/ajv@8.16.0';
import ValidateFunction from 'https://esm.sh/ajv@8.16.0';
import JSONSchemaType from 'https://esm.sh/ajv@8.16.0';
export { ValidateFunction, JSONSchemaType }
rwblair marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions bids-validator/src/issues/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,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
42 changes: 41 additions & 1 deletion bids-validator/src/schema/applyRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ function schemaObjectTypeCheck(
if (value === "n/a") {
return true;
}

if ("anyOf" in schemaObject) {
return schemaObject.anyOf.some((x) =>
schemaObjectTypeCheck(x, value, schema)
Expand All @@ -179,6 +180,7 @@ function schemaObjectTypeCheck(
if ("enum" in schemaObject && schemaObject.enum) {
return schemaObject.enum.some((x) => x === value);
}

// @ts-expect-error
const format = schema.objects.formats[schemaObject.type];
const re = new RegExp(`^${format.pattern}$`);
Expand Down Expand Up @@ -424,7 +426,7 @@ function evalJsonCheck(
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 keyName: string = schema.objects.metadata[key].name;
if (severity && severity !== "ignore" && !(keyName in context.sidecar)) {
if (requirement.issue?.code && requirement.issue?.message) {
context.issues.add({
Expand All @@ -442,6 +444,44 @@ function evalJsonCheck(
]);
}
}
let originFileKey = ''
if (keyName in context.sidecarKeyOrigin) {
originFileKey = `${context.sidecarKeyOrigin[keyName]}:${keyName}`
}
rwblair marked this conversation as resolved.
Show resolved Hide resolved
if (context.dataset.sidecarKeyValidated.has(originFileKey)) {
return
}

if (keyName in context.sidecar && schema.objects.metadata && keyName in schema.objects.metadata) {
rwblair marked this conversation as resolved.
Show resolved Hide resolved
// @ts-expect-error
const validate = context.dataset.ajv.compile(schema.objects.metadata[keyName])
effigies marked this conversation as resolved.
Show resolved Hide resolved
const result = validate(context.sidecar[keyName])
if (result === false) {
const evidenceBase = `Failed for this file.key: ${originFileKey} Schema path: ${schemaPath}`
Copy link
Member Author

Choose a reason for hiding this comment

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

Errors could take schema path as a seperate argument and print the entire rule. https://deno.land/x/json2yaml@v1.0.1

let messages = []
rwblair marked this conversation as resolved.
Show resolved Hide resolved
if (!validate.errors) {
rwblair marked this conversation as resolved.
Show resolved Hide resolved
context.issues.addNonSchemaIssue("JSON_SCHEMA_VALIDATION_ERROR", [
{
...context.file,
evidence: evidenceBase
}
])
} else {
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
40 changes: 39 additions & 1 deletion bids-validator/src/schema/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import { loadHeader } from '../files/nifti.ts'
import { buildAssociations } from './associations.ts'
import { ValidatorOptions } from '../setup/options.ts'
import { logger } from '../utils/logger.ts'
import {Ajv, ValidateFunction, JSONSchemaType} 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 +30,22 @@ export class BIDSContextDataset implements ContextDataset {
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 +58,27 @@ export class BIDSContextDataset implements ContextDataset {
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.`,
)
return
}
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`,
)
continue
}
this.ajv.addFormat(key, schemaFormats[key]['pattern'])
}
}
}

export class BIDSContextDatasetSubjects implements ContextDatasetSubjects {
Expand Down Expand Up @@ -78,6 +113,7 @@ export class BIDSContext implements Context {
datatype: string
modality: string
sidecar: Record<string, any>
sidecarKeyOrigin: Record<string, string>
json: object
columns: ColumnsMap
associations: ContextAssociations
Expand All @@ -102,6 +138,7 @@ export class BIDSContext implements Context {
this.datatype = ''
this.modality = ''
this.sidecar = {}
this.sidecarKeyOrigin = {}
this.columns = new ColumnsMap()
this.json = {}
this.associations = {} as ContextAssociations
Expand Down Expand Up @@ -163,6 +200,7 @@ export class BIDSContext implements Context {
.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 @@ -97,6 +100,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 hasProp = <K extends PropertyKey, T>(
export const objectPathHandler = {
get(target: unknown, property: string) {
let res = target
if (typeof property === "symbol") {
return res
}
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
Loading