diff --git a/.gitignore b/.gitignore index b512c09..bd5ce71 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ -node_modules \ No newline at end of file +node_modules +test/tmp \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e065da..f460a45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +[1.3.0] - 2018-06-07 +--------------------- +###### Added +- Appearance validation. + +###### Changed +- Updated Enketo libraries. + [1.2.2] - 2018-05-01 --------------------- ##### Changed diff --git a/README.md b/README.md index d932fed..43638b9 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,7 @@ In it's current iteration, the validator does the following: * It checks whether the XForm is a valid XML document. * It performs some basic ODK XForm structure checks. * It checks if each bind `nodeset` exists in the primary instance. +* It checks if appearance values are supported. * It checks for each `` whether the `relevant`, `constraint`, `calculate`, and `required` expressions are supported and valid\* XPath. \* Note, that `/path/to/nonexisting/node` is perfectly valid XPath. diff --git a/package.json b/package.json index b2be429..e71087a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "enketo-validate", - "version": "1.2.2", + "version": "1.3.0", "description": "An XForm validator around Enketo's form engine", "main": "src/validator.js", "scripts": { @@ -37,4 +37,4 @@ "mocha": "^5.0.1", "pkg": "^4.3.0" } -} +} \ No newline at end of file diff --git a/src/appearances.json b/src/appearances.json new file mode 100644 index 0000000..8322869 --- /dev/null +++ b/src/appearances.json @@ -0,0 +1,120 @@ +{ + "minimal": { + "controls": [ "select", "select1", "group" ] + }, + "label": { + "controls": [ "select", "select1" ] + }, + "list-nolabel": { + "controls": [ "select", "select1" ] + }, + "autocomplete": { + "controls": [ "select1" ] + }, + "search": { + "controls": [ "select1" ], + "preferred": "autocomplete" + }, + "likert": { + "controls": [ "select1" ] + }, + "image-map": { + "controls": [ "select1", "select" ] + }, + "horizontal": { + "controls": [ "select", "select1", "range" ] + }, + "vertical": "horizontal", + "compact": { + "controls": [ "select", "select1", "group" ] + }, + "no-collapse": { + "controls": [ "group" ] + }, + "horizontal-compact": { + "controls": [ "select", "select1" ], + "preferred": "compact" + }, + "compact-1": "compact", + "compact-2": "compact", + "compact-3": "compact", + "compact-4": "compact", + "compact-5": "compact", + "compact-6": "compact", + "compact-7": "compact", + "compact-8": "compact", + "compact-9": "compact", + "compact-10": "compact", + "quick": { + "controls": [ "select", "select1" ] + }, + "quickcompact": { + "controls": [ "select", "select1" ] + }, + "numbers": { + "controls": [ "input" ], + "types": [ "string", "xsd:string" ] + }, + "thousands-sep": { + "controls": [ "input" ], + "types": [ "string", "xsd:string", "int", "xsd:int", "decimal", "xsd:decimal" ] + }, + "multiline": "numbers", + "url": "numbers", + "distress": { + "controls": [ "range", "input" ], + "types": [ "int", "xsd:int" ] + }, + "bearing": { + "controls": [ "input" ], + "types": [ "decimal", "xsd:decimal" ] + }, + "month-year": { + "controls": [ "input" ], + "types": [ "date", "xsd:date" ] + }, + "year": "month-year", + "no-calendar": { + "controls": [ "input" ], + "types": [ "date", "xsd:date", "dateTime", "xsd:dateTime" ] + }, + "ethiopian": "no-calendar", + "coptic": "no-calendar", + "islamic": "no-calendar", + "new": { + "controls": [ "upload" ], + "types": [ "binary" ] + }, + "new-front": "new", + "new-rear": "new", + "annotate": "new", + "draw": "new", + "signature": "new", + "maps": { + "controls": [ "input" ], + "types": [ "geopoint", "geotrace", "geoshape" ] + }, + "hide-input": "maps", + "streets": "maps", + "satellite": "maps", + "terrain": "maps", + "placement-map": "maps", + "field-list": { + "controls": [ "group" ] + }, + "picker": { + "controls": [ "range" ] + }, + "w1": { + "controls": [ "input", "select1", "select", "upload", "trigger", "range", "odk:rank", "group" ] + }, + "w2": "w1", + "w3": "w1", + "w4": "w1", + "w5": "w1", + "w6": "w1", + "w7": "w1", + "w8": "w1", + "w9": "w1", + "w10": "w1" +} \ No newline at end of file diff --git a/src/validator.js b/src/validator.js index f417065..8315f2e 100644 --- a/src/validator.js +++ b/src/validator.js @@ -15,9 +15,14 @@ let validate = ( xformStr, options = {} ) => { if ( xform ) { xform.checkStructure( warnings, errors ); - xform.checkRules( warnings, errors ); + xform.checkBinds( warnings, errors ); + if ( options.openclinica ) { xform.checkOpenClinicaRules( warnings, errors ); + // OpenClinica would like all appearance warnings to be output as errors, for now + xform.checkAppearances( errors, errors ); + } else { + xform.checkAppearances( warnings, errors ); } } diff --git a/src/xform.js b/src/xform.js index 4df41a6..f86831a 100644 --- a/src/xform.js +++ b/src/xform.js @@ -10,6 +10,7 @@ const libxmljs = libxslt.libxmljs; const sheets = require( 'enketo-xslt' ); const xslModelSheet = libxslt.parse( sheets.xslModel ); const addXPathExtensionsOc = require( 'enketo-xpath-extensions-oc' ); +const appearanceRules = require( './appearances' ); class XForm { @@ -33,6 +34,47 @@ class XForm { return this._bindsWithCalc; } + get formControls() { + // TODO: wrong to use h: namespace prefix without resolver here! + // fix in JSDom might be forthcoming: + // * https://github.com/jsdom/jsdom/issues/2159, + // * https://github.com/jsdom/jsdom/issues/2028 + // doc.evaluate does not support namespaces at all (nsResolver is not used) in JSDom, hence this clever not() trick + // to use querySelectorAll instead. + this._formControls = this._formControls || [ ...this.doc.querySelectorAll( 'h\\:body *:not(item):not(label):not(hint):not(value):not(itemset):not(output)' ) ]; + return this._formControls; + } + + get NAMESPACES() { + return { + '': 'http://www.w3.org/2002/xforms', + h: 'http://www.w3.org/1999/xhtml', + oc: 'http://openclinica.org/xforms', + odk: 'http://opendatakit.org/xforms', + enk: 'http://enketo.org/xforms', + orx: 'http://openrosa.org/xforms', + xsd: 'http://www.w3.org/2001/XMLSchema', + }; + } + + bind( nodeset ) { + return this.doc.querySelector( `bind[nodeset="${nodeset}"]` ); + } + + nsPrefixResolver( ns ) { + let prefix = null; + if ( !ns ) { + return prefix; + } + Object.entries( this.NAMESPACES ).some( obj => { + if ( obj[ 1 ] === ns ) { + prefix = obj[ 0 ]; + return true; + } + } ); + return prefix; + } + // The reason this is not included in the constructor is to separate different types of errors, // and keep the constructor just for XML parse errors. parseModel() { @@ -81,15 +123,12 @@ class XForm { } checkStructure( warnings, errors ) { - const htmlNamespace = 'http://www.w3.org/1999/xhtml'; - const xformsNamespace = 'http://www.w3.org/2002/xforms'; - const rootEl = this.doc.documentElement; const rootElNodeName = rootEl.nodeName; if ( !( /^[A-z]+:html$/.test( rootElNodeName ) ) ) { errors.push( 'Root element should be .' ); } - if ( rootEl.namespaceURI !== htmlNamespace ) { + if ( rootEl.namespaceURI !== this.NAMESPACES.h ) { errors.push( 'Root element has incorrect namespace.' ); } @@ -105,13 +144,13 @@ class XForm { if ( !headEl ) { errors.push( 'No head element found as child of .' ); } - if ( headEl && headEl.namespaceURI !== htmlNamespace ) { + if ( headEl && headEl.namespaceURI !== this.NAMESPACES.h ) { errors.push( 'Head element has incorrect namespace.' ); } if ( !bodyEl ) { errors.push( 'No body element found as child of .' ); } - if ( bodyEl && bodyEl.namespaceURI !== htmlNamespace ) { + if ( bodyEl && bodyEl.namespaceURI !== this.NAMESPACES.h ) { errors.push( 'Body element has incorrect namespace.' ); } @@ -126,7 +165,7 @@ class XForm { if ( !modelEl ) { errors.push( 'No model element found as child of .' ); } - if ( modelEl && modelEl.namespaceURI !== xformsNamespace ) { + if ( modelEl && modelEl.namespaceURI !== this.NAMESPACES[ '' ] ) { errors.push( 'Model element has incorrect namespace.' ); } } @@ -142,7 +181,7 @@ class XForm { if ( !primInstanceEl ) { errors.push( 'No primary instance element found as first instance child of .' ); } - if ( primInstanceEl && primInstanceEl.namespaceURI !== xformsNamespace ) { + if ( primInstanceEl && primInstanceEl.namespaceURI !== this.NAMESPACES[ '' ] ) { errors.push( 'Primary instance element has incorrect namespace.' ); } } @@ -171,7 +210,7 @@ class XForm { } } - checkRules( warnings, errors ) { + checkBinds( warnings, errors ) { // Check for use of form controls with calculations that are not readonly this.bindsWithCalc .filter( this._withFormControl.bind( this ) ) @@ -185,8 +224,58 @@ class XForm { .forEach( nodeName => errors.push( `Question "${nodeName}" has a calculation that is not set to readonly.` ) ); } + checkAppearances( warnings, errors ) { + this.formControls + .forEach( control => { + const appearanceVal = control.getAttribute( 'appearance' ); + if ( !appearanceVal || appearanceVal.indexOf( 'ex:' ) === 0 ) { + return; + } + const appearances = appearanceVal.split( ' ' ); + appearances.forEach( appearance => { + let rules = appearanceRules[ appearance ]; + if ( typeof rules === 'string' ) { + rules = appearanceRules[ rules ]; + } + const ref = control.getAttribute( 'ref' ); + if ( !ref ) { + errors.push( 'Question found in body that has no ref attribute' ); + return; + } + const nodeName = ref.substring( ref.lastIndexOf( '/' ) + 1 ); // in model! + const controlNsPrefix = this.nsPrefixResolver( control.namespaceURI ); + const bindEl = this.bind( ref ); + const controlName = controlNsPrefix && /:/.test( control.nodeName ) ? controlNsPrefix + ':' + control.nodeName.split( ':' )[ 1 ] : control.nodeName; + let dataType = bindEl ? bindEl.getAttribute( 'type' ) : 'string'; + // Convert ns prefix to properly evaluate XML Schema datatypes regardless of namespace prefix used in XForm. + const typeValNs = /:/.test( dataType ) ? bindEl.lookupNamespaceURI( dataType.split( ':' )[ 0 ] ) : null; + dataType = typeValNs ? `${this.nsPrefixResolver(typeValNs)}:${dataType.split(':')[1]}` : dataType; + if ( !rules ) { + warnings.push( `Appearance "${appearance}" for question "${nodeName}" is not supported` ); + return; + } + if ( rules.controls && !rules.controls.includes( controlName ) ) { + warnings.push( `Appearance "${appearance}" for question "${nodeName}" is not valid for this question type (${control.nodeName})` ); + return; + } + if ( rules.types && !rules.types.includes( dataType ) ) { + // Only check types if controls check passed. + // TODO check namespaced types when it becomes applicable (for XML Schema types). + warnings.push( `Appearance "${appearance}" for question "${nodeName}" is not valid for this data type (${dataType})` ); + return; + } + if ( rules.preferred ) { + warnings.push( `Appearance "${appearance}" for question "${nodeName}" is deprecated, use "${rules.preferred}" instead` ); + } + // Possibilities for future additions: + // - check accept/mediaType + // - check conflicting combinations of appearances + } ); + + } ); + } + checkOpenClinicaRules( warnings, errors ) { - const OC_NS = 'http://openclinica.org/xforms'; const CLINICALDATA_REF = /instance\(\s*(["'])((?:(?!\1)clinicaldata))\1\s*\)/; // Check for use of external data in instance "clinicaldata" @@ -195,14 +284,14 @@ class XForm { .filter( bind => { // If both are true we have found an error (in an efficient manner) return CLINICALDATA_REF.test( bind.getAttribute( 'calculate' ) ) && - bind.getAttributeNS( OC_NS, 'external' ) !== 'clinicaldata'; + bind.getAttributeNS( this.NAMESPACES.oc, 'external' ) !== 'clinicaldata'; } ) .map( this._nodeNames.bind( this ) ) .forEach( nodeName => errors.push( `Found calculation for "${nodeName}" that refers to ` + 'external clinicaldata without the required "external" attribute in the correct namespace.' ) ); this.bindsWithCalc - .filter( bind => bind.getAttributeNS( OC_NS, 'external' ) === 'clinicaldata' ) + .filter( bind => bind.getAttributeNS( this.NAMESPACES.oc, 'external' ) === 'clinicaldata' ) .filter( bind => { const calculation = bind.getAttribute( 'calculate' ); return !calculation || !CLINICALDATA_REF.test( calculation ); diff --git a/test/spec/xform.spec.js b/test/spec/xform.spec.js index dfff9eb..bdcdf52 100644 --- a/test/spec/xform.spec.js +++ b/test/spec/xform.spec.js @@ -104,4 +104,37 @@ describe( 'XForm', () => { } ); } ); + describe( 'with incorrect appearance usage', () => { + const xf = loadXForm( 'appearances.xml' ); + const result = validator.validate( xf ); + const resultOc = validator.validate( xf, { openclinica: true } ); + const ISSUES = 12; + + it( 'outputs warnings', () => { + expect( result.warnings.length ).to.equal( ISSUES ); + expect( arrContains( result.warnings, /"minimal" for question "b"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"compact-2" for question "b"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"maximal" for question "c"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"hide-input" for question "d"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"search" for question "d" .+ deprecated.+"autocomplete"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"compact" for question "e"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"compact-19" for question "f"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"numbers" for question "g"/i ) ).to.equal( true ); + expect( arrContains( result.warnings, /"field-list" for question "two"/i ) ).to.equal( true ); + } ); + + it( 'outputs no errors', () => { + expect( result.errors.length ).to.equal( 0 ); + } ); + + it( 'outputs no warnings with --oc flag', () => { + expect( resultOc.warnings.length ).to.equal( 0 ); + } ); + + it( 'outputs errors with --oc flag', () => { + expect( resultOc.errors.length ).to.equal( ISSUES ); + } ); + + } ); + } ); \ No newline at end of file diff --git a/test/xform/appearances.xml b/test/xform/appearances.xml new file mode 100644 index 0000000..55e55f9 --- /dev/null +++ b/test/xform/appearances.xml @@ -0,0 +1,91 @@ + + + + appearances + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + AK + + + + + + + + + + AK + + + + + + + + + + + + + + AK + + + + + \ No newline at end of file diff --git a/validate b/validate index d2c78f6..e419266 100755 --- a/validate +++ b/validate @@ -52,7 +52,7 @@ if ( program.me ) { console.log( result.warnings.join( '\n\n' ) ); } if ( result.errors.length ) { - console.error( result.errors.join( '\n\n' ) ); + console.error( '\n' + result.errors.join( '\n\n' ) ); console.error( '\nResult: Invalid' ); process.exit( 1 ); } else {