Added contract validation #8
Conversation
Change-type: patch
nazrhom
changed the title
lib/validation.js
Outdated
| * @memberof module:validation | ||
| * @public | ||
| * | ||
| * } |
There was a problem hiding this comment.
There is a weird closing bracket here
| * @summary Checks if a contract is valid. | ||
| * @function | ||
| * @memberof module:validation | ||
| * @public |
lib/validation.js
Outdated
| * | ||
| * } | ||
| */ | ||
| exports.checkValidContract = (contract, schema) => { |
There was a problem hiding this comment.
Maybe .checkContract() is better since its shorter and the meaning is preserved?
| * @summary Checks if a contract is valid. | ||
| * @function | ||
| * @memberof module:validation | ||
| * @public |
There was a problem hiding this comment.
Can you define the arguments and return values?
lib/validation.js
Outdated
| if (success) { | ||
| return true | ||
| } | ||
| throw new Error(ajv.errorsText()) |
There was a problem hiding this comment.
I wonder if it'd be better to return an object with a success boolean flag or something like that, and an array of potential errors or something like that, so we don't have to unnecessary enclose this in try/catch for certain use cases.
| tags: [] | ||
| } | ||
|
|
||
| ava.test('should validate base contract', (test) => { |
There was a problem hiding this comment.
Can we create a JSON file in ie test/contracts.js that includes an array of potential contracts and their expected results, and then this file can require that, iterate through the test cases, and generate the ava.test calls over it?
| "type": "string", | ||
| "pattern": "^[a-z0-9-]+$" | ||
| }, | ||
| "version": { |
There was a problem hiding this comment.
We should probably add a regex here, since we control contract versions
There was a problem hiding this comment.
Done, I used a regex which includes both fixed version and some of the ranges we support as atm we are using the same version field to validate both cases.
lib/schema/contract.json
Outdated
| "conflicts": { | ||
| "type": "array", | ||
| "additionalItems": false, | ||
| "items": { |
There was a problem hiding this comment.
This should have the same structure as requires
| ) | ||
| }) | ||
|
|
||
| ava.test('Should reject invald tagged contract', (test) => { |
There was a problem hiding this comment.
Typo in "invald" (same in test below)
| ava.test('should validate require contract', (test) => { | ||
| test.deepEqual( | ||
| { | ||
| success: true, errors: [] |
There was a problem hiding this comment.
Can we put keys in different lines?
| @@ -0,0 +1,111 @@ | |||
| /* | |||
| * Copyright 2017 resin.io | |||
lib/index.js
Outdated
| @@ -24,6 +24,7 @@ | |||
| exports.Contract = require('./contract') | |||
| exports.Blueprint = require('./blueprint') | |||
| exports.buildTemplate = require('./partials').buildTemplate | |||
| exports.validation = require('./validation') | |||
There was a problem hiding this comment.
We should probably just expose a top level .isValid(contract) function or something like that.
Added
checkValidContractfunction.It takes a contract and an optional schema. If no schema is supplied it will validate the contract against the base contract schema. If the schema is supplied it will validate against both schemas. The user supplied schema can reference the base contract schema via
{ $ref: 'contract.json' }The function will throw if the contract is not valid
Fixes #7
Change-type: patch