Merge pull request #1351 from apiaryio/honzajavorek/upgrade-dt

Upgrade Dredd Transactions and refactor how annotations are processed

lib/Dredd.js

@@ -3,13 +3,13 @@ const parse = require('dredd-transactions/parse');
 const compile = require('dredd-transactions/compile');
 
 const configureReporters = require('./configureReporters');
-const handleRuntimeProblems = require('./handleRuntimeProblems');
 const resolveLocations = require('./resolveLocations');
 const readLocation = require('./readLocation');
 const resolveModule = require('./resolveModule');
 const logger = require('./logger');
 const TransactionRunner = require('./TransactionRunner');
 const { applyConfiguration } = require('./configuration');
+const annotationToLoggerInfo = require('./annotationToLoggerInfo');
 
 
 function prefixError(error, prefix) {
@@ -89,6 +89,14 @@ function toTransactions(apiDescriptions) {
 }
 
 
+function toLoggerInfos(apiDescriptions) {
+  return apiDescriptions
+    .map(apiDescription => apiDescription.annotations
+      .map(annotation => annotationToLoggerInfo(apiDescription.location, annotation)))
+    .reduce((flatAnnotations, annotations) => flatAnnotations.concat(annotations), []);
+}
+
+
 class Dredd {
   constructor(config) {
     this.configuration = applyConfiguration(config);
@@ -165,13 +173,21 @@ class Dredd {
     this.logger.debug('Preparing API description documents');
     this.prepareAPIdescriptions((error, apiDescriptions) => {
       if (error) { callback(error, this.stats); return; }
-      this.configuration.apiDescriptions = apiDescriptions;
 
-      // TODO https://github.com/apiaryio/dredd/issues/1191
-      const annotationsError = handleRuntimeProblems(apiDescriptions, this.logger);
-      if (annotationsError) { callback(annotationsError, this.stats); return; }
+      const loggerInfos = toLoggerInfos(apiDescriptions);
+      // FIXME: Winston 3.x supports calling .log() directly with the loggerInfo
+      // object as it's sole argument, but that's not the case with Winston 2.x
+      // Once we upgrade Winston, the line below can be simplified to .log(loggerInfo)
+      //
+      // Watch https://github.com/apiaryio/dredd/issues/1225 for updates
+      loggerInfos.forEach(({ level, message }) => this.logger.log(level, message));
+      if (loggerInfos.find(loggerInfo => loggerInfo.level === 'error')) {
+        callback(new Error('API description processing error'), this.stats);
+        return;
+      }
 
       this.logger.debug('Starting the transaction runner');
+      this.configuration.apiDescriptions = apiDescriptions;
       this.transactionRunner.config(this.configuration);
       const transactions = toTransactions(apiDescriptions);
       this.transactionRunner.run(transactions, (runError) => {

lib/annotationToLoggerInfo.js

@@ -0,0 +1,96 @@
+const compileTransactionName = require('./compileTransactionName');
+
+
+/**
+ * Turns annotation type into a log level
+ */
+function typeToLogLevel(annotationType) {
+  const level = { error: 'error', warning: 'warn' }[annotationType];
+  if (!level) {
+    throw new Error(`Invalid annotation type: '${annotationType}'`);
+  }
+  return level;
+}
+
+
+/**
+ * Takes a component identifier and turns it into something user can understand
+ *
+ * @param {string} component
+ */
+function formatComponent(component) {
+  switch (component) {
+    case 'apiDescriptionParser':
+      return 'API description parser';
+    case 'parametersValidation':
+      return 'API description URI parameters validation';
+    case 'uriTemplateExpansion':
+      return 'API description URI template expansion';
+    default:
+      return 'API description';
+  }
+}
+
+
+/**
+ * Formats given location data as something user can understand
+ *
+ * @param {string} apiDescriptionLocation API description location name
+ * @param {array} annotationLocation See 'dredd-transactions' docs
+ */
+function formatLocation(apiDescriptionLocation, annotationLocation) {
+  if (!annotationLocation) {
+    return apiDescriptionLocation;
+  }
+
+  const [[startLine, startColumn], [endLine, endColumn]] = annotationLocation;
+  const editorLink = `${apiDescriptionLocation}:${startLine}`;
+  const from = `line ${startLine} column ${startColumn}`;
+
+  if (startLine === endLine && startColumn === endColumn) {
+    return `${editorLink} (${from})`;
+  }
+
+  const to = startLine === endLine
+    ? `column ${endColumn}`
+    : `line ${endLine} column ${endColumn}`;
+  return `${editorLink} (from ${from} to ${to})`;
+}
+
+
+/**
+ * @typedef {Object} LoggerInfo A plain object winston.log() accepts as input
+ * @property {string} level
+ * @property {string} message
+ */
+
+/**
+ * Takes API description parser or compiler annotation returned from
+ * the 'dredd-transactions' library and transforms it into a message
+ * Dredd can show to the user. Returns an object logger accepts as input.
+ *
+ * @param {string} apiDescriptionLocation API description location name
+ * @param {Object} annotation the annotation object from Dredd Transactions
+ * @return {LoggerInfo}
+ */
+module.exports = function annotationToLoggerInfo(apiDescriptionLocation, annotation) {
+  const level = typeToLogLevel(annotation.type);
+
+  if (annotation.component === 'apiDescriptionParser') {
+    const message = (
+      `${formatComponent(annotation.component)} ${annotation.type}`
+      + ` in ${formatLocation(apiDescriptionLocation, annotation.location)}:`
+      + ` ${annotation.message}`
+    );
+    return { level, message };
+  }
+
+  // See https://github.com/apiaryio/dredd-transactions/issues/275 why this
+  // is handled in a different way than parser annotations
+  const message = (
+    `${formatComponent(annotation.component)} ${annotation.type}`
+    + ` in ${apiDescriptionLocation} (${compileTransactionName(annotation.origin)}):`
+    + ` ${annotation.message}`
+  );
+  return { level, message };
+};

lib/compileTransactionName.js

@@ -0,0 +1,14 @@
+// This file is copy-pasted "as is" from the Dredd Transactions library, where
+// it's also tested. This is a temporary solution,
+// see https://github.com/apiaryio/dredd-transactions/issues/276
+
+
+module.exports = function compileTransactionName(origin) {
+  const segments = [];
+  if (origin.apiName) { segments.push(origin.apiName); }
+  if (origin.resourceGroupName) { segments.push(origin.resourceGroupName); }
+  if (origin.resourceName) { segments.push(origin.resourceName); }
+  if (origin.actionName) { segments.push(origin.actionName); }
+  if (origin.exampleName) { segments.push(origin.exampleName); }
+  return segments.join(' > ');
+};

package.json

@@ -40,7 +40,7 @@
     "chai": "4.2.0",
     "clone": "2.1.2",
     "cross-spawn": "6.0.5",
-    "dredd-transactions": "7.0.0",
+    "dredd-transactions": "8.1.3",
     "gavel": "3.0.1",
     "glob": "7.1.4",
     "html": "1.0.0",

test/integration/annotations-test.js

@@ -14,7 +14,7 @@ function compileTransactions(apiDescription, logger, callback) {
 
 describe('Parser and compiler annotations', () => {
   describe('when processing a file with parser warnings', () => {
-    const logger = { debug: sinon.spy(), warn: sinon.spy() };
+    const logger = { debug: sinon.spy(), log: sinon.spy() };
     let error;
 
     before((done) => {
@@ -32,16 +32,19 @@ FORMAT: 1A
     it("doesn't abort Dredd", () => {
       assert.isUndefined(error);
     });
+    it('logs warnings', () => {
+      assert.equal(logger.log.getCall(0).args[0], 'warn');
+    });
     it('logs the warnings with line numbers', () => {
       assert.match(
-        logger.warn.getCall(0).args[0],
-        /^parser warning in 'configuration\.apiDescriptions\[0\]': [\s\S]+ on line 5$/i
+        logger.log.getCall(0).args[1],
+        /parser warning in configuration\.apiDescriptions\[0\]:5 \(from line 5 column 3 to column 11\)/i
       );
     });
   });
 
   describe('when processing a file with parser errors', () => {
-    const logger = { debug: sinon.spy(), error: sinon.spy() };
+    const logger = { debug: sinon.spy(), log: sinon.spy() };
     let error;
 
     before((done) => {
@@ -60,16 +63,19 @@ FORMAT: 1A
     it('aborts Dredd', () => {
       assert.instanceOf(error, Error);
     });
+    it('logs errors', () => {
+      assert.equal(logger.log.getCall(0).args[0], 'error');
+    });
     it('logs the errors with line numbers', () => {
       assert.match(
-        logger.error.getCall(0).args[0],
-        /^parser error in 'configuration\.apiDescriptions\[0\]': [\s\S]+ on line 6$/i
+        logger.log.getCall(0).args[1],
+        /parser error in configuration\.apiDescriptions\[0\]:6 \(line 6 column 1\)/i
       );
     });
   });
 
   describe('when processing a file with compilation warnings', () => {
-    const logger = { debug: sinon.spy(), warn: sinon.spy() };
+    const logger = { debug: sinon.spy(), log: sinon.spy() };
     let error;
 
     before((done) => {
@@ -87,16 +93,19 @@ FORMAT: 1A
     it("doesn't abort Dredd", () => {
       assert.isUndefined(error);
     });
+    it('logs warnings', () => {
+      assert.equal(logger.log.getCall(0).args[0], 'warn');
+    });
     it('logs the warnings with a transaction path', () => {
       assert.match(
-        logger.warn.getCall(0).args[0],
-        /^compilation warning in 'configuration\.apiDescriptions\[0\]': [\s\S]+ \(Dummy API > Index > Index\)$/i
+        logger.log.getCall(0).args[1],
+        /uri template expansion warning in configuration\.apiDescriptions\[0\] \(Dummy API > Index > Index\)/i
       );
     });
   });
 
   describe('when processing a file with compilation errors', () => {
-    const logger = { debug: sinon.spy(), error: sinon.spy(), warn: sinon.spy() };
+    const logger = { debug: sinon.spy(), log: sinon.spy() };
     let error;
 
     before((done) => {
@@ -116,10 +125,13 @@ FORMAT: 1A
     it('aborts Dredd', () => {
       assert.instanceOf(error, Error);
     });
+    it('logs errors', () => {
+      assert.equal(logger.log.getCall(0).args[0], 'error');
+    });
     it('logs the errors with a transaction path', () => {
       assert.match(
-        logger.error.getCall(0).args[0],
-        /^compilation error in 'configuration\.apiDescriptions\[0\]': [\s\S]+ \(Dummy API > Index > Index\)$/i
+        logger.log.getCall(0).args[1],
+        /uri parameters validation error in configuration\.apiDescriptions\[0\] \(Dummy API > Index > Index\)/i
       );
     });
   });