Repository: ogen-go/ogen
Pull request #688 (2022-11-23 14:55:08)

refactor(parser): move response pattern parsing to `parser` package


Patch:
diff --git a/gen/gen_responses.go b/gen/gen_responses.go
index cad418ec..6eb97e17 100644
--- a/gen/gen_responses.go
+++ b/gen/gen_responses.go
@@ -2,46 +2,22 @@ package gen
 
 import (
 	"fmt"
-	"strconv"
-	"strings"
 
 	"github.com/go-faster/errors"
-	"golang.org/x/exp/slices"
 
 	"github.com/ogen-go/ogen/gen/ir"
 	"github.com/ogen-go/ogen/internal/xmaps"
 	"github.com/ogen-go/ogen/openapi"
 )
 
-func (g *Generator) generateResponses(ctx *genctx, opName string, responses map[string]*openapi.Response) (_ *ir.Responses, err error) {
+func (g *Generator) generateResponses(ctx *genctx, opName string, responses openapi.Responses) (_ *ir.Responses, err error) {
 	name := opName + "Res"
 	result := &ir.Responses{
-		StatusCode: make(map[int]*ir.Response, len(responses)),
+		StatusCode: make(map[int]*ir.Response, len(responses.StatusCode)),
 	}
 
 	// Sort responses by status code.
-	statusCodes := make([]int, 0, len(responses))
-	patterns := make([]string, 0, 5)
-	for status := range responses {
-		if status == "default" {
-			continue // Ignore default response.
-		}
-		switch strings.ToUpper(status) {
-		case "1XX", "2XX", "3XX", "4XX", "5XX":
-			patterns = append(patterns, status)
-			continue // Ignore pattern responses.
-		default:
-			code, err := strconv.Atoi(status)
-			if err != nil {
-				return nil, errors.Wrap(err, "parse response status code")
-			}
-
-			statusCodes = append(statusCodes, code)
-		}
-	}
-	slices.Sort(statusCodes)
-	slices.Sort(patterns)
-
+	statusCodes := xmaps.SortedKeys(responses.StatusCode)
 	for _, code := range statusCodes {
 		respName, err := pascal(opName, statusText(code))
 		if err != nil {
@@ -49,7 +25,7 @@ func (g *Generator) generateResponses(ctx *genctx, opName string, responses map[
 		}
 
 		var (
-			resp = responses[strconv.Itoa(code)]
+			resp = responses.StatusCode[code]
 			doc  = fmt.Sprintf("%s is response for %s operation.", respName, opName)
 		)
 
@@ -59,34 +35,26 @@ func (g *Generator) generateResponses(ctx *genctx, opName string, responses map[
 		}
 	}
 
-	for _, pattern := range patterns {
+	for idx, resp := range responses.Pattern {
+		if resp == nil {
+			continue
+		}
+		pattern := fmt.Sprintf("%dXX", idx+1)
+
 		respName, err := pascal(opName, pattern)
 		if err != nil {
 			return nil, errors.Wrapf(err, "%s: %s: response name", opName, pattern)
 		}
 
-		// Convert first digit to byte.
-		n := pattern[0] - '0'
-		switch n {
-		case 1, 2, 3, 4, 5:
-		default:
-			panic(fmt.Sprintf("unexpected pattern %q: code %d", pattern, n))
-		}
-		// Index starts from 0.
-		n--
-
-		var (
-			resp = responses[pattern]
-			doc  = fmt.Sprintf("%s is %s pattern response for %s operation.", respName, pattern, opName)
-		)
+		var doc = fmt.Sprintf("%s is %s pattern response for %s operation.", respName, pattern, opName)
 
-		result.Pattern[n], err = g.responseToIR(ctx, respName, doc, resp, true)
+		result.Pattern[idx], err = g.responseToIR(ctx, respName, doc, resp, true)
 		if err != nil {
 			return nil, errors.Wrapf(err, "pattern %q", pattern)
 		}
 	}
 
-	if def, ok := responses["default"]; ok && g.errType == nil {
+	if def := responses.Default; def != nil && g.errType == nil {
 		var (
 			respName = opName + "Def"
 			doc      = fmt.Sprintf("%s is default response for %s operation.", respName, opName)
diff --git a/gen/reduce.go b/gen/reduce.go
index 88e17dce..b8d2c82f 100644
--- a/gen/reduce.go
+++ b/gen/reduce.go
@@ -17,16 +17,16 @@ func (g *Generator) reduceDefault(ops []*openapi.Operation) error {
 
 	// Compare first default response to others.
 	first := ops[0]
-	if first.Responses == nil || first.Responses["default"] == nil {
+	d := first.Responses.Default
+	if d == nil {
 		return nil
 	}
-	d := first.Responses["default"]
 	if d.Ref.IsZero() {
 		// Not supported.
 		return nil
 	}
 	for _, spec := range ops[1:] {
-		if !reflect.DeepEqual(spec.Responses["default"], d) {
+		if !reflect.DeepEqual(spec.Responses.Default, d) {
 			return nil
 		}
 	}
diff --git a/openapi/operation.go b/openapi/operation.go
index c28393ec..59015423 100644
--- a/openapi/operation.go
+++ b/openapi/operation.go
@@ -1,6 +1,10 @@
 package openapi
 
 import (
+	"strconv"
+
+	"github.com/go-faster/errors"
+
 	"github.com/ogen-go/ogen/internal/location"
 )
 
@@ -20,13 +24,7 @@ type Operation struct {
 	Security []SecurityRequirement
 
 	// Operation responses.
-	// Map is always non-nil.
-	//
-	// Key can be:
-	//  * HTTP Status code
-	//  * default
-	//  * 1XX, 2XX, 3XX, 4XX, 5XX
-	Responses map[string]*Response
+	Responses Responses
 
 	location.Pointer `json:"-" yaml:"-"`
 }
@@ -54,3 +52,42 @@ type Response struct {
 
 	location.Pointer `json:"-" yaml:"-"`
 }
+
+// Responses contains a list of parsed OpenAPI Responses.
+type Responses struct {
+	StatusCode map[int]*Response
+	Pattern    [5]*Response
+	Default    *Response
+}
+
+// Add adds a response to the Responses.
+func (r *Responses) Add(pattern string, resp *Response) error {
+	switch pattern {
+	case "default":
+		r.Default = resp
+	case "1XX":
+		r.Pattern[0] = resp
+	case "2XX":
+		r.Pattern[1] = resp
+	case "3XX":
+		r.Pattern[2] = resp
+	case "4XX":
+		r.Pattern[3] = resp
+	case "5XX":
+		r.Pattern[4] = resp
+	default:
+		code, err := strconv.Atoi(pattern)
+		if err != nil {
+			// Do not return parsing error, it could be a bit confusing.
+			return errors.Errorf("invalid response pattern %q", pattern)
+		}
+		if code < 100 || code > 599 {
+			return errors.Errorf("invalid status code: %d", code)
+		}
+		if r.StatusCode == nil {
+			r.StatusCode = make(map[int]*Response)
+		}
+		r.StatusCode[code] = resp
+	}
+	return nil
+}
diff --git a/openapi/parser/parse_response.go b/openapi/parser/parse_response.go
index 30af3e91..1e5ebc41 100644
--- a/openapi/parser/parse_response.go
+++ b/openapi/parser/parse_response.go
@@ -1,8 +1,6 @@
 package parser
 
 import (
-	"strconv"
-
 	"github.com/go-faster/errors"
 
 	"github.com/ogen-go/ogen"
@@ -15,24 +13,24 @@ func (p *parser) parseResponses(
 	responses ogen.Responses,
 	locator location.Locator,
 	ctx *jsonpointer.ResolveCtx,
-) (_ map[string]*openapi.Response, err error) {
+) (result openapi.Responses, _ error) {
 	if len(responses) == 0 {
-		return nil, errors.New("no responses")
+		return result, errors.New("no responses")
 	}
 
-	result := make(map[string]*openapi.Response, len(responses))
-	for status, response := range responses {
-		if err := validateStatusCode(status); err != nil {
-			return nil, p.wrapLocation(p.file(ctx), locator.Key(status), err)
-		}
-
+	result = openapi.Responses{
+		StatusCode: map[int]*openapi.Response{},
+	}
+	for pattern, response := range responses {
 		resp, err := p.parseResponse(response, ctx)
 		if err != nil {
-			err := errors.Wrap(err, status)
-			return nil, p.wrapLocation(p.file(ctx), locator.Field(status), err)
+			err := errors.Wrap(err, pattern)
+			return result, p.wrapLocation(p.file(ctx), locator.Field(pattern), err)
 		}
 
-		result[status] = resp
+		if err := result.Add(pattern, resp); err != nil {
+			return result, p.wrapLocation(p.file(ctx), locator.Key(pattern), err)
+		}
 	}
 
 	return result, nil
@@ -73,21 +71,3 @@ func (p *parser) parseResponse(resp *ogen.Response, ctx *jsonpointer.ResolveCtx)
 		Pointer:     locator.Pointer(p.file(ctx)),
 	}, nil
 }
-
-func validateStatusCode(v string) error {
-	switch v {
-	case "default", "1XX", "2XX", "3XX", "4XX", "5XX":
-		return nil
-
-	default:
-		code, err := strconv.Atoi(v)
-		if err != nil {
-			return errors.Wrap(err, "parse status code")
-		}
-
-		if code < 100 || code > 599 {
-			return errors.Errorf("unknown status code: %d", code)
-		}
-		return nil
-	}
-}


Test patch:
diff --git a/openapi/parser/resolve_external_test.go b/openapi/parser/resolve_external_test.go
index e28fb9a6..7bb18950 100644
--- a/openapi/parser/resolve_external_test.go
+++ b/openapi/parser/resolve_external_test.go
@@ -284,8 +284,10 @@ func TestExternalReference(t *testing.T) {
 				Parameters:  []*openapi.Parameter{param},
 				RequestBody: requestBody,
 				Security:    []openapi.SecurityRequirement{},
-				Responses: map[string]*openapi.Response{
-					"200": response,
+				Responses: openapi.Responses{
+					StatusCode: map[int]*openapi.Response{
+						200: response,
+					},
 				},
 			},
 			{
@@ -298,8 +300,10 @@ func TestExternalReference(t *testing.T) {
 				Parameters:  []*openapi.Parameter{},
 				RequestBody: nil,
 				Security:    []openapi.SecurityRequirement{},
-				Responses: map[string]*openapi.Response{
-					"200": response,
+				Responses: openapi.Responses{
+					StatusCode: map[int]*openapi.Response{
+						200: response,
+					},
 				},
 			},
 		},
@@ -402,7 +406,7 @@ func TestExternalErrors(t *testing.T) {
 		File:     location.NewFile("root.json", "root.json", nil),
 		RootURL:  testRootURL,
 	})
-	a.ErrorContains(err, "parse status code")
+	a.ErrorContains(err, "invalid response pattern")
 
 	var locErr *location.Error
 	a.ErrorAs(err, &locErr)
@@ -435,7 +439,7 @@ func TestInitialLocation(t *testing.T) {
 		op := ops[0]
 		a.Equal("/get", op.Path.String())
 
-		resp, ok := op.Responses["200"]
+		resp, ok := op.Responses.StatusCode[200]
 		a.True(ok)
 		content, ok := resp.Content["application/json"]
 		a.True(ok)
diff --git a/openapi/parser/resolve_test.go b/openapi/parser/resolve_test.go
index 11248311..74ef975c 100644
--- a/openapi/parser/resolve_test.go
+++ b/openapi/parser/resolve_test.go
@@ -159,32 +159,34 @@ func TestComplicatedReference(t *testing.T) {
 				},
 			},
 			Security: []openapi.SecurityRequirement{},
-			Responses: map[string]*openapi.Response{
-				"200": {
-					Ref: refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/responses/200"},
-					Headers: map[string]*openapi.Header{
-						"ResponseHeader": responseHeader,
-					},
-					Content: map[string]*openapi.MediaType{
-						"application/json": {
-							Schema: &jsonschema.Schema{
-								Ref:  refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/requestBody/content/application~1json/schema"},
-								Type: "string",
+			Responses: openapi.Responses{
+				StatusCode: map[int]*openapi.Response{
+					200: {
+						Ref: refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/responses/200"},
+						Headers: map[string]*openapi.Header{
+							"ResponseHeader": responseHeader,
+						},
+						Content: map[string]*openapi.MediaType{
+							"application/json": {
+								Schema: &jsonschema.Schema{
+									Ref:  refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/requestBody/content/application~1json/schema"},
+									Type: "string",
+								},
+								Examples: map[string]*openapi.Example{},
+								Encoding: map[string]*openapi.Encoding{},
 							},
-							Examples: map[string]*openapi.Example{},
-							Encoding: map[string]*openapi.Encoding{},
 						},
 					},
-				},
-				"201": {
-					Headers: map[string]*openapi.Header{
-						"ResponseHeader": responseHeader,
-					},
-					Content: map[string]*openapi.MediaType{
-						"application/json": {
-							Schema:   &jsonschema.Schema{Type: "string"},
-							Examples: map[string]*openapi.Example{},
-							Encoding: map[string]*openapi.Encoding{},
+					201: {
+						Headers: map[string]*openapi.Header{
+							"ResponseHeader": responseHeader,
+						},
+						Content: map[string]*openapi.MediaType{
+							"application/json": {
+								Schema:   &jsonschema.Schema{Type: "string"},
+								Examples: map[string]*openapi.Example{},
+								Encoding: map[string]*openapi.Encoding{},
+							},
 						},
 					},
 				},
@@ -220,19 +222,21 @@ func TestComplicatedReference(t *testing.T) {
 				},
 			},
 			Security: []openapi.SecurityRequirement{},
-			Responses: map[string]*openapi.Response{
-				"200": {
-					Headers: map[string]*openapi.Header{
-						"ResponseHeader": responseHeader,
-					},
-					Content: map[string]*openapi.MediaType{
-						"application/json": {
-							Schema: &jsonschema.Schema{
-								Ref:  refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/requestBody/content/application~1json/schema"},
-								Type: "string",
+			Responses: openapi.Responses{
+				StatusCode: map[int]*openapi.Response{
+					200: {
+						Headers: map[string]*openapi.Header{
+							"ResponseHeader": responseHeader,
+						},
+						Content: map[string]*openapi.MediaType{
+							"application/json": {
+								Schema: &jsonschema.Schema{
+									Ref:  refKey{Loc: "/root.json", Ptr: "#/paths/~1post/post/requestBody/content/application~1json/schema"},
+									Type: "string",
+								},
+								Examples: map[string]*openapi.Example{},
+								Encoding: map[string]*openapi.Encoding{},
 							},
-							Examples: map[string]*openapi.Example{},
-							Encoding: map[string]*openapi.Encoding{},
 						},
 					},
 				},