From 347d481930011e59374dbe4e4d045a66d51bcde6 Mon Sep 17 00:00:00 2001
From: Jim Kalafut <jim@kalafut.net>
Date: Fri, 4 Jan 2019 11:46:54 -0800
Subject: [PATCH] Add tests for OpenAPI operation ids (#5998)

---
 logical/framework/openapi.go               | 10 +++-
 logical/framework/openapi_test.go          | 68 ++++++++++++++++++++++
 logical/framework/testdata/legacy.json     |  2 +
 logical/framework/testdata/operations.json |  2 +
 logical/framework/testdata/responses.json  |  2 +
 5 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/logical/framework/openapi.go b/logical/framework/openapi.go
index e6d467304..adc4e38a3 100644
--- a/logical/framework/openapi.go
+++ b/logical/framework/openapi.go
@@ -625,8 +625,16 @@ func cleanResponse(resp *logical.Response) (*cleanedResponse, error) {
 // An optional user-provided suffix ("context") may also be appended.
 func (d *OASDocument) CreateOperationIDs(context string) {
 	opIDCount := make(map[string]int)
+	var paths []string
 
-	for path, pi := range d.Paths {
+	// traverse paths in a stable order to ensure stable output
+	for path := range d.Paths {
+		paths = append(paths, path)
+	}
+	sort.Strings(paths)
+
+	for _, path := range paths {
+		pi := d.Paths[path]
 		for _, method := range []string{"get", "post", "delete"} {
 			var oasOperation *OASOperation
 			switch method {
diff --git a/logical/framework/openapi_test.go b/logical/framework/openapi_test.go
index 99f4c299c..444d0d308 100644
--- a/logical/framework/openapi_test.go
+++ b/logical/framework/openapi_test.go
@@ -399,6 +399,60 @@ func TestOpenAPI_Paths(t *testing.T) {
 	})
 }
 
+func TestOpenAPI_OperationID(t *testing.T) {
+	path1 := &Path{
+		Pattern: "foo/" + GenericNameRegex("id"),
+		Fields: map[string]*FieldSchema{
+			"id": {Type: TypeString},
+		},
+		Operations: map[logical.Operation]OperationHandler{
+			logical.ReadOperation:   &PathOperation{},
+			logical.UpdateOperation: &PathOperation{},
+			logical.DeleteOperation: &PathOperation{},
+		},
+	}
+
+	path2 := &Path{
+		Pattern: "Foo/" + GenericNameRegex("id"),
+		Fields: map[string]*FieldSchema{
+			"id": {Type: TypeString},
+		},
+		Operations: map[logical.Operation]OperationHandler{
+			logical.ReadOperation: &PathOperation{},
+		},
+	}
+
+	for _, context := range []string{"", "bar"} {
+		doc := NewOASDocument()
+		documentPath(path1, nil, logical.TypeLogical, doc)
+		documentPath(path2, nil, logical.TypeLogical, doc)
+		doc.CreateOperationIDs(context)
+
+		tests := []struct {
+			path string
+			op   string
+			opID string
+		}{
+			{"/Foo/{id}", "get", "getFooId"},
+			{"/foo/{id}", "get", "getFooId_2"},
+			{"/foo/{id}", "post", "postFooId"},
+			{"/foo/{id}", "delete", "deleteFooId"},
+		}
+
+		for _, test := range tests {
+			actual := getPathOp(doc.Paths[test.path], test.op).OperationID
+			expected := test.opID
+			if context != "" {
+				expected += "_" + context
+			}
+
+			if actual != expected {
+				t.Fatalf("expected %v, got %v", expected, actual)
+			}
+		}
+	}
+}
+
 func TestOpenAPI_CustomDecoder(t *testing.T) {
 	p := &Path{
 		Pattern:      "foo",
@@ -452,6 +506,7 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string)
 
 	doc := NewOASDocument()
 	documentPath(path, sp, logical.TypeLogical, doc)
+	doc.CreateOperationIDs("")
 
 	docJSON, err := json.MarshalIndent(doc, "", "  ")
 	if err != nil {
@@ -474,6 +529,19 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string)
 	}
 }
 
+func getPathOp(pi *OASPathItem, op string) *OASOperation {
+	switch op {
+	case "get":
+		return pi.Get
+	case "post":
+		return pi.Post
+	case "delete":
+		return pi.Delete
+	default:
+		panic("unexpected operation: " + op)
+	}
+}
+
 func expected(name string) string {
 	data, err := ioutil.ReadFile(filepath.Join("testdata", name+".json"))
 	if err != nil {
diff --git a/logical/framework/testdata/legacy.json b/logical/framework/testdata/legacy.json
index e3e5ee2a1..2fcb26882 100644
--- a/logical/framework/testdata/legacy.json
+++ b/logical/framework/testdata/legacy.json
@@ -24,6 +24,7 @@
         }
       ],
       "get": {
+        "operationId": "getLookupId",
         "summary": "Synopsis",
         "tags": ["secrets"],
         "responses": {
@@ -33,6 +34,7 @@
         }
       },
       "post": {
+        "operationId": "postLookupId",
         "summary": "Synopsis",
         "tags": ["secrets"],
         "requestBody": {
diff --git a/logical/framework/testdata/operations.json b/logical/framework/testdata/operations.json
index 95d3185e6..8f1222ab9 100644
--- a/logical/framework/testdata/operations.json
+++ b/logical/framework/testdata/operations.json
@@ -34,6 +34,7 @@
         }
       ],
       "get": {
+        "operationId": "getFooId",
         "tags": ["secrets"],
         "summary": "My Summary",
         "description": "My Description",
@@ -54,6 +55,7 @@
         ]
       },
       "post": {
+        "operationId": "postFooId",
         "tags": ["secrets"],
         "summary": "Update Summary",
         "description": "Update Description",
diff --git a/logical/framework/testdata/responses.json b/logical/framework/testdata/responses.json
index c7d9c9435..bd332054c 100644
--- a/logical/framework/testdata/responses.json
+++ b/logical/framework/testdata/responses.json
@@ -14,6 +14,7 @@
       "description": "Synopsis",
       "x-vault-unauthenticated": true,
       "delete": {
+        "operationId": "deleteFoo",
         "tags": ["secrets"],
         "summary": "Delete stuff",
         "responses": {
@@ -23,6 +24,7 @@
         }
       },
       "get": {
+        "operationId": "getFoo",
         "tags": ["secrets"],
         "summary": "My Summary",
         "description": "My Description",