From 9245e2593bdf368ae07bbb8ac9bdd889350be35b Mon Sep 17 00:00:00 2001 From: Manuel Imperiale Date: Tue, 23 Mar 2021 11:48:05 +0100 Subject: [PATCH] MF-1368 - Add internal http api package for query params reading (#1384) * MF-1368 - Add internal http api package for query params reading Signed-off-by: Manuel Imperiale * Fix comments Signed-off-by: Manuel Imperiale * Fix comments Signed-off-by: Manuel Imperiale * Fix reviews Signed-off-by: Manuel Imperiale * Use internal/http and internalhttp alias Signed-off-by: Manuel Imperiale * Mv errors types to pkg Signed-off-by: Manuel Imperiale * Use httputil/query.go and remove aliases Signed-off-by: Manuel Imperiale * Add blank lines after error definitions Signed-off-by: Manuel Imperiale * Add ReadBoolValueQuery Signed-off-by: Manuel Imperiale * Mv readBoolValueQuery Signed-off-by: Manuel Imperiale * User ErrNotFoundParam instead of pointer Signed-off-by: Manuel Imperiale * Revert ReadUintQuery to use default values Signed-off-by: Manuel Imperiale * Use default values for all query readers Signed-off-by: Manuel Imperiale --- auth/api/http/groups/transport.go | 105 +++----------- bootstrap/api/endpoint_test.go | 2 +- bootstrap/api/transport.go | 29 ++-- certs/api/transport.go | 53 ++----- consumers/notifiers/api/endpoint_test.go | 3 +- consumers/notifiers/api/transport.go | 50 ++----- internal/httputil/query.go | 106 ++++++++++++++ opcua/api/transport.go | 39 ++---- pkg/errors/types.go | 18 +++ pkg/sdk/go/channels.go | 4 +- pkg/sdk/go/channels_test.go | 167 +++++++++++------------ pkg/sdk/go/things.go | 4 +- pkg/sdk/go/things_test.go | 167 +++++++++++------------ provision/api/requests.go | 4 +- provision/api/requests_test.go | 2 +- provision/api/transport.go | 19 ++- readers/api/requests.go | 9 +- readers/api/transport.go | 122 ++++++----------- things/api/auth/http/transport.go | 15 +- things/api/things/http/requests.go | 18 ++- things/api/things/http/transport.go | 141 +++++-------------- things/mocks/channels.go | 4 +- things/mocks/things.go | 4 +- things/postgres/channels.go | 30 ++-- things/postgres/channels_test.go | 81 +++++------ things/postgres/things.go | 34 ++--- things/postgres/things_test.go | 69 +++++----- things/redis/streams_test.go | 8 +- things/service.go | 16 +-- things/service_test.go | 158 ++++++++++----------- twins/api/http/transport.go | 95 +++---------- users/api/endpoint_test.go | 15 +- users/api/transport.go | 122 +++++------------ 33 files changed, 712 insertions(+), 1001 deletions(-) create mode 100644 internal/httputil/query.go create mode 100644 pkg/errors/types.go diff --git a/auth/api/http/groups/transport.go b/auth/api/http/groups/transport.go index 031f16aee2..a34db01084 100644 --- a/auth/api/http/groups/transport.go +++ b/auth/api/http/groups/transport.go @@ -5,7 +5,6 @@ import ( "encoding/json" "io" "net/http" - "strconv" "strings" kitot "github.com/go-kit/kit/tracing/opentracing" @@ -13,6 +12,7 @@ import ( "github.com/go-zoo/bone" "github.com/mainflux/mainflux" "github.com/mainflux/mainflux/auth" + "github.com/mainflux/mainflux/internal/httputil" "github.com/mainflux/mainflux/pkg/errors" "github.com/opentracing/opentracing-go" ) @@ -23,6 +23,7 @@ var ( ) const ( + contentType = "application/json" maxNameSize = 254 offsetKey = "offset" limitKey = "limit" @@ -30,13 +31,12 @@ const ( metadataKey = "metadata" treeKey = "tree" groupType = "type" - contentType = "application/json" - - defOffset = 0 - defLimit = 10 - defLevel = 1 + defOffset = 0 + defLimit = 10 + defLevel = 1 ) +// MakeHandler returns a HTTP handler for API endpoints. func MakeHandler(svc auth.Service, mux *bone.Mux, tracer opentracing.Tracer) *bone.Mux { opts := []kithttp.ServerOption{ kithttp.ServerErrorEncoder(encodeError), @@ -127,17 +127,17 @@ func decodeListGroupsRequest(_ context.Context, r *http.Request) (interface{}, e return nil, auth.ErrUnsupportedContentType } - l, err := readUintQuery(r, levelKey, defLevel) + l, err := httputil.ReadUintQuery(r, levelKey, defLevel) if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } - t, err := readBoolQuery(r, treeKey) + t, err := httputil.ReadBoolQuery(r, treeKey, false) if err != nil { return nil, err } @@ -157,27 +157,27 @@ func decodeListMembersRequest(_ context.Context, r *http.Request) (interface{}, return nil, auth.ErrUnsupportedContentType } - o, err := readUintQuery(r, offsetKey, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } - tree, err := readBoolQuery(r, treeKey) + tree, err := httputil.ReadBoolQuery(r, treeKey, false) if err != nil { return nil, err } - t, err := readStringQuery(r, groupType) + t, err := httputil.ReadStringQuery(r, groupType, "") if err != nil { return nil, err } @@ -199,22 +199,22 @@ func decodeListMembershipsRequest(_ context.Context, r *http.Request) (interface return nil, auth.ErrUnsupportedContentType } - o, err := readUintQuery(r, offsetKey, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } - tree, err := readBoolQuery(r, treeKey) + tree, err := httputil.ReadBoolQuery(r, treeKey, false) if err != nil { return nil, err } @@ -282,75 +282,6 @@ func decodeAssignRequest(_ context.Context, r *http.Request) (interface{}, error return req, nil } -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return val, nil -} - -func readMetadataQuery(r *http.Request, key string) (map[string]interface{}, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return nil, errInvalidQueryParams - } - - if len(vals) == 0 { - return nil, nil - } - - m := make(map[string]interface{}) - err := json.Unmarshal([]byte(vals[0]), &m) - if err != nil { - return nil, errors.Wrap(errInvalidQueryParams, err) - } - - return m, nil -} - -func readBoolQuery(r *http.Request, key string) (bool, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return true, errInvalidQueryParams - } - - if len(vals) == 0 { - return false, nil - } - - b, err := strconv.ParseBool(vals[0]) - if err != nil { - return false, errInvalidQueryParams - } - - return b, nil -} - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidQueryParams - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} - func encodeResponse(_ context.Context, w http.ResponseWriter, response interface{}) error { w.Header().Set("Content-Type", contentType) diff --git a/bootstrap/api/endpoint_test.go b/bootstrap/api/endpoint_test.go index d1c655b737..a05f8bd12c 100644 --- a/bootstrap/api/endpoint_test.go +++ b/bootstrap/api/endpoint_test.go @@ -892,7 +892,7 @@ func TestList(t *testing.T) { res: configPage{}, }, { - desc: "view list with invalid query params", + desc: "view list with invalid query parameters", auth: validToken, url: fmt.Sprintf("%s?offset=%d&limit=%d&state=%d&key=%%", path, 10, 10, bootstrap.Inactive), status: http.StatusBadRequest, diff --git a/bootstrap/api/transport.go b/bootstrap/api/transport.go index f6732704a2..b16834edd4 100644 --- a/bootstrap/api/transport.go +++ b/bootstrap/api/transport.go @@ -27,12 +27,10 @@ const ( ) var ( - errUnsupportedContentType = errors.New("unsupported content type") - errInvalidQueryParams = errors.New("invalid query params") - errInvalidLimitParam = errors.New("invalid limit query param") - errInvalidOffsetParam = errors.New("invalid offset query param") - fullMatch = []string{"state", "external_id", "mainflux_id", "mainflux_key"} - partialMatch = []string{"name"} + errInvalidLimitParam = errors.New("invalid limit query param") + errInvalidOffsetParam = errors.New("invalid offset query param") + fullMatch = []string{"state", "external_id", "mainflux_id", "mainflux_key"} + partialMatch = []string{"name"} ) // MakeHandler returns a HTTP handler for API endpoints. @@ -110,7 +108,7 @@ func MakeHandler(svc bootstrap.Service, reader bootstrap.ConfigReader) http.Hand func decodeAddRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := addReq{token: r.Header.Get("Authorization")} @@ -123,7 +121,7 @@ func decodeAddRequest(_ context.Context, r *http.Request) (interface{}, error) { func decodeUpdateRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateReq{key: r.Header.Get("Authorization")} @@ -137,7 +135,7 @@ func decodeUpdateRequest(_ context.Context, r *http.Request) (interface{}, error func decodeUpdateCertRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateCertReq{ @@ -154,7 +152,7 @@ func decodeUpdateCertRequest(_ context.Context, r *http.Request) (interface{}, e func decodeUpdateConnRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateConnReq{key: r.Header.Get("Authorization")} @@ -169,7 +167,7 @@ func decodeUpdateConnRequest(_ context.Context, r *http.Request) (interface{}, e func decodeListRequest(_ context.Context, r *http.Request) (interface{}, error) { q, err := url.ParseQuery(r.URL.RawQuery) if err != nil { - return nil, errInvalidQueryParams + return nil, errors.ErrInvalidQueryParams } offset, limit, err := parsePagePrams(q) @@ -200,7 +198,7 @@ func decodeBootstrapRequest(_ context.Context, r *http.Request) (interface{}, er func decodeStateRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := changeStateReq{key: r.Header.Get("Authorization")} @@ -254,10 +252,11 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { case errors.Error: w.Header().Set("Content-Type", contentType) switch { - case errors.Contains(errorVal, errUnsupportedContentType): + case errors.Contains(errorVal, errors.ErrUnsupportedContentType): w.WriteHeader(http.StatusUnsupportedMediaType) - case errors.Contains(errorVal, errInvalidQueryParams): + case errors.Contains(errorVal, errors.ErrInvalidQueryParams): w.WriteHeader(http.StatusBadRequest) + case errors.Contains(errorVal, bootstrap.ErrMalformedEntity): w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, bootstrap.ErrNotFound): @@ -292,7 +291,7 @@ func parseUint(s string) (uint64, error) { ret, err := strconv.ParseUint(s, 10, 64) if err != nil { - return 0, errInvalidQueryParams + return 0, errors.ErrInvalidQueryParams } return ret, nil diff --git a/certs/api/transport.go b/certs/api/transport.go index df1d2f8c09..847b6b4276 100644 --- a/certs/api/transport.go +++ b/certs/api/transport.go @@ -8,32 +8,27 @@ import ( "encoding/json" "io" "net/http" - "strconv" - - "github.com/mainflux/mainflux/certs" - "github.com/mainflux/mainflux/pkg/errors" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/certs" + "github.com/mainflux/mainflux/internal/httputil" + "github.com/mainflux/mainflux/pkg/errors" "github.com/prometheus/client_golang/prometheus/promhttp" ) const ( contentType = "application/json" - offset = "offset" - limit = "limit" - - defOffset = 0 - defLimit = 10 + offsetKey = "offset" + limitKey = "limit" + defOffset = 0 + defLimit = 10 ) var ( - errUnsupportedContentType = errors.New("unsupported content type") - errUnauthorized = errors.New("missing or invalid credentials provided") - errInvalidQueryParams = errors.New("invalid query params") - errMalformedEntity = errors.New("malformed entity") - errConflict = errors.New("entity already exists") + errUnauthorized = errors.New("missing or invalid credentials provided") + errConflict = errors.New("entity already exists") ) // MakeHandler returns a HTTP handler for API endpoints. @@ -90,11 +85,11 @@ func encodeResponse(_ context.Context, w http.ResponseWriter, response interface } func decodeListCerts(_ context.Context, r *http.Request) (interface{}, error) { - l, err := readUintQuery(r, limit, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - o, err := readUintQuery(r, offset, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } @@ -107,28 +102,9 @@ func decodeListCerts(_ context.Context, r *http.Request) (interface{}, error) { return req, nil } -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return val, nil -} - func decodeCerts(_ context.Context, r *http.Request) (interface{}, error) { if r.Header.Get("Content-Type") != contentType { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := addCertsReq{token: r.Header.Get("Authorization")} @@ -152,9 +128,10 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.Header().Set("Content-Type", contentType) switch err { - case errUnsupportedContentType: + case errors.ErrUnsupportedContentType: w.WriteHeader(http.StatusUnsupportedMediaType) - case io.EOF, errMalformedEntity: + case io.EOF, errors.ErrMalformedEntity, + errors.ErrInvalidQueryParams: w.WriteHeader(http.StatusBadRequest) case errConflict: w.WriteHeader(http.StatusConflict) diff --git a/consumers/notifiers/api/endpoint_test.go b/consumers/notifiers/api/endpoint_test.go index 555346ec58..57c997a4d2 100644 --- a/consumers/notifiers/api/endpoint_test.go +++ b/consumers/notifiers/api/endpoint_test.go @@ -17,6 +17,7 @@ import ( notifiers "github.com/mainflux/mainflux/consumers/notifiers" httpapi "github.com/mainflux/mainflux/consumers/notifiers/api" "github.com/mainflux/mainflux/consumers/notifiers/mocks" + "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/pkg/uuid" "github.com/opentracing/opentracing-go/mocktracer" "github.com/stretchr/testify/assert" @@ -36,7 +37,7 @@ const ( var ( notFoundRes = toJSON(errorRes{notifiers.ErrNotFound.Error()}) unauthRes = toJSON(errorRes{notifiers.ErrUnauthorizedAccess.Error()}) - invalidRes = toJSON(errorRes{"invalid query parameters"}) + invalidRes = toJSON(errorRes{errors.ErrInvalidQueryParams.Error()}) ) type testRequest struct { diff --git a/consumers/notifiers/api/transport.go b/consumers/notifiers/api/transport.go index 1212aa7780..979fef0c1f 100644 --- a/consumers/notifiers/api/transport.go +++ b/consumers/notifiers/api/transport.go @@ -8,28 +8,21 @@ import ( "encoding/json" "io" "net/http" - "strconv" "strings" - notifiers "github.com/mainflux/mainflux/consumers/notifiers" - "github.com/mainflux/mainflux/pkg/errors" - kitot "github.com/go-kit/kit/tracing/opentracing" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + notifiers "github.com/mainflux/mainflux/consumers/notifiers" + "github.com/mainflux/mainflux/internal/httputil" + "github.com/mainflux/mainflux/pkg/errors" opentracing "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus/promhttp" ) const contentType = "application/json" -var ( - errMalformedEntity = errors.New("failed to decode request body") - errInvalidQueryParams = errors.New("invalid query parameters") - errUnsupportedContentType = errors.New("unsupported content type") -) - // MakeHandler returns a HTTP handler for API endpoints. func MakeHandler(svc notifiers.Service, tracer opentracing.Tracer) http.Handler { opts := []kithttp.ServerOption{ @@ -74,11 +67,11 @@ func MakeHandler(svc notifiers.Service, tracer opentracing.Tracer) http.Handler func decodeCreate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } var req createSubReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return nil, errors.Wrap(errMalformedEntity, err) + return nil, errors.Wrap(errors.ErrMalformedEntity, err) } req.token = r.Header.Get("Authorization") @@ -108,40 +101,21 @@ func decodeList(_ context.Context, r *http.Request) (interface{}, error) { req.contact = vals[0] } - offset, err := readUintQuery(r, "offset", 0) + offset, err := httputil.ReadUintQuery(r, "offset", 0) if err != nil { return listSubsReq{}, err } - req.offset = offset + req.offset = uint(offset) - limit, err := readUintQuery(r, "limit", 20) + limit, err := httputil.ReadUintQuery(r, "limit", 20) if err != nil { return listSubsReq{}, err } - req.limit = limit + req.limit = uint(limit) return req, nil } -func readUintQuery(r *http.Request, key string, def uint) (uint, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return uint(val), nil -} - func encodeResponse(_ context.Context, w http.ResponseWriter, response interface{}) error { if ar, ok := response.(mainflux.Response); ok { for k, v := range ar.Headers() { @@ -163,10 +137,10 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { case errors.Error: w.Header().Set("Content-Type", contentType) switch { - case errors.Contains(errorVal, errMalformedEntity), + case errors.Contains(errorVal, errors.ErrMalformedEntity), errors.Contains(errorVal, errInvalidContact), errors.Contains(errorVal, errInvalidTopic), - errors.Contains(errorVal, errInvalidQueryParams): + errors.Contains(errorVal, errors.ErrInvalidQueryParams): w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, notifiers.ErrNotFound), errors.Contains(errorVal, errNotFound): @@ -175,7 +149,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusUnauthorized) case errors.Contains(errorVal, notifiers.ErrConflict): w.WriteHeader(http.StatusConflict) - case errors.Contains(errorVal, errUnsupportedContentType): + case errors.Contains(errorVal, errors.ErrUnsupportedContentType): w.WriteHeader(http.StatusUnsupportedMediaType) case errors.Contains(errorVal, io.ErrUnexpectedEOF): w.WriteHeader(http.StatusBadRequest) diff --git a/internal/httputil/query.go b/internal/httputil/query.go new file mode 100644 index 0000000000..455df8dc00 --- /dev/null +++ b/internal/httputil/query.go @@ -0,0 +1,106 @@ +// Copyright (c) Mainflux +// SPDX-License-Identifier: Apache-2.0 + +package httputil + +import ( + "encoding/json" + "net/http" + "strconv" + + "github.com/go-zoo/bone" + "github.com/mainflux/mainflux/pkg/errors" +) + +// ReadUintQuery reads the value of uint64 http query parameters for a given key +func ReadUintQuery(r *http.Request, key string, def uint64) (uint64, error) { + vals := bone.GetQuery(r, key) + if len(vals) > 1 { + return 0, errors.ErrInvalidQueryParams + } + + if len(vals) == 0 { + return def, nil + } + + strval := vals[0] + val, err := strconv.ParseUint(strval, 10, 64) + if err != nil { + return 0, errors.ErrInvalidQueryParams + } + + return val, nil +} + +// ReadStringQuery reads the value of string http query parameters for a given key +func ReadStringQuery(r *http.Request, key string, def string) (string, error) { + vals := bone.GetQuery(r, key) + if len(vals) > 1 { + return "", errors.ErrInvalidQueryParams + } + + if len(vals) == 0 { + return def, nil + } + + return vals[0], nil +} + +// ReadMetadataQuery reads the value of json http query parameters for a given key +func ReadMetadataQuery(r *http.Request, key string, def map[string]interface{}) (map[string]interface{}, error) { + vals := bone.GetQuery(r, key) + if len(vals) > 1 { + return nil, errors.ErrInvalidQueryParams + } + + if len(vals) == 0 { + return def, nil + } + + m := make(map[string]interface{}) + err := json.Unmarshal([]byte(vals[0]), &m) + if err != nil { + return nil, errors.Wrap(errors.ErrInvalidQueryParams, err) + } + + return m, nil +} + +// ReadBoolQuery reads boolean query parameters in a given http request +func ReadBoolQuery(r *http.Request, key string, def bool) (bool, error) { + vals := bone.GetQuery(r, key) + if len(vals) > 1 { + return false, errors.ErrInvalidQueryParams + } + + if len(vals) == 0 { + return def, nil + } + + b, err := strconv.ParseBool(vals[0]) + if err != nil { + return false, errors.ErrInvalidQueryParams + } + + return b, nil +} + +// ReadFloatQuery reads the value of float64 http query parameters for a given key +func ReadFloatQuery(r *http.Request, key string, def float64) (float64, error) { + vals := bone.GetQuery(r, key) + if len(vals) > 1 { + return 0, errors.ErrInvalidQueryParams + } + + if len(vals) == 0 { + return def, nil + } + + fval := vals[0] + val, err := strconv.ParseFloat(fval, 64) + if err != nil { + return 0, errors.ErrInvalidQueryParams + } + + return val, nil +} diff --git a/opcua/api/transport.go b/opcua/api/transport.go index b116f3c96c..733a7cfd87 100644 --- a/opcua/api/transport.go +++ b/opcua/api/transport.go @@ -6,13 +6,14 @@ package api import ( "context" "encoding/json" - "errors" "net/http" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/internal/httputil" "github.com/mainflux/mainflux/opcua" + "github.com/mainflux/mainflux/pkg/errors" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -21,17 +22,10 @@ const ( serverParam = "server" namespaceParam = "namespace" identifierParam = "identifier" - - defOffset = 0 - defLimit = 10 - - defNamespace = "ns=0" // Standard root namespace - defIdentifier = "i=84" // Standard root identifier -) - -var ( - errUnsupportedContentType = errors.New("unsupported content type") - errInvalidQueryParams = errors.New("invalid query params") + defOffset = 0 + defLimit = 10 + defNamespace = "ns=0" // Standard root namespace + defIdentifier = "i=84" // Standard root identifier ) // MakeHandler returns a HTTP handler for API endpoints. @@ -56,17 +50,17 @@ func MakeHandler(svc opcua.Service) http.Handler { } func decodeBrowse(_ context.Context, r *http.Request) (interface{}, error) { - s, err := readStringQuery(r, serverParam) + s, err := httputil.ReadStringQuery(r, serverParam, "") if err != nil { return nil, err } - n, err := readStringQuery(r, namespaceParam) + n, err := httputil.ReadStringQuery(r, namespaceParam, "") if err != nil { return nil, err } - i, err := readStringQuery(r, identifierParam) + i, err := httputil.ReadStringQuery(r, identifierParam, "") if err != nil { return nil, err } @@ -109,22 +103,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { switch err { case opcua.ErrMalformedEntity: w.WriteHeader(http.StatusBadRequest) - case errInvalidQueryParams: + case errors.ErrInvalidQueryParams: w.WriteHeader(http.StatusBadRequest) default: w.WriteHeader(http.StatusInternalServerError) } } - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidQueryParams - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} diff --git a/pkg/errors/types.go b/pkg/errors/types.go new file mode 100644 index 0000000000..6dd668d7ad --- /dev/null +++ b/pkg/errors/types.go @@ -0,0 +1,18 @@ +// Copyright (c) Mainflux +// SPDX-License-Identifier: Apache-2.0 + +package errors + +var ( + // ErrUnsupportedContentType indicates unacceptable or lack of Content-Type + ErrUnsupportedContentType = New("unsupported content type") + + // ErrInvalidQueryParams indicates invalid query parameters + ErrInvalidQueryParams = New("invalid query parameters") + + // ErrNotFoundParam indicates that the parameter was not found in the query + ErrNotFoundParam = New("parameter not found in the query") + + // ErrMalformedEntity indicates a malformed entity specification + ErrMalformedEntity = New("malformed entity specification") +) diff --git a/pkg/sdk/go/channels.go b/pkg/sdk/go/channels.go index e947781eff..9b3ec3c946 100644 --- a/pkg/sdk/go/channels.go +++ b/pkg/sdk/go/channels.go @@ -110,8 +110,8 @@ func (sdk mfSDK) Channels(token string, offset, limit uint64, name string) (Chan return cp, nil } -func (sdk mfSDK) ChannelsByThing(token, thingID string, offset, limit uint64, connected bool) (ChannelsPage, error) { - endpoint := fmt.Sprintf("things/%s/channels?offset=%d&limit=%d&connected=%t", thingID, offset, limit, connected) +func (sdk mfSDK) ChannelsByThing(token, thingID string, offset, limit uint64, disconn bool) (ChannelsPage, error) { + endpoint := fmt.Sprintf("things/%s/channels?offset=%d&limit=%d&disconnected=%t", thingID, offset, limit, disconn) url := createURL(sdk.baseURL, sdk.thingsPrefix, endpoint) req, err := http.NewRequest(http.MethodGet, url, nil) diff --git a/pkg/sdk/go/channels_test.go b/pkg/sdk/go/channels_test.go index b51c5da273..d0238f6600 100644 --- a/pkg/sdk/go/channels_test.go +++ b/pkg/sdk/go/channels_test.go @@ -336,99 +336,92 @@ func TestChannelsByThing(t *testing.T) { } cases := []struct { - desc string - thing string - token string - offset uint64 - limit uint64 - connected bool - err error - response []sdk.Channel + desc string + thing string + token string + offset uint64 + limit uint64 + disconnected bool + err error + response []sdk.Channel }{ { - desc: "get a list of channels by thing", - thing: tid, - token: token, - offset: 0, - limit: 5, - connected: true, - err: nil, - response: channels[0:5], - }, - { - desc: "get a list of channels by thing with invalid token", - thing: tid, - token: wrongValue, - offset: 0, - limit: 5, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), - response: nil, - }, - { - desc: "get a list of channels by thing with empty token", - thing: tid, - token: "", - offset: 0, - limit: 5, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), - response: nil, - }, - { - desc: "get a list of channels by thing with zero limit", - thing: tid, - token: token, - offset: 0, - limit: 0, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of channels by thing with limit greater than max", - thing: tid, - token: token, - offset: 0, - limit: 110, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of channels by thing with offset greater than max", - thing: tid, - token: token, - offset: 110, - limit: 5, - connected: true, - err: nil, - response: []sdk.Channel{}, - }, - { - desc: "get a list of channels by thing with invalid args (zero limit) and invalid token", - thing: tid, - token: wrongValue, - offset: 0, - limit: 0, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of not connected channels by thing", - thing: tid, - token: token, - offset: 0, - limit: 100, - connected: false, - err: nil, - response: []sdk.Channel{channels[n-chsDiscoNum]}, + desc: "get a list of channels by thing", + thing: tid, + token: token, + offset: 0, + limit: 5, + err: nil, + response: channels[0:5], + }, + { + desc: "get a list of channels by thing with invalid token", + thing: tid, + token: wrongValue, + offset: 0, + limit: 5, + err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), + response: nil, + }, + { + desc: "get a list of channels by thing with empty token", + thing: tid, + token: "", + offset: 0, + limit: 5, + err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), + response: nil, + }, + { + desc: "get a list of channels by thing with zero limit", + thing: tid, + token: token, + offset: 0, + limit: 0, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of channels by thing with limit greater than max", + thing: tid, + token: token, + offset: 0, + limit: 110, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of channels by thing with offset greater than max", + thing: tid, + token: token, + offset: 110, + limit: 5, + err: nil, + response: []sdk.Channel{}, + }, + { + desc: "get a list of channels by thing with invalid args (zero limit) and invalid token", + thing: tid, + token: wrongValue, + offset: 0, + limit: 0, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of not connected channels by thing", + thing: tid, + token: token, + offset: 0, + limit: 100, + disconnected: true, + err: nil, + response: []sdk.Channel{channels[n-chsDiscoNum]}, }, } for _, tc := range cases { - page, err := mainfluxSDK.ChannelsByThing(tc.token, tc.thing, tc.offset, tc.limit, tc.connected) + page, err := mainfluxSDK.ChannelsByThing(tc.token, tc.thing, tc.offset, tc.limit, tc.disconnected) assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page.Channels, fmt.Sprintf("%s: expected response channel %s, got %s", tc.desc, tc.response, page.Channels)) } diff --git a/pkg/sdk/go/things.go b/pkg/sdk/go/things.go index 4f2386d18f..e720fccd9b 100644 --- a/pkg/sdk/go/things.go +++ b/pkg/sdk/go/things.go @@ -113,8 +113,8 @@ func (sdk mfSDK) Things(token string, offset, limit uint64, name string) (Things return tp, nil } -func (sdk mfSDK) ThingsByChannel(token, chanID string, offset, limit uint64, connected bool) (ThingsPage, error) { - endpoint := fmt.Sprintf("channels/%s/things?offset=%d&limit=%d&connected=%t", chanID, offset, limit, connected) +func (sdk mfSDK) ThingsByChannel(token, chanID string, offset, limit uint64, disconn bool) (ThingsPage, error) { + endpoint := fmt.Sprintf("channels/%s/things?offset=%d&limit=%d&disconnected=%t", chanID, offset, limit, disconn) url := createURL(sdk.baseURL, sdk.thingsPrefix, endpoint) req, err := http.NewRequest(http.MethodGet, url, nil) diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index b775855ccc..239f3cf356 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -382,98 +382,91 @@ func TestThingsByChannel(t *testing.T) { } cases := []struct { - desc string - channel string - token string - offset uint64 - limit uint64 - connected bool - err error - response []sdk.Thing + desc string + channel string + token string + offset uint64 + limit uint64 + disconnected bool + err error + response []sdk.Thing }{ { - desc: "get a list of things by channel", - channel: cid, - token: token, - offset: 0, - limit: 5, - connected: true, - err: nil, - response: things[0:5], - }, - { - desc: "get a list of things by channel with invalid token", - channel: cid, - token: wrongValue, - offset: 0, - limit: 5, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), - response: nil, - }, - { - desc: "get a list of things by channel with empty token", - channel: cid, - token: "", - offset: 0, - limit: 5, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), - response: nil, - }, - { - desc: "get a list of things by channel with zero limit", - channel: cid, - token: token, - offset: 0, - limit: 0, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of things by channel with limit greater than max", - channel: cid, - token: token, - offset: 0, - limit: 110, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of things by channel with offset greater than max", - channel: cid, - token: token, - offset: 110, - limit: 5, - connected: true, - err: nil, - response: []sdk.Thing{}, - }, - { - desc: "get a list of things by channel with invalid args (zero limit) and invalid token", - channel: cid, - token: wrongValue, - offset: 0, - limit: 0, - connected: true, - err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), - response: nil, - }, - { - desc: "get a list of not connected things by channel", - channel: cid, - token: token, - offset: 0, - limit: 100, - connected: false, - err: nil, - response: []sdk.Thing{things[n-thsDiscoNum]}, + desc: "get a list of things by channel", + channel: cid, + token: token, + offset: 0, + limit: 5, + err: nil, + response: things[0:5], + }, + { + desc: "get a list of things by channel with invalid token", + channel: cid, + token: wrongValue, + offset: 0, + limit: 5, + err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), + response: nil, + }, + { + desc: "get a list of things by channel with empty token", + channel: cid, + token: "", + offset: 0, + limit: 5, + err: createError(sdk.ErrFailedFetch, http.StatusUnauthorized), + response: nil, + }, + { + desc: "get a list of things by channel with zero limit", + channel: cid, + token: token, + offset: 0, + limit: 0, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of things by channel with limit greater than max", + channel: cid, + token: token, + offset: 0, + limit: 110, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of things by channel with offset greater than max", + channel: cid, + token: token, + offset: 110, + limit: 5, + err: nil, + response: []sdk.Thing{}, + }, + { + desc: "get a list of things by channel with invalid args (zero limit) and invalid token", + channel: cid, + token: wrongValue, + offset: 0, + limit: 0, + err: createError(sdk.ErrFailedFetch, http.StatusBadRequest), + response: nil, + }, + { + desc: "get a list of not connected things by channel", + channel: cid, + token: token, + offset: 0, + limit: 100, + disconnected: true, + err: nil, + response: []sdk.Thing{things[n-thsDiscoNum]}, }, } for _, tc := range cases { - page, err := mainfluxSDK.ThingsByChannel(tc.token, tc.channel, tc.offset, tc.limit, tc.connected) + page, err := mainfluxSDK.ThingsByChannel(tc.token, tc.channel, tc.offset, tc.limit, tc.disconnected) assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err)) assert.Equal(t, tc.response, page.Things, fmt.Sprintf("%s: expected response channel %s, got %s", tc.desc, tc.response, page.Things)) } diff --git a/provision/api/requests.go b/provision/api/requests.go index d0ee06945f..192919306b 100644 --- a/provision/api/requests.go +++ b/provision/api/requests.go @@ -1,5 +1,7 @@ package api +import "github.com/mainflux/mainflux/pkg/errors" + type provisionReq struct { token string Name string `json:"name"` @@ -9,7 +11,7 @@ type provisionReq struct { func (req provisionReq) validate() error { if req.ExternalID == "" || req.ExternalKey == "" { - return errMalformedEntity + return errors.ErrMalformedEntity } return nil } diff --git a/provision/api/requests_test.go b/provision/api/requests_test.go index a705630132..d2ad993c7b 100644 --- a/provision/api/requests_test.go +++ b/provision/api/requests_test.go @@ -21,7 +21,7 @@ func TestValidate(t *testing.T) { err: nil, }, "external id for device empty": { - err: errMalformedEntity, + err: errors.ErrMalformedEntity, }, } diff --git a/provision/api/transport.go b/provision/api/transport.go index ca0ace3414..3fcc1987ff 100644 --- a/provision/api/transport.go +++ b/provision/api/transport.go @@ -6,12 +6,11 @@ import ( "io" "net/http" - "github.com/mainflux/mainflux/pkg/errors" - "github.com/mainflux/mainflux/provision" - kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/pkg/errors" + "github.com/mainflux/mainflux/provision" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -20,10 +19,8 @@ const ( ) var ( - errUnsupportedContentType = errors.New("unsupported content type") - errUnauthorized = errors.New("missing or invalid credentials provided") - errMalformedEntity = errors.New("malformed entity") - errConflict = errors.New("entity already exists") + errUnauthorized = errors.New("missing or invalid credentials provided") + errConflict = errors.New("entity already exists") ) // MakeHandler returns a HTTP handler for API endpoints. @@ -75,7 +72,7 @@ func encodeResponse(_ context.Context, w http.ResponseWriter, response interface func decodeProvisionRequest(_ context.Context, r *http.Request) (interface{}, error) { if r.Header.Get("Content-Type") != contentType { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := provisionReq{token: r.Header.Get("Authorization")} @@ -88,7 +85,7 @@ func decodeProvisionRequest(_ context.Context, r *http.Request) (interface{}, er func decodeMappingRequest(_ context.Context, r *http.Request) (interface{}, error) { if r.Header.Get("Content-Type") != contentType { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := mappingReq{token: r.Header.Get("Authorization")} @@ -100,9 +97,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.Header().Set("Content-Type", contentType) switch err { - case errUnsupportedContentType: + case errors.ErrUnsupportedContentType: w.WriteHeader(http.StatusUnsupportedMediaType) - case io.EOF, errMalformedEntity: + case io.EOF, errors.ErrMalformedEntity: w.WriteHeader(http.StatusBadRequest) case errConflict: w.WriteHeader(http.StatusConflict) diff --git a/readers/api/requests.go b/readers/api/requests.go index 186604ba30..86e7f1098e 100644 --- a/readers/api/requests.go +++ b/readers/api/requests.go @@ -3,7 +3,10 @@ package api -import "github.com/mainflux/mainflux/readers" +import ( + "github.com/mainflux/mainflux/pkg/errors" + "github.com/mainflux/mainflux/readers" +) type apiReq interface { validate() error @@ -16,7 +19,7 @@ type listMessagesReq struct { func (req listMessagesReq) validate() error { if req.pageMeta.Limit < 1 || req.pageMeta.Offset < 0 { - return errInvalidRequest + return errors.ErrInvalidQueryParams } if req.pageMeta.Comparator != "" && req.pageMeta.Comparator != readers.EqualKey && @@ -24,7 +27,7 @@ func (req listMessagesReq) validate() error { req.pageMeta.Comparator != readers.LowerThanEqualKey && req.pageMeta.Comparator != readers.GreaterThanKey && req.pageMeta.Comparator != readers.GreaterThanEqualKey { - return errInvalidRequest + return errors.ErrInvalidQueryParams } return nil diff --git a/readers/api/transport.go b/readers/api/transport.go index b64e515e64..0f85330ab7 100644 --- a/readers/api/transport.go +++ b/readers/api/transport.go @@ -13,6 +13,7 @@ import ( kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/internal/httputil" "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/readers" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -21,17 +22,27 @@ import ( ) const ( - contentType = "application/json" - defLimit = 10 - defOffset = 0 - format = "format" - defFormat = "messages" + contentType = "application/json" + offsetKey = "offset" + limitKey = "limit" + formatKey = "format" + subtopicKey = "subtopic" + publisherKey = "publisher" + protocolKey = "protocol" + nameKey = "name" + valueKey = "v" + stringValueKey = "vs" + dataValueKey = "vd" + comparatorKey = "comparator" + fromKey = "from" + toKey = "to" + defLimit = 10 + defOffset = 0 + defFormat = "messages" ) var ( - errInvalidRequest = errors.New("received invalid request") errUnauthorizedAccess = errors.New("missing or invalid credentials provided") - errNotInQuery = errors.New("parameter missing in the query") auth mainflux.ThingsServiceClient ) @@ -60,77 +71,74 @@ func MakeHandler(svc readers.MessageRepository, tc mainflux.ThingsServiceClient, func decodeList(_ context.Context, r *http.Request) (interface{}, error) { chanID := bone.GetValue(r, "chanID") if chanID == "" { - return nil, errInvalidRequest + return nil, errors.ErrInvalidQueryParams } if err := authorize(r, chanID); err != nil { return nil, err } - offset, err := readUintQuery(r, "offset", defOffset) + offset, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - limit, err := readUintQuery(r, "limit", defLimit) + limit, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - format, err := readStringQuery(r, "format") + format, err := httputil.ReadStringQuery(r, formatKey, defFormat) if err != nil { return nil, err } - if format != "" { - format = defFormat - } - subtopic, err := readStringQuery(r, "subtopic") + subtopic, err := httputil.ReadStringQuery(r, subtopicKey, "") if err != nil { return nil, err } - publisher, err := readStringQuery(r, "publisher") + publisher, err := httputil.ReadStringQuery(r, publisherKey, "") if err != nil { return nil, err } - protocol, err := readStringQuery(r, "protocol") + protocol, err := httputil.ReadStringQuery(r, protocolKey, "") if err != nil { return nil, err } - name, err := readStringQuery(r, "name") + name, err := httputil.ReadStringQuery(r, nameKey, "") if err != nil { return nil, err } - v, err := readFloatQuery(r, "v") + v, err := httputil.ReadFloatQuery(r, valueKey, 0) if err != nil { return nil, err } - comparator, err := readStringQuery(r, "comparator") + comparator, err := httputil.ReadStringQuery(r, comparatorKey, "") if err != nil { return nil, err } - vs, err := readStringQuery(r, "vs") + vs, err := httputil.ReadStringQuery(r, stringValueKey, "") if err != nil { return nil, err } - vd, err := readStringQuery(r, "vd") + vd, err := httputil.ReadStringQuery(r, dataValueKey, "") if err != nil { return nil, err } - from, err := readFloatQuery(r, "from") + from, err := httputil.ReadFloatQuery(r, fromKey, 0) if err != nil { return nil, err } - to, err := readFloatQuery(r, "to") + to, err := httputil.ReadFloatQuery(r, toKey, 0) if err != nil { return nil, err } @@ -154,9 +162,8 @@ func decodeList(_ context.Context, r *http.Request) (interface{}, error) { }, } - vb, err := readBoolQuery(r, "vb") - // Check if vb is in the query - if err != nil && err != errNotInQuery { + vb, err := readBoolValueQuery(r, "vb") + if err != nil && err != errors.ErrNotFoundParam { return nil, err } if err == nil { @@ -187,7 +194,7 @@ func encodeResponse(_ context.Context, w http.ResponseWriter, response interface func encodeError(_ context.Context, err error, w http.ResponseWriter) { switch { case errors.Contains(err, nil): - case errors.Contains(err, errInvalidRequest): + case errors.Contains(err, errors.ErrInvalidQueryParams): w.WriteHeader(http.StatusBadRequest) case errors.Contains(err, errUnauthorizedAccess): w.WriteHeader(http.StatusForbidden) @@ -224,70 +231,19 @@ func authorize(r *http.Request, chanID string) error { return nil } -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidRequest - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidRequest - } - - return val, nil -} - -func readFloatQuery(r *http.Request, key string) (float64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidRequest - } - - if len(vals) == 0 { - return 0, nil - } - - fval := vals[0] - val, err := strconv.ParseFloat(fval, 64) - if err != nil { - return 0, errInvalidRequest - } - - return val, nil -} - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidRequest - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} - -func readBoolQuery(r *http.Request, key string) (bool, error) { +func readBoolValueQuery(r *http.Request, key string) (bool, error) { vals := bone.GetQuery(r, key) if len(vals) > 1 { - return false, errInvalidRequest + return false, errors.ErrInvalidQueryParams } if len(vals) == 0 { - return false, errNotInQuery + return false, errors.ErrNotFoundParam } b, err := strconv.ParseBool(vals[0]) if err != nil { - return false, errInvalidRequest + return false, errors.ErrInvalidQueryParams } return b, nil diff --git a/things/api/auth/http/transport.go b/things/api/auth/http/transport.go index 80de696ea8..68b054cafe 100644 --- a/things/api/auth/http/transport.go +++ b/things/api/auth/http/transport.go @@ -10,20 +10,17 @@ import ( "net/http" "strings" - "github.com/mainflux/mainflux/pkg/errors" - kitot "github.com/go-kit/kit/tracing/opentracing" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/things" opentracing "github.com/opentracing/opentracing-go" ) const contentType = "application/json" -var errUnsupportedContentType = errors.New("unsupported content type") - // MakeHandler returns a HTTP handler for auth API endpoints. func MakeHandler(tracer opentracing.Tracer, svc things.Service) http.Handler { opts := []kithttp.ServerOption{ @@ -58,7 +55,7 @@ func MakeHandler(tracer opentracing.Tracer, svc things.Service) http.Handler { func decodeIdentify(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := identifyReq{} @@ -71,7 +68,7 @@ func decodeIdentify(_ context.Context, r *http.Request) (interface{}, error) { func decodeCanAccessByKey(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := canAccessByKeyReq{ @@ -86,7 +83,7 @@ func decodeCanAccessByKey(_ context.Context, r *http.Request) (interface{}, erro func decodeCanAccessByID(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := canAccessByIDReq{ @@ -125,8 +122,10 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusNotFound) case things.ErrEntityConnected: w.WriteHeader(http.StatusForbidden) - case errUnsupportedContentType: + + case errors.ErrUnsupportedContentType: w.WriteHeader(http.StatusUnsupportedMediaType) + case io.ErrUnexpectedEOF: w.WriteHeader(http.StatusBadRequest) case io.EOF: diff --git a/things/api/things/http/requests.go b/things/api/things/http/requests.go index c4a9bb86dc..4459649899 100644 --- a/things/api/things/http/requests.go +++ b/things/api/things/http/requests.go @@ -8,8 +8,14 @@ import ( "github.com/mainflux/mainflux/things" ) -const maxLimitSize = 100 -const maxNameSize = 1024 +const ( + maxLimitSize = 100 + maxNameSize = 1024 + nameOrder = "name" + idOrder = "id" + ascDir = "asc" + descDir = "desc" +) type createThingReq struct { token string @@ -198,12 +204,12 @@ func (req *listResourcesReq) validate() error { } if req.pageMetadata.Order != "" && - req.pageMetadata.Order != "name" && req.pageMetadata.Order != "id" { + req.pageMetadata.Order != nameOrder && req.pageMetadata.Order != idOrder { return things.ErrMalformedEntity } if req.pageMetadata.Dir != "" && - req.pageMetadata.Dir != "asc" && req.pageMetadata.Dir != "desc" { + req.pageMetadata.Dir != ascDir && req.pageMetadata.Dir != descDir { return things.ErrMalformedEntity } @@ -230,12 +236,12 @@ func (req listByConnectionReq) validate() error { } if req.pageMetadata.Order != "" && - req.pageMetadata.Order != "name" && req.pageMetadata.Order != "id" { + req.pageMetadata.Order != nameOrder && req.pageMetadata.Order != idOrder { return things.ErrMalformedEntity } if req.pageMetadata.Dir != "" && - req.pageMetadata.Dir != "asc" && req.pageMetadata.Dir != "desc" { + req.pageMetadata.Dir != ascDir && req.pageMetadata.Dir != descDir { return things.ErrMalformedEntity } diff --git a/things/api/things/http/transport.go b/things/api/things/http/transport.go index b559a4d1fa..07fc45491d 100644 --- a/things/api/things/http/transport.go +++ b/things/api/things/http/transport.go @@ -8,7 +8,6 @@ import ( "encoding/json" "io" "net/http" - "strconv" "strings" kitot "github.com/go-kit/kit/tracing/opentracing" @@ -16,6 +15,7 @@ import ( "github.com/go-zoo/bone" "github.com/mainflux/mainflux" "github.com/mainflux/mainflux/auth" + "github.com/mainflux/mainflux/internal/httputil" "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/things" opentracing "github.com/opentracing/opentracing-go" @@ -30,15 +30,9 @@ const ( orderKey = "order" dirKey = "dir" metadataKey = "metadata" - connKey = "connected" - - defOffset = 0 - defLimit = 10 -) - -var ( - errUnsupportedContentType = errors.New("unsupported content type") - errInvalidQueryParams = errors.New("invalid query params") + disconnKey = "disconnected" + defOffset = 0 + defLimit = 10 ) // MakeHandler returns a HTTP handler for API endpoints. @@ -197,7 +191,7 @@ func MakeHandler(tracer opentracing.Tracer, svc things.Service) http.Handler { func decodeThingCreation(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := createThingReq{token: r.Header.Get("Authorization")} @@ -210,7 +204,7 @@ func decodeThingCreation(_ context.Context, r *http.Request) (interface{}, error func decodeThingsCreation(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := createThingsReq{token: r.Header.Get("Authorization")} @@ -223,7 +217,7 @@ func decodeThingsCreation(_ context.Context, r *http.Request) (interface{}, erro func decodeThingUpdate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateThingReq{ @@ -239,7 +233,7 @@ func decodeThingUpdate(_ context.Context, r *http.Request) (interface{}, error) func decodeKeyUpdate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateKeyReq{ @@ -255,7 +249,7 @@ func decodeKeyUpdate(_ context.Context, r *http.Request) (interface{}, error) { func decodeChannelCreation(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := createChannelReq{token: r.Header.Get("Authorization")} @@ -268,7 +262,7 @@ func decodeChannelCreation(_ context.Context, r *http.Request) (interface{}, err func decodeChannelsCreation(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := createChannelsReq{token: r.Header.Get("Authorization")} @@ -282,7 +276,7 @@ func decodeChannelsCreation(_ context.Context, r *http.Request) (interface{}, er func decodeChannelUpdate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateChannelReq{ @@ -306,32 +300,32 @@ func decodeView(_ context.Context, r *http.Request) (interface{}, error) { } func decodeList(_ context.Context, r *http.Request) (interface{}, error) { - o, err := readUintQuery(r, offsetKey, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - n, err := readStringQuery(r, nameKey) + n, err := httputil.ReadStringQuery(r, nameKey, "") if err != nil { return nil, err } - or, err := readStringQuery(r, orderKey) + or, err := httputil.ReadStringQuery(r, orderKey, "") if err != nil { return nil, err } - d, err := readStringQuery(r, dirKey) + d, err := httputil.ReadStringQuery(r, dirKey, "") if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } @@ -361,27 +355,27 @@ func decodeListByMetadata(_ context.Context, r *http.Request) (interface{}, erro } func decodeListByConnection(_ context.Context, r *http.Request) (interface{}, error) { - o, err := readUintQuery(r, offsetKey, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - c, err := readBoolQuery(r, connKey) + c, err := httputil.ReadBoolQuery(r, disconnKey, false) if err != nil { return nil, err } - or, err := readStringQuery(r, orderKey) + or, err := httputil.ReadStringQuery(r, orderKey, "") if err != nil { return nil, err } - d, err := readStringQuery(r, dirKey) + d, err := httputil.ReadStringQuery(r, dirKey, "") if err != nil { return nil, err } @@ -390,11 +384,11 @@ func decodeListByConnection(_ context.Context, r *http.Request) (interface{}, er token: r.Header.Get("Authorization"), id: bone.GetValue(r, "id"), pageMetadata: things.PageMetadata{ - Offset: o, - Limit: l, - Connected: c, - Order: or, - Dir: d, + Offset: o, + Limit: l, + Disconnected: c, + Order: or, + Dir: d, }, } @@ -413,7 +407,7 @@ func decodeConnection(_ context.Context, r *http.Request) (interface{}, error) { func decodeCreateConnections(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := createConnectionsReq{token: r.Header.Get("Authorization")} @@ -425,17 +419,17 @@ func decodeCreateConnections(_ context.Context, r *http.Request) (interface{}, e } func decodeListThingsGroupRequest(_ context.Context, r *http.Request) (interface{}, error) { - o, err := readUintQuery(r, offsetKey, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } @@ -479,9 +473,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(errorVal, things.ErrEntityConnected): w.WriteHeader(http.StatusUnauthorized) - case errors.Contains(errorVal, errInvalidQueryParams): + case errors.Contains(errorVal, errors.ErrInvalidQueryParams): w.WriteHeader(http.StatusBadRequest) - case errors.Contains(errorVal, errUnsupportedContentType): + case errors.Contains(errorVal, errors.ErrUnsupportedContentType): w.WriteHeader(http.StatusUnsupportedMediaType) case errors.Contains(errorVal, things.ErrMalformedEntity): @@ -523,72 +517,3 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusInternalServerError) } } - -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return val, nil -} - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidQueryParams - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} - -func readMetadataQuery(r *http.Request, key string) (map[string]interface{}, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return nil, errInvalidQueryParams - } - - if len(vals) == 0 { - return nil, nil - } - - m := make(map[string]interface{}) - err := json.Unmarshal([]byte(vals[0]), &m) - if err != nil { - return nil, errors.Wrap(errInvalidQueryParams, err) - } - - return m, nil -} - -func readBoolQuery(r *http.Request, key string) (bool, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return true, errInvalidQueryParams - } - - if len(vals) == 0 { - return true, nil - } - - b, err := strconv.ParseBool(vals[0]) - if err != nil { - return true, errInvalidQueryParams - } - - return b, nil -} diff --git a/things/mocks/channels.go b/things/mocks/channels.go index d859d19716..01bc52fe23 100644 --- a/things/mocks/channels.go +++ b/things/mocks/channels.go @@ -133,8 +133,8 @@ func (crm *channelRepositoryMock) RetrieveByThing(_ context.Context, owner, thID var chs []things.Channel // Append connected or not connected channels - switch pm.Connected { - case true: + switch pm.Disconnected { + case false: for _, co := range crm.cconns[thID] { id, _ := strconv.ParseUint(co.ID, 10, 64) if id >= first && id < last { diff --git a/things/mocks/things.go b/things/mocks/things.go index 3c07ccad06..3df7932bd7 100644 --- a/things/mocks/things.go +++ b/things/mocks/things.go @@ -202,8 +202,8 @@ func (trm *thingRepositoryMock) RetrieveByChannel(_ context.Context, owner, chID var ths []things.Thing // Append connected or not connected channels - switch pm.Connected { - case true: + switch pm.Disconnected { + case false: for _, co := range trm.tconns[chID] { id, _ := strconv.ParseUint(co.ID, 10, 64) if id >= first && id < last { diff --git a/things/postgres/channels.go b/things/postgres/channels.go index 3ae32b43ec..8c1450a660 100644 --- a/things/postgres/channels.go +++ b/things/postgres/channels.go @@ -187,22 +187,8 @@ func (cr channelRepository) RetrieveByThing(ctx context.Context, owner, thID str } var q, qc string - switch pm.Connected { + switch pm.Disconnected { case true: - q = fmt.Sprintf(`SELECT id, name, metadata FROM channels ch - INNER JOIN connections conn - ON ch.id = conn.channel_id - WHERE ch.owner = :owner AND conn.thing_id = :thing - ORDER BY %s %s - LIMIT :limit - OFFSET :offset;`, oq, dq) - - qc = `SELECT COUNT(*) - FROM channels ch - INNER JOIN connections conn - ON ch.id = conn.channel_id - WHERE ch.owner = $1 AND conn.thing_id = $2` - default: q = fmt.Sprintf(`SELECT id, name, metadata FROM channels ch WHERE ch.owner = :owner AND ch.id NOT IN @@ -221,6 +207,20 @@ func (cr channelRepository) RetrieveByThing(ctx context.Context, owner, thID str INNER JOIN connections conn ON ch.id = conn.channel_id WHERE ch.owner = $1 AND conn.thing_id = $2);` + default: + q = fmt.Sprintf(`SELECT id, name, metadata FROM channels ch + INNER JOIN connections conn + ON ch.id = conn.channel_id + WHERE ch.owner = :owner AND conn.thing_id = :thing + ORDER BY %s %s + LIMIT :limit + OFFSET :offset;`, oq, dq) + + qc = `SELECT COUNT(*) + FROM channels ch + INNER JOIN connections conn + ON ch.id = conn.channel_id + WHERE ch.owner = $1 AND conn.thing_id = $2` } params := map[string]interface{}{ diff --git a/things/postgres/channels_test.go b/things/postgres/channels_test.go index 8102d2cd72..0b0843b5ca 100644 --- a/things/postgres/channels_test.go +++ b/things/postgres/channels_test.go @@ -166,8 +166,7 @@ func TestSingleChannelRetrieval(t *testing.T) { } chs, _ := chanRepo.Save(context.Background(), ch) ch.ID = chs[0].ID - err = chanRepo.Connect(context.Background(), email, []string{ch.ID}, []string{th.ID}) - require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) + chanRepo.Connect(context.Background(), email, []string{ch.ID}, []string{th.ID}) nonexistentChanID, err := idProvider.ID() require.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err)) @@ -420,9 +419,8 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: n - chsDisconNum, }, @@ -430,9 +428,8 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: n / 2, - Limit: n, - Connected: true, + Offset: n / 2, + Limit: n, }, size: (n / 2) - chsDisconNum, }, @@ -440,9 +437,8 @@ func TestRetrieveByThing(t *testing.T) { owner: wrongValue, thID: thID, pageMetadata: things.PageMetadata{ - Offset: n / 2, - Limit: n, - Connected: true, + Offset: n / 2, + Limit: n, }, size: 0, }, @@ -450,9 +446,8 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: nonexistentThingID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, }, @@ -460,9 +455,8 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: wrongValue, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, err: things.ErrNotFound, @@ -471,9 +465,9 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, + Offset: 0, + Limit: n, + Disconnected: true, }, size: chsDisconNum, }, @@ -481,11 +475,10 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "asc", }, size: n - chsDisconNum, }, @@ -493,11 +486,11 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "asc", }, size: chsDisconNum, }, @@ -505,11 +498,10 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "desc", }, size: n - chsDisconNum, }, @@ -517,11 +509,11 @@ func TestRetrieveByThing(t *testing.T) { owner: email, thID: thID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "desc", }, size: chsDisconNum, }, @@ -678,8 +670,7 @@ func TestDisconnect(t *testing.T) { }) require.Nil(t, err, fmt.Sprintf("unexpected error: %s\n", err)) chID = chs[0].ID - err = chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) - require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) + chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) nonexistentThingID, err := idProvider.ID() require.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err)) @@ -765,8 +756,7 @@ func TestHasThing(t *testing.T) { }) require.Nil(t, err, fmt.Sprintf("unexpected error: %s\n", err)) chID = chs[0].ID - err = chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) - require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) + chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) nonexistentChanID, err := idProvider.ID() require.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err)) @@ -841,8 +831,7 @@ func TestHasThingByID(t *testing.T) { }) require.Nil(t, err, fmt.Sprintf("unexpected error: %s\n", err)) chID = chs[0].ID - err = chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) - require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err)) + chanRepo.Connect(context.Background(), email, []string{chID}, []string{thID}) nonexistentChanID, err := idProvider.ID() require.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err)) diff --git a/things/postgres/things.go b/things/postgres/things.go index 1e7e4c049e..42caa0d716 100644 --- a/things/postgres/things.go +++ b/things/postgres/things.go @@ -192,7 +192,7 @@ func (tr thingRepository) RetrieveByIDs(ctx context.Context, thingIDs []string, return things.Page{}, errors.Wrap(things.ErrSelectEntity, err) } - q := fmt.Sprintf(`SELECT id, owner, name, key, metadata FROM things + q := fmt.Sprintf(`SELECT id, owner, name, key, metadata FROM things %s%s%s ORDER BY %s %s LIMIT :limit OFFSET :offset;`, idq, mq, nq, oq, dq) params := map[string]interface{}{ @@ -315,23 +315,8 @@ func (tr thingRepository) RetrieveByChannel(ctx context.Context, owner, chID str } var q, qc string - switch pm.Connected { + switch pm.Disconnected { case true: - q = fmt.Sprintf(`SELECT id, name, key, metadata - FROM things th - INNER JOIN connections conn - ON th.id = conn.thing_id - WHERE th.owner = :owner AND conn.channel_id = :channel - ORDER BY %s %s - LIMIT :limit - OFFSET :offset;`, oq, dq) - - qc = `SELECT COUNT(*) - FROM things th - INNER JOIN connections conn - ON th.id = conn.thing_id - WHERE th.owner = $1 AND conn.channel_id = $2;` - default: q = fmt.Sprintf(`SELECT id, name, key, metadata FROM things th WHERE th.owner = :owner AND th.id NOT IN @@ -350,6 +335,21 @@ func (tr thingRepository) RetrieveByChannel(ctx context.Context, owner, chID str INNER JOIN connections conn ON th.id = conn.thing_id WHERE th.owner = $1 AND conn.channel_id = $2);` + default: + q = fmt.Sprintf(`SELECT id, name, key, metadata + FROM things th + INNER JOIN connections conn + ON th.id = conn.thing_id + WHERE th.owner = :owner AND conn.channel_id = :channel + ORDER BY %s %s + LIMIT :limit + OFFSET :offset;`, oq, dq) + + qc = `SELECT COUNT(*) + FROM things th + INNER JOIN connections conn + ON th.id = conn.thing_id + WHERE th.owner = $1 AND conn.channel_id = $2;` } params := map[string]interface{}{ diff --git a/things/postgres/things_test.go b/things/postgres/things_test.go index f84f809ee9..90cf8507fb 100644 --- a/things/postgres/things_test.go +++ b/things/postgres/things_test.go @@ -612,9 +612,8 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: n - thsDisconNum, }, @@ -622,9 +621,8 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: n / 2, - Limit: n, - Connected: true, + Offset: n / 2, + Limit: n, }, size: (n / 2) - thsDisconNum, }, @@ -632,9 +630,8 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: wrongValue, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, }, @@ -642,9 +639,8 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: nonexistentChanID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, }, @@ -652,9 +648,8 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: wrongValue, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, err: things.ErrNotFound, @@ -663,9 +658,9 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, + Offset: 0, + Limit: n, + Disconnected: true, }, size: thsDisconNum, }, @@ -673,11 +668,10 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "asc", }, size: n - thsDisconNum, }, @@ -685,11 +679,11 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "asc", }, size: thsDisconNum, }, @@ -697,11 +691,10 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "desc", }, size: n - thsDisconNum, }, @@ -709,11 +702,11 @@ func TestMultiThingRetrievalByChannel(t *testing.T) { owner: email, chID: chID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "desc", }, size: thsDisconNum, }, diff --git a/things/redis/streams_test.go b/things/redis/streams_test.go index 0dfa5cb141..28bf57f7df 100644 --- a/things/redis/streams_test.go +++ b/things/redis/streams_test.go @@ -214,8 +214,8 @@ func TestListThingsByChannel(t *testing.T) { require.Nil(t, err, fmt.Sprintf("unexpected error %s", err)) essvc := redis.NewEventStoreMiddleware(svc, redisClient) - esths, eserr := essvc.ListThingsByChannel(context.Background(), token, sch.ID, things.PageMetadata{Offset: 0, Limit: 10, Connected: true}) - thps, err := svc.ListThingsByChannel(context.Background(), token, sch.ID, things.PageMetadata{Offset: 0, Limit: 10, Connected: true}) + esths, eserr := essvc.ListThingsByChannel(context.Background(), token, sch.ID, things.PageMetadata{Offset: 0, Limit: 10}) + thps, err := svc.ListThingsByChannel(context.Background(), token, sch.ID, things.PageMetadata{Offset: 0, Limit: 10}) assert.Equal(t, thps, esths, fmt.Sprintf("event sourcing changed service behaviour: expected %v got %v", thps, esths)) assert.Equal(t, err, eserr, fmt.Sprintf("event sourcing changed service behaviour: expected %v got %v", err, eserr)) } @@ -450,8 +450,8 @@ func TestListChannelsByThing(t *testing.T) { require.Nil(t, err, fmt.Sprintf("unexpected error %s", err)) essvc := redis.NewEventStoreMiddleware(svc, redisClient) - eschs, eserr := essvc.ListChannelsByThing(context.Background(), token, sth.ID, things.PageMetadata{Offset: 0, Limit: 10, Connected: true}) - chps, err := svc.ListChannelsByThing(context.Background(), token, sth.ID, things.PageMetadata{Offset: 0, Limit: 10, Connected: true}) + eschs, eserr := essvc.ListChannelsByThing(context.Background(), token, sth.ID, things.PageMetadata{Offset: 0, Limit: 10}) + chps, err := svc.ListChannelsByThing(context.Background(), token, sth.ID, things.PageMetadata{Offset: 0, Limit: 10}) assert.Equal(t, chps, eschs, fmt.Sprintf("event sourcing changed service behaviour: expected %v got %v", chps, eschs)) assert.Equal(t, err, eserr, fmt.Sprintf("event sourcing changed service behaviour: expected %v got %v", err, eserr)) } diff --git a/things/service.go b/things/service.go index 4b24fa65d4..17e3e725f5 100644 --- a/things/service.go +++ b/things/service.go @@ -125,14 +125,14 @@ type Service interface { // PageMetadata contains page metadata that helps navigation. type PageMetadata struct { - Total uint64 - Offset uint64 `json:"offset,omitempty"` - Limit uint64 `json:"limit,omitempty"` - Name string `json:"name,omitempty"` - Order string `json:"order,omitempty"` - Dir string `json:"dir,omitempty"` - Metadata map[string]interface{} `json:"metadata,omitempty"` - Connected bool // Used for connected or disconnected lists + Total uint64 + Offset uint64 `json:"offset,omitempty"` + Limit uint64 `json:"limit,omitempty"` + Name string `json:"name,omitempty"` + Order string `json:"order,omitempty"` + Dir string `json:"dir,omitempty"` + Metadata map[string]interface{} `json:"metadata,omitempty"` + Disconnected bool // Used for connected or disconnected lists } var _ Service = (*thingsService)(nil) diff --git a/things/service_test.go b/things/service_test.go index 1e106b944f..6a98855304 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -352,9 +352,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: n - thsDisconNum, err: nil, @@ -363,9 +362,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: n / 2, - Limit: n, - Connected: true, + Offset: n / 2, + Limit: n, }, size: (n / 2) - thsDisconNum, err: nil, @@ -374,9 +372,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: n - 1 - thsDisconNum, - Limit: n, - Connected: true, + Offset: n - 1 - thsDisconNum, + Limit: n, }, size: 1, err: nil, @@ -385,9 +382,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: n + 1, - Limit: n, - Connected: true, + Offset: n + 1, + Limit: n, }, size: 0, err: nil, @@ -396,9 +392,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 1, - Limit: 0, - Connected: true, + Offset: 1, + Limit: 0, }, size: 0, err: nil, @@ -407,9 +402,8 @@ func TestListThingsByChannel(t *testing.T) { token: wrongValue, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: 0, - Connected: true, + Offset: 0, + Limit: 0, }, size: 0, err: things.ErrUnauthorizedAccess, @@ -418,9 +412,8 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: "non-existent", pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, err: nil, @@ -429,9 +422,9 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, + Offset: 0, + Limit: n, + Disconnected: true, }, size: thsDisconNum, err: nil, @@ -440,11 +433,10 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "asc", }, size: n - thsDisconNum, err: nil, @@ -453,11 +445,11 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "asc", }, size: thsDisconNum, err: nil, @@ -466,11 +458,10 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "desc", }, size: n - thsDisconNum, err: nil, @@ -479,11 +470,11 @@ func TestListThingsByChannel(t *testing.T) { token: token, chID: ch.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "desc", }, size: thsDisconNum, err: nil, @@ -836,9 +827,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: n - chsDisconNum, err: nil, @@ -847,9 +837,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: (n - chsDisconNum) / 2, - Limit: n, - Connected: true, + Offset: (n - chsDisconNum) / 2, + Limit: n, }, size: (n - chsDisconNum) / 2, err: nil, @@ -858,9 +847,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: n - 1 - chsDisconNum, - Limit: n, - Connected: true, + Offset: n - 1 - chsDisconNum, + Limit: n, }, size: 1, err: nil, @@ -869,9 +857,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: n + 1, - Limit: n, - Connected: true, + Offset: n + 1, + Limit: n, }, size: 0, err: nil, @@ -880,9 +867,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 1, - Limit: 0, - Connected: true, + Offset: 1, + Limit: 0, }, size: 0, err: nil, @@ -891,9 +877,8 @@ func TestListChannelsByThing(t *testing.T) { token: wrongValue, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: 0, - Connected: true, + Offset: 0, + Limit: 0, }, size: 0, err: things.ErrUnauthorizedAccess, @@ -902,9 +887,8 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: "non-existent", pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, + Offset: 0, + Limit: n, }, size: 0, err: nil, @@ -913,9 +897,9 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, + Offset: 0, + Limit: n, + Disconnected: true, }, size: chsDisconNum, err: nil, @@ -924,11 +908,10 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "asc", }, size: n - chsDisconNum, err: nil, @@ -937,11 +920,11 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "asc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "asc", }, size: chsDisconNum, err: nil, @@ -950,11 +933,10 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: true, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Order: "name", + Dir: "desc", }, size: n - chsDisconNum, err: nil, @@ -963,11 +945,11 @@ func TestListChannelsByThing(t *testing.T) { token: token, thID: th.ID, pageMetadata: things.PageMetadata{ - Offset: 0, - Limit: n, - Connected: false, - Order: "name", - Dir: "desc", + Offset: 0, + Limit: n, + Disconnected: true, + Order: "name", + Dir: "desc", }, size: chsDisconNum, err: nil, diff --git a/twins/api/http/transport.go b/twins/api/http/transport.go index 6da6f8c3d8..7e0b66f6e9 100644 --- a/twins/api/http/transport.go +++ b/twins/api/http/transport.go @@ -8,15 +8,14 @@ import ( "encoding/json" "io" "net/http" - "strconv" "strings" - "github.com/mainflux/mainflux/pkg/errors" - kitot "github.com/go-kit/kit/tracing/opentracing" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/internal/httputil" + "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/twins" opentracing "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -24,19 +23,12 @@ import ( const ( contentType = "application/json" - - offset = "offset" - limit = "limit" - name = "name" - metadata = "metadata" - - defLimit = 10 - defOffset = 0 -) - -var ( - errUnsupportedContentType = errors.New("unsupported content type") - errInvalidQueryParams = errors.New("invalid query params") + offsetKey = "offset" + limitKey = "limit" + nameKey = "name" + metadataKey = "metadata" + defLimit = 10 + defOffset = 0 ) // MakeHandler returns a HTTP handler for API endpoints. @@ -97,7 +89,7 @@ func MakeHandler(tracer opentracing.Tracer, svc twins.Service) http.Handler { func decodeTwinCreation(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := addTwinReq{token: r.Header.Get("Authorization")} @@ -110,7 +102,7 @@ func decodeTwinCreation(_ context.Context, r *http.Request) (interface{}, error) func decodeTwinUpdate(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, errUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } req := updateTwinReq{ @@ -134,22 +126,22 @@ func decodeView(_ context.Context, r *http.Request) (interface{}, error) { } func decodeList(_ context.Context, r *http.Request) (interface{}, error) { - l, err := readUintQuery(r, limit, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - o, err := readUintQuery(r, offset, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } - n, err := readStringQuery(r, name) + n, err := httputil.ReadStringQuery(r, nameKey, "") if err != nil { return nil, err } - m, err := readMetadataQuery(r, "metadata") + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } @@ -166,12 +158,12 @@ func decodeList(_ context.Context, r *http.Request) (interface{}, error) { } func decodeListStates(_ context.Context, r *http.Request) (interface{}, error) { - l, err := readUintQuery(r, limit, defLimit) + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) if err != nil { return nil, err } - o, err := readUintQuery(r, offset, defOffset) + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) if err != nil { return nil, err } @@ -216,9 +208,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusNotFound) case twins.ErrConflict: w.WriteHeader(http.StatusUnprocessableEntity) - case errUnsupportedContentType: + case errors.ErrUnsupportedContentType: w.WriteHeader(http.StatusUnsupportedMediaType) - case errInvalidQueryParams: + case errors.ErrInvalidQueryParams: w.WriteHeader(http.StatusBadRequest) case io.ErrUnexpectedEOF: w.WriteHeader(http.StatusBadRequest) @@ -235,54 +227,3 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { } } } - -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return val, nil -} - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidQueryParams - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} - -func readMetadataQuery(r *http.Request, key string) (map[string]interface{}, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return nil, errInvalidQueryParams - } - - if len(vals) == 0 { - return nil, nil - } - - m := make(map[string]interface{}) - err := json.Unmarshal([]byte(vals[0]), &m) - if err != nil { - return nil, errInvalidQueryParams - } - - return m, nil -} diff --git a/users/api/endpoint_test.go b/users/api/endpoint_test.go index 6bac3b4920..30ccc0d8ec 100644 --- a/users/api/endpoint_test.go +++ b/users/api/endpoint_test.go @@ -16,6 +16,7 @@ import ( "testing" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/pkg/uuid" "github.com/mainflux/mainflux/users" "github.com/mainflux/mainflux/users/api" @@ -40,8 +41,8 @@ var ( unauthRes = toJSON(errorRes{users.ErrUnauthorizedAccess.Error()}) malformedRes = toJSON(errorRes{users.ErrMalformedEntity.Error()}) weakPassword = toJSON(errorRes{users.ErrPasswordFormat.Error()}) - unsupportedRes = toJSON(errorRes{api.ErrUnsupportedContentType.Error()}) - failDecodeRes = toJSON(errorRes{api.ErrFailedDecode.Error()}) + unsupportedRes = toJSON(errorRes{errors.ErrUnsupportedContentType.Error()}) + failDecodeRes = toJSON(errorRes{errors.ErrMalformedEntity.Error()}) passRegex = regexp.MustCompile("^.{8,}$") ) @@ -262,9 +263,9 @@ func TestPasswordResetRequest(t *testing.T) { }{ {"password reset request with valid email", data, contentType, http.StatusCreated, expectedExisting}, {"password reset request with invalid email", nonexistentData, contentType, http.StatusBadRequest, notFoundRes}, - {"password reset request with invalid request format", "{", contentType, http.StatusBadRequest, failDecodeRes}, + {"password reset request with invalid request format", "{", contentType, http.StatusBadRequest, malformedRes}, {"password reset request with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes}, - {"password reset request with empty request", "", contentType, http.StatusBadRequest, failDecodeRes}, + {"password reset request with empty request", "", contentType, http.StatusBadRequest, malformedRes}, {"password reset request with missing content type", data, "", http.StatusUnsupportedMediaType, unsupportedRes}, } @@ -345,9 +346,9 @@ func TestPasswordReset(t *testing.T) { {"password reset with valid token", reqExisting, contentType, http.StatusCreated, expectedSuccess, token}, {"password reset with invalid token", reqNoExist, contentType, http.StatusForbidden, unauthRes, token}, {"password reset with confirm password not matching", reqPassNoMatch, contentType, http.StatusBadRequest, malformedRes, token}, - {"password reset request with invalid request format", "{", contentType, http.StatusBadRequest, failDecodeRes, token}, + {"password reset request with invalid request format", "{", contentType, http.StatusBadRequest, malformedRes, token}, {"password reset request with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes, token}, - {"password reset request with empty request", "", contentType, http.StatusBadRequest, failDecodeRes, token}, + {"password reset request with empty request", "", contentType, http.StatusBadRequest, malformedRes, token}, {"password reset request with missing content type", reqExisting, "", http.StatusUnsupportedMediaType, unsupportedRes, token}, {"password reset with weak password", reqPassWeak, contentType, http.StatusBadRequest, weakPassword, token}, } @@ -427,7 +428,7 @@ func TestPasswordChange(t *testing.T) { {"password change with invalid old password", reqWrongPass, contentType, http.StatusForbidden, unauthRes, token}, {"password change with invalid new password", reqWeakPass, contentType, http.StatusBadRequest, weakPassword, token}, {"password change with empty JSON request", "{}", contentType, http.StatusBadRequest, malformedRes, token}, - {"password change empty request", "", contentType, http.StatusBadRequest, failDecodeRes, token}, + {"password change empty request", "", contentType, http.StatusBadRequest, malformedRes, token}, {"password change missing content type", dataResExisting, "", http.StatusUnsupportedMediaType, unsupportedRes, token}, } diff --git a/users/api/transport.go b/users/api/transport.go index 4eb85d62f0..c3821052be 100644 --- a/users/api/transport.go +++ b/users/api/transport.go @@ -8,15 +8,14 @@ import ( "encoding/json" "io" "net/http" - "strconv" "strings" - "github.com/mainflux/mainflux/pkg/errors" - kitot "github.com/go-kit/kit/tracing/opentracing" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-zoo/bone" "github.com/mainflux/mainflux" + "github.com/mainflux/mainflux/internal/httputil" + "github.com/mainflux/mainflux/pkg/errors" "github.com/mainflux/mainflux/users" opentracing "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -24,24 +23,12 @@ import ( const ( contentType = "application/json" - offsetKey = "offset" limitKey = "limit" emailKey = "email" metadataKey = "metadata" - - defOffset = 0 - defLimit = 10 -) - -var ( - errInvalidQueryParams = errors.New("invalid query params") - - // ErrUnsupportedContentType indicates unacceptable or lack of Content-Type - ErrUnsupportedContentType = errors.New("unsupported content type") - - // ErrFailedDecode indicates failed to decode request body - ErrFailedDecode = errors.New("failed to decode request body") + defOffset = 0 + defLimit = 10 ) // MakeHandler returns a HTTP handler for API endpoints. @@ -144,22 +131,25 @@ func decodeViewProfile(_ context.Context, r *http.Request) (interface{}, error) } func decodeListUsers(_ context.Context, r *http.Request) (interface{}, error) { - o, err := readUintQuery(r, offsetKey, defOffset) - if err != nil { + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) + if err != nil && err != errors.ErrNotFoundParam { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) - if err != nil { + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) + if err != nil && err != errors.ErrNotFoundParam { return nil, err } + if err == errors.ErrNotFoundParam { + l = defLimit + } - e, err := readStringQuery(r, emailKey) + e, err := httputil.ReadStringQuery(r, emailKey, "") if err != nil { return nil, err } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } @@ -186,12 +176,12 @@ func decodeUpdateUser(_ context.Context, r *http.Request) (interface{}, error) { func decodeCredentials(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, ErrUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } var user users.User if err := json.NewDecoder(r.Body).Decode(&user); err != nil { - return nil, errors.Wrap(users.ErrMalformedEntity, err) + return nil, errors.Wrap(errors.ErrMalformedEntity, err) } return userReq{user}, nil @@ -199,13 +189,13 @@ func decodeCredentials(_ context.Context, r *http.Request) (interface{}, error) func decodePasswordResetRequest(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, ErrUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } var req passwResetReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return nil, errors.Wrap(ErrFailedDecode, err) + return nil, errors.Wrap(errors.ErrMalformedEntity, err) } req.Host = r.Header.Get("Referer") @@ -214,12 +204,12 @@ func decodePasswordResetRequest(_ context.Context, r *http.Request) (interface{} func decodePasswordReset(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, ErrUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } var req resetTokenReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return nil, errors.Wrap(ErrFailedDecode, err) + return nil, errors.Wrap(errors.ErrMalformedEntity, err) } return req, nil @@ -227,12 +217,12 @@ func decodePasswordReset(_ context.Context, r *http.Request) (interface{}, error func decodePasswordChange(_ context.Context, r *http.Request) (interface{}, error) { if !strings.Contains(r.Header.Get("Content-Type"), contentType) { - return nil, ErrUnsupportedContentType + return nil, errors.ErrUnsupportedContentType } var req passwChangeReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - return nil, errors.Wrap(ErrFailedDecode, err) + return nil, errors.Wrap(errors.ErrMalformedEntity, err) } req.Token = r.Header.Get("Authorization") @@ -241,17 +231,20 @@ func decodePasswordChange(_ context.Context, r *http.Request) (interface{}, erro } func decodeListMemberGroupRequest(_ context.Context, r *http.Request) (interface{}, error) { - o, err := readUintQuery(r, offsetKey, defOffset) - if err != nil { + o, err := httputil.ReadUintQuery(r, offsetKey, defOffset) + if err != nil && err != errors.ErrNotFoundParam { return nil, err } - l, err := readUintQuery(r, limitKey, defLimit) - if err != nil { + l, err := httputil.ReadUintQuery(r, limitKey, defLimit) + if err != nil && err != errors.ErrNotFoundParam { return nil, err } + if err == errors.ErrNotFoundParam { + l = defLimit + } - m, err := readMetadataQuery(r, metadataKey) + m, err := httputil.ReadMetadataQuery(r, metadataKey, nil) if err != nil { return nil, err } @@ -287,6 +280,8 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { case errors.Error: w.Header().Set("Content-Type", contentType) switch { + case errors.Contains(errorVal, errors.ErrInvalidQueryParams): + w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, users.ErrMalformedEntity): w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, users.ErrUnauthorizedAccess): @@ -295,9 +290,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusConflict) case errors.Contains(errorVal, users.ErrGroupConflict): w.WriteHeader(http.StatusConflict) - case errors.Contains(errorVal, ErrUnsupportedContentType): + case errors.Contains(errorVal, errors.ErrUnsupportedContentType): w.WriteHeader(http.StatusUnsupportedMediaType) - case errors.Contains(errorVal, ErrFailedDecode): + case errors.Contains(errorVal, errors.ErrMalformedEntity): w.WriteHeader(http.StatusBadRequest) case errors.Contains(errorVal, io.ErrUnexpectedEOF): w.WriteHeader(http.StatusBadRequest) @@ -321,54 +316,3 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusInternalServerError) } } - -func readUintQuery(r *http.Request, key string, def uint64) (uint64, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return 0, errInvalidQueryParams - } - - if len(vals) == 0 { - return def, nil - } - - strval := vals[0] - val, err := strconv.ParseUint(strval, 10, 64) - if err != nil { - return 0, errInvalidQueryParams - } - - return val, nil -} - -func readStringQuery(r *http.Request, key string) (string, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return "", errInvalidQueryParams - } - - if len(vals) == 0 { - return "", nil - } - - return vals[0], nil -} - -func readMetadataQuery(r *http.Request, key string) (map[string]interface{}, error) { - vals := bone.GetQuery(r, key) - if len(vals) > 1 { - return nil, errInvalidQueryParams - } - - if len(vals) == 0 { - return nil, nil - } - - m := make(map[string]interface{}) - err := json.Unmarshal([]byte(vals[0]), &m) - if err != nil { - return nil, errors.Wrap(errInvalidQueryParams, err) - } - - return m, nil -}