From bc0222dcb430b982d2ac3f791f5f98be774136be Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Wed, 23 Aug 2023 16:47:02 +0300 Subject: [PATCH] [#3176] skip fields query param transformations for non 20x responses --- CHANGELOG.md | 2 + tools/rest/json_serializer.go | 4 +- tools/rest/json_serializer_test.go | 72 +++++++++++++++++++++++------- 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de9b252..5285648a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ - Fill the `LastVerificationSentAt` and `LastResetSentAt` fields only after a successfull email send ([#3121](https://github.com/pocketbase/pocketbase/issues/3121)). +- Skip API `fields` json transformations for non 20x responses ([#3176](https://github.com/pocketbase/pocketbase/issues/3176)). + - (@todo docs) Added new "Strip urls domain" `editor` field option to allow controlling the default TinyMCE imported urls behavior (_default to `false` for new content_). - Reduced the default JSVM prewarmed pool size to 25 to reduce the initial memory consumptions (_you can manually adjust the pool size with `--hooksPool=50` if you need to, but the default should suffice for most cases_). diff --git a/tools/rest/json_serializer.go b/tools/rest/json_serializer.go index 2e46423d..b3ad35c9 100644 --- a/tools/rest/json_serializer.go +++ b/tools/rest/json_serializer.go @@ -25,8 +25,10 @@ func (s *Serializer) Serialize(c echo.Context, i any, indent string) error { fieldsParam = "fields" } + statusCode := c.Response().Status + param := c.QueryParam(fieldsParam) - if param == "" { + if param == "" || statusCode < 200 || statusCode > 299 { return s.DefaultJSONSerializer.Serialize(c, i, indent) } diff --git a/tools/rest/json_serializer_test.go b/tools/rest/json_serializer_test.go index 3abb6f1c..94685f76 100644 --- a/tools/rest/json_serializer_test.go +++ b/tools/rest/json_serializer_test.go @@ -16,6 +16,7 @@ func TestSerialize(t *testing.T) { scenarios := []struct { name string serializer rest.Serializer + statusCode int data any query string expected string @@ -23,6 +24,7 @@ func TestSerialize(t *testing.T) { { "empty query", rest.Serializer{}, + 200, map[string]any{"a": 1, "b": 2, "c": "test"}, "", `{"a":1,"b":2,"c":"test"}`, @@ -30,6 +32,7 @@ func TestSerialize(t *testing.T) { { "empty fields", rest.Serializer{}, + 200, map[string]any{"a": 1, "b": 2, "c": "test"}, "fields=", `{"a":1,"b":2,"c":"test"}`, @@ -37,6 +40,7 @@ func TestSerialize(t *testing.T) { { "missing fields", rest.Serializer{}, + 200, map[string]any{"a": 1, "b": 2, "c": "test"}, "fields=missing", `{}`, @@ -44,6 +48,7 @@ func TestSerialize(t *testing.T) { { "non map response", rest.Serializer{}, + 200, "test", "fields=a,b,test", `"test"`, @@ -51,13 +56,39 @@ func TestSerialize(t *testing.T) { { "non slice of map response", rest.Serializer{}, + 200, []any{"a", "b", "test"}, "fields=a,test", `["a","b","test"]`, }, + { + "map with no matching field", + rest.Serializer{}, + 200, + map[string]any{"a": 1, "b": 2, "c": "test"}, + "fields=missing", // test individual fields trim + `{}`, + }, + { + ">299 response", + rest.Serializer{}, + 300, + map[string]any{"a": 1, "b": 2, "c": "test"}, + "fields=missing", + `{"a":1,"b":2,"c":"test"}`, + }, + { + "<200 response", + rest.Serializer{}, + 199, + map[string]any{"a": 1, "b": 2, "c": "test"}, + "fields=missing", + `{"a":1,"b":2,"c":"test"}`, + }, { "map with existing and missing fields", rest.Serializer{}, + 200, map[string]any{"a": 1, "b": 2, "c": "test"}, "fields=a, c ,missing", // test individual fields trim `{"a":1,"c":"test"}`, @@ -65,6 +96,7 @@ func TestSerialize(t *testing.T) { { "custom fields param", rest.Serializer{FieldsParam: "custom"}, + 200, map[string]any{"a": 1, "b": 2, "c": "test"}, "custom=a, c ,missing", // test individual fields trim `{"a":1,"c":"test"}`, @@ -72,6 +104,7 @@ func TestSerialize(t *testing.T) { { "slice of maps with existing and missing fields", rest.Serializer{}, + 200, []any{ map[string]any{"a": 11, "b": 11, "c": "test1"}, map[string]any{"a": 22, "b": 22, "c": "test2"}, @@ -82,6 +115,7 @@ func TestSerialize(t *testing.T) { { "nested fields with mixed map and any slices", rest.Serializer{}, + 200, map[string]any{ "a": 1, "b": 2, @@ -139,6 +173,7 @@ func TestSerialize(t *testing.T) { { "SearchResult", rest.Serializer{}, + 200, search.Result{ Page: 1, PerPage: 10, @@ -155,6 +190,7 @@ func TestSerialize(t *testing.T) { { "*SearchResult", rest.Serializer{}, + 200, &search.Result{ Page: 1, PerPage: 10, @@ -171,25 +207,27 @@ func TestSerialize(t *testing.T) { } for _, s := range scenarios { - e := echo.New() - req := httptest.NewRequest(http.MethodPost, "/", nil) - req.URL.RawQuery = s.query - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) + t.Run(s.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.URL.RawQuery = s.query + rec := httptest.NewRecorder() - if err := s.serializer.Serialize(c, s.data, ""); err != nil { - t.Errorf("[%s] Serialize failure: %v", s.name, err) - continue - } + e := echo.New() + c := e.NewContext(req, rec) + c.Response().Status = s.statusCode - rawBody, err := io.ReadAll(rec.Result().Body) - if err != nil { - t.Errorf("[%s] Failed to read request body: %v", s.name, err) - continue - } + if err := s.serializer.Serialize(c, s.data, ""); err != nil { + t.Fatalf("[%s] Serialize failure: %v", s.name, err) + } - if v := strings.TrimSpace(string(rawBody)); v != s.expected { - t.Fatalf("[%s] Expected body\n%v \ngot: \n%v", s.name, s.expected, v) - } + rawBody, err := io.ReadAll(rec.Result().Body) + if err != nil { + t.Fatalf("[%s] Failed to read request body: %v", s.name, err) + } + + if v := strings.TrimSpace(string(rawBody)); v != s.expected { + t.Fatalf("[%s] Expected body\n%v \ngot: \n%v", s.name, s.expected, v) + } + }) } }