swagger-tools - npm Package Compare versions

Comparing version 0.1.3 to 0.1.4

index.js

@@ -177,7 +177,2 @@ /*
 var validateModels = function validateModels (spec, resource) {
-  var modelIds = _.map(resource.models || {}, function (model) {
-    return model.id;
-  });
-  var modelRefs = {};
-  var primitives = _.union(spec.primitives, ['array', 'void', 'File']);
   var addModelRef = function (modelId, modelRef) {
@@ -191,2 +186,88 @@ if (Object.keys(modelRefs).indexOf(modelId) === -1) {
   var errors = [];
+  var identifyModelInheritanceIssues = function (modelDeps) {
+    var circular = {};
+    var composed = {};
+    var resolved = {};
+    var unresolved = {};
+    var addModelProps = function (parentModel, modelName) {
+      var model = models[modelName];
+
+      if (model && model.properties && _.isObject(model.properties)) {
+        _.each(model.properties, function (prop, propName) {
+          if (composed[propName]) {
+            errors.push({
+              code: 'CHILD_MODEL_REDECLARES_PROPERTY',
+              message: 'Child model declares property already declared by ancestor: ' + propName,
+              data: prop,
+              path: '$.models[\'' + parentModel + '\'].properties[\'' + propName + '\']'
+            });
+          } else {
+            composed[propName] = propName;
+          }
+        });
+      }
+    };
+    var getPath = function (parent, unresolved) {
+      var parentVisited = false;
+
+      return Object.keys(unresolved).filter(function (dep) {
+        if (dep === parent) {
+          parentVisited = true;
+        }
+        return parentVisited && unresolved[dep];
+      });
+    };
+    var resolver = function (id, deps, circular, resolved, unresolved) {
+      var model = models[id];
+      var modelDeps = deps[id];
+
+      unresolved[id] = true;
+
+      if (modelDeps) {
+        if (modelDeps.length > 1) {
+          errors.push({
+            code: 'MULTIPLE_MODEL_INHERITANCE',
+            message: 'Child model is sub type of multiple models: ' + modelDeps.join(' && '),
+            data: model,
+            path: '$.models[\'' + id + '\']'
+          });
+        }
+
+        modelDeps.forEach(function (dep) {
+          if (!resolved[dep]) {
+            if (unresolved[dep]) {
+              circular[id] = getPath(dep, unresolved);
+
+              errors.push({
+                code: 'CYCLICAL_MODEL_INHERITANCE',
+                message: 'Model has a circular inheritance: ' + id + ' -> ' + circular[id].join(' -> '),
+                data: model.subTypes || [],
+                path: '$.models[\'' + id + '\'].subTypes'
+              });
+              return;
+            }
+
+            addModelProps(id, dep);
+
+            resolver(dep, deps, circular, resolved, unresolved);
+          }
+        });
+      }
+
+      resolved[id] = true;
+      unresolved[id] = false;
+    };
+
+    Object.keys(modelDeps).forEach(function (modelName) {
+      composed = {};
+      addModelProps(modelName, modelName);
+      resolver(modelName, modelDeps, circular, resolved, unresolved);
+    });
+  };
+  var modelDeps = {};
+  var modelIds = [];
+  var modelProps = {};
+  var modelRefs = {};
+  var models = resource.models || {};
+  var primitives = _.union(spec.primitives, ['array', 'void', 'File']);
   var warnings = [];
@@ -238,6 +319,43 @@
     // Find references defined in the models themselves (Validation happens elsewhere but we have to be smart)
-    if (resource.models && _.isObject(resource.models)) {
-      _.each(resource.models, function (model, name) {
+    if (models && _.isObject(models)) {
+      _.each(models, function (model, name) {
         var modelPath = '$.models[\'' + name + '\']'; // Always use bracket notation just to be safe
+        var modelId = model.id;
+        var seenSubTypes = [];
 
+        // Keep track of model children and properties and duplicate models
+        if (modelIds.indexOf(modelId) > -1) {
+          errors.push({
+            code: 'DUPLICATE_MODEL_DEFINITION',
+            message: 'Model already defined: ' + modelId,
+            data: modelId,
+            path: '$.models[\'' + name + '\'].id'
+          });
+        } else {
+          modelIds.push(modelId);
+
+          modelProps[name] = Object.keys(model.properties || {});
+
+          (model.subTypes || []).forEach(function (subType, index) {
+            var deps = modelDeps[subType];
+
+            if (deps) {
+              if (seenSubTypes.indexOf(subType) > -1) {
+                warnings.push({
+                  code: 'DUPLICATE_MODEL_SUBTYPE_DEFINITION',
+                  message: 'Model already has subType defined: ' + subType,
+                  data: subType,
+                  path: '$.models[\'' + name + '\'].subTypes[' + index + ']'
+                });
+              } else {
+                modelDeps[subType].push(name);
+              }
+            } else {
+              modelDeps[subType] = [name];
+            }
+
+            seenSubTypes.push(subType);
+          });
+        }
+
         // References in model properties
@@ -270,3 +388,3 @@ if (model.properties && _.isObject(model.properties)) {
 
-  // Handle missing models
+  // Identify missing models (referenced but not declared)
   _.difference(Object.keys(modelRefs), modelIds).forEach(function (missing) {
@@ -283,3 +401,3 @@ modelRefs[missing].forEach(function (modelRef) {
 
-  // Handle unused models
+  // Identify unused models (declared but not referenced)
   _.difference(modelIds, Object.keys(modelRefs)).forEach(function (unused) {
@@ -294,5 +412,8 @@ warnings.push({
 
-  // TODO: Validate subTypes are not cyclical
-  // TODO: Validate subTypes do not override parent properties
-  // TODO: Validate subTypes do not include discriminiator
+  // Identify cyclical model dependencies
+  // Identify model multiple inheritance
+  // Identify model duplicate subType entries
+  // Identify model redeclares property of ancestor
+  identifyModelInheritanceIssues(modelDeps);
+
   // TODO: Validate discriminitor property exists
@@ -299,0 +420,0 @@ // TODO: Validate required properties exist

package.json

 {
   "name": "swagger-tools",
-  "version": "0.1.3",
+  "version": "0.1.4",
   "description": "Various tools for using and integrating with Swagger.",
@@ -5,0 +5,0 @@ "main": "index.js",

test/test-v1_2.js

@@ -42,2 +42,4 @@ /* global describe, it */
 var allSampleFiles = {};
+var invalidModelRefsJson = require('./v1_2-invalid-model-refs.json');
+var invalidModelsJson = require('./v1_2-invalid-models.json');
 
@@ -185,5 +187,4 @@ // Load the sample files from disk
 
-    it('should return errors for missing model references in apiDeclaration/resource files', function () {
-      var json = require('./v1_2-invalid-models.json');
-      var result = spec.validate(json);
+    it('should return errors for missing model references in apiDeclaration files', function () {
+      var result = spec.validate(invalidModelRefsJson);
       var expectedMissingModelRefs = {
@@ -209,5 +210,4 @@ 'MissingParamRef': '$.apis[0].operations[0].parameters[0].type',
 
-    it('should return warnings for unused models in apiDeclaration/resource files', function () {
-      var json = require('./v1_2-invalid-models.json');
-      var result = spec.validate(json);
+    it('should return warnings for unused models in apiDeclaration files', function () {
+      var result = spec.validate(invalidModelRefsJson);
 
@@ -223,3 +223,104 @@ assert.equal(1, result.warnings.length);
     });
+
+    it('should return errors for duplicate model ids in apiDeclaration files', function () {
+      var errors = [];
+
+      spec.validate(invalidModelsJson).errors.forEach(function (error) {
+        if (error.code === 'DUPLICATE_MODEL_DEFINITION') {
+          errors.push(error);
+        }
+      });
+
+      assert.deepEqual(errors, [
+        {
+          code: 'DUPLICATE_MODEL_DEFINITION',
+          message: 'Model already defined: A',
+          data: 'A',
+          path: '$.models[\'J\'].id'
+        }
+      ]);
+    });
+
+    it('should return errors for cyclical model subTypes in apiDeclaration files', function () {
+      var errors = [];
+
+      spec.validate(invalidModelsJson).errors.forEach(function (error) {
+        if (error.code === 'CYCLICAL_MODEL_INHERITANCE') {
+          errors.push(error);
+        }
+      });
+
+      assert.deepEqual(errors, [
+        {
+          code: 'CYCLICAL_MODEL_INHERITANCE',
+          message: 'Model has a circular inheritance: C -> A -> D -> C',
+          data: ['D'],
+          path: '$.models[\'C\'].subTypes'
+        },
+        {
+          code: 'CYCLICAL_MODEL_INHERITANCE',
+          message: 'Model has a circular inheritance: H -> I -> H',
+          data: ['I', 'I'],
+          path: '$.models[\'H\'].subTypes'
+        }
+      ]);
+    });
+
+    it('should return errors for model multiple inheritance in apiDeclaration files', function () {
+      var errors = [];
+
+      spec.validate(invalidModelsJson).errors.forEach(function (error) {
+        if (error.code === 'MULTIPLE_MODEL_INHERITANCE') {
+          errors.push(error);
+        }
+      });
+
+      assert.deepEqual(errors, [
+        {
+          code: 'MULTIPLE_MODEL_INHERITANCE',
+          message: 'Child model is sub type of multiple models: A && E',
+          data: invalidModelsJson.models.B,
+          path: '$.models[\'B\']'
+        }
+      ]);
+    });
+
+    it('should return errors for model subTypes redeclaring ancestor properties apiDeclaration files', function () {
+      var errors = [];
+
+      spec.validate(invalidModelsJson).errors.forEach(function (error) {
+        if (error.code === 'CHILD_MODEL_REDECLARES_PROPERTY') {
+          errors.push(error);
+        }
+      });
+
+      assert.deepEqual(errors, [
+        {
+          code: 'CHILD_MODEL_REDECLARES_PROPERTY',
+          message: 'Child model declares property already declared by ancestor: fId',
+          data: invalidModelsJson.models.G.properties.fId,
+          path: '$.models[\'G\'].properties[\'fId\']'
+        }
+      ]);
+    });
+
+    it('should return warning for model subTypes with duplicate entries apiDeclaration files', function () {
+      var warnings = [];
+
+      spec.validate(invalidModelsJson).warnings.forEach(function (warning) {
+        if (warning.code === 'DUPLICATE_MODEL_SUBTYPE_DEFINITION') {
+          warnings.push(warning);
+        }
+      });
+
+      assert.deepEqual(warnings, [
+        {
+          code: 'DUPLICATE_MODEL_SUBTYPE_DEFINITION',
+          message: 'Model already has subType defined: I',
+          data: 'I',
+          path: '$.models[\'H\'].subTypes[1]'
+        }
+      ]);
+    });
   });
 });

test/v1_2-invalid-models.json

@@ -18,45 +18,10 @@ {
               "required": true,
-              "type": "MissingParamRef"
-            },
-            {
-              "allowMultiple": true,
-              "description": "Some fake stuff",
-              "name": "fake",
-              "paramType": "query",
-              "required": false,
-              "type": "array",
-              "items": {
-                "$ref": "MissingParamItemsRef"
-              }
+              "type": "string"
             }
           ],
-          "responseMessages": [
-            {
-              "code": 400,
-              "message": "Missing name",
-              "responseModel": "MissingResponseMessageRef"
-            }
-          ],
-          "summary": "Find pet by ID",
-          "type": "MissingTypeRef"
+          "summary": "Create a greeting",
+          "type": "string"
         }
       ],
       "path": "/greeting/{name}"
-    },
-    {
-      "operations": [
-        {
-          "authorizations": {},
-          "method": "GET",
-          "nickname": "getGreetings",
-          "notes": "Returns all greetings",
-          "parameters": [],
-          "summary": "Find pet by ID",
-          "type": "array",
-          "items": {
-            "$ref": "MissingTypeItemsRef"
-          }
-        }
-      ],
-      "path": "/greetings/"
     }
@@ -66,40 +31,138 @@ ],
   "models": {
-    "Animal": {
-      "id": "Animal",
-      "name": "Animal",
+    "A": {
+      "id": "A",
+      "name": "A",
       "required": [
-        "id",
-        "type"
+        "aId"
       ],
       "properties": {
-        "id": {
-          "type": "long"
-        },
-        "type": {
+        "aId": {
           "type": "string"
-        },
-        "breeds": {
-          "type": "array",
-          "items": {
-            "$ref": "MissingPropertyItemsRef"
-          }
         }
       },
-      "subTypes": ["Cat", "MissingSubTypeRef"],
-      "discriminator": "type"
+      "subTypes": ["B", "C"],
+      "discriminator": "aId"
     },
-    "Cat": {
-      "id": "Cat",
-      "name": "Cat",
+    "B": {
+      "id": "B",
+      "name": "B",
       "required": [
-        "likesMilk"
+        "bId"
       ],
       "properties": {
-        "likesMilk": {
-          "type": "boolean"
+        "bId": {
+          "type": "string"
+        }
+      }
+    },
+    "C": {
+      "id": "C",
+      "name": "C",
+      "required": [
+        "cId"
+      ],
+      "properties": {
+        "cId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["D"],
+      "discriminator": "cId"
+    },
+    "D": {
+      "id": "D",
+      "name": "D",
+      "required": [
+        "dId"
+      ],
+      "properties": {
+        "dId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["A"],
+      "discriminator": "dId"
+    },
+    "E": {
+      "id": "E",
+      "name": "E",
+      "required": [
+        "eId"
+      ],
+      "properties": {
+        "eId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["B"],
+      "discriminator": "eId"
+    },
+    "F": {
+      "id": "F",
+      "name": "F",
+      "required": [
+        "fId"
+      ],
+      "properties": {
+        "fId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["G"],
+      "discriminator": "fId"
+    },
+    "G": {
+      "id": "G",
+      "name": "G",
+      "required": [
+        "gId"
+      ],
+      "properties": {
+        "gId": {
+          "type": "string"
         },
-        "address": {
-          "$ref": "MissingPropertyRef"
+        "fId": {
+          "type": "string"
         }
       }
+    },
+    "H": {
+      "id": "H",
+      "name": "H",
+      "required": [
+        "hId"
+      ],
+      "properties": {
+        "hId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["I", "I"],
+      "discriminator": "hId"
+    },
+    "I": {
+      "id": "I",
+      "name": "I",
+      "required": [
+        "iId"
+      ],
+      "properties": {
+        "iId": {
+          "type": "string"
+        }
+      },
+      "subTypes": ["F", "H"],
+      "discriminator": "iId"
+    },
+    "J": {
+      "id": "A",
+      "name": "A",
+      "required": [
+        "jId"
+      ],
+      "properties": {
+        "jId": {
+          "type": "string"
+        }
+      }
     }
@@ -106,0 +169,0 @@ },

No alert changes

Improved metrics

Total package byte prevSize

90948

increased by12.41%

Number of package files

26

increased by4%

Lines of code

2518

increased by16.68%

No dependency changes