[echo] support reference parameter types, fix #673

[jxsl13]

Summary:

After the second day of implementation and reading the source code, I'm more confident in this solution.

The tests provide additional evidence but we might need to tweak somemore configuration knobs, especially ones mentioned in the templates like IsStyle or IsJson, etc. to see whether the generator behaves as expected.

---

We introduce a new struct field called IsRef in order to differentiate whether the (query) parameter is defined globally or inline.

type ParameterDefinition struct {
	ParamName string // The original json parameter name, eg param_name
	In        string // Where the parameter is defined - path, header, cookie, query
	Required  bool   // Is this a required parameter?
	Spec      *openapi3.Parameter
	Schema    Schema
	IsRef     bool
}

We set IsRef to true in case we know that it is a reference type (#/components/parameters)

In the template we extend the interface with initially the params object/type which consists of inline definitions and at the end we add all of the parameter types that are defined globally in the parameters section.

// ServerInterface represents all server handlers.
type ServerInterface interface {
{{range .}}{{.SummaryAsComment }}
// ({{.Method}} {{.Path}})
{{.OperationId}}(ctx echo.Context{{genParamArgs .PathParams}}{{if .RequiresParamObject}}, params {{.OperationId}}Params{{ end }}{{ if .RequiresReferenceParams }}{{genParamArgs .ReferenceParams}}{{end}}) error
{{end}}
}

The other template has been changed in order not to set struct fields but to create local variables that are passed to the handler methods.

[jxsl13]

@jamietanna Would love if you could take a look, as you have been pretty active recently, so you might know and understand the source code better than me. Would be happy if this could be merged at some point by deepmap-marcinr (once someone reviewed it, until then I'm not gonna ping him :))

[jxsl13]

@deepmap-marcinr could you take a look :)

[jxsl13]

@deepmap-marcinr any further feedback?

[jxsl13]

@jamietanna would appreciate if someone could take a look.

this prevents the generation of redundant query parameter code in case query parameters are references to globally defined parameters.

The old master branch behavior creates a new struct type or every api endpoint that references global query parameter definitions. This pr reuses global query patameters.

[deepmap-marcinr]

Yes, I see what you have done, and it's a good change, downside is, it's a breaking change to generated code. If some people use those redundant types, their code will break after generation, and there will be many issues filed here.

How do you feel about feature flagging this in the output options structure?

[jxsl13]

I will add a flag

[jxsl13]

@jamietanna @deepmap-marcinr

Added an option for reference parameter deduplication, which is off by default.

[jxsl13]

@deepmap-marcinr @jamietanna added an option to enable this feature

[jxsl13]

@deepmap-marcinr @jamietanna can this be merged?

[jxsl13]

@deepmap-marcinr when you get the time, pls review.

[deepmap-marcinr]

I'm still looking through this, because the implications are complex, and this code is fragile.

The issue you are trying to solve is that you have parameter lists that are the same in different handlers, but due to how oapi-codegen works, you get a params type per handler, and they are not convertible, so it's annoying to work with.

I wonder if this issue can be solved much more simply via type aliases, eg.

type Params1 = struct {
     Arg1 string
     Arg2 string
}

Type Params2 = struct {
    Arg1 string
    Arg2 string
}

In your code, anything of type Params1 and Params2 would be the same type, and mutually assignable.