Merge pull request #1434 from apiaryio/update-gavel-7

Updates Gavel to version 7
apiaryio · Jul 16, 2019 · cd9538f · cd9538f
2 parents 243f376 + d5c595a
commit cd9538f
Show file tree

Hide file tree

Showing 9 changed files with 133 additions and 159 deletions.
diff --git a/lib/TransactionRunner.js b/lib/TransactionRunner.js
@@ -505,13 +505,6 @@ Not performing HTTP request for '${transaction.name}'.\
       }
       transaction.real = real;
 
-      if (!transaction.real.body && transaction.expected.body) {
-        // Leaving body as undefined skips its validation completely. In case
-        // there is no real body, but there is one expected, the empty string
-        // ensures Gavel does the validation.
-        transaction.real.body = '';
-      }
-
       logger.debug('Running \'beforeEachValidation\' hooks');
       this.runHooksForData(hooks && hooks.beforeEachValidationHooks, transaction, () => {
         if (this.hookHandlerError) { return callback(this.hookHandlerError); }
@@ -526,99 +519,107 @@ Not performing HTTP request for '${transaction.name}'.\
     });
   }
 
+
+  // TODO Rewrite this entire method.
+  // Motivations:
+  // 1. Mutations at place.
+  // 2. Constant shadowing and reusage of "validationOutput" object where it could be avoided.
+  // 3. Ambiguity between internal "results" and legacy "gavelResult[name].results".
+  // 4. Mapping with for/of that affects prototype properties.
   validateTransaction(test, transaction, callback) {
     logger.debug('Validating HTTP transaction by Gavel.js');
+    let gavelResult = { fields: {} };
 
-    gavel.validate(transaction.real, transaction.expected, 'response', (validateError, gavelResult) => {
-      if (validateError) {
-        logger.debug('Gavel.js validation errored:', validateError);
-        this.emitError(validateError, test);
-      }
+    try {
+      gavelResult = gavel.validate(transaction.expected, transaction.real);
+    } catch (validationError) {
+      logger.debug('Gavel.js validation errored:', validationError);
+      this.emitError(validationError, test);
+    }
 
-      test.title = transaction.id;
-      test.actual = transaction.real;
-      test.expected = transaction.expected;
-      test.request = transaction.request;
+    test.title = transaction.id;
+    test.actual = transaction.real;
+    test.expected = transaction.expected;
+    test.request = transaction.request;
 
-      const isValid = gavelResult ? gavelResult.isValid : false;
+    // TODO
+    // Gavel result MUST NOT be undefined. Check transaction runner tests
+    // to find where and why it is.
+    const { valid: isValid } = gavelResult;
 
-      if (isValid) {
-        test.status = 'pass';
-      } else {
-        test.status = 'fail';
-      }
+    if (isValid) {
+      test.status = 'pass';
+    } else {
+      test.status = 'fail';
+    }
 
-      // Warn about empty responses
-      // Expected is as string, actual is as integer :facepalm:
-      const isExpectedResponseStatusCodeEmpty = ['204', '205'].includes(
-        test.expected.statusCode ? test.expected.statusCode.toString() : undefined
-      );
-      const isActualResponseStatusCodeEmpty = ['204', '205'].includes(
-        test.actual.statusCode ? test.actual.statusCode.toString() : undefined
-      );
-      const hasBody = (test.expected.body || test.actual.body);
-      if ((isExpectedResponseStatusCodeEmpty || isActualResponseStatusCodeEmpty) && hasBody) {
-        logger.warn(`\
+    // Warn about empty responses
+    // Expected is as string, actual is as integer :facepalm:
+    const isExpectedResponseStatusCodeEmpty = ['204', '205'].includes(
+      test.expected.statusCode ? test.expected.statusCode.toString() : undefined
+    );
+    const isActualResponseStatusCodeEmpty = ['204', '205'].includes(
+      test.actual.statusCode ? test.actual.statusCode.toString() : undefined
+    );
+    const hasBody = (test.expected.body || test.actual.body);
+    if ((isExpectedResponseStatusCodeEmpty || isActualResponseStatusCodeEmpty) && hasBody) {
+      logger.warn(`\
 ${test.title} HTTP 204 and 205 responses must not \
 include a message body: https://tools.ietf.org/html/rfc7231#section-6.3\
 `);
-      }
+    }
 
-      // Create test message from messages of all validation errors
-      let message = '';
-      const object = gavelResult || {};
-      let validatorOutput;
+    // Create test message from messages of all validation errors
+    let message = '';
 
-      // Order-sensitive list of validation sections to output in the log
-      const resultSections = ['headers', 'body', 'statusCode']
-        .filter(sectionName => Object.prototype.hasOwnProperty.call(object, sectionName));
+    // Order-sensitive list of Gavel validation fields to output in the log
+    // Note that Dredd asserts EXACTLY this order. Make sure to adjust tests upon change.
+    const loggedFields = ['headers', 'body', 'statusCode']
+      .filter(fieldName => Object.prototype.hasOwnProperty.call(gavelResult.fields, fieldName));
 
-      resultSections.forEach((sectionName) => {
-        validatorOutput = object[sectionName];
-        (validatorOutput.results || []).forEach((gavelError) => {
-          message += `${sectionName}: ${gavelError.message}\n`;
-        });
+    loggedFields.forEach((fieldName) => {
+      const fieldResult = gavelResult.fields[fieldName];
+      (fieldResult.errors || []).forEach((gavelError) => {
+        message += `${fieldName}: ${gavelError.message}\n`;
       });
+    });
 
-      test.message = message;
-
-      // Record raw validation output to transaction results object
-      //
-      // It looks like the transaction object can already contain 'results'.
-      // (Needs to be prooved, the assumption is based just on previous
-      // version of the code.) In that case, we want to save the new validation
-      // output, but we want to keep at least the original array of Gavel errors.
-      const results = transaction.results || {};
-      for (const sectionName of Object.keys(gavelResult || {})) {
-        // Section names are 'statusCode', 'headers', 'body' (and 'version', which is irrelevant)
-        const rawValidatorOutput = gavelResult[sectionName];
-        if (sectionName !== 'version') {
-          if (!results[sectionName]) { results[sectionName] = {}; }
-
-          // We don't want to modify the object and we want to get rid of some
-          // custom Gavel.js types ('clone' will keep just plain JS objects).
-          validatorOutput = clone(rawValidatorOutput);
-
-          // If transaction already has the 'results' object, ...
-          if (results[sectionName].results) {
-            // ...then take all Gavel errors it contains and add them to the array
-            // of Gavel errors in the new validator output object...
-            validatorOutput.results = validatorOutput.results.concat(results[sectionName].results);
-          }
-          // ...and replace the original validator object with the new one.
-          results[sectionName] = validatorOutput;
-        }
+    test.message = message;
+
+    // Record raw validation output to transaction results object
+    //
+    // It looks like the transaction object can already contain 'results'.
+    // (Needs to be prooved, the assumption is based just on previous
+    // version of the code.) In that case, we want to save the new validation
+    // output, but we want to keep at least the original array of Gavel errors.
+    const results = transaction.results || {};
+    for (const sectionName of Object.keys(gavelResult.fields)) {
+      const rawValidatorOutput = gavelResult.fields[sectionName];
+      if (!results[sectionName]) { results[sectionName] = {}; }
+
+      // We don't want to modify the object and we want to get rid of some
+      // custom Gavel.js types ('clone' will keep just plain JS objects).
+      const validatorOutput = clone(rawValidatorOutput);
+
+      // If transaction already has the 'results' object, ...
+      if (results[sectionName].results) {
+        // ...then take all Gavel errors it contains and add them to the array
+        // of Gavel errors in the new validator output object...
+        validatorOutput.results = validatorOutput.errors.concat(results[sectionName].results);
       }
-      transaction.results = results;
+      // ...and replace the original validator object with the new one.
+      results[sectionName] = validatorOutput;
+    }
+    transaction.results = results;
 
-      // Set the validation results and the boolean verdict to the test object
-      test.results = transaction.results;
-      test.valid = isValid;
+    // Set the validation results and the boolean verdict to the test object
+    test.results = transaction.results;
+    test.valid = isValid;
 
-      // Propagate test object so 'after' hooks can modify it
-      transaction.test = test;
-      callback();
-    });
+    // Propagate test object so 'after' hooks can modify it
+    transaction.test = test;
+
+    callback();
   }
 
   emitEnd(callback) {

diff --git a/package.json b/package.json
@@ -44,7 +44,7 @@
     "clone": "2.1.2",
     "cross-spawn": "6.0.5",
     "dredd-transactions": "9.0.4",
-    "gavel": "5.0.0",
+    "gavel": "7.0.1",
     "glob": "7.1.4",
     "html": "1.0.0",
     "htmlencode": "0.0.4",

diff --git a/test/fixtures/sanitation/plain-text-response-body.js b/test/fixtures/sanitation/plain-text-response-body.js
@@ -3,15 +3,16 @@ const hooks = require('hooks');
 const tokenPattern = /([0-9]|[a-f]){24,}/g;
 
 hooks.after('Resource > Update Resource', (transaction, done) => {
-  let body;
+  const replaceToken = body => body.replace(tokenPattern, '--- CENSORED ---');
 
-  body = transaction.test.actual.body;
-  transaction.test.actual.body = body.replace(tokenPattern, '--- CENSORED ---');
+  // Remove sensitive data from Dredd transaction
+  transaction.test.actual.body = replaceToken(transaction.test.actual.body);
+  transaction.test.expected.body = replaceToken(transaction.test.expected.body);
 
-  body = transaction.test.expected.body;
-  transaction.test.expected.body = body.replace(tokenPattern, '--- CENSORED ---');
+  // Remove sensitive data from the Gavel validation result
+  const bodyResult = transaction.test.results.body;
+  bodyResult.values.expected = replaceToken(bodyResult.values.expected);
+  bodyResult.values.actual = replaceToken(bodyResult.values.actual);
 
-  // Sanitation of diff in the patch format
-  delete transaction.test.results.body.results.rawData;
   done();
 });
diff --git a/test/fixtures/sanitation/response-body-attribute.js b/test/fixtures/sanitation/response-body-attribute.js
@@ -1,40 +1,27 @@
 const hooks = require('hooks');
 
-hooks.after('Resource > Update Resource', (transaction, done) => {
-  // Sanitation of the attribute in body
-  let body;
+const unfold = (jsonString, transform) => JSON.stringify(transform(JSON.parse(jsonString)));
 
-  body = JSON.parse(transaction.test.actual.body);
-  delete body.token;
-  transaction.test.actual.body = JSON.stringify(body);
+hooks.after('Resource > Update Resource', (transaction, done) => {
+  const deleteToken = (obj) => {
+    delete obj.token;
+    return obj;
+  };
 
-  body = JSON.parse(transaction.test.expected.body);
-  delete body.token;
-  transaction.test.expected.body = JSON.stringify(body);
+  // Removes sensitive data from the Dredd transaction
+  transaction.test.actual.body = unfold(transaction.test.actual.body, deleteToken);
+  transaction.test.expected.body = unfold(transaction.test.expected.body, deleteToken);
 
   // Sanitation of the attribute in JSON Schema
   const bodySchema = JSON.parse(transaction.test.expected.bodySchema);
   delete bodySchema.properties.token;
   transaction.test.expected.bodySchema = JSON.stringify(bodySchema);
 
-  // Sanitation of the attribute in validation output
-  const validationOutput = transaction.test.results.body;
-
-  const errors = [];
-  for (let i = 0; i < validationOutput.results.length; i++) {
-    if (validationOutput.results[i].pointer !== '/token') {
-      errors.push(validationOutput.results[i]);
-    }
-  }
-  validationOutput.results = errors;
-
-  const rawData = [];
-  for (let i = 0; i < validationOutput.rawData.length; i++) {
-    if (validationOutput.rawData[i].property[0] !== 'token') {
-      rawData.push(validationOutput.rawData[i]);
-    }
-  }
-  validationOutput.rawData = rawData;
+  // Removes sensitive data from the Gavel validation result
+  const bodyResult = transaction.test.results.body;
+  bodyResult.errors = bodyResult.errors.filter(error => error.location.pointer !== '/token');
+  bodyResult.values.expected = unfold(bodyResult.values.expected, deleteToken);
+  bodyResult.values.actual = unfold(bodyResult.values.actual, deleteToken);
 
   transaction.test.message = '';
   done();

diff --git a/test/fixtures/sanitation/response-headers.js b/test/fixtures/sanitation/response-headers.js
@@ -2,37 +2,20 @@ const caseless = require('caseless');
 const hooks = require('hooks');
 
 hooks.after('Resource > Update Resource', (transaction, done) => {
-  let headers;
-  let name;
+  const deleteSensitiveHeader = (headerName, headers) => {
+    const name = caseless(headers).has(headerName);
+    delete headers[name];
+    return headers;
+  };
 
-  headers = transaction.test.actual.headers;
-  name = caseless(headers).has('authorization');
-  delete headers[name];
-  transaction.test.actual.headers = headers;
+  // Remove sensitive data from the Dredd transaction
+  transaction.test.actual.headers = deleteSensitiveHeader('authorization', transaction.test.actual.headers);
+  transaction.test.expected.headers = deleteSensitiveHeader('authorization', transaction.test.expected.headers);
 
-  headers = transaction.test.expected.headers;
-  name = caseless(headers).has('authorization');
-  delete headers[name];
-  transaction.test.expected.headers = headers;
-
-  // Sanitation of the header in validation output
-  const validationOutput = transaction.test.results.headers;
-
-  const errors = [];
-  for (let i = 0; i < validationOutput.results.length; i++) {
-    if (validationOutput.results[i].pointer.toLowerCase() !== '/authorization') {
-      errors.push(validationOutput.results[i]);
-    }
-  }
-  validationOutput.results = errors;
-
-  const rawData = [];
-  for (let i = 0; i < validationOutput.rawData.length; i++) {
-    if (validationOutput.rawData[i].property[0].toLowerCase() !== 'authorization') {
-      rawData.push(validationOutput.rawData[i]);
-    }
-  }
-  validationOutput.rawData = rawData;
+  // Remove sensitive data from the Gavel validation result
+  const headersResult = transaction.test.results.headers;
+  headersResult.errors = headersResult.errors.filter(error => error.location.pointer.toLowerCase() !== '/authorization');
+  headersResult.values.expected = deleteSensitiveHeader('authorization', headersResult.values.expected);
 
   transaction.test.message = '';
   done();

diff --git a/test/integration/apiary-reporter-test.js b/test/integration/apiary-reporter-test.js
@@ -146,9 +146,9 @@ describe('Apiary reporter', () => {
       assert.nestedProperty(receivedRequest, 'resultData.request');
       assert.nestedProperty(receivedRequest, 'resultData.realResponse');
       assert.nestedProperty(receivedRequest, 'resultData.expectedResponse');
-      assert.nestedProperty(receivedRequest, 'resultData.result.body.validator');
-      assert.nestedProperty(receivedRequest, 'resultData.result.headers.validator');
-      assert.nestedProperty(receivedRequest, 'resultData.result.statusCode.validator');
+      assert.nestedProperty(receivedRequest, 'resultData.result.body.kind');
+      assert.nestedProperty(receivedRequest, 'resultData.result.headers.kind');
+      assert.nestedProperty(receivedRequest, 'resultData.result.statusCode.kind');
 
       it('prints out an error message', () => assert.notEqual(exitStatus, 0));
     });
@@ -269,9 +269,9 @@ describe('Apiary reporter', () => {
         assert.nestedProperty(receivedRequest, 'resultData.request');
         assert.nestedProperty(receivedRequest, 'resultData.realResponse');
         assert.nestedProperty(receivedRequest, 'resultData.expectedResponse');
-        assert.nestedProperty(receivedRequest, 'resultData.result.body.validator');
-        assert.nestedProperty(receivedRequest, 'resultData.result.headers.validator');
-        assert.nestedProperty(receivedRequest, 'resultData.result.statusCode.validator');
+        assert.nestedProperty(receivedRequest, 'resultData.result.body.kind');
+        assert.nestedProperty(receivedRequest, 'resultData.result.headers.kind');
+        assert.nestedProperty(receivedRequest, 'resultData.result.statusCode.kind');
       });
     });
   });

diff --git a/test/integration/cli/reporters-cli-test.js b/test/integration/cli/reporters-cli-test.js
@@ -105,9 +105,9 @@ describe('CLI - Reporters', () => {
         assert.nestedProperty(stepRequest.body, 'resultData.request');
         assert.nestedProperty(stepRequest.body, 'resultData.realResponse');
         assert.nestedProperty(stepRequest.body, 'resultData.expectedResponse');
-        assert.nestedProperty(stepRequest.body, 'resultData.result.body.validator');
-        assert.nestedProperty(stepRequest.body, 'resultData.result.headers.validator');
-        assert.nestedProperty(stepRequest.body, 'resultData.result.statusCode.validator');
+        assert.nestedProperty(stepRequest.body, 'resultData.result.body.kind');
+        assert.nestedProperty(stepRequest.body, 'resultData.result.headers.kind');
+        assert.nestedProperty(stepRequest.body, 'resultData.result.statusCode.kind');
       });
     });
 

diff --git a/test/integration/response-test.js b/test/integration/response-test.js
@@ -233,7 +233,7 @@ const Dredd = require('../../lib/Dredd');
       /HTTP 204 and 205 responses must not include a message body/g
     ).length, 4));
     it('prints four failures for each non-matching status code', () => assert.equal(runtimeInfo.dredd.logging.match(
-      /fail: statusCode: Status code is '200' instead of/g
+      /fail: statusCode: Expected status code '\d+', but got '200'./g
     ).length, 4));
     it('does not print any failures regarding response bodies', () => assert.isNull(runtimeInfo.dredd.logging.match(/fail: body:/g)));
   });
@@ -262,10 +262,10 @@ const Dredd = require('../../lib/Dredd');
       /HTTP 204 and 205 responses must not include a message body/g
     ).length, 2));
     it('prints two failures for each non-matching body (and status code)', () => assert.equal(runtimeInfo.dredd.logging.match(
-      /fail: body: Real and expected data does not match.\nstatusCode: Status code is '200' instead of/g
+      /fail: body: Actual and expected data do not match.\nstatusCode: Expected status code '\d+', but got '200'./g
     ).length, 2));
     it('prints two failures for each non-matching status code', () => assert.equal(runtimeInfo.dredd.logging.match(
-      /fail: statusCode: Status code is '200' instead of/g
+      /fail: statusCode: Expected status code '\d+', but got '200'./g
     ).length, 2));
   });
 }));