openapi3: introduce (Paths).InMatchingOrder() paths iterator (#719)

fenollp · web-flow · commit 1490eae89ee4 · 2022-12-19T15:48:40.000+01:00
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
@@ -105,7 +105,7 @@ jobs:
     - if: runner.os == 'Linux'
       name: Missing specification object link to definition
       run: |
-        [[ 30 -eq $(git grep -InE '^// See https:.+OpenAPI-Specification.+3[.]0[.]3[.]md#.+bject$' openapi3/*.go | grep -v _test.go | grep -v doc.go | wc -l) ]]
+        [[ 31 -eq $(git grep -InE '^// See https:.+OpenAPI-Specification.+3[.]0[.]3[.]md#.+bject$' openapi3/*.go | grep -v _test.go | grep -v doc.go | wc -l) ]]
 
     - if: runner.os == 'Linux'
       name: Style around ExtensionProps embedding
diff --git a/openapi2/openapi2.go b/openapi2/openapi2.go
@@ -40,15 +40,13 @@ func (doc *T) UnmarshalJSON(data []byte) error {
 }
 
 func (doc *T) AddOperation(path string, method string, operation *Operation) {
-	paths := doc.Paths
-	if paths == nil {
-		paths = make(map[string]*PathItem)
-		doc.Paths = paths
+	if doc.Paths == nil {
+		doc.Paths = make(map[string]*PathItem)
 	}
-	pathItem := paths[path]
+	pathItem := doc.Paths[path]
 	if pathItem == nil {
 		pathItem = &PathItem{}
-		paths[path] = pathItem
+		doc.Paths[path] = pathItem
 	}
 	pathItem.SetOperation(method, operation)
 }
diff --git a/openapi2conv/openapi2_conv.go b/openapi2conv/openapi2_conv.go
@@ -1204,15 +1204,13 @@ func stripNonCustomExtensions(extensions map[string]interface{}) {
 }
 
 func addPathExtensions(doc2 *openapi2.T, path string, extensionProps openapi3.ExtensionProps) {
-	paths := doc2.Paths
-	if paths == nil {
-		paths = make(map[string]*openapi2.PathItem)
-		doc2.Paths = paths
+	if doc2.Paths == nil {
+		doc2.Paths = make(map[string]*openapi2.PathItem)
 	}
-	pathItem := paths[path]
+	pathItem := doc2.Paths[path]
 	if pathItem == nil {
 		pathItem = &openapi2.PathItem{}
-		paths[path] = pathItem
+		doc2.Paths[path] = pathItem
 	}
 	pathItem.ExtensionProps = extensionProps
 }
diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go
@@ -193,14 +193,11 @@ func (doc *T) addCallbackToSpec(c *CallbackRef, refNameResolver RefNameResolver,
 		return false
 	}
 	name := refNameResolver(c.Ref)
-	if _, ok := doc.Components.Callbacks[name]; ok {
-		c.Ref = "#/components/callbacks/" + name
-	}
 	if doc.Components.Callbacks == nil {
 		doc.Components.Callbacks = make(Callbacks)
 	}
-	doc.Components.Callbacks[name] = &CallbackRef{Value: c.Value}
 	c.Ref = "#/components/callbacks/" + name
+	doc.Components.Callbacks[name] = &CallbackRef{Value: c.Value}
 	return true
 }
 
diff --git a/openapi3/loader.go b/openapi3/loader.go
@@ -281,12 +281,7 @@ func isSingleRefElement(ref string) bool {
 	return !strings.Contains(ref, "#")
 }
 
-func (loader *Loader) resolveComponent(
-	doc *T,
-	ref string,
-	path *url.URL,
-	resolved interface{},
-) (
+func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolved interface{}) (
 	componentDoc *T,
 	componentPath *url.URL,
 	err error,
@@ -928,11 +923,10 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen
 					}
 					id := unescapeRefString(rest)
 
-					definitions := doc.Components.Callbacks
-					if definitions == nil {
+					if doc.Components.Callbacks == nil {
 						return failedToResolveRefFragmentPart(ref, "callbacks")
 					}
-					resolved := definitions[id]
+					resolved := doc.Components.Callbacks[id]
 					if resolved == nil {
 						return failedToResolveRefFragmentPart(ref, id)
 					}
@@ -1022,15 +1016,13 @@ func (loader *Loader) resolvePathItemRef(doc *T, entrypoint string, pathItem *Pa
 			}
 			id := unescapeRefString(rest)
 
-			definitions := doc.Paths
-			if definitions == nil {
+			if doc.Paths == nil {
 				return failedToResolveRefFragmentPart(ref, "paths")
 			}
-			resolved := definitions[id]
+			resolved := doc.Paths[id]
 			if resolved == nil {
 				return failedToResolveRefFragmentPart(ref, id)
 			}
-
 			*pathItem = *resolved
 		}
 	}
diff --git a/openapi3/openapi3.go b/openapi3/openapi3.go
@@ -36,15 +36,13 @@ func (doc *T) UnmarshalJSON(data []byte) error {
 }
 
 func (doc *T) AddOperation(path string, method string, operation *Operation) {
-	paths := doc.Paths
-	if paths == nil {
-		paths = make(Paths)
-		doc.Paths = paths
+	if doc.Paths == nil {
+		doc.Paths = make(Paths)
 	}
-	pathItem := paths[path]
+	pathItem := doc.Paths[path]
 	if pathItem == nil {
 		pathItem = &PathItem{}
-		paths[path] = pathItem
+		doc.Paths[path] = pathItem
 	}
 	pathItem.SetOperation(method, operation)
 }
diff --git a/openapi3/paths.go b/openapi3/paths.go
@@ -29,8 +29,8 @@ func (paths Paths) Validate(ctx context.Context, opts ...ValidationOption) error
 		}
 
 		if pathItem == nil {
-			paths[path] = &PathItem{}
-			pathItem = paths[path]
+			pathItem = &PathItem{}
+			paths[path] = pathItem
 		}
 
 		normalizedPath, _, varsInPath := normalizeTemplatedPath(path)
@@ -109,6 +109,37 @@ func (paths Paths) Validate(ctx context.Context, opts ...ValidationOption) error
 	return nil
 }
 
+// InMatchingOrder returns paths in the order they are matched against URLs.
+// See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#paths-object
+// When matching URLs, concrete (non-templated) paths would be matched
+// before their templated counterparts.
+func (paths Paths) InMatchingOrder() []string {
+	// NOTE: sorting by number of variables ASC then by descending lexicographical
+	// order seems to be a good heuristic.
+	if paths == nil {
+		return nil
+	}
+
+	vars := make(map[int][]string)
+	max := 0
+	for path := range paths {
+		count := strings.Count(path, "}")
+		vars[count] = append(vars[count], path)
+		if count > max {
+			max = count
+		}
+	}
+
+	ordered := make([]string, 0, len(paths))
+	for c := 0; c <= max; c++ {
+		if ps, ok := vars[c]; ok {
+			sort.Sort(sort.Reverse(sort.StringSlice(ps)))
+			ordered = append(ordered, ps...)
+		}
+	}
+	return ordered
+}
+
 // Find returns a path that matches the key.
 //
 // The method ignores differences in template variable names (except possible "*" suffix).
diff --git a/routers/gorillamux/router.go b/routers/gorillamux/router.go
@@ -56,7 +56,7 @@ func NewRouter(doc *openapi3.T) (routers.Router, error) {
 
 	muxRouter := mux.NewRouter().UseEncodedPath()
 	r := &Router{}
-	for _, path := range orderedPaths(doc.Paths) {
+	for _, path := range doc.Paths.InMatchingOrder() {
 		pathItem := doc.Paths[path]
 		if len(pathItem.Servers) > 0 {
 			if servers, err = makeServers(pathItem.Servers); err != nil {
@@ -203,31 +203,6 @@ func newSrv(serverURL string, server *openapi3.Server, varsUpdater varsf) (srv,
 	return svr, nil
 }
 
-func orderedPaths(paths map[string]*openapi3.PathItem) []string {
-	// https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#pathsObject
-	// When matching URLs, concrete (non-templated) paths would be matched
-	// before their templated counterparts.
-	// NOTE: sorting by number of variables ASC then by descending lexicographical
-	// order seems to be a good heuristic.
-	vars := make(map[int][]string)
-	max := 0
-	for path := range paths {
-		count := strings.Count(path, "}")
-		vars[count] = append(vars[count], path)
-		if count > max {
-			max = count
-		}
-	}
-	ordered := make([]string, 0, len(paths))
-	for c := 0; c <= max; c++ {
-		if ps, ok := vars[c]; ok {
-			sort.Sort(sort.Reverse(sort.StringSlice(ps)))
-			ordered = append(ordered, ps...)
-		}
-	}
-	return ordered
-}
-
 // Magic strings that temporarily replace "{}" so net/url.Parse() works
 var blURL, brURL = strings.Repeat("-", 50), strings.Repeat("_", 50)
 

Original file line number	Diff line number	Diff line change
`@@ -40,15 +40,13 @@ func (doc *T) UnmarshalJSON(data []byte) error {`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`func (doc T) AddOperation(path string, method string, operation Operation) {`
`43`		`- paths := doc.Paths`
`44`		`- if paths == nil {`
`45`		`- paths = make(map[string]*PathItem)`
`46`		`- doc.Paths = paths`
	`43`	`+ if doc.Paths == nil {`
	`44`	`+ doc.Paths = make(map[string]*PathItem)`
`47`	`45`	`}`
`48`		`- pathItem := paths[path]`
	`46`	`+ pathItem := doc.Paths[path]`
`49`	`47`	`if pathItem == nil {`
`50`	`48`	`pathItem = &PathItem{}`
`51`		`- paths[path] = pathItem`
	`49`	`+ doc.Paths[path] = pathItem`
`52`	`50`	`}`
`53`	`51`	`pathItem.SetOperation(method, operation)`
`54`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1204,15 +1204,13 @@ func stripNonCustomExtensions(extensions map[string]interface{}) {`
`1204`	`1204`	`}`
`1205`	`1205`
`1206`	`1206`	`func addPathExtensions(doc2 *openapi2.T, path string, extensionProps openapi3.ExtensionProps) {`
`1207`		`- paths := doc2.Paths`
`1208`		`- if paths == nil {`
`1209`		`- paths = make(map[string]*openapi2.PathItem)`
`1210`		`- doc2.Paths = paths`
	`1207`	`+ if doc2.Paths == nil {`
	`1208`	`+ doc2.Paths = make(map[string]*openapi2.PathItem)`
`1211`	`1209`	`}`
`1212`		`- pathItem := paths[path]`
	`1210`	`+ pathItem := doc2.Paths[path]`
`1213`	`1211`	`if pathItem == nil {`
`1214`	`1212`	`pathItem = &openapi2.PathItem{}`
`1215`		`- paths[path] = pathItem`
	`1213`	`+ doc2.Paths[path] = pathItem`
`1216`	`1214`	`}`
`1217`	`1215`	`pathItem.ExtensionProps = extensionProps`
`1218`	`1216`	`}`
Original file line number	Diff line number	Diff line change
`@@ -193,14 +193,11 @@ func (doc T) addCallbackToSpec(c CallbackRef, refNameResolver RefNameResolver,`
`193`	`193`	`return false`
`194`	`194`	`}`
`195`	`195`	`name := refNameResolver(c.Ref)`
`196`		`- if _, ok := doc.Components.Callbacks[name]; ok {`
`197`		`- c.Ref = "#/components/callbacks/" + name`
`198`		`- }`
`199`	`196`	`if doc.Components.Callbacks == nil {`
`200`	`197`	`doc.Components.Callbacks = make(Callbacks)`
`201`	`198`	`}`
`202`		`- doc.Components.Callbacks[name] = &CallbackRef{Value: c.Value}`
`203`	`199`	`c.Ref = "#/components/callbacks/" + name`
	`200`	`+ doc.Components.Callbacks[name] = &CallbackRef{Value: c.Value}`
`204`	`201`	`return true`
`205`	`202`	`}`
`206`	`203`
Original file line number	Diff line number	Diff line change
`@@ -281,12 +281,7 @@ func isSingleRefElement(ref string) bool {`
`281`	`281`	`return !strings.Contains(ref, "#")`
`282`	`282`	`}`
`283`	`283`
`284`		`-func (loader *Loader) resolveComponent(`
`285`		`- doc *T,`
`286`		`- ref string,`
`287`		`- path *url.URL,`
`288`		`- resolved interface{},`
`289`		`-) (`
	`284`	`+func (loader Loader) resolveComponent(doc T, ref string, path *url.URL, resolved interface{}) (`
`290`	`285`	`componentDoc *T,`
`291`	`286`	`componentPath *url.URL,`
`292`	`287`	`err error,`
`@@ -928,11 +923,10 @@ func (loader Loader) resolveCallbackRef(doc T, component *CallbackRef, documen`
`928`	`923`	`}`
`929`	`924`	`id := unescapeRefString(rest)`
`930`	`925`
`931`		`- definitions := doc.Components.Callbacks`
`932`		`- if definitions == nil {`
	`926`	`+ if doc.Components.Callbacks == nil {`
`933`	`927`	`return failedToResolveRefFragmentPart(ref, "callbacks")`
`934`	`928`	`}`
`935`		`- resolved := definitions[id]`
	`929`	`+ resolved := doc.Components.Callbacks[id]`
`936`	`930`	`if resolved == nil {`
`937`	`931`	`return failedToResolveRefFragmentPart(ref, id)`
`938`	`932`	`}`
`@@ -1022,15 +1016,13 @@ func (loader Loader) resolvePathItemRef(doc T, entrypoint string, pathItem *Pa`
`1022`	`1016`	`}`
`1023`	`1017`	`id := unescapeRefString(rest)`
`1024`	`1018`
`1025`		`- definitions := doc.Paths`
`1026`		`- if definitions == nil {`
	`1019`	`+ if doc.Paths == nil {`
`1027`	`1020`	`return failedToResolveRefFragmentPart(ref, "paths")`
`1028`	`1021`	`}`
`1029`		`- resolved := definitions[id]`
	`1022`	`+ resolved := doc.Paths[id]`
`1030`	`1023`	`if resolved == nil {`
`1031`	`1024`	`return failedToResolveRefFragmentPart(ref, id)`
`1032`	`1025`	`}`
`1033`		`-`
`1034`	`1026`	`pathItem = resolved`
`1035`	`1027`	`}`
`1036`	`1028`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,15 +36,13 @@ func (doc *T) UnmarshalJSON(data []byte) error {`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`func (doc T) AddOperation(path string, method string, operation Operation) {`
`39`		`- paths := doc.Paths`
`40`		`- if paths == nil {`
`41`		`- paths = make(Paths)`
`42`		`- doc.Paths = paths`
	`39`	`+ if doc.Paths == nil {`
	`40`	`+ doc.Paths = make(Paths)`
`43`	`41`	`}`
`44`		`- pathItem := paths[path]`
	`42`	`+ pathItem := doc.Paths[path]`
`45`	`43`	`if pathItem == nil {`
`46`	`44`	`pathItem = &PathItem{}`
`47`		`- paths[path] = pathItem`
	`45`	`+ doc.Paths[path] = pathItem`
`48`	`46`	`}`
`49`	`47`	`pathItem.SetOperation(method, operation)`
`50`	`48`	`}`