Use a single implementation of struct traversal reflection for API requests
dnephin opened this issue · 1 comments
Background
Today we use reflection on api Request
types in 3 places:
- in
readRequest
we usegin/binding
to copy values fromhttp.Request
(path parameters, query parameters, and request body) to our api request structs - in
readRequest
we useinternal/validate
to validate the api request struct after "binding" - in
openapi.go
we use another version for inspecting the request and response structs to build API docs
These 3 uses are not identical:
- validate doesn't need to iterate over all fields, it only needs to find embedded and nested structs
- binding only applies to path and query parameters,
encoding/json
is used for the request body - openapi doc generation has to mimic the traversal of
encoding/json
for the request and response body, which we don't need for the other 2 cases
Despite those differences, there is some overlap. Both openapi doc generation and binding need the same rules for traversal of path and query parameters.
Proposal
In #4031 I looked to reduce gin/binding
to just the parts we need (~200 lines of production code, plus tests). I realized that this traversal needs to match the logic in openapi
doc generation, but it doesn't exactly today. Instead of 3 separate implementations of traversal, I think we could use a single one.
Given a struct, with structs tags on the fields to identify which fields come from which part of the request (form
, uri
, json- we can probably change
formto
query` after this is done), use reflection to find all the fields.
Each time it finds a field or a nested struct it should call a method on an interface and pass it all the details about that field. Something like this:
type NestedStruct struct {
Name string
Value reflect.Value
Section string // one of: json, uri, form
}
type Field struct {
Name string
Value reflect.Value
Section string // one of: json, uri, form
}
type Walker interface {
NestedStruct(ns NestedStruct) error
Field(f Field) error
}
The exact interface may change as we work through the implementation. Both openapi
and binding
would implement Walker
, and they can both call validate methods as necessary each time they receive a NestedStruct
.
I think this would allow us to replace the 3 different implementations of struct traversal with this one.
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.