Sort OpenAPI operation and path parameters

pkg/genericapiserver/openapi/openapi.go

@@ -183,8 +183,10 @@ func (o *openAPI) buildPaths() error {
 			for _, p := range inPathCommonParamsMap {
 				pathItem.Parameters = append(pathItem.Parameters, p)
 			}
+			sortParameters(pathItem.Parameters)
 			for _, route := range routes {
 				op, err := o.buildOperations(route, inPathCommonParamsMap)
+				sortParameters(op.Parameters)
 				if err != nil {
 					return err
 				}
@@ -287,28 +289,6 @@ func (o *openAPI) buildResponse(model interface{}, description string) (spec.Res
 	}, nil
 }
 
-func groupRoutesByPath(routes []restful.Route) (ret map[string][]restful.Route) {
-	ret = make(map[string][]restful.Route)
-	for _, r := range routes {
-		route, exists := ret[r.Path]
-		if !exists {
-			route = make([]restful.Route, 0, 1)
-		}
-		ret[r.Path] = append(route, r)
-	}
-	return ret
-}
-
-func mapKeyFromParam(param *restful.Parameter) interface{} {
-	return struct {
-		Name string
-		Kind int
-	}{
-		Name: param.Data().Name,
-		Kind: param.Data().Kind,
-	}
-}
-
 func (o *openAPI) findCommonParameters(routes []restful.Route) (map[interface{}]spec.Parameter, error) {
 	commonParamsMap := make(map[interface{}]spec.Parameter, 0)
 	paramOpsCountByName := make(map[interface{}]int, 0)
@@ -412,54 +392,3 @@ func (o *openAPI) buildParameters(restParam []*restful.Parameter) (ret []spec.Pa
 	}
 	return ret, nil
 }
-
-// A simple trie implementation with Add an HasPrefix methods only.
-type trie struct {
-	children map[byte]*trie
-	wordTail bool
-}
-
-func createTrie(list []string) trie {
-	ret := trie{
-		children: make(map[byte]*trie),
-		wordTail: false,
-	}
-	for _, v := range list {
-		ret.Add(v)
-	}
-	return ret
-}
-
-func (t *trie) Add(v string) {
-	root := t
-	for _, b := range []byte(v) {
-		child, exists := root.children[b]
-		if !exists {
-			child = &trie{
-				children: make(map[byte]*trie),
-				wordTail: false,
-			}
-			root.children[b] = child
-		}
-		root = child
-	}
-	root.wordTail = true
-}
-
-func (t *trie) HasPrefix(v string) bool {
-	root := t
-	if root.wordTail {
-		return true
-	}
-	for _, b := range []byte(v) {
-		child, exists := root.children[b]
-		if !exists {
-			return false
-		}
-		if child.wordTail {
-			return true
-		}
-		root = child
-	}
-	return false
-}

pkg/genericapiserver/openapi/openapi_test.go

@@ -25,7 +25,6 @@ import (
 	"github.com/go-openapi/spec"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
-	"sort"
 )
 
 // setUp is a convenience function for setting up for (most) tests.
@@ -322,35 +321,6 @@ func getAdditionalTestParameters() []spec.Parameter {
 	return ret
 }
 
-type Parameters []spec.Parameter
-
-func (s Parameters) Len() int      { return len(s) }
-func (s Parameters) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-
-type ByName struct {
-	Parameters
-}
-
-func (s ByName) Less(i, j int) bool {
-	return s.Parameters[i].Name < s.Parameters[j].Name
-}
-
-// TODO(mehdy): Consider sort parameters in actual spec generation for more predictable spec generation
-func sortParameters(s *spec.Swagger) *spec.Swagger {
-	for k, p := range s.Paths.Paths {
-		sort.Sort(ByName{p.Parameters})
-		sort.Sort(ByName{p.Get.Parameters})
-		sort.Sort(ByName{p.Put.Parameters})
-		sort.Sort(ByName{p.Post.Parameters})
-		sort.Sort(ByName{p.Head.Parameters})
-		sort.Sort(ByName{p.Delete.Parameters})
-		sort.Sort(ByName{p.Options.Parameters})
-		sort.Sort(ByName{p.Patch.Parameters})
-		s.Paths.Paths[k] = p // Unnecessary?! Magic!!!
-	}
-	return s
-}
-
 func getTestInputDefinition() spec.Schema {
 	return spec.Schema{
 		SchemaProps: spec.SchemaProps{
@@ -434,8 +404,6 @@ func TestBuildSwaggerSpec(t *testing.T) {
 	}
 	err := o.init()
 	if assert.NoError(err) {
-		sortParameters(expected)
-		sortParameters(o.swagger)
 		assert.Equal(expected, o.swagger)
 	}
 }

pkg/genericapiserver/openapi/util.go

@@ -0,0 +1,116 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package openapi
+
+import (
+	"sort"
+
+	"github.com/emicklei/go-restful"
+	"github.com/go-openapi/spec"
+)
+
+type parameters []spec.Parameter
+
+func (s parameters) Len() int      { return len(s) }
+func (s parameters) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+
+// byNameIn used in sorting parameters by Name and In fields.
+type byNameIn struct {
+	parameters
+}
+
+func (s byNameIn) Less(i, j int) bool {
+	return s.parameters[i].Name < s.parameters[j].Name || (s.parameters[i].Name == s.parameters[j].Name && s.parameters[i].In < s.parameters[j].In)
+}
+
+// SortParameters sorts parameters by Name and In fields.
+func sortParameters(p []spec.Parameter) {
+	sort.Sort(byNameIn{p})
+}
+
+func groupRoutesByPath(routes []restful.Route) (ret map[string][]restful.Route) {
+	ret = make(map[string][]restful.Route)
+	for _, r := range routes {
+		route, exists := ret[r.Path]
+		if !exists {
+			route = make([]restful.Route, 0, 1)
+		}
+		ret[r.Path] = append(route, r)
+	}
+	return ret
+}
+
+func mapKeyFromParam(param *restful.Parameter) interface{} {
+	return struct {
+		Name string
+		Kind int
+	}{
+		Name: param.Data().Name,
+		Kind: param.Data().Kind,
+	}
+}
+
+// A simple trie implementation with Add an HasPrefix methods only.
+type trie struct {
+	children map[byte]*trie
+	wordTail bool
+}
+
+func createTrie(list []string) trie {
+	ret := trie{
+		children: make(map[byte]*trie),
+		wordTail: false,
+	}
+	for _, v := range list {
+		ret.Add(v)
+	}
+	return ret
+}
+
+func (t *trie) Add(v string) {
+	root := t
+	for _, b := range []byte(v) {
+		child, exists := root.children[b]
+		if !exists {
+			child = &trie{
+				children: make(map[byte]*trie),
+				wordTail: false,
+			}
+			root.children[b] = child
+		}
+		root = child
+	}
+	root.wordTail = true
+}
+
+func (t *trie) HasPrefix(v string) bool {
+	root := t
+	if root.wordTail {
+		return true
+	}
+	for _, b := range []byte(v) {
+		child, exists := root.children[b]
+		if !exists {
+			return false
+		}
+		if child.wordTail {
+			return true
+		}
+		root = child
+	}
+	return false
+}