Merge pull request #130 from holidayextras/swagger-fixes

Swagger Improvements

Author: Oliver Rumbelow

.travis.yml

@@ -8,5 +8,6 @@ env:
     - TEST_STEP=lint
     - TEST_STEP=flow
     - TEST_STEP=test
+    - TEST_STEP=swagger
 notifications:
   email: false

lib/swagger/paths.js

@@ -1,6 +1,9 @@
 "use strict";
 var swaggerPaths = module.exports = { };
 var jsonApi = require("../../");
+var _ = {
+  uniq: require("lodash.uniq")
+};
 
 
 swaggerPaths.getPathDefinitions = function() {
@@ -40,31 +43,36 @@ swaggerPaths._addBasicPaths = function(paths, resourceName, resourceConfig) {
       handler: "search",
       resourceName: resourceName,
       description: "Search for " + resourceName,
-      parameters: resourceConfig.searchParams
+      parameters: resourceConfig.searchParams,
+      hasPathId: false
     }),
     post: swaggerPaths._getPathOperationObject({
       handler: "create",
       resourceName: resourceName,
       description: "Create a new instance of " + resourceName,
-      parameters: resourceConfig.attributes
+      parameters: resourceConfig.attributes,
+      hasPathId: false
     })
   };
 
   paths["/" + resourceName + "/{id}"] = {
     get: swaggerPaths._getPathOperationObject({
       handler: "find",
       resourceName: resourceName,
-      description: "Get a specific instance of " + resourceName
+      description: "Get a specific instance of " + resourceName,
+      hasPathId: true
     }),
     delete: swaggerPaths._getPathOperationObject({
       handler: "delete",
       resourceName: resourceName,
-      description: "Delete an instance of " + resourceName
+      description: "Delete an instance of " + resourceName,
+      hasPathId: true
     }),
     patch: swaggerPaths._getPathOperationObject({
       handler: "update",
       resourceName: resourceName,
-      description: "Update an instance of " + resourceName
+      description: "Update an instance of " + resourceName,
+      hasPathId: true
     })
   };
 };
@@ -73,7 +81,8 @@ swaggerPaths._addDeepPaths = function(paths, resourceName, resourceConfig, relat
   paths["/" + resourceName + "/{id}/" + relationName] = {
     get: swaggerPaths._getPathOperationObject({
       handler: "find",
-      resourceName: relation
+      resourceName: relation,
+      hasPathId: true
     })
   };
 
@@ -83,25 +92,29 @@ swaggerPaths._addDeepPaths = function(paths, resourceName, resourceConfig, relat
       handler: "find",
       resourceName: relation,
       relationType: relationType,
-      extraTags: resourceName
+      extraTags: resourceName,
+      hasPathId: true
     }),
     post: swaggerPaths._getPathOperationObject({
       handler: "create",
       resourceName: relation,
       relationType: relationType,
-      extraTags: resourceName
+      extraTags: resourceName,
+      hasPathId: true
     }),
     patch: swaggerPaths._getPathOperationObject({
       handler: "update",
       resourceName: relation,
       relationType: relationType,
-      extraTags: resourceName
+      extraTags: resourceName,
+      hasPathId: true
     }),
     delete: swaggerPaths._getPathOperationObject({
       handler: "delete",
       resourceName: relation,
       relationType: relationType,
-      extraTags: resourceName
+      extraTags: resourceName,
+      hasPathId: true
     })
   };
 };
@@ -116,7 +129,7 @@ swaggerPaths._getPathOperationObject = function(options) {
         description: options.resourceName + " " + options.handler + " response",
         schema: {
           type: "object",
-          required: [ "jsonapi", "meta, links" ],
+          required: [ "jsonapi", "meta", "links" ],
           properties: {
             jsonapi: {
               type: "object",
@@ -164,6 +177,7 @@ swaggerPaths._getPathOperationObject = function(options) {
   };
   if (options.extraTags) {
     pathDefinition.tags = pathDefinition.tags.concat(options.extraTags);
+    pathDefinition.tags = _.uniq(pathDefinition.tags);
   }
 
   var responseShortcut = pathDefinition.responses["200"].schema.properties;
@@ -180,7 +194,10 @@ swaggerPaths._getPathOperationObject = function(options) {
   if (((options.handler === "search") || (options.handler === "find")) && !options.relation) {
     pathDefinition.parameters = pathDefinition.parameters.concat(swaggerPaths._optionalJsonApiParameters());
     responseShortcut.included = {
-      type: "array"
+      type: "array",
+      items: {
+        type: "object"
+      }
     };
   }
 
@@ -214,7 +231,7 @@ swaggerPaths._getPathOperationObject = function(options) {
     pathDefinition.responses["200"] = undefined;
   }
 
-  if ((options.handler !== "search") && (options.handler !== "create")) {
+  if (options.hasPathId) {
     pathDefinition.parameters.push({
       name: "id",
       in: "path",
@@ -258,7 +275,8 @@ swaggerPaths._optionalJsonApiParameters = function() {
     { "$ref": "#/parameters/sort" },
     { "$ref": "#/parameters/include" },
     { "$ref": "#/parameters/filter" },
-    { "$ref": "#/parameters/fields" }
+    { "$ref": "#/parameters/fields" },
+    { "$ref": "#/parameters/page" }
   ];
 };
 

lib/swagger/resources.js

@@ -30,12 +30,10 @@ swaggerPaths._getResourceDefinition = function(resourceConfig) {
       },
       "attributes": {
         type: "object",
-        required: [ ],
         properties: { }
       },
       "relationships": {
         type: "object",
-        required: [ ],
         properties: { }
       },
       "links": {
@@ -74,9 +72,10 @@ swaggerPaths._getResourceDefinition = function(resourceConfig) {
       }
       attributeShortcut[attribute] = swaggerScheme;
 
-      // if ((joiScheme._flags || { }).presence === "required") {
-      //   resourceDefinition.properties.attributes.required.push(attribute);
-      // }
+      if ((joiScheme._flags || { }).presence === "required") {
+        resourceDefinition.properties.attributes.required = resourceDefinition.properties.attributes.required || [ ];
+        resourceDefinition.properties.attributes.required.push(attribute);
+      }
     } else {
       if (joiScheme._settings.as) continue;
 

package.json

@@ -45,6 +45,7 @@
     "mocha-performance": "0.1.0",
     "node-inspector": "0.12.5",
     "plato": "1.5.0",
+    "swagger-tools": "^0.10.1",
     "v8-profiler": "5.5.0"
   },
   "scripts": {
@@ -56,7 +57,8 @@
     "performance": "node --allow-natives-syntax --harmony ./node_modules/mocha/bin/_mocha -S --reporter mocha-performance ./test/*.js",
     "lint": "node ./node_modules/eslint/bin/eslint ./example ./lib ./test --quiet && echo '✔ All good!'",
     "jscpd": "jscpd --blame -p ./lib/ || echo 'Finished!'",
-    "flow": "node ./node_modules/flow-bin/cli.js && echo '✔ All good!'"
+    "flow": "node ./node_modules/flow-bin/cli.js && echo '✔ All good!'",
+    "swagger": "node ./node_modules/mocha/bin/mocha -S -R spec ./swaggerValidator.js --timeout 60000"
   },
   "config": {
     "blanket": {

swaggerValidator.js

@@ -0,0 +1,48 @@
+"use strict";
+var jsonApiTestServer = require("./example/server.js");
+var request = require("request");
+var assert = require("assert");
+
+describe("Use a tool to validate the generated swagger document", function() {
+  it("should not contain any errors", function(done) {
+    var validator = require("swagger-tools").specs.v2;
+
+    var uri = "http://localhost:16006/rest/swagger.json";
+    request(uri, function(meh, res, swaggerObject) {
+      swaggerObject = JSON.parse(swaggerObject);
+
+      validator.validate(swaggerObject, function (err, result) {
+        assert.ifError(err);
+
+        if (!result) {
+          console.log("Swagger document is valid");
+          return done();
+        }
+
+        if (result.errors.length > 0) {
+          console.log("The Swagger document is invalid...");
+          console.log("");
+          console.log("Errors");
+          console.log("------");
+          console.log(result.errors);
+          console.log("");
+        }
+
+        if (result.warnings.length > 0) {
+          console.log("Warnings");
+          console.log("--------");
+          console.log(result.warnings);
+        }
+
+        done(new Error("Invalid swagger.json!"));
+      });
+    });
+  });
+
+  before(function() {
+    jsonApiTestServer.start();
+  });
+  after(function() {
+    jsonApiTestServer.close();
+  });
+});

test/get-resource-id.js

@@ -1,6 +1,7 @@
 "use strict";
 var assert = require("assert");
 var helpers = require("./helpers.js");
+var request = require("request");
 var jsonApiTestServer = require("../example/server.js");
 
 
@@ -38,7 +39,7 @@ describe("Testing jsonapi-server", function() {
 
     it("with fields", function(done) {
       var url = "http://localhost:16006/rest/articles/de305d54-75b4-431b-adb2-eb6b9e546014?fields[articles]=title";
-      helpers.request({
+      request({
         method: "GET",
         url: url
       }, function(err, res, json) {

test/get-resource.js

@@ -1,6 +1,7 @@
 "use strict";
 var assert = require("assert");
 var helpers = require("./helpers.js");
+var request = require("request");
 var jsonApiTestServer = require("../example/server.js");
 
 
@@ -365,7 +366,7 @@ describe("Testing jsonapi-server", function() {
 
       it("just title", function(done) {
         var url = "http://localhost:16006/rest/articles?fields[articles]=title";
-        helpers.request({
+        request({
           method: "GET",
           url: url
         }, function(err, res, json) {
@@ -384,7 +385,7 @@ describe("Testing jsonapi-server", function() {
 
       it("title AND content", function(done) {
         var url = "http://localhost:16006/rest/articles?fields[articles]=title,content";
-        helpers.request({
+        request({
           method: "GET",
           url: url
         }, function(err, res, json) {

test/patch-resource-id.js

@@ -166,7 +166,7 @@ describe("Testing jsonapi-server", function() {
           }
         })
       };
-      helpers.request(data, function(err, res, json) {
+      request(data, function(err, res, json) {
         assert.equal(err, null);
         json = helpers.validateJson(json);
 

test/post-resource.js

@@ -36,7 +36,7 @@ describe("Testing jsonapi-server", function() {
           }
         })
       };
-      helpers.request(data, function(err, res, json) {
+      request(data, function(err, res, json) {
         assert.equal(err, null);
         json = helpers.validateError(json);
         assert.equal(json.errors[0].detail.length, 2, "Expecting several validation errors");