From 17227f5fbe6ffcd6d29a57c716233f99fde1ae27 Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Sun, 4 Sep 2022 09:27:30 -0700 Subject: [PATCH] feat: customizable round-trip behavior for read-only fields --- README.md | 11 +++++++ operation.go | 13 ++++++-- resolver.go | 33 +++++++++++++++++-- router.go | 29 ++++++++++++++++- router_test.go | 87 +++++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 153 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 898befc8..6bb4b5e2 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,7 @@ Features include: - Annotated Go types for input and output models - Generates JSON Schema from Go types - Automatic input model validation & error handling +- Customizable round-trip behavior for read-only fields - Documentation generation using [RapiDoc](https://mrin9.github.io/RapiDoc/), [ReDoc](https://github.com/Redocly/redoc), or [SwaggerUI](https://swagger.io/tools/swagger-ui/) - CLI built-in, configured via arguments or environment variables - Set via e.g. `-p 8000`, `--port=8000`, or `SERVICE_PORT=8000` @@ -661,6 +662,16 @@ Parameters have some additional validation tags: | ---------- | ------------------------------ | ----------------- | | `internal` | Internal-only (not documented) | `internal:"true"` | +### Round Trip Behavior + +Since validation allows for read-only fields there is an interesting case of round trip complications when you `GET` a resource and then try to `PUT` or `POST` with those read-only fields intact. Huma's behavior is configurable: + +1. `huma.RoundTripRemove` (default): read-only fields are allowed but removed from the request by being set to their zero value. +2. `huma.RoundTripReject`: read-only fields are not allowed to be set to anything but their zero value. Round trips will generally fail if read-only fields were present in the response. +3. `huma.RoundTripIgnore`: read-only fields are allowed and completely ignored by Huma, allowing the implementer to provide additional logic (e.g. allow if and only if they are zero _or_ the same as the value currently on the server). + +> :whale: Using `RoundTripIgnore` is faster than removal (no additional processing of request bodies) but has implications for writing/overwriting read-only fields into your data store, so be careful with this option. + ## Middleware Standard [Go HTTP middleware](https://justinas.org/writing-http-middleware-in-go) is supported. It can be attached to the main router/app or to individual resources, but **must** be added _before_ operation handlers are added. diff --git a/operation.go b/operation.go index f273400f..be1f1a6f 100644 --- a/operation.go +++ b/operation.go @@ -239,7 +239,11 @@ func (o *Operation) Run(handler interface{}) { ) if o.requestSchema == nil { - o.requestSchema, err = schema.GenerateWithMode(body.Type, schema.ModeWrite, nil) + mode := schema.ModeWrite + if o.resource != nil && o.resource.router != nil && o.resource.router.roundTripBehavior != RoundTripReject { + mode = schema.ModeAll + } + o.requestSchema, err = schema.GenerateWithMode(body.Type, mode, nil) if o.resource != nil && o.resource.router != nil && !o.resource.router.disableSchemaProperty { o.requestSchema.AddSchemaField() } @@ -314,7 +318,12 @@ func (o *Operation) Run(handler interface{}) { } } - setFields(ctx, ctx.r, input, inputType) + removeReadOnly := false + if o.resource != nil && o.resource.router != nil && o.resource.router.roundTripBehavior == RoundTripRemove { + removeReadOnly = true + } + + setFields(ctx, ctx.r, input, inputType, removeReadOnly) if !ctx.HasError() { // No errors yet, so any errors that come after should be treated as a // semantic rather than structural error. diff --git a/resolver.go b/resolver.go index 9f81e65f..e3148ab9 100644 --- a/resolver.go +++ b/resolver.go @@ -170,7 +170,7 @@ func parseParamValue(ctx Context, location string, name string, typ reflect.Type return pv } -func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect.Type) { +func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect.Type, removeReadOnly bool) { if t.Kind() == reflect.Ptr { t = t.Elem() } @@ -189,7 +189,7 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect. if f.Anonymous { // Embedded struct - setFields(ctx, req, inField, f.Type) + setFields(ctx, req, inField, f.Type, removeReadOnly) continue } @@ -252,6 +252,10 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect. }) } + if removeReadOnly { + removeReadOnlyFields(inField) + } + // If requested, also provide access to the raw body bytes. if _, ok := t.FieldByName("RawBody"); ok { input.FieldByName("RawBody").Set(reflect.ValueOf(data)) @@ -350,6 +354,31 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect. } } +func removeReadOnlyFields(v reflect.Value) { + switch v.Kind() { + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + ro := v.Type().Field(i).Tag.Get("readOnly") + if ro == "true" { + // Set the field to its zero value! + f.Set(reflect.Zero(f.Type())) + } else { + removeReadOnlyFields(f) + } + } + case reflect.Slice, reflect.Array: + for i := 0; i < v.Len(); i++ { + removeReadOnlyFields(v.Index(i)) + } + case reflect.Map: + iter := v.MapRange() + for iter.Next() { + removeReadOnlyFields(iter.Value()) + } + } +} + // A smart join for JSONPath func pathJoin(prefix string, parts ...string) string { joined := prefix diff --git a/router.go b/router.go index 9ee76177..6e0fff4b 100644 --- a/router.go +++ b/router.go @@ -25,6 +25,23 @@ var connContextKey contextKey = "huma-request-conn" // has finished. var opIDContextKey contextKey = "huma-operation-id" +// RoundTripBehavior defines how the server handles a request which has +// fields set from a response that were read-only, for example the created +// date/time for the resource. These fields are not writeable, but you can +// choose to reject, or allow them. +type RoundTripBehavior int + +const ( + // RoundTripRemove will cause request validation to succeed and any read-only fields will be set to their zero value before calling the operation's handler function. This is the default behavior. + RoundTripRemove RoundTripBehavior = 0 + + // RoundTripReject will cause request validation to fail when read-only fields are present. + RoundTripReject RoundTripBehavior = 1 + + // RoundTripIgnore will cause request validation to succeed and any read-only fields will be ignored. This enables service implementers to use custom logic, such as rejecting read-only fields if they do not match the value stored on the server. + RoundTripIgnore RoundTripBehavior = 2 +) + // GetConn gets the underlying `net.Conn` from a context. func GetConn(ctx context.Context) net.Conn { conn := ctx.Value(connContextKey) @@ -69,8 +86,13 @@ type Router struct { defaultServerIdleTimeout time.Duration // Information for creating non-relative links & schema refs. - urlPrefix string + urlPrefix string + + // Disable the auto-generated `$schema` property in requests/responses. disableSchemaProperty bool + + // Specify whether resources with read-only fields can be round-tripped (i.e. GET then PUT, or POSTed with a read-only `created` field). + roundTripBehavior RoundTripBehavior } // OpenAPI returns an OpenAPI 3 representation of the API, which can be @@ -474,6 +496,11 @@ func (r *Router) DisableSchemaProperty() { r.disableSchemaProperty = true } +// RoundTripBehavior controls how the server handles resources which +func (r *Router) RoundTripBehavior(behavior RoundTripBehavior) { + r.roundTripBehavior = behavior +} + const ( DefaultDocsSuffix = "docs" DefaultSchemasSuffix = "schemas" diff --git a/router_test.go b/router_test.go index 642cf2b1..f260d8a5 100644 --- a/router_test.go +++ b/router_test.go @@ -501,13 +501,11 @@ func TestDefaultResponse(t *testing.T) { func TestRoundTrip(t *testing.T) { app := newTestRouter() - // This should not crash. - resource := app.Resource("/") - type Thing struct { Name string `json:"name"` } + resource := app.Resource("/") resource.Get("get-root", "docs", NewResponse(0, "").Model(Thing{})).Run(func(ctx Context) { ctx.WriteModel(http.StatusOK, Thing{Name: "Test"}) }) @@ -519,16 +517,6 @@ func TestRoundTrip(t *testing.T) { assert.Equal(t, "Test", input.Body.Name) }) - w1 := httptest.NewRecorder() - req1, _ := http.NewRequest(http.MethodGet, "/openapi.json", nil) - app.ServeHTTP(w1, req1) - t.Log(w1.Body.String()) - - w1 = httptest.NewRecorder() - req1, _ = http.NewRequest(http.MethodGet, "/schemas/Thing.json", nil) - app.ServeHTTP(w1, req1) - t.Log(w1.Body.String()) - w := httptest.NewRecorder() req, _ := http.NewRequest(http.MethodGet, "/", nil) app.ServeHTTP(w, req) @@ -542,7 +530,76 @@ func TestRoundTrip(t *testing.T) { req, _ = http.NewRequest(http.MethodPut, "/", thing) app.ServeHTTP(w, req) - t.Log(w.Body.String()) - assert.Equal(t, http.StatusOK, w.Result().StatusCode) } + +func TestRoundTripReadOnlyReject(t *testing.T) { + type Item struct { + Value string `json:"value" readOnly:"true"` + } + + type Thing struct { + Name string `json:"name"` + Created time.Time `json:"created" readOnly:"true"` + Nested map[string][]Item `json:"nested"` + } + + created := time.Date(2022, 01, 01, 00, 00, 00, 0, time.UTC) + + tests := []struct { + Name string + Behavior RoundTripBehavior + Response int + Handler func(thing Thing) + }{ + {"Reject", RoundTripReject, http.StatusUnprocessableEntity, func(thing Thing) { t.Fail() }}, + {"Ignore", RoundTripIgnore, http.StatusOK, func(thing Thing) { + assert.Equal(t, created, thing.Created) + assert.Equal(t, "value", thing.Nested["test"][0].Value) + }}, + {"Remove", RoundTripRemove, http.StatusOK, func(thing Thing) { + assert.True(t, thing.Created.IsZero()) + assert.Equal(t, "", thing.Nested["test"][0].Value) + }}, + } + + for _, example := range tests { + t.Run(example.Name, func(t *testing.T) { + app := newTestRouter() + app.RoundTripBehavior(example.Behavior) + + resource := app.Resource("/") + resource.Get("get-root", "docs", NewResponse(0, "").Model(Thing{})).Run(func(ctx Context) { + ctx.WriteModel(http.StatusOK, Thing{ + Name: "Test", + Created: created, + Nested: map[string][]Item{ + "test": {{Value: "value"}}, + }, + }) + }) + + resource.Put("put-root", "", NewResponse(200, "")).Run(func(ctx Context, input struct { + Body Thing + }) { + example.Handler(input.Body) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/", nil) + app.ServeHTTP(w, req) + + // This should not panic and should return the 200 OK + assert.Equal(t, http.StatusOK, w.Result().StatusCode) + + thing := w.Body + t.Logf("Sending %s", w.Body.String()) + + w = httptest.NewRecorder() + req, _ = http.NewRequest(http.MethodPut, "/", thing) + app.ServeHTTP(w, req) + + assert.Equal(t, example.Response, w.Result().StatusCode, w.Body.String()) + }) + } +}