infrahq/infra

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:

  1. in readRequest we use gin/binding to copy values from http.Request (path parameters, query parameters, and request body) to our api request structs
  2. in readRequest we use internal/validate to validate the api request struct after "binding"
  3. 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 changeformtoquery` 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.

stale commented

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.