From 0c2035addd12387326977b06433408876dd17255 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Fri, 12 Jul 2024 14:40:31 +0100 Subject: [PATCH 1/2] chore: make it possible to provide detailed config validation errors As part of future changes, we want to introduce the ability to give us more control over validating the configuration provided by users. This follows the usage of a `map[string]string` via [0], which can then allow us to provide much more useful sets of error messages. [0]: https://grafana.com/blog/2024/02/09/how-i-write-http-services-in-go-after-13-years/ --- pkg/codegen/configuration.go | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/codegen/configuration.go b/pkg/codegen/configuration.go index 89c950ae81..b0f3284fbb 100644 --- a/pkg/codegen/configuration.go +++ b/pkg/codegen/configuration.go @@ -2,6 +2,7 @@ package codegen import ( "errors" + "fmt" "reflect" ) @@ -55,6 +56,10 @@ type GenerateOptions struct { EmbeddedSpec bool `yaml:"embedded-spec,omitempty"` } +func (oo GenerateOptions) Validate() map[string]string { + return nil +} + // CompatibilityOptions specifies backward compatibility settings for the // code generator. type CompatibilityOptions struct { @@ -105,6 +110,10 @@ type CompatibilityOptions struct { CircularReferenceLimit int `yaml:"circular-reference-limit"` } +func (co CompatibilityOptions) Validate() map[string]string { + return nil +} + // OutputOptions are used to modify the output code in some way. type OutputOptions struct { // Whether to skip go imports on the generated code @@ -142,6 +151,10 @@ type OutputOptions struct { NameNormalizer string `yaml:"name-normalizer,omitempty"` } +func (oo OutputOptions) Validate() map[string]string { + return nil +} + // UpdateDefaults sets reasonable default values for unset fields in Configuration func (o Configuration) UpdateDefaults() Configuration { if reflect.ValueOf(o.Generate).IsZero() { @@ -186,5 +199,30 @@ func (o Configuration) Validate() error { if nServers > 1 { return errors.New("only one server type is supported at a time") } + + var errs []error + if problems := o.Generate.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`generate` configuration for %v was incorrect: %v", k, v)) + } + } + + if problems := o.Compatibility.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`compatibility-options` configuration for %v was incorrect: %v", k, v)) + } + } + + if problems := o.OutputOptions.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`output-options` configuration for %v was incorrect: %v", k, v)) + } + } + + err := errors.Join(errs...) + if err != nil { + return fmt.Errorf("failed to validate configuration: %w", err) + } + return nil } From e0ddd715a270cae4e39b9136b8d70dc0dfbb5e88 Mon Sep 17 00:00:00 2001 From: Jamie Tanna Date: Fri, 12 Jul 2024 14:45:39 +0100 Subject: [PATCH 2/2] refactor: move `Configuration` methods closer to struct --- pkg/codegen/configuration.go | 144 +++++++++++++++++------------------ 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/pkg/codegen/configuration.go b/pkg/codegen/configuration.go index b0f3284fbb..637d7ccecc 100644 --- a/pkg/codegen/configuration.go +++ b/pkg/codegen/configuration.go @@ -30,6 +30,78 @@ type Configuration struct { NoVCSVersionOverride *string `yaml:"-"` } +// Validate checks whether Configuration represent a valid configuration +func (o Configuration) Validate() error { + if o.PackageName == "" { + return errors.New("package name must be specified") + } + + // Only one server type should be specified at a time. + nServers := 0 + if o.Generate.IrisServer { + nServers++ + } + if o.Generate.ChiServer { + nServers++ + } + if o.Generate.FiberServer { + nServers++ + } + if o.Generate.EchoServer { + nServers++ + } + if o.Generate.GorillaServer { + nServers++ + } + if o.Generate.StdHTTPServer { + nServers++ + } + if o.Generate.GinServer { + nServers++ + } + if nServers > 1 { + return errors.New("only one server type is supported at a time") + } + + var errs []error + if problems := o.Generate.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`generate` configuration for %v was incorrect: %v", k, v)) + } + } + + if problems := o.Compatibility.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`compatibility-options` configuration for %v was incorrect: %v", k, v)) + } + } + + if problems := o.OutputOptions.Validate(); problems != nil { + for k, v := range problems { + errs = append(errs, fmt.Errorf("`output-options` configuration for %v was incorrect: %v", k, v)) + } + } + + err := errors.Join(errs...) + if err != nil { + return fmt.Errorf("failed to validate configuration: %w", err) + } + + return nil +} + +// UpdateDefaults sets reasonable default values for unset fields in Configuration +func (o Configuration) UpdateDefaults() Configuration { + if reflect.ValueOf(o.Generate).IsZero() { + o.Generate = GenerateOptions{ + EchoServer: true, + Models: true, + EmbeddedSpec: true, + } + } + return o +} + // GenerateOptions specifies which supported output formats to generate. type GenerateOptions struct { // IrisServer specifies whether to generate iris server boilerplate @@ -154,75 +226,3 @@ type OutputOptions struct { func (oo OutputOptions) Validate() map[string]string { return nil } - -// UpdateDefaults sets reasonable default values for unset fields in Configuration -func (o Configuration) UpdateDefaults() Configuration { - if reflect.ValueOf(o.Generate).IsZero() { - o.Generate = GenerateOptions{ - EchoServer: true, - Models: true, - EmbeddedSpec: true, - } - } - return o -} - -// Validate checks whether Configuration represent a valid configuration -func (o Configuration) Validate() error { - if o.PackageName == "" { - return errors.New("package name must be specified") - } - - // Only one server type should be specified at a time. - nServers := 0 - if o.Generate.IrisServer { - nServers++ - } - if o.Generate.ChiServer { - nServers++ - } - if o.Generate.FiberServer { - nServers++ - } - if o.Generate.EchoServer { - nServers++ - } - if o.Generate.GorillaServer { - nServers++ - } - if o.Generate.StdHTTPServer { - nServers++ - } - if o.Generate.GinServer { - nServers++ - } - if nServers > 1 { - return errors.New("only one server type is supported at a time") - } - - var errs []error - if problems := o.Generate.Validate(); problems != nil { - for k, v := range problems { - errs = append(errs, fmt.Errorf("`generate` configuration for %v was incorrect: %v", k, v)) - } - } - - if problems := o.Compatibility.Validate(); problems != nil { - for k, v := range problems { - errs = append(errs, fmt.Errorf("`compatibility-options` configuration for %v was incorrect: %v", k, v)) - } - } - - if problems := o.OutputOptions.Validate(); problems != nil { - for k, v := range problems { - errs = append(errs, fmt.Errorf("`output-options` configuration for %v was incorrect: %v", k, v)) - } - } - - err := errors.Join(errs...) - if err != nil { - return fmt.Errorf("failed to validate configuration: %w", err) - } - - return nil -}