Added semantic validation for unique operationIds

docs/versions/2.0.md

@@ -13,6 +13,7 @@ Swagger 2.0 as well.
 | Operations cannot have both a `body` parameter and a `formData` parameter | Error |
 | Operations must have only one `body` parameter | Error |
 | Operations must have unique *(`name` + `in` combination)* parameters | Error |
+| Operations must have unique `operationId` | Error |
 | Path parameters declared in the path string need matching parameter definitions *(Either at the path-level or the operation)* | Error |
 | Path parameters definition *(Either at the path-level or the operation)* need matching paramater declarations | Error |
 | Path strings must be *(equivalently)* different *(Example: `/pet/{petId}` and `/pet/{petId2}` are equivalently the same and would generate an error)* | Error |

lib/versions/2.0/validators.js

@@ -399,6 +399,7 @@ function validateSchemaObjects (api) {
  * * Ensure that an operation only has one body parameter
  * * Ensure that an operation has only a body or formData parameter but not both
  * * Ensure that all operation parameters are unique (in + name)
+ * * Ensure that all operation ids are unique
  *
  * @param {SwaggerApi} api - The SwaggerApi object
  *
@@ -427,7 +428,7 @@ function validatePathsAndOperations (api) {
     return seenParameters;
   }
 
-  _.reduce(api.resolved.paths, function (paths, pathDef, path) {
+  _.reduce(api.resolved.paths, function (metadata, pathDef, path) {
     var declaredPathParameters = [];
     var normalizedPath = path;
     var pPath = ['paths', path];
@@ -441,14 +442,14 @@ function validatePathsAndOperations (api) {
     });
 
     // Idenfity paths that are functionally the same
-    if (_.indexOf(paths, normalizedPath) > -1) {
+    if (_.indexOf(metadata.paths, normalizedPath) > -1) {
       response.errors.push({
         code: 'EQUIVALENT_PATH',
         message: 'Equivalent path already exists: ' + path,
         path: pPath
       });
     } else {
-      paths.push(normalizedPath);
+      metadata.paths.push(normalizedPath);
     }
 
     // Identify duplicate path-level parameters (We do this manually since SwaggerApi#getOperation consolidates them)
@@ -459,6 +460,7 @@ function validatePathsAndOperations (api) {
     _.forEach(pathDef, function (operationDef, method) {
       var definedPathParameters = {};
       var oPath = pPath.concat(method);
+      var operationId = operationDef.operationId;
       var pathMetadata;
       var parameters;
 
@@ -467,6 +469,19 @@ function validatePathsAndOperations (api) {
         return;
       }
 
+      // Identify duplicate operationIds
+      if (!_.isUndefined(operationId)) {
+        if (_.indexOf(metadata.operationIds, operationId) !== -1) {
+          response.errors.push({
+            code: 'DUPLICATE_OPERATIONID',
+            message: 'Cannot have multiple operations with the same operationId: ' + operationId,
+            path: oPath.concat(['operationId'])
+          });
+        } else {
+          metadata.operationIds.push(operationId);
+        }
+      }
+
       // Identify duplicate operation-level parameters (We do this manually for the same reasons above)
       _.reduce(operationDef.parameters, function (seenParameters, parameter, index) {
         return validateDuplicateParameter(seenParameters, parameter, oPath.concat(['parameters', index.toString()]));
@@ -475,17 +490,17 @@ function validatePathsAndOperations (api) {
       // Use SwaggerApi#getOperation to avoid having to consolidate parameters
       parameters = api.getOperation(path, method).getParameters();
 
-      pathMetadata = _.reduce(parameters, function (metadata, parameter) {
+      pathMetadata = _.reduce(parameters, function (pMetadata, parameter) {
         // Record path parameters
         if (parameter.in === 'path') {
           definedPathParameters[parameter.name] = parameter.ptr;
         } else if (parameter.in === 'body') {
-          metadata.bodyParameteters += 1;
+          pMetadata.bodyParameteters += 1;
         } else if (parameter.in === 'formData') {
-          metadata.formParameters += 1;
+          pMetadata.formParameters += 1;
         }
 
-        return metadata;
+        return pMetadata;
       }, {bodyParameteters: 0, formParameters: 0});
 
       // Identify multiple body parameters
@@ -525,8 +540,8 @@ function validatePathsAndOperations (api) {
       });
     });
 
-    return paths;
+    return metadata;
-  }, []);
+  }, {paths: [], operationIds: []});
 
   return response;
 }

test/test-2.0.js

@@ -1229,30 +1229,6 @@ describe('swagger-core-api (Swagger 2.0)', function () {
           });
         });
 
-        it('multiple equivalent paths', function (done) {
-          var cSwagger = _.cloneDeep(swaggerDoc);
-
-          cSwagger.paths['/pet/{notPetId}'] = {};
-
-          swaggerApi.create({
-            definition: cSwagger
-          })
-            .then(function (api) {
-              var result = api.validate();
-
-              assert.ok(result === false);
-              assert.deepEqual([], api.getLastWarnings());
-              assert.deepEqual([
-                {
-                  code: 'EQUIVALENT_PATH',
-                  message: 'Equivalent path already exists: /pet/{notPetId}',
-                  path: ['paths', '/pet/{notPetId}']
-                }
-              ], api.getLastErrors());
-            })
-            .then(done, done);
-        });
-
         it('missing path parameter declaration', function (done) {
           var cSwagger = _.cloneDeep(swaggerDoc);
 
@@ -1319,6 +1295,55 @@ describe('swagger-core-api (Swagger 2.0)', function () {
             .then(done, done);
         });
 
+        it('multiple equivalent paths', function (done) {
+          var cSwagger = _.cloneDeep(swaggerDoc);
+
+          cSwagger.paths['/pet/{notPetId}'] = {};
+
+          swaggerApi.create({
+            definition: cSwagger
+          })
+            .then(function (api) {
+              var result = api.validate();
+
+              assert.ok(result === false);
+              assert.deepEqual([], api.getLastWarnings());
+              assert.deepEqual([
+                {
+                  code: 'EQUIVALENT_PATH',
+                  message: 'Equivalent path already exists: /pet/{notPetId}',
+                  path: ['paths', '/pet/{notPetId}']
+                }
+              ], api.getLastErrors());
+            })
+            .then(done, done);
+        });
+
+        it('multiple operations with the same operationId', function (done) {
+          var cSwagger = _.cloneDeep(swaggerDoc);
+          var operationId = cSwagger.paths['/pet'].post.operationId;
+
+          cSwagger.paths['/pet'].put.operationId = operationId;
+
+          swaggerApi.create({
+            definition: cSwagger
+          })
+            .then(function (api) {
+              var result = api.validate();
+
+              assert.ok(result === false);
+              assert.deepEqual([], api.getLastWarnings());
+              assert.deepEqual([
+                {
+                  code: 'DUPLICATE_OPERATIONID',
+                  message: 'Cannot have multiple operations with the same operationId: ' + operationId,
+                  path: ['paths', '/pet', 'put', 'operationId']
+                }
+              ], api.getLastErrors());
+            })
+            .then(done, done);
+        });
+
         it('operation has multiple body parameters', function (done) {
           var cSwagger = _.cloneDeep(swaggerDoc);
           var dBodyParam = _.cloneDeep(cSwagger.paths['/pet'].post.parameters[0]);