From 43d8160dcd48d1c66ae900e088bdae08107dc7c2 Mon Sep 17 00:00:00 2001 From: "nicolas.revelant" Date: Wed, 9 Nov 2022 17:37:01 +0100 Subject: [PATCH] fix(handler): requested format with Accept header and suffix --- internal/api/net.go | 93 +++++----- internal/service/handler.go | 26 +-- .../service/mock_test/handler_get_test.go | 163 +++++++++++++++--- .../service/mock_test/runner_mock_test.go | 40 ++++- 4 files changed, 240 insertions(+), 82 deletions(-) diff --git a/internal/api/net.go b/internal/api/net.go index 23e92c56..fe89cf09 100644 --- a/internal/api/net.go +++ b/internal/api/net.go @@ -77,48 +77,59 @@ func RequestedFormat(r *http.Request) string { // Accept header value hdrAcceptValue := r.Header.Get("Accept") - - // Extension value - splittedPath := strings.Split(path, "/") - pathEnd := splittedPath[len(splittedPath)-1] - extension := "" - pos := strings.LastIndex(pathEnd, ".") - if pos != -1 { - extension = pathEnd[pos+1:] - } - - // TODO: case when extension and header Accept are provided at the same time - // -> Bad Request ? - - if extension != "" && hdrAcceptValue == "" { - switch extension { - case "json": - return FormatJSON - case "html": - return FormatHTML - case "txt": - return FormatText - case "svg": - return FormatSVG - default: - return extension - } - } - if hdrAcceptValue != "" { - switch hdrAcceptValue { - case ContentTypeJSON: - return FormatJSON - case ContentTypeSchemaJSON, ContentTypeSchemaPatchJSON: - return FormatSchemaJSON - case ContentTypeHTML: - return FormatHTML - case ContentTypeText: - return FormatText - case ContentTypeSVG: - return FormatSVG - default: - return hdrAcceptValue + // Accept header fields preferences: + // -> https://www.rfc-editor.org/rfc/rfc9110.html#section-12.5.1 + + // Examples: + // "Accept: application/json" + // "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8" + preferredFormats := strings.Split(hdrAcceptValue, ",") + + for _, value := range preferredFormats { + mediaTypeValue := value + lastSemicolon := strings.LastIndex(value, ";") + if lastSemicolon > 0 { + mediaTypeValue = value[:lastSemicolon] // 'q' quality parameter not used + } + switch mediaTypeValue { + case ContentTypeJSON: + return FormatJSON + case ContentTypeSchemaJSON, ContentTypeSchemaPatchJSON: + return FormatSchemaJSON + case ContentTypeHTML: + return FormatHTML + case ContentTypeText: + return FormatText + case ContentTypeSVG: + return FormatSVG + default: + return hdrAcceptValue + } + } + } else { + + // Extension value + splittedPath := strings.Split(path, "/") + pathEnd := splittedPath[len(splittedPath)-1] + extension := "" + pos := strings.LastIndex(pathEnd, ".") + if pos != -1 { + extension = pathEnd[pos+1:] + } + if extension != "" { + switch extension { + case "json": + return FormatJSON + case "html": + return FormatHTML + case "txt": + return FormatText + case "svg": + return FormatSVG + default: + return extension + } } } return FormatJSON diff --git a/internal/service/handler.go b/internal/service/handler.go index acf1bd9d..47fc1cc1 100644 --- a/internal/service/handler.go +++ b/internal/service/handler.go @@ -52,8 +52,10 @@ func InitRouter(basePath string) *mux.Router { Subrouter() addRoute(router, "/", handleRoot) + addRoute(router, "/home", handleRoot) addRoute(router, "/home{.fmt}", handleRoot) // consistent with pg_tileserv + addRoute(router, "/index", handleRoot) addRoute(router, "/index{.fmt}", handleRoot) addRoute(router, "/etags/decodestrong/{etag}", handleDecodeStrongEtag) @@ -449,20 +451,22 @@ func handleCollectionItems(w http.ResponseWriter, r *http.Request) *appError { if tbl == nil { return appErrorNotFound(err1, api.ErrMsgCollectionNotFound, name) } - param, err := createQueryParams(&reqParam, tbl.Columns, tbl.Srid) - if err != nil { - return appErrorBadRequest(err, err.Error()) - } + param, errQuery := createQueryParams(&reqParam, tbl.Columns, tbl.Srid) param.Filter = parseFilter(reqParam.Values, tbl.DbTypes) - ctx := r.Context() - switch format { - case api.FormatJSON: - return writeItemsJSON(ctx, w, name, param, urlBase) - case api.FormatHTML: - return writeItemsHTML(w, tbl, name, query, urlBase) + if errQuery == nil { + ctx := r.Context() + switch format { + case api.FormatJSON: + return writeItemsJSON(ctx, w, name, param, urlBase) + case api.FormatHTML: + return writeItemsHTML(w, tbl, name, query, urlBase) + default: + return appErrorNotAcceptable(nil, api.ErrMsgNotSupportedFormat, format) + } + } else { + return appErrorBadRequest(errQuery, api.ErrMsgInvalidQuery) } - return nil } func writeCreateItemSchemaJSON(ctx context.Context, w http.ResponseWriter, table *api.Table) *appError { diff --git a/internal/service/mock_test/handler_get_test.go b/internal/service/mock_test/handler_get_test.go index 66f1255d..97ba97b4 100644 --- a/internal/service/mock_test/handler_get_test.go +++ b/internal/service/mock_test/handler_get_test.go @@ -1,7 +1,7 @@ package mock_test /* - Copyright 2019 Crunchy Data Solutions, Inc. + Copyright 2022 Crunchy Data Solutions, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at @@ -11,6 +11,11 @@ package mock_test WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + + Date : September 2022 + Authors : Benoit De Mezzo (benoit dot de dot mezzo at oslandia dot com) + Jean-philippe Bazonnais (jean-philippe dot bazonnais at ign dot fr) + Nicolas Revelant (nicolas dot revelant at ign dot fr) */ import ( @@ -43,6 +48,135 @@ func (t *MockTests) TestRoot() { }) } +func (t *MockTests) TestGetFormatHandlingWithAcceptHeader() { + t.Test.Run("TestGetFormatHandlingWithAcceptHeader", func(t *testing.T) { + // This test targets the RequestedFormat() function from the net.go file + + // route / + "Accept: application/json" + jsonBody := checkRouteWithAcceptHeader(t, "/", "application/json", http.StatusOK, api.ContentTypeJSON) + jsonMap := new(map[string]interface{}) + errUnMarsh := json.Unmarshal(jsonBody, &jsonMap) + util.Assert(t, errUnMarsh == nil, fmt.Sprintf("%v", errUnMarsh)) + + checkRouteWithAcceptHeader(t, "/", "text/html", http.StatusOK, api.ContentTypeHTML) + + checkRouteWithAcceptHeader(t, "/api", "*/*", http.StatusOK, api.ContentTypeJSON) + + // Browser tests + // ------------- + // route /api + default Accept header from Firefox 92 + firefoxAcceptHdr := "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + checkRouteWithAcceptHeader(t, "/api", firefoxAcceptHdr, http.StatusOK, "") + + // route /api + default Accept header from Safari/Chrome + chromeAcceptHdr := "text/html, application/xhtml+xml, image/jxr, */*" + checkRouteWithAcceptHeader(t, "/api", chromeAcceptHdr, http.StatusOK, "") + + // route /api + default Accept header from Opera + operaAcceptHdr := "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + checkRouteWithAcceptHeader(t, "/api", operaAcceptHdr, http.StatusOK, "") + }) +} + +func (t *MockTests) TestGetFormatHandlingSuffix() { + t.Test.Run("TestGetFormatHandlingSuffix", func(t *testing.T) { + + // checking supported suffixes HTML and JSON, and missing suffix + checkRouteResponseFormat(t, "/home", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/home.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/home.json", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/index", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/index.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/index.json", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/api", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/api.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/api.json", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/collections", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/collections/mock_a", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/collections/mock_a.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/collections/mock_a.json", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/collections/mock_a/items", api.ContentTypeGeoJSON) + checkRouteResponseFormat(t, "/collections/mock_a/items.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/collections/mock_a/items.json", api.ContentTypeGeoJSON) + checkRouteResponseFormat(t, "/collections/mock_a/items/1", api.ContentTypeGeoJSON) + checkRouteResponseFormat(t, "/collections/mock_a/items/1.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/collections/mock_a/items/1.json", api.ContentTypeGeoJSON) + checkRouteResponseFormat(t, "/functions", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/functions.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/functions.json", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/functions/fun_a", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/functions/fun_a.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/functions/fun_a.json", api.ContentTypeJSON) + // TODO : /functions/{id}/items + checkRouteResponseFormat(t, "/conformance", api.ContentTypeJSON) + checkRouteResponseFormat(t, "/conformance.html", api.ContentTypeHTML) + checkRouteResponseFormat(t, "/conformance.json", api.ContentTypeJSON) + + }) +} + +func (t *MockTests) TestGetFormatHeaderAcceptUnsupportedMimeType() { + t.Test.Run("TestGetFormatHeaderAcceptUnsupportedMimeType", func(t *testing.T) { + + gifAccept := "image/gif" + xmlAccept := "application/xml" + var dummyAcceptHdr = make(http.Header) + dummyAcceptHdr.Add("Accept", "dummy/format") + // Root + checkRouteWithAcceptHeader(t, "/", gifAccept, http.StatusOK, api.ContentTypeJSON) + checkRouteWithAcceptHeader(t, "/", xmlAccept, http.StatusOK, api.ContentTypeJSON) + + // Api + checkRouteWithAcceptHeader(t, "/api", gifAccept, http.StatusOK, api.ContentTypeJSON) + + // Collections + checkRouteWithAcceptHeader(t, "/collections", gifAccept, http.StatusOK, api.ContentTypeJSON) + checkRouteWithAcceptHeader(t, "/collections/mock_a", gifAccept, http.StatusOK, api.ContentTypeJSON) + + // GET item(s) + hTest.DoRequestMethodStatus(t, "GET", "/collections/mock_a/items", nil, dummyAcceptHdr, http.StatusNotAcceptable) + hTest.DoRequestMethodStatus(t, "GET", "/collections/mock_a/items/1", nil, dummyAcceptHdr, http.StatusNotAcceptable) + + }) +} + +func (t *MockTests) TestGetFormatHeaderAcceptSupersedesSuffix() { + t.Test.Run("TestGetFormatHeaderAcceptSupersedesSuffix", func(t *testing.T) { + + htmlAccept := "text/html" + checkRouteWithAcceptHeader(t, "/api.json", htmlAccept, http.StatusOK, api.ContentTypeHTML) + checkRouteWithAcceptHeader(t, "/api.html", htmlAccept, http.StatusOK, api.ContentTypeHTML) + + }) +} + +func (t *MockTests) TestFeatureFormats() { + t.Test.Run("TestFeatureFormats", func(t *testing.T) { + + hTest.DoRequestStatus(t, "/collections/mock_a/items/1.dummyformat", http.StatusNotAcceptable) + + path := "/collections/mock_a/items/1" + + // From header Accept + var header = make(http.Header) + header.Add("Accept", "json") + resp := hTest.DoRequestMethodStatus(t, "GET", path, nil, header, http.StatusOK) + var geoJsonStruct api.GeojsonFeatureData + errUnMarsh := json.Unmarshal(hTest.ReadBody(resp), &geoJsonStruct) + util.Assert(t, errUnMarsh == nil, fmt.Sprintf("%v", errUnMarsh)) + + // TODO HTML + // TODO SVG + // TODO TEXT + + // From URL extension + resp2 := hTest.DoRequestStatus(t, "/collections/mock_a/items/1.json", http.StatusOK) + errUnMarsh2 := json.Unmarshal(hTest.ReadBody(resp2), &geoJsonStruct) + util.Assert(t, errUnMarsh == nil, fmt.Sprintf("%v", errUnMarsh2)) + + }) +} + func (t *MockTests) TestCollectionsResponse() { t.Test.Run("TestCollectionsResponse", func(t *testing.T) { path := "/collections" @@ -115,33 +249,6 @@ func (t *MockTests) TestCollectionItem() { }) } -func (t *MockTests) TestFeatureFormats() { - t.Test.Run("TestFeatureFormats", func(t *testing.T) { - - hTest.DoRequestStatus(t, "/collections/mock_a/items/1.dummyformat", http.StatusNotAcceptable) - - path := "/collections/mock_a/items/1" - - // From header Accept - var header = make(http.Header) - header.Add("Accept", "json") - resp := hTest.DoRequestMethodStatus(t, "GET", path, nil, header, http.StatusOK) - var geoJsonStruct api.GeojsonFeatureData - errUnMarsh := json.Unmarshal(hTest.ReadBody(resp), &geoJsonStruct) - util.Assert(t, errUnMarsh == nil, fmt.Sprintf("%v", errUnMarsh)) - - // TODO HTML - // TODO SVG - // TODO TEXT - - // From URL extension - resp2 := hTest.DoRequestStatus(t, "/collections/mock_a/items/1.json", http.StatusOK) - errUnMarsh2 := json.Unmarshal(hTest.ReadBody(resp2), &geoJsonStruct) - util.Assert(t, errUnMarsh == nil, fmt.Sprintf("%v", errUnMarsh2)) - - }) -} - func (t *MockTests) TestCollectionItemPropertiesEmpty() { t.Test.Run("TestCollectionItemPropertiesEmpty", func(t *testing.T) { rr := hTest.DoRequest(t, "/collections/mock_a/items/1?properties=") diff --git a/internal/service/mock_test/runner_mock_test.go b/internal/service/mock_test/runner_mock_test.go index c5ee6a72..dad90cc2 100644 --- a/internal/service/mock_test/runner_mock_test.go +++ b/internal/service/mock_test/runner_mock_test.go @@ -1,7 +1,7 @@ package mock_test /* - Copyright 2019 Crunchy Data Solutions, Inc. + Copyright 2022 Crunchy Data Solutions, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at @@ -11,14 +11,19 @@ package mock_test WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + Date : October 2022 + Authors : Jean-philippe Bazonnais (jean-philippe dot bazonnais at ign dot fr) + Nicolas Revelant (nicolas dot revelant at ign dot fr) */ import ( "encoding/json" "fmt" "io/ioutil" + "net/http" "os" "strconv" + "strings" "testing" "github.com/CrunchyData/pg_featureserv/internal/api" @@ -53,6 +58,12 @@ func TestRunnerHandlerMock(t *testing.T) { t.Run("GET", func(t *testing.T) { m := MockTests{Test: t} + m.TestRoot() + m.TestGetFormatHandlingWithAcceptHeader() + m.TestGetFormatHandlingSuffix() + m.TestGetFormatHeaderAcceptUnsupportedMimeType() + m.TestGetFormatHeaderAcceptSupersedesSuffix() + m.TestFeatureFormats() m.TestCollectionItem() m.TestCollectionItemsResponse() m.TestCollectionMissingItemsNotFound() @@ -61,7 +72,6 @@ func TestRunnerHandlerMock(t *testing.T) { m.TestCollectionResponse() m.TestCollectionsResponse() m.TestFeatureNotFound() - m.TestFeatureFormats() }) t.Run("CACHE AND ETAGS", func(t *testing.T) { m := MockTests{Test: t} @@ -293,3 +303,29 @@ func checkItem(t *testing.T, id int) []byte { return body } + +func checkRouteResponseFormat(t *testing.T, url string, expectedContentType string) { + + resp := hTest.DoRequestStatus(t, url, http.StatusOK) + respContentType := resp.Result().Header["Content-Type"][0] + util.Assert(t, respContentType == expectedContentType, fmt.Sprintf("wrong Content-Type: %s", respContentType)) + +} + +func checkRouteWithAcceptHeader(t *testing.T, url string, acceptValue string, expectedStatus int, expectedFormat string) []byte { + + var acceptHeader = make(http.Header) + acceptHeader.Add("Accept", acceptValue) + + resp := hTest.DoRequestMethodStatus(t, "GET", url, nil, acceptHeader, expectedStatus) + contentType := resp.Result().Header["Content-Type"][0] + + if expectedFormat != "" { + util.Assert(t, contentType == expectedFormat, fmt.Sprintf("Content-Type: %s", contentType)) + } else { + util.Assert(t, strings.Contains(acceptValue, contentType), fmt.Sprintf("Content-Type: %s", contentType)) + } + + body, _ := ioutil.ReadAll(resp.Body) + return body +}