Skip to content

Commit cb2fb0c

Browse files
effigieseffigies
committed
rf: Move context encapsulation, and only apply once
1 parent 3a1b37a commit cb2fb0c

File tree

4 files changed

+115
-72
lines changed

4 files changed

+115
-72
lines changed

‎src/schema/applyRules.test.ts‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @ts-nocheck
22
import { assert, assertEquals } from '@std/assert'
3-
import { applyRules, evalCheck, evalConstructor } from './applyRules.ts'
3+
import { applyRules, evalCheck } from './applyRules.ts'
44
import { DatasetIssues } from '../issues/datasetIssues.ts'
55
import { expressionFunctions } from './expressionLanguage.ts'
66

@@ -107,48 +107,3 @@ Deno.test(
107107
assertEquals(context.dataset.issues.get({ code: 'CHECK_ERROR' }).length, 1)
108108
},
109109
)
110-
111-
Deno.test('evalConstructor test', async (t) => {
112-
const match = expressionFunctions.match
113-
await t.step('check veridical reconstruction of match expressions', () => {
114-
// match() functions frequently contain escapes in regex patterns
115-
// We receive these from the schema as written in YAML, so we need to ensure
116-
// that they are correctly reconstructed in the evalConstructor() function
117-
//
118-
// The goal is to avoid schema authors needing to double-escape their regex patterns
119-
// and other implementations to account for the double-escaping
120-
let pattern = String.raw`^\.nii(\.gz)?$`
121-
122-
// Check both a literal and a variable pattern produce the same result
123-
for (const check of ['match(extension, pattern)', `match(extension, '${pattern}')`]) {
124-
const niftiCheck = evalConstructor(check)
125-
for (
126-
const [extension, expected] of [
127-
['.nii', true],
128-
['.nii.gz', true],
129-
['.tsv', false],
130-
[',nii,gz', false], // Check that . is not treated as a wildcard
131-
]
132-
) {
133-
assert(match(extension, pattern) === expected)
134-
// Pass in a context object to provide any needed variables
135-
assert(niftiCheck({ match, extension, pattern }) === expected)
136-
}
137-
}
138-
139-
pattern = String.raw`\S`
140-
for (const check of ['match(json.Name, pattern)', `match(json.Name, '${pattern}')`]) {
141-
const nonEmptyCheck = evalConstructor(check)
142-
for (
143-
const [Name, expected] of [
144-
['test', true],
145-
['', false],
146-
[' ', false],
147-
]
148-
) {
149-
assert(match(Name, pattern) === expected)
150-
assert(nonEmptyCheck({ match, json: { Name }, pattern }) === expected)
151-
}
152-
}
153-
})
154-
})

‎src/schema/applyRules.ts‎

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import type { GenericRule, GenericSchema, SchemaFields, SchemaTypeLike } from '../types/schema.ts'
22
import type { Severity } from '../types/issues.ts'
33
import type { BIDSContext } from './context.ts'
4-
import { expressionFunctions } from './expressionLanguage.ts'
4+
import { contextFunction, prepareContext } from './expressionLanguage.ts'
55
import { logger } from '../utils/logger.ts'
6-
import { memoize } from '../utils/memoize.ts'
76
import { compile } from '../validators/json.ts'
87
import type { DefinedError } from '@ajv'
98
import {
@@ -29,6 +28,35 @@ export function applyRules(
2928
context: BIDSContext,
3029
rootSchema?: GenericSchema,
3130
schemaPath?: string,
31+
): Promise<void> {
32+
_applyRules(schema, prepareContext(context), rootSchema, schemaPath)
33+
return Promise.resolve()
34+
}
35+
36+
/** Evaluate a single expression in a BIDSContext.
37+
*/
38+
export function evalCheck(src: string, context: BIDSContext) {
39+
return _evalCheck(src, prepareContext(context))
40+
}
41+
42+
/**
43+
* Classic rules interpreted like selectors. Examples in specification:
44+
* schema/rules/checks/*
45+
*/
46+
export function evalRuleChecks(
47+
rule: GenericRule,
48+
context: BIDSContext,
49+
schema: GenericSchema,
50+
schemaPath: string,
51+
): boolean {
52+
return _evalRuleChecks(rule, prepareContext(context), schema, schemaPath)
53+
}
54+
55+
function _applyRules(
56+
schema: GenericSchema,
57+
context: BIDSContext,
58+
rootSchema?: GenericSchema,
59+
schemaPath?: string,
3260
) {
3361
if (!rootSchema) {
3462
rootSchema = schema
@@ -45,9 +73,6 @@ export function applyRules(
4573
schemaPath = ''
4674
}
4775
}
48-
Object.assign(context, expressionFunctions)
49-
// @ts-expect-error
50-
context.exists.bind(context)
5176
for (const key in schema) {
5277
if (!(schema[key].constructor === Object)) {
5378
continue
@@ -60,29 +85,20 @@ export function applyRules(
6085
`${schemaPath}.${key}`,
6186
)
6287
} else if (schema[key].constructor === Object) {
63-
applyRules(
88+
_applyRules(
6489
schema[key] as GenericSchema,
6590
context,
6691
rootSchema,
6792
`${schemaPath}.${key}`,
6893
)
6994
}
7095
}
71-
return Promise.resolve()
7296
}
7397

74-
export const evalConstructor = (src: string): Function =>
75-
new Function('context', `with (context) { return ${src.replace(/\\/g, '\\\\')} }`)
76-
const safeHas = () => true
77-
const safeGet = (target: any, prop: any) => prop === Symbol.unscopables ? undefined : target[prop]
78-
79-
const memoizedEvalConstructor = memoize(evalConstructor)
80-
81-
export function evalCheck(src: string, context: BIDSContext) {
82-
const test = memoizedEvalConstructor(src)
83-
const safeContext = new Proxy(context, { has: safeHas, get: safeGet })
98+
function _evalCheck(src: string, context: BIDSContext) {
99+
const test = contextFunction(src)
84100
try {
85-
return test(safeContext)
101+
return test(context)
86102
} catch (error) {
87103
logger.debug(error)
88104
return null
@@ -103,7 +119,7 @@ const evalMap: Record<
103119
schemaPath: string,
104120
) => boolean | void
105121
> = {
106-
checks: evalRuleChecks,
122+
checks: _evalRuleChecks,
107123
// @ts-expect-error
108124
columns: evalColumns,
109125
// @ts-expect-error
@@ -139,14 +155,10 @@ function evalRule(
139155
}
140156

141157
function mapEvalCheck(statements: string[], context: BIDSContext): boolean {
142-
return statements.every((x) => evalCheck(x, context))
158+
return statements.every((x) => _evalCheck(x, context))
143159
}
144160

145-
/**
146-
* Classic rules interpreted like selectors. Examples in specification:
147-
* schema/rules/checks/*
148-
*/
149-
export function evalRuleChecks(
161+
function _evalRuleChecks(
150162
rule: GenericRule,
151163
context: BIDSContext,
152164
schema: GenericSchema,

‎src/schema/expressionLanguage.test.ts‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { assert } from '@std/assert'
2-
import { expressionFunctions } from './expressionLanguage.ts'
2+
import { contextFunction, expressionFunctions } from './expressionLanguage.ts'
33
import { dataFile, rootFileTree } from './fixtures.test.ts'
44
import { BIDSContext } from './context.ts'
55
import type { DatasetIssues } from '../issues/datasetIssues.ts'
@@ -235,3 +235,50 @@ Deno.test('test expression functions', async (t) => {
235235
assert(allequal([1, 2, 1, 2], [1, 2, 1]) === false)
236236
})
237237
})
238+
239+
Deno.test('contextFunction test', async (t) => {
240+
const match = expressionFunctions.match
241+
await t.step('check veridical reconstruction of match expressions', () => {
242+
// match() functions frequently contain escapes in regex patterns
243+
// We receive these from the schema as written in YAML, so we need to ensure
244+
// that they are correctly reconstructed in the contextFunction() function
245+
//
246+
// The goal is to avoid schema authors needing to double-escape their regex patterns
247+
// and other implementations to account for the double-escaping
248+
let pattern = String.raw`^\.nii(\.gz)?$`
249+
250+
// Check both a literal and a variable pattern produce the same result
251+
for (const check of ['match(extension, pattern)', `match(extension, '${pattern}')`]) {
252+
const niftiCheck = contextFunction(check)
253+
for (
254+
const { extension, expected } of [
255+
{ extension: '.nii', expected: true },
256+
{ extension: '.nii.gz', expected: true },
257+
{ extension: '.tsv', expected: false },
258+
{ extension: ',nii,gz', expected: false }, // Check that . is not treated as a wildcard
259+
]
260+
) {
261+
assert(match(extension, pattern) === expected)
262+
// Pass in a context object to provide any needed variables
263+
assert(niftiCheck({ match, extension, pattern } as unknown as BIDSContext) === expected)
264+
}
265+
}
266+
267+
pattern = String.raw`\S`
268+
for (const check of ['match(json.Name, pattern)', `match(json.Name, '${pattern}')`]) {
269+
const nonEmptyCheck = contextFunction(check)
270+
for (
271+
const { Name, expected } of [
272+
{ Name: 'test', expected: true },
273+
{ Name: '', expected: false },
274+
{ Name: ' ', expected: false },
275+
]
276+
) {
277+
assert(match(Name, pattern) === expected)
278+
assert(
279+
nonEmptyCheck({ match, json: { Name }, pattern } as unknown as BIDSContext) === expected,
280+
)
281+
}
282+
}
283+
})
284+
})

‎src/schema/expressionLanguage.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { BIDSContext } from './context.ts'
2+
import { memoize } from '../utils/memoize.ts'
23

34
function exists(this: BIDSContext, list: string[], rule: string = 'dataset'): number {
45
if (list == null) {
@@ -124,3 +125,31 @@ export const expressionFunctions = {
124125
return (a != null && b != null) && a.length === b.length && a.every((v, i) => v === b[i])
125126
},
126127
}
128+
129+
/**
130+
* Generate a function that evaluates an expression using the BIDS context.
131+
*/
132+
function _contextFunction(expr: string): (context: BIDSContext) => any {
133+
return new Function('context', `with (context) { return ${expr.replace(/\\/g, '\\\\')} }`) as (
134+
context: BIDSContext,
135+
) => any
136+
}
137+
138+
/**
139+
* Generate a function that evaluates an expression using the BIDS context.
140+
*/
141+
export const contextFunction = memoize(_contextFunction)
142+
143+
function safeContext(context: BIDSContext): BIDSContext {
144+
return new Proxy(context, {
145+
has: () => true,
146+
get: (target: any, prop: any) => prop === Symbol.unscopables ? undefined : target[prop],
147+
})
148+
}
149+
150+
export function prepareContext(context: BIDSContext): BIDSContext {
151+
Object.assign(context, expressionFunctions)
152+
// @ts-expect-error
153+
context.exists.bind(context)
154+
return safeContext(context)
155+
}

0 commit comments

Comments
 (0)