From 6219e5713558e0b0c8c704d39f2324dab68fba9b Mon Sep 17 00:00:00 2001 From: "Gerasimos (Makis) Maropoulos" Date: Thu, 22 Apr 2021 14:00:00 +0300 Subject: [PATCH] New APIContainer.EnableStrictMode(bool) method. Read HISTORY.md --- HISTORY.md | 2 ++ core/router/api_builder.go | 20 ++++++++--- core/router/api_container.go | 16 +++++++++ hero/binding.go | 25 +++++++++----- hero/binding_test.go | 25 ++++++++++++-- hero/container.go | 65 +++++++++++++++++++++++------------- hero/dependency.go | 14 +++++--- hero/handler.go | 2 +- hero/struct.go | 2 +- 9 files changed, 124 insertions(+), 47 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 811f6a48..2a694e9a 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -28,6 +28,8 @@ The codebase for Dependency Injection, Internationalization and localization and ## Fixes and Improvements +- New `APIContainer.EnableStrictMode(bool)` to disable automatic payload binding and panic on missing dependencies for exported struct'sfields or function's input parameters on MVC controller or hero function or PartyConfigurator. + - New `Party.PartyConfigure(relativePath string, partyReg ...PartyConfigurator) Party` helper, registers a children Party like `Party` and `PartyFunc` but instead it accepts a structure value which may contain one or more of the dependencies registered by `RegisterDependency` or `ConfigureContainer().RegisterDependency` methods and fills the unset/zero exported struct's fields respectfully (useful when the api's dependencies amount are too much to pass on a function). - **New feature:** add the ability to set custom error handlers on path type parameters errors (existing or custom ones). Example Code: diff --git a/core/router/api_builder.go b/core/router/api_builder.go index 7f23294e..554076a1 100644 --- a/core/router/api_builder.go +++ b/core/router/api_builder.go @@ -893,11 +893,21 @@ func (api *APIBuilder) PartyFunc(relativePath string, partyBuilderFunc func(p Pa return p } -// PartyConfigurator is an interface which all child parties that are registered -// through `PartyConfigure` should implement. -type PartyConfigurator interface { - Configure(parent Party) -} +type ( + // PartyConfigurator is an interface which all child parties that are registered + // through `PartyConfigure` should implement. + PartyConfigurator interface { + Configure(parent Party) + } + + // StrictlyPartyConfigurator is an optional interface which a `PartyConfigurator` + // can implement to make sure that all exported fields having a not-nin, non-zero + // value before server starts. + // StrictlyPartyConfigurator interface { + // Strict() bool + // } + // Good idea but a `mvc or bind:"required"` is a better one I think. +) // PartyConfigure like `Party` and `PartyFunc` registers a new children Party // but instead it accepts a struct value which should implement the PartyConfigurator interface. diff --git a/core/router/api_container.go b/core/router/api_container.go index 0504eeb9..d566b0c5 100644 --- a/core/router/api_container.go +++ b/core/router/api_container.go @@ -79,6 +79,22 @@ func (api *APIContainer) UseResultHandler(handler func(next hero.ResultHandler) return api } +// EnableStrictMode sets the container's DisablePayloadAutoBinding and MarkExportedFieldsAsRequired to true. +// Meaning that all struct's fields (or function's parameters) should be binded manually (except the path parameters). +// +// Note that children will clone the same properties. +// Call the same method with `false` for children +// to enable automatic binding on missing dependencies. +// +// Strict mode is disabled by default; +// structs or path parameters that don't match to registered dependencies +// are automatically binded from the request context (body and url path parameters respectfully). +func (api *APIContainer) EnableStrictMode(strictMode bool) *APIContainer { + api.Container.DisablePayloadAutoBinding = strictMode + api.Container.MarkExportedFieldsAsRequired = strictMode + return api +} + // convertHandlerFuncs accepts Iris hero handlers and returns a slice of native Iris handlers. func (api *APIContainer) convertHandlerFuncs(relativePath string, handlersFn ...interface{}) context.Handlers { fullpath := api.Self.GetRelPath() + relativePath diff --git a/hero/binding.go b/hero/binding.go index 137743ff..46ade3d4 100644 --- a/hero/binding.go +++ b/hero/binding.go @@ -118,7 +118,7 @@ func matchDependency(dep *Dependency, in reflect.Type) bool { return dep.DestType == nil || equalTypes(dep.DestType, in) } -func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramsCount int) (bindings []*binding) { +func getBindingsFor(inputs []reflect.Type, deps []*Dependency, disablePayloadAutoBinding bool, paramsCount int) (bindings []*binding) { // Path parameter start index is the result of [total path parameters] - [total func path parameters inputs], // moving from last to first path parameters and first to last (probably) available input args. // @@ -208,15 +208,17 @@ func getBindingsFor(inputs []reflect.Type, deps []*Dependency, paramsCount int) } if prevN == len(bindings) { - if canBePathParameter { + if canBePathParameter { // Let's keep that option just for "payload": disablePayloadAutoBinding // no new dependency added for this input, // let's check for path parameters. bindings = append(bindings, paramBinding(i, getParamIndex(), in)) continue } - // else add builtin bindings that may be registered by user too, but they didn't. - if isPayloadType(in) { + // else, if payload binding is not disabled, + // add builtin request bindings that + // could be registered by end-dev but they didn't + if !disablePayloadAutoBinding && isPayloadType(in) { bindings = append(bindings, payloadBinding(i, in)) continue } @@ -235,7 +237,7 @@ func isPayloadType(in reflect.Type) bool { } } -func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramsCount int) []*binding { +func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, disablePayloadAutoBinding bool, paramsCount int) []*binding { fnTyp := fn.Type() if !isFunc(fnTyp) { panic("bindings: unresolved: not a func type") @@ -247,7 +249,7 @@ func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramsCoun inputs[i] = fnTyp.In(i) } - bindings := getBindingsFor(inputs, dependencies, paramsCount) + bindings := getBindingsFor(inputs, dependencies, disablePayloadAutoBinding, paramsCount) if expected, got := n, len(bindings); expected != got { expectedInputs := "" missingInputs := "" @@ -276,7 +278,7 @@ func getBindingsForFunc(fn reflect.Value, dependencies []*Dependency, paramsCoun return bindings } -func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramsCount int, sorter Sorter) (bindings []*binding) { +func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, markExportedFieldsAsRequired bool, disablePayloadAutoBinding bool, paramsCount int, sorter Sorter) (bindings []*binding) { typ := indirectType(v.Type()) if typ.Kind() != reflect.Struct { panic("bindings: unresolved: no struct type") @@ -288,7 +290,7 @@ func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramsCou for _, f := range nonZero { // fmt.Printf("Controller [%s] | NonZero | Field Index: %v | Field Type: %s\n", typ, f.Index, f.Type) bindings = append(bindings, &binding{ - Dependency: NewDependency(elem.FieldByIndex(f.Index).Interface()), + Dependency: newDependency(elem.FieldByIndex(f.Index).Interface(), disablePayloadAutoBinding), Input: newStructFieldInput(f), }) } @@ -308,13 +310,18 @@ func getBindingsForStruct(v reflect.Value, dependencies []*Dependency, paramsCou inputs[i] = fields[i].Type } - exportedBindings := getBindingsFor(inputs, dependencies, paramsCount) + exportedBindings := getBindingsFor(inputs, dependencies, disablePayloadAutoBinding, paramsCount) + // fmt.Printf("Controller [%s] | Inputs length: %d vs Bindings length: %d | NonZero: %d | Stateless : %d\n", // typ, n, len(exportedBindings), len(nonZero), stateless) // for i, b := range exportedBindings { // fmt.Printf("[%d] [Static=%v] %#+v\n", i, b.Dependency.Static, b.Dependency.OriginalValue) // } + if markExportedFieldsAsRequired && len(exportedBindings) != n { + panic(fmt.Sprintf("MarkExportedFieldsAsRequired is true and at least one of struct's (%s) field was not binded to a dependency.\nFields length: %d, matched exported bindings length: %d.\nUse the Reporter for further details", typ.String(), n, len(exportedBindings))) + } + if stateless == 0 && len(nonZero) >= len(exportedBindings) { // if we have not a single stateless and fields are defined then just return. // Note(@kataras): this can accept further improvements. diff --git a/hero/binding_test.go b/hero/binding_test.go index 40d05c52..95a736e6 100644 --- a/hero/binding_test.go +++ b/hero/binding_test.go @@ -269,7 +269,7 @@ func TestGetBindingsForFunc(t *testing.T) { } for i, tt := range tests { - bindings := getBindingsForFunc(reflect.ValueOf(tt.Func), c.Dependencies, 0) + bindings := getBindingsForFunc(reflect.ValueOf(tt.Func), c.Dependencies, c.DisablePayloadAutoBinding, 0) if expected, got := len(tt.Expected), len(bindings); expected != got { t.Fatalf("[%d] expected bindings length to be: %d but got: %d of: %s", i, expected, got, bindings) @@ -524,7 +524,7 @@ func TestBindingsForStruct(t *testing.T) { } for i, tt := range tests { - bindings := getBindingsForStruct(reflect.ValueOf(tt.Value), tt.Registered, 0, nil) + bindings := getBindingsForStruct(reflect.ValueOf(tt.Value), tt.Registered, false, false, 0, nil) if expected, got := len(tt.Expected), len(bindings); expected != got { t.Logf("[%d] expected bindings length to be: %d but got: %d:\n", i, expected, got) @@ -546,3 +546,24 @@ func TestBindingsForStruct(t *testing.T) { } } + +func TestBindingsForStructMarkExportedFieldsAsRequred(t *testing.T) { + type ( + Embedded struct { + Val string + } + + controller struct { + MyService service + Embedded *Embedded + } + ) + + dependencies := []*Dependency{ + NewDependency(&Embedded{"test"}), + NewDependency(&serviceImpl{}), + } + + // should panic if fail. + _ = getBindingsForStruct(reflect.ValueOf(new(controller)), dependencies, true, true, 0, nil) +} diff --git a/hero/container.go b/hero/container.go index ded50532..f368207f 100644 --- a/hero/container.go +++ b/hero/container.go @@ -38,6 +38,21 @@ type Container struct { Sorter Sorter // The dependencies entries. Dependencies []*Dependency + + // MarkExportedFieldsAsRequired reports whether all struct's fields + // MUST be binded to a dependency from the `Dependencies` list field. + // In-short, if it is set to true and if at least one exported field + // of a struct is not binded to a dependency then + // the entire application will exit with a panic message before server startup. + MarkExportedFieldsAsRequired bool + // DisablePayloadAutoBinding reports whether + // a function's parameter or struct's field of struct type + // should not be binded automatically to the request body (e.g. JSON) + // if a dependency for that type is missing. + // By default the binder will bind structs to request body, + // set to true to disable that kind of behavior. + DisablePayloadAutoBinding bool + // GetErrorHandler should return a valid `ErrorHandler` to handle bindings AND handler dispatch errors. // Defaults to a functon which returns the `DefaultErrorHandler`. GetErrorHandler func(*context.Context) ErrorHandler // cannot be nil. @@ -124,13 +139,13 @@ func (c *Container) fillReport(fullName string, bindings []*binding) { // Contains the iris context, standard context, iris sessions and time dependencies. var BuiltinDependencies = []*Dependency{ // iris context dependency. - NewDependency(func(ctx *context.Context) *context.Context { return ctx }).Explicitly(), + newDependency(func(ctx *context.Context) *context.Context { return ctx }, true).Explicitly(), // standard context dependency. - NewDependency(func(ctx *context.Context) stdContext.Context { + newDependency(func(ctx *context.Context) stdContext.Context { return ctx.Request().Context() - }).Explicitly(), + }, true).Explicitly(), // iris session dependency. - NewDependency(func(ctx *context.Context) *sessions.Session { + newDependency(func(ctx *context.Context) *sessions.Session { session := sessions.Get(ctx) if session == nil { ctx.Application().Logger().Debugf("binding: session is nil\nMaybe inside HandleHTTPError? Register it with app.UseRouter(sess.Handler()) to fix it") @@ -141,52 +156,52 @@ var BuiltinDependencies = []*Dependency{ } return session - }).Explicitly(), + }, true).Explicitly(), // application's logger. - NewDependency(func(ctx *context.Context) *golog.Logger { + newDependency(func(ctx *context.Context) *golog.Logger { return ctx.Application().Logger() - }).Explicitly(), + }, true).Explicitly(), // time.Time to time.Now dependency. - NewDependency(func(ctx *context.Context) time.Time { + newDependency(func(ctx *context.Context) time.Time { return time.Now() - }).Explicitly(), + }, true).Explicitly(), // standard http Request dependency. - NewDependency(func(ctx *context.Context) *http.Request { + newDependency(func(ctx *context.Context) *http.Request { return ctx.Request() - }).Explicitly(), + }, true).Explicitly(), // standard http ResponseWriter dependency. - NewDependency(func(ctx *context.Context) http.ResponseWriter { + newDependency(func(ctx *context.Context) http.ResponseWriter { return ctx.ResponseWriter() - }).Explicitly(), + }, true).Explicitly(), // http headers dependency. - NewDependency(func(ctx *context.Context) http.Header { + newDependency(func(ctx *context.Context) http.Header { return ctx.Request().Header - }).Explicitly(), + }, true).Explicitly(), // Client IP. - NewDependency(func(ctx *context.Context) net.IP { + newDependency(func(ctx *context.Context) net.IP { return net.ParseIP(ctx.RemoteAddr()) - }).Explicitly(), + }, true).Explicitly(), // Status Code (special type for MVC HTTP Error handler to not conflict with path parameters) - NewDependency(func(ctx *context.Context) Code { + newDependency(func(ctx *context.Context) Code { return Code(ctx.GetStatusCode()) - }).Explicitly(), + }, true).Explicitly(), // Context Error. May be nil - NewDependency(func(ctx *context.Context) Err { + newDependency(func(ctx *context.Context) Err { err := ctx.GetErr() if err == nil { return nil } return err - }).Explicitly(), + }, true).Explicitly(), // Context User, e.g. from basic authentication. - NewDependency(func(ctx *context.Context) context.User { + newDependency(func(ctx *context.Context) context.User { u := ctx.User() if u == nil { return nil } return u - }), + }, true), // payload and param bindings are dynamically allocated and declared at the end of the `binding` source file. } @@ -228,6 +243,8 @@ func (c *Container) Clone() *Container { clonedDeps := make([]*Dependency, len(c.Dependencies)) copy(clonedDeps, c.Dependencies) cloned.Dependencies = clonedDeps + cloned.DisablePayloadAutoBinding = c.DisablePayloadAutoBinding + cloned.MarkExportedFieldsAsRequired = c.MarkExportedFieldsAsRequired cloned.resultHandlers = c.resultHandlers // Reports are not cloned. return cloned @@ -264,7 +281,7 @@ func Register(dependency interface{}) *Dependency { // - Register(func(ctx iris.Context) User {...}) // - Register(func(User) OtherResponse {...}) func (c *Container) Register(dependency interface{}) *Dependency { - d := NewDependency(dependency, c.Dependencies...) + d := newDependency(dependency, c.DisablePayloadAutoBinding, c.Dependencies...) if d.DestType == nil { // prepend the dynamic dependency so it will be tried at the end // (we don't care about performance here, design-time) diff --git a/hero/dependency.go b/hero/dependency.go index 9ecabbaa..e87689de 100644 --- a/hero/dependency.go +++ b/hero/dependency.go @@ -51,6 +51,10 @@ func (d *Dependency) String() string { // // See `Container.Handler` for more. func NewDependency(dependency interface{}, funcDependencies ...*Dependency) *Dependency { + return newDependency(dependency, false, funcDependencies...) +} + +func newDependency(dependency interface{}, disablePayloadAutoBinding bool, funcDependencies ...*Dependency) *Dependency { if dependency == nil { panic(fmt.Sprintf("bad value: nil: %T", dependency)) } @@ -70,7 +74,7 @@ func NewDependency(dependency interface{}, funcDependencies ...*Dependency) *Dep OriginalValue: dependency, } - if !resolveDependency(v, dest, funcDependencies...) { + if !resolveDependency(v, disablePayloadAutoBinding, dest, funcDependencies...) { panic(fmt.Sprintf("bad value: could not resolve a dependency from: %#+v", dependency)) } @@ -80,11 +84,11 @@ func NewDependency(dependency interface{}, funcDependencies ...*Dependency) *Dep // DependencyResolver func(v reflect.Value, dest *Dependency) bool // Resolver DependencyResolver -func resolveDependency(v reflect.Value, dest *Dependency, funcDependencies ...*Dependency) bool { +func resolveDependency(v reflect.Value, disablePayloadAutoBinding bool, dest *Dependency, funcDependencies ...*Dependency) bool { return fromDependencyHandler(v, dest) || fromStructValue(v, dest) || fromFunc(v, dest) || - len(funcDependencies) > 0 && fromDependentFunc(v, dest, funcDependencies) + len(funcDependencies) > 0 && fromDependentFunc(v, disablePayloadAutoBinding, dest, funcDependencies) } func fromDependencyHandler(_ reflect.Value, dest *Dependency) bool { @@ -197,7 +201,7 @@ func handlerFromFunc(v reflect.Value, typ reflect.Type) DependencyHandler { } } -func fromDependentFunc(v reflect.Value, dest *Dependency, funcDependencies []*Dependency) bool { +func fromDependentFunc(v reflect.Value, disablePayloadAutoBinding bool, dest *Dependency, funcDependencies []*Dependency) bool { // * func(...) returns // * func(...) returns error // * func(...) returns , error @@ -207,7 +211,7 @@ func fromDependentFunc(v reflect.Value, dest *Dependency, funcDependencies []*De return false } - bindings := getBindingsForFunc(v, funcDependencies, -1 /* parameter bindings are disabled for depent dependencies */) + bindings := getBindingsForFunc(v, funcDependencies, disablePayloadAutoBinding, -1 /* parameter bindings are disabled for depent dependencies */) numIn := typ.NumIn() numOut := typ.NumOut() diff --git a/hero/handler.go b/hero/handler.go index 92444fb6..8a35ae89 100644 --- a/hero/handler.go +++ b/hero/handler.go @@ -107,7 +107,7 @@ func makeHandler(fn interface{}, c *Container, paramsCount int) context.Handler typ := v.Type() numIn := typ.NumIn() - bindings := getBindingsForFunc(v, c.Dependencies, paramsCount) + bindings := getBindingsForFunc(v, c.Dependencies, c.DisablePayloadAutoBinding, paramsCount) c.fillReport(context.HandlerName(fn), bindings) resultHandler := defaultResultHandler diff --git a/hero/struct.go b/hero/struct.go index 2a9d2af0..c3ae376b 100644 --- a/hero/struct.go +++ b/hero/struct.go @@ -51,7 +51,7 @@ func makeStruct(structPtr interface{}, c *Container, partyParamsCount int) *Stru } // get struct's fields bindings. - bindings := getBindingsForStruct(v, c.Dependencies, partyParamsCount, c.Sorter) + bindings := getBindingsForStruct(v, c.Dependencies, c.MarkExportedFieldsAsRequired, c.DisablePayloadAutoBinding, partyParamsCount, c.Sorter) // length bindings of 0, means that it has no fields or all mapped deps are static. // If static then Struct.Acquire will return the same "value" instance, otherwise it will create a new one.