[go-server] Support min/max/defaults for values#15185
[go-server] Support min/max/defaults for values#15185wing328 merged 7 commits intoOpenAPITools:7.0.xfrom
Conversation
|
can you please resolve the merge conflicts when you've time? |
There was a problem hiding this comment.
This seems to return pointers up the stack which may result in heap allocation and slow perf.
As much as possible try to not use ptr of basic types
There was a problem hiding this comment.
Is there a case where a body param is not a schema object?
If multiple body param references the same schema object, this will be duplicated repeatedly.
Could this be moved into the model.mustache?
There was a problem hiding this comment.
instead of using nil all over, and requiring ptr. you can use functional options pattern
|
Let me know if you need any help |
Thx for the review. What's the status on using generics in the go-server generated code? I'd like to use generics for the option pattern to avoid having all the combination of With(Float|Int|String|Bool)(Default|Minimum|Maximum) I see #15087 was merged in 7.0.x, does it mean generics won't appear in any 6.x releases? If so, what's your recommandation? target 7.0.x for this PR? |
|
Yup, generics is breaking cause of the go version bump, so will not be in 6. According to wing328, it’s going to be a tentative June release. If you don’t mind waiting for the new major release or building 7 yourself to use this new feature for the time being. You can create a new branch from 7.0.0, port the changes over, and create a new PR targeting that branch. |
Enforce, for the go-server, to check the minimum and maximum values specified in the openapi description. Also apply the default if the parameter is not passed. Fix OpenAPITools#14013
c83d644 to
e618103
Compare
| {{/-last}}{{/imports}}{{#model}}{{#isEnum}}{{#description}}// {{{classname}}} : {{{description}}}{{/description}} | ||
| {{/-last}}{{/imports}} | ||
|
|
||
| import "errors" // FIXME: why not in #imports |
There was a problem hiding this comment.
Not sure how to have the imports automatically added. Any help welcome.
There was a problem hiding this comment.
For this, you will have to look at postProcessModels()
may have to override it in modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoServerCodegen.java
Let me know if there's any issues
|
New version of the PR, now targeting 7.0.x. I had one trouble with imports (see the comment). For testing, as I don't see how to test against the public instance, I wrote my own server/client implementation, I uploaded in another branch: rledisez@c83d644 |
lwj5
left a comment
There was a problem hiding this comment.
Just some minor inputs, I'll look at routers in the coming week
|
Also help to check the indentations, I found some with 4 spaces. Should be tabs, if not linter will complain |
|
For router go you can use something like this // GetOrderById - Find purchase order by ID
func (c *StoreApiController) GetOrderById(w http.ResponseWriter, r *http.Request) {
orderIdParam, err := parseNumericParameter[int64](
chi.URLParam(r, "orderId"),
WithRequire[int64](parseInt64),
WithMinimum[int64](1),
WithMaximum[int64](5),
)
if err != nil {
c.errorHandler(w, r, &ParsingError{Err: err}, nil)
return
}
...
}with something like this type Number interface {
constraints.Integer | constraints.Float
}
type ParseString[T Number | string | bool] func(v string) (T, error)
// parseFloat64 parses a string parameter to an float64.
func parseFloat64(param string) (float64, error) {
return strconv.ParseFloat(param, 64)
}
// parseFloat32 parses a string parameter to an float32.
func parseFloat32(param string) (float32, error) {
v, err := strconv.ParseFloat(param, 32)
return float32(v), err
}
// parseInt64 parses a string parameter to an int64.
func parseInt64(param string) (int64, error) {
return strconv.ParseInt(param, 10, 64)
}
// parseInt32 parses a string parameter to an int32.
func parseInt32(param string) (int32, error) {
val, err := strconv.ParseInt(param, 10, 32)
return int32(val), err
}
// parseBool parses a string parameter to an bool.
func parseBool(param string) (bool, error) {
return strconv.ParseBool(param)
}
type Operation[T Number | string | bool] func(actual string) (T, bool, error)
func WithRequire[T Number | string | bool](parse ParseString[T]) Operation[T] {
var empty T
return func(actual string) (T, bool, error) {
if actual == "" {
return empty, false, errors.New(errMsgRequiredMissing)
}
v, err := parse(actual)
return v, false, err
}
}
func WithDefaultOrParse[T Number | string | bool](def T, parse ParseString[T]) Operation[T] {
return func(actual string) (T, bool, error) {
if actual == "" {
return def, true, nil
}
v, err := parse(actual)
return v, false, err
}
}
func WithParse[T Number | string | bool](parse ParseString[T]) Operation[T] {
return func(actual string) (T, bool, error) {
v, err := parse(actual)
return v, false, err
}
}
type Constraint[T Number | string | bool] func(actual T) error
func WithMinimum[T Number](expected T) Constraint[T] {
return func(actual T) error {
if actual < expected {
return errors.New(errMsgMinValueConstraint)
}
return nil
}
}
func WithMaximum[T Number](expected T) Constraint[T] {
return func(actual T) error {
if actual > expected {
return errors.New(errMsgMaxValueConstraint)
}
return nil
}
}
// parseNumericParameter parses a numeric parameter to an its respective type.
func parseNumericParameter[T Number](param string, fn Operation[T], check ...Constraint[T]) (T, error) {
v, ok, err := fn(param)
if err != nil {
return 0, err
}
if !ok {
for _, o := range check {
if err := o(v); err != nil {
return 0, err
}
}
}
return v, nil
}
// parseBoolParameter parses a string parameter to a bool
func parseBoolParameter(param string, fn Operation[bool]) (bool, error) {
v, _, err := fn(param)
return v, err
}please do a sanity check if you can |
Co-authored-by: Ween Jiann <[email protected]>
Co-authored-by: Ween Jiann <[email protected]>
Co-authored-by: Ween Jiann <[email protected]>
|
I think I did all suggested changes. For the java change, the IDE (vscode) added a bunch of imports, otherwise it is complaining that "TYPE cannot be resolved", so I let it as-is for now, let me know if I should not. I did separate commits for now, let met know if they should be squashed when you'll be satisfied of the PR. |
|
@rledisez could you unmark this as draft |
|
Sure, sorry for that. It's done. |
|
LGTM. @wing328 I think failure is related to the parallel test issue, but not sure |
|
the circleci failure has been fixed in the master |
* [go-server] Support min/max/defaults for values Enforce, for the go-server, to check the minimum and maximum values specified in the openapi description. Also apply the default if the parameter is not passed. Fix #14013 * Fix merge conflict Co-authored-by: Ween Jiann <[email protected]> * Improve UnmarshalJSON implementation Co-authored-by: Ween Jiann <[email protected]> * Improve default value handling for string Co-authored-by: Ween Jiann <[email protected]> * Fix suggested changes * rework option pattern * add imports based on types/min max values --------- Co-authored-by: Ween Jiann <[email protected]>
|
FYI. I've cherry-pick 6b7d9ff into master as the latest master is the upcoming v7.0.0 release. |
| Boolean importErrors = false; | ||
|
|
||
| for (CodegenProperty param : Iterables.concat(model.vars, model.allVars, model.requiredVars, model.optionalVars)) { | ||
| if (param.isNumeric && ((param.minimum != null && param.minimum != "") || (param.maximum != null && param.maximum != ""))) { |
Enforce, for the go-server, to check the minimum and maximum values specified in the openapi description. Also apply the default if the parameter is not passed.
Fix #14013
@antihax @grokify @kemokemo @jirikuncar @ph4r5h4d @lwj5