Fix OpenAPI 3 validation: operationId must be unique (#504)

Author: vasayxtx

openapi3/paths.go

@@ -82,6 +82,11 @@ func (value Paths) Validate(ctx context.Context) error {
 			return err
 		}
 	}
+
+	if err := value.validateUniqueOperationIDs(); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -114,6 +119,30 @@ func (paths Paths) Find(key string) *PathItem {
 	return nil
 }
 
+func (value Paths) validateUniqueOperationIDs() error {
+	operationIDs := make(map[string]string)
+	for urlPath, pathItem := range value {
+		if pathItem == nil {
+			continue
+		}
+		for httpMethod, operation := range pathItem.Operations() {
+			if operation == nil || operation.OperationID == "" {
+				continue
+			}
+			endpoint := httpMethod + " " + urlPath
+			if endpointDup, ok := operationIDs[operation.OperationID]; ok {
+				if endpoint > endpointDup { // For make error message a bit more deterministic. May be useful for tests.
+					endpoint, endpointDup = endpointDup, endpoint
+				}
+				return fmt.Errorf("operations %q and %q have the same operation id %q",
+					endpoint, endpointDup, operation.OperationID)
+			}
+			operationIDs[operation.OperationID] = endpoint
+		}
+	}
+	return nil
+}
+
 func normalizeTemplatedPath(path string) (string, uint, map[string]struct{}) {
 	if strings.IndexByte(path, '{') < 0 {
 		return path, 0, nil

openapi3/paths_test.go

@@ -7,22 +7,88 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var emptyPathSpec = `
+func TestPathsValidate(t *testing.T) {
+	tests := []struct {
+		name    string
+		spec    string
+		wantErr string
+	}{
+		{
+			name: "ok, empty paths",
+			spec: `
 openapi: "3.0.0"
 info:
   version: 1.0.0
   title: Swagger Petstore
   license:
     name: MIT
-servers:
-  - url: http://petstore.swagger.io/v1
 paths:
   /pets:
-`
+`,
+		},
+		{
+			name: "operation ids are not unique, same path",
+			spec: `
+openapi: "3.0.0"
+info:
+  version: 1.0.0
+  title: Swagger Petstore
+  license:
+    name: MIT
+paths:
+  /pets:
+    post:
+      operationId: createPet
+      responses:
+        201:
+          description: "entity created"
+    delete:
+      operationId: createPet
+      responses:
+        204:
+          description: "entity deleted"
+`,
+			wantErr: `operations "DELETE /pets" and "POST /pets" have the same operation id "createPet"`,
+		},
+		{
+			name: "operation ids are not unique, different paths",
+			spec: `
+openapi: "3.0.0"
+info:
+  version: 1.0.0
+  title: Swagger Petstore
+  license:
+    name: MIT
+paths:
+  /pets:
+    post:
+      operationId: createPet
+      responses:
+        201:
+          description: "entity created"
+  /users:
+    post:
+      operationId: createPet
+      responses:
+        201:
+          description: "entity created"
+`,
+			wantErr: `operations "POST /pets" and "POST /users" have the same operation id "createPet"`,
+		},
+	}
+
+	for i := range tests {
+		tt := tests[i]
+		t.Run(tt.name, func(t *testing.T) {
+			doc, err := NewLoader().LoadFromData([]byte(tt.spec))
+			require.NoError(t, err)
 
-func TestPathValidate(t *testing.T) {
-	doc, err := NewLoader().LoadFromData([]byte(emptyPathSpec))
-	require.NoError(t, err)
-	err = doc.Paths.Validate(context.Background())
-	require.NoError(t, err)
+			err = doc.Paths.Validate(context.Background())
+			if tt.wantErr == "" {
+				require.NoError(t, err)
+				return
+			}
+			require.Equal(t, tt.wantErr, err.Error())
+		})
+	}
 }

openapi3filter/fixtures/petstore.json

@@ -121,7 +121,7 @@
         ],
         "summary": "Add a new pet to the store",
         "description": "",
-        "operationId": "addPet",
+        "operationId": "addPet2",
         "responses": {
           "405": {
             "description": "Invalid input"