From cd4b5f56a2c4fb05521ba856c236ca0e502b71de Mon Sep 17 00:00:00 2001 From: David Subiros Date: Thu, 17 Dec 2020 12:38:51 +0000 Subject: [PATCH 1/8] First implementation of get options by IDs --- api/api.go | 1 - api/dimensions.go | 64 +++++++++++++++++++++++++------- apierrors/errors.go | 2 + go.mod | 2 + go.sum | 5 +++ mongo/dimension_store.go | 42 +++++++++++++++++++++ service/mock/initialiser.go | 9 ++--- store/datastore.go | 1 + store/datastoretest/datastore.go | 60 ++++++++++++++++++++++++++++++ store/datastoretest/mongo.go | 60 ++++++++++++++++++++++++++++++ 10 files changed, 226 insertions(+), 20 deletions(-) diff --git a/api/api.go b/api/api.go index cf89e7e8..5376c4c4 100644 --- a/api/api.go +++ b/api/api.go @@ -1,6 +1,5 @@ package api -//go:generate moq -out ../mocks/generated_auth_mocks.go -pkg mocks . AuthHandler //go:generate moq -out ../mocks/mocks.go -pkg mocks . DownloadsGenerator import ( diff --git a/api/dimensions.go b/api/dimensions.go index 26f67be8..0e15c875 100644 --- a/api/dimensions.go +++ b/api/dimensions.go @@ -15,6 +15,8 @@ import ( "github.com/gorilla/mux" ) +const maxIDs = 1000 + func (api *DatasetAPI) getDimensions(w http.ResponseWriter, r *http.Request) { ctx := r.Context() vars := mux.Vars(r) @@ -141,6 +143,18 @@ func getPositiveIntQueryParameter(queryVars url.Values, varKey string, defaultVa return val, nil } +// getStringListFromQueryParameter obtains a list of strings from the provided query paramters, up to maxNumItems +func getStringListFromQueryParameter(queryVars url.Values, varKey string, maxNumItems int) ([]string, error) { + items, found := queryVars[varKey] + if !found { + return []string{}, nil + } + if len(items) > maxNumItems { + return []string{}, errs.ErrTooManyQueryParameters + } + return items, nil +} + func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Request) { ctx := r.Context() vars := mux.Vars(r) @@ -157,20 +171,32 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques state = models.PublishedState } - // get limit from query parameters, or default value - limit, err := getPositiveIntQueryParameter(r.URL.Query(), "limit", api.defaultLimit) + // get list of option IDs that we want to get + ids, err := getStringListFromQueryParameter(r.URL.Query(), "ids", maxIDs) if err != nil { logData["query_params"] = r.URL.RawQuery - handleDimensionsErr(ctx, w, "failed to obtain limit from request query paramters", err, logData) + handleDimensionsErr(ctx, w, "failed to obtain list of IDs from request query paramters", err, logData) return } - // get offset from query parameters, or default value - offset, err := getPositiveIntQueryParameter(r.URL.Query(), "offset", api.defaultOffset) - if err != nil { - logData["query_params"] = r.URL.RawQuery - handleDimensionsErr(ctx, w, "failed to obtain offset from request query paramters", err, logData) - return + // if no list of IDs is provided, get offset and limit + var offset, limit int + if len(ids) == 0 { + // get limit from query parameters, or default value + limit, err = getPositiveIntQueryParameter(r.URL.Query(), "limit", api.defaultLimit) + if err != nil { + logData["query_params"] = r.URL.RawQuery + handleDimensionsErr(ctx, w, "failed to obtain limit from request query paramters", err, logData) + return + } + + // get offset from query parameters, or default value + offset, err = getPositiveIntQueryParameter(r.URL.Query(), "offset", api.defaultOffset) + if err != nil { + logData["query_params"] = r.URL.RawQuery + handleDimensionsErr(ctx, w, "failed to obtain offset from request query paramters", err, logData) + return + } } // ger version for provided dataset, edition and versionID @@ -187,11 +213,21 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques return } - // get sorted dimension options, starting at offset index, with a limit on the number of items - results, err := api.dataStore.Backend.GetDimensionOptions(version, dimension, offset, limit) - if err != nil { - handleDimensionsErr(ctx, w, "failed to get a list of dimension options", err, logData) - return + var results *models.DimensionOptionResults + if len(ids) == 0 { + // get sorted dimension options, starting at offset index, with a limit on the number of items + results, err = api.dataStore.Backend.GetDimensionOptions(version, dimension, offset, limit) + if err != nil { + handleDimensionsErr(ctx, w, "failed to get a list of dimension options", err, logData) + return + } + } else { + // get dimension options from the provided list of IDs, sorted by option + results, err = api.dataStore.Backend.GetDimensionOptionsFromIDs(version, dimension, ids) + if err != nil { + handleDimensionsErr(ctx, w, "failed to get a list of dimension options", err, logData) + return + } } // populate links diff --git a/apierrors/errors.go b/apierrors/errors.go index 93cbd46a..8c69a6d1 100644 --- a/apierrors/errors.go +++ b/apierrors/errors.go @@ -26,6 +26,7 @@ var ( ErrInternalServer = errors.New("internal error") ErrInsertedObservationsInvalidSyntax = errors.New("inserted observation request parameter not an integer") ErrInvalidQueryParameter = errors.New("invalid query parameter") + ErrTooManyQueryParameters = errors.New("too many query parameters have been provided") ErrMetadataVersionNotFound = errors.New("version not found") ErrMissingJobProperties = errors.New("missing job properties") ErrMissingParameters = errors.New("missing properties in JSON") @@ -63,6 +64,7 @@ var ( BadRequestMap = map[error]bool{ ErrInsertedObservationsInvalidSyntax: true, ErrInvalidQueryParameter: true, + ErrTooManyQueryParameters: true, ErrMissingJobProperties: true, ErrMissingParameters: true, ErrUnableToParseJSON: true, diff --git a/go.mod b/go.mod index ad31c946..527a70f9 100644 --- a/go.mod +++ b/go.mod @@ -22,4 +22,6 @@ require ( github.com/satori/go.uuid v1.2.0 github.com/smartystreets/goconvey v1.6.4 gopkg.in/avro.v0 v0.0.0-20171217001914-a730b5802183 // indirect + gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index 30df9138..38a9b3cc 100644 --- a/go.sum +++ b/go.sum @@ -33,6 +33,7 @@ github.com/ONSdigital/go-ns v0.0.0-20191104121206-f144c4ec2e58/go.mod h1:iWos35i github.com/ONSdigital/go-ns v0.0.0-20200205115900-a11716f93bad/go.mod h1:uHT6LaUlRbJsJRrIlN31t+QLUB80tAbk6ZR9sfoHL8Y= github.com/ONSdigital/go-ns v0.0.0-20200511161740-afc39066ee62 h1:DyGIxcRIEEHGGb/FuvrWoYXvpbR0WDbnvEn9kBF4tvM= github.com/ONSdigital/go-ns v0.0.0-20200511161740-afc39066ee62/go.mod h1:uHT6LaUlRbJsJRrIlN31t+QLUB80tAbk6ZR9sfoHL8Y= +github.com/ONSdigital/go-ns v0.0.0-20200902154605-290c8b5ba5eb h1:JQyVnHu+gr8NL+QTd2Dt+/03WRs21UWoU3HqOmTKnJE= github.com/ONSdigital/golang-neo4j-bolt-driver v0.0.0-20190228153339-da534111531d h1:Z0FsB7q0SG3tG4O/WGv0hh1MyxScyZ5JWjECEgVCIzM= github.com/ONSdigital/golang-neo4j-bolt-driver v0.0.0-20190228153339-da534111531d/go.mod h1:75Sxr59AMz2RiPskqSymLFxdeaIEhnkNaJE5lonMS3M= github.com/ONSdigital/graphson v0.0.0-20190718134034-c13ceacd109d h1:yrCtEGlohmA3OnXtke0nOOp/m9O83orpSnTGOfYOw1Q= @@ -191,6 +192,10 @@ gopkg.in/jcmturner/gokrb5.v7 v7.5.0 h1:a9tsXlIDD9SKxotJMK3niV7rPZAJeX2aD/0yg3qlI gopkg.in/jcmturner/gokrb5.v7 v7.5.0/go.mod h1:l8VISx+WGYp+Fp7KRbsiUuXTTOnxIc3Tuvyavf11/WM= gopkg.in/jcmturner/rpc.v1 v1.1.0 h1:QHIUxTX1ISuAv9dD2wJ9HWQVuWDX/Zc0PfeC2tjc4rU= gopkg.in/jcmturner/rpc.v1 v1.1.0/go.mod h1:YIdkC4XfD6GXbzje11McwsDuOlZQSb9W4vfLvuNnlv8= +gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 h1:VpOs+IwYnYBaFnrNAeB8UUWtL3vEUnzSCL1nVjPhqrw= +gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200601152816-913338de1bd2 h1:VEmvx0P+GVTgkNu2EdTN988YCZPcD3lo9AoczZpucwc= gopkg.in/yaml.v3 v3.0.0-20200601152816-913338de1bd2/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/mongo/dimension_store.go b/mongo/dimension_store.go index 336324b9..9cb5b7d4 100644 --- a/mongo/dimension_store.go +++ b/mongo/dimension_store.go @@ -11,6 +11,7 @@ import ( ) const dimensionOptions = "dimension.options" +const maxIDs = 1000 // GetDimensionsFromInstance returns a list of dimensions and their options for an instance resource func (m *Mongo) GetDimensionsFromInstance(id string) (*models.DimensionNodeResults, error) { @@ -120,3 +121,44 @@ func (m *Mongo) GetDimensionOptions(version *models.Version, dimension string, o Limit: limit, }, nil } + +// GetDimensionOptionsFromIDs returns dimension options for a dimension within a dataset, whose IDs match the provided list of IDs +func (m *Mongo) GetDimensionOptionsFromIDs(version *models.Version, dimension string, IDs []string) (*models.DimensionOptionResults, error) { + if len(IDs) > maxIDs { + return nil, errors.New("too many IDs provided") + } + + s := m.Session.Copy() + defer s.Close() + + selectorAll := bson.M{"instance_id": version.ID, "name": dimension} + selectorInList := bson.M{"instance_id": version.ID, "name": dimension, "option": bson.M{"$in": IDs}} + + // count total number of options in dimension + q := s.DB(m.Database).C(dimensionOptions).Find(selectorAll) + totalCount, err := q.Count() + if err != nil { + return nil, err + } + + // obtain only options matching the provided IDs + q = s.DB(m.Database).C(dimensionOptions).Find(selectorInList) + + var values []models.PublicDimensionOption + iter := q.Sort("option").Iter() + if err := iter.All(&values); err != nil { + return nil, err + } + + for i := 0; i < len(values); i++ { + values[i].Links.Version = *version.Links.Self + } + + return &models.DimensionOptionResults{ + Items: values, + Count: len(values), + TotalCount: totalCount, + Offset: 0, + Limit: 0, + }, nil +} diff --git a/service/mock/initialiser.go b/service/mock/initialiser.go index 20221b3c..f4b5d785 100644 --- a/service/mock/initialiser.go +++ b/service/mock/initialiser.go @@ -5,13 +5,12 @@ package mock import ( "context" - "net/http" - "sync" - "github.com/ONSdigital/dp-dataset-api/config" "github.com/ONSdigital/dp-dataset-api/service" "github.com/ONSdigital/dp-dataset-api/store" - kafka "github.com/ONSdigital/dp-kafka/v2" + "github.com/ONSdigital/dp-kafka/v2" + "net/http" + "sync" ) var ( @@ -22,7 +21,7 @@ var ( lockInitialiserMockDoGetMongoDB sync.RWMutex ) -// Ensure, that InitialiserMock does implement Initialiser. +// Ensure, that InitialiserMock does implement service.Initialiser. // If this is not the case, regenerate this file with moq. var _ service.Initialiser = &InitialiserMock{} diff --git a/store/datastore.go b/store/datastore.go index 78551f9c..0f086853 100644 --- a/store/datastore.go +++ b/store/datastore.go @@ -30,6 +30,7 @@ type dataMongoDB interface { GetDimensionsFromInstance(ID string) (*models.DimensionNodeResults, error) GetDimensions(datasetID, versionID string) ([]bson.M, error) GetDimensionOptions(version *models.Version, dimension string, offset, limit int) (*models.DimensionOptionResults, error) + GetDimensionOptionsFromIDs(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) GetEdition(ID, editionID, state string) (*models.EditionUpdate, error) GetEditions(ctx context.Context, ID, state string) (*models.EditionUpdateResults, error) GetInstances(ctx context.Context, states []string, datasets []string) (*models.InstanceResults, error) diff --git a/store/datastoretest/datastore.go b/store/datastoretest/datastore.go index d10cbaa6..760f6dee 100755 --- a/store/datastoretest/datastore.go +++ b/store/datastoretest/datastore.go @@ -6,6 +6,7 @@ package storetest import ( "context" "github.com/ONSdigital/dp-dataset-api/models" + "github.com/ONSdigital/dp-dataset-api/store" "github.com/ONSdigital/dp-graph/v2/observation" "github.com/globalsign/mgo/bson" "sync" @@ -23,6 +24,7 @@ var ( lockStorerMockGetDataset sync.RWMutex lockStorerMockGetDatasets sync.RWMutex lockStorerMockGetDimensionOptions sync.RWMutex + lockStorerMockGetDimensionOptionsFromIDs sync.RWMutex lockStorerMockGetDimensions sync.RWMutex lockStorerMockGetDimensionsFromInstance sync.RWMutex lockStorerMockGetEdition sync.RWMutex @@ -50,6 +52,10 @@ var ( lockStorerMockUpsertVersion sync.RWMutex ) +// Ensure, that StorerMock does implement store.Storer. +// If this is not the case, regenerate this file with moq. +var _ store.Storer = &StorerMock{} + // StorerMock is a mock implementation of store.Storer. // // func TestSomethingThatUsesStorer(t *testing.T) { @@ -89,6 +95,9 @@ var ( // GetDimensionOptionsFunc: func(version *models.Version, dimension string, offset int, limit int) (*models.DimensionOptionResults, error) { // panic("mock out the GetDimensionOptions method") // }, +// GetDimensionOptionsFromIDsFunc: func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { +// panic("mock out the GetDimensionOptionsFromIDs method") +// }, // GetDimensionsFunc: func(datasetID string, versionID string) ([]bson.M, error) { // panic("mock out the GetDimensions method") // }, @@ -204,6 +213,9 @@ type StorerMock struct { // GetDimensionOptionsFunc mocks the GetDimensionOptions method. GetDimensionOptionsFunc func(version *models.Version, dimension string, offset int, limit int) (*models.DimensionOptionResults, error) + // GetDimensionOptionsFromIDsFunc mocks the GetDimensionOptionsFromIDs method. + GetDimensionOptionsFromIDsFunc func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) + // GetDimensionsFunc mocks the GetDimensions method. GetDimensionsFunc func(datasetID string, versionID string) ([]bson.M, error) @@ -358,6 +370,15 @@ type StorerMock struct { // Limit is the limit argument value. Limit int } + // GetDimensionOptionsFromIDs holds details about calls to the GetDimensionOptionsFromIDs method. + GetDimensionOptionsFromIDs []struct { + // Version is the version argument value. + Version *models.Version + // Dimension is the dimension argument value. + Dimension string + // Ids is the ids argument value. + Ids []string + } // GetDimensions holds details about calls to the GetDimensions method. GetDimensions []struct { // DatasetID is the datasetID argument value. @@ -949,6 +970,45 @@ func (mock *StorerMock) GetDimensionOptionsCalls() []struct { return calls } +// GetDimensionOptionsFromIDs calls GetDimensionOptionsFromIDsFunc. +func (mock *StorerMock) GetDimensionOptionsFromIDs(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { + if mock.GetDimensionOptionsFromIDsFunc == nil { + panic("StorerMock.GetDimensionOptionsFromIDsFunc: method is nil but Storer.GetDimensionOptionsFromIDs was just called") + } + callInfo := struct { + Version *models.Version + Dimension string + Ids []string + }{ + Version: version, + Dimension: dimension, + Ids: ids, + } + lockStorerMockGetDimensionOptionsFromIDs.Lock() + mock.calls.GetDimensionOptionsFromIDs = append(mock.calls.GetDimensionOptionsFromIDs, callInfo) + lockStorerMockGetDimensionOptionsFromIDs.Unlock() + return mock.GetDimensionOptionsFromIDsFunc(version, dimension, ids) +} + +// GetDimensionOptionsFromIDsCalls gets all the calls that were made to GetDimensionOptionsFromIDs. +// Check the length with: +// len(mockedStorer.GetDimensionOptionsFromIDsCalls()) +func (mock *StorerMock) GetDimensionOptionsFromIDsCalls() []struct { + Version *models.Version + Dimension string + Ids []string +} { + var calls []struct { + Version *models.Version + Dimension string + Ids []string + } + lockStorerMockGetDimensionOptionsFromIDs.RLock() + calls = mock.calls.GetDimensionOptionsFromIDs + lockStorerMockGetDimensionOptionsFromIDs.RUnlock() + return calls +} + // GetDimensions calls GetDimensionsFunc. func (mock *StorerMock) GetDimensions(datasetID string, versionID string) ([]bson.M, error) { if mock.GetDimensionsFunc == nil { diff --git a/store/datastoretest/mongo.go b/store/datastoretest/mongo.go index e4535555..0c018a10 100644 --- a/store/datastoretest/mongo.go +++ b/store/datastoretest/mongo.go @@ -6,6 +6,7 @@ package storetest import ( "context" "github.com/ONSdigital/dp-dataset-api/models" + "github.com/ONSdigital/dp-dataset-api/store" "github.com/ONSdigital/dp-healthcheck/healthcheck" "github.com/globalsign/mgo/bson" "sync" @@ -24,6 +25,7 @@ var ( lockMongoDBMockGetDataset sync.RWMutex lockMongoDBMockGetDatasets sync.RWMutex lockMongoDBMockGetDimensionOptions sync.RWMutex + lockMongoDBMockGetDimensionOptionsFromIDs sync.RWMutex lockMongoDBMockGetDimensions sync.RWMutex lockMongoDBMockGetDimensionsFromInstance sync.RWMutex lockMongoDBMockGetEdition sync.RWMutex @@ -49,6 +51,10 @@ var ( lockMongoDBMockUpsertVersion sync.RWMutex ) +// Ensure, that MongoDBMock does implement store.MongoDB. +// If this is not the case, regenerate this file with moq. +var _ store.MongoDB = &MongoDBMock{} + // MongoDBMock is a mock implementation of store.MongoDB. // // func TestSomethingThatUsesMongoDB(t *testing.T) { @@ -91,6 +97,9 @@ var ( // GetDimensionOptionsFunc: func(version *models.Version, dimension string, offset int, limit int) (*models.DimensionOptionResults, error) { // panic("mock out the GetDimensionOptions method") // }, +// GetDimensionOptionsFromIDsFunc: func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { +// panic("mock out the GetDimensionOptionsFromIDs method") +// }, // GetDimensionsFunc: func(datasetID string, versionID string) ([]bson.M, error) { // panic("mock out the GetDimensions method") // }, @@ -203,6 +212,9 @@ type MongoDBMock struct { // GetDimensionOptionsFunc mocks the GetDimensionOptions method. GetDimensionOptionsFunc func(version *models.Version, dimension string, offset int, limit int) (*models.DimensionOptionResults, error) + // GetDimensionOptionsFromIDsFunc mocks the GetDimensionOptionsFromIDs method. + GetDimensionOptionsFromIDsFunc func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) + // GetDimensionsFunc mocks the GetDimensions method. GetDimensionsFunc func(datasetID string, versionID string) ([]bson.M, error) @@ -350,6 +362,15 @@ type MongoDBMock struct { // Limit is the limit argument value. Limit int } + // GetDimensionOptionsFromIDs holds details about calls to the GetDimensionOptionsFromIDs method. + GetDimensionOptionsFromIDs []struct { + // Version is the version argument value. + Version *models.Version + // Dimension is the dimension argument value. + Dimension string + // Ids is the ids argument value. + Ids []string + } // GetDimensions holds details about calls to the GetDimensions method. GetDimensions []struct { // DatasetID is the datasetID argument value. @@ -940,6 +961,45 @@ func (mock *MongoDBMock) GetDimensionOptionsCalls() []struct { return calls } +// GetDimensionOptionsFromIDs calls GetDimensionOptionsFromIDsFunc. +func (mock *MongoDBMock) GetDimensionOptionsFromIDs(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { + if mock.GetDimensionOptionsFromIDsFunc == nil { + panic("MongoDBMock.GetDimensionOptionsFromIDsFunc: method is nil but MongoDB.GetDimensionOptionsFromIDs was just called") + } + callInfo := struct { + Version *models.Version + Dimension string + Ids []string + }{ + Version: version, + Dimension: dimension, + Ids: ids, + } + lockMongoDBMockGetDimensionOptionsFromIDs.Lock() + mock.calls.GetDimensionOptionsFromIDs = append(mock.calls.GetDimensionOptionsFromIDs, callInfo) + lockMongoDBMockGetDimensionOptionsFromIDs.Unlock() + return mock.GetDimensionOptionsFromIDsFunc(version, dimension, ids) +} + +// GetDimensionOptionsFromIDsCalls gets all the calls that were made to GetDimensionOptionsFromIDs. +// Check the length with: +// len(mockedMongoDB.GetDimensionOptionsFromIDsCalls()) +func (mock *MongoDBMock) GetDimensionOptionsFromIDsCalls() []struct { + Version *models.Version + Dimension string + Ids []string +} { + var calls []struct { + Version *models.Version + Dimension string + Ids []string + } + lockMongoDBMockGetDimensionOptionsFromIDs.RLock() + calls = mock.calls.GetDimensionOptionsFromIDs + lockMongoDBMockGetDimensionOptionsFromIDs.RUnlock() + return calls +} + // GetDimensions calls GetDimensionsFunc. func (mock *MongoDBMock) GetDimensions(datasetID string, versionID string) ([]bson.M, error) { if mock.GetDimensionsFunc == nil { From 68ee24fa0c103c8342f7f7c999f2d0ac41af2747 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Mon, 21 Dec 2020 09:48:19 +0100 Subject: [PATCH 2/8] set default limit to 100 items --- config/config.go | 2 +- config/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 741930e7..484b700c 100644 --- a/config/config.go +++ b/config/config.go @@ -68,7 +68,7 @@ func Get() (*Configuration, error) { BindAddr: "localhost:27017", Collection: "datasets", Database: "datasets", - Limit: 0, + Limit: 100, Offset: 0, }, } diff --git a/config/config_test.go b/config/config_test.go index d56bcce7..5e62c954 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -33,7 +33,7 @@ func TestSpec(t *testing.T) { So(cfg.MongoConfig.BindAddr, ShouldEqual, "localhost:27017") So(cfg.MongoConfig.Collection, ShouldEqual, "datasets") So(cfg.MongoConfig.Database, ShouldEqual, "datasets") - So(cfg.MongoConfig.Limit, ShouldEqual, 0) + So(cfg.MongoConfig.Limit, ShouldEqual, 100) So(cfg.MongoConfig.Offset, ShouldEqual, 0) So(cfg.EnablePermissionsAuth, ShouldBeFalse) So(cfg.HealthCheckCriticalTimeout, ShouldEqual, 90*time.Second) From e87da754223b513a52335d65b8eeece793762ee0 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Mon, 21 Dec 2020 10:00:12 +0100 Subject: [PATCH 3/8] Changed description of default limit in swagger --- swagger.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swagger.yaml b/swagger.yaml index f8c8cbb6..9e1c9f10 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -162,7 +162,7 @@ parameters: $ref: '#/definitions/UpdateVersion' limit: name: limit - description: "Maximum number of items that will be returned. A value of zero (default) will return all available items, without limit." + description: "Maximum number of items that will be returned. A value of zero will return all available items, without limit. The default value is 100." in: query required: false type: integer From f5fe244ce6fc6c77fd3ba28103ae662544e7e1bb Mon Sep 17 00:00:00 2001 From: David Subiros Date: Mon, 21 Dec 2020 13:41:59 +0100 Subject: [PATCH 4/8] get parameters as comma separated values from query param 'ids', added unit tests --- .gitignore | 3 +- api/dimensions.go | 13 +++++--- api/dimensions_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 9b06bd83..100deb8b 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ debug *.tar.gz build -.idea \ No newline at end of file +.idea +.vscode diff --git a/api/dimensions.go b/api/dimensions.go index 0e15c875..ed9a2964 100644 --- a/api/dimensions.go +++ b/api/dimensions.go @@ -2,11 +2,13 @@ package api import ( "context" + "encoding/csv" "encoding/json" "fmt" "net/http" "net/url" "strconv" + "strings" errs "github.com/ONSdigital/dp-dataset-api/apierrors" "github.com/ONSdigital/dp-dataset-api/models" @@ -143,12 +145,15 @@ func getPositiveIntQueryParameter(queryVars url.Values, varKey string, defaultVa return val, nil } -// getStringListFromQueryParameter obtains a list of strings from the provided query paramters, up to maxNumItems -func getStringListFromQueryParameter(queryVars url.Values, varKey string, maxNumItems int) ([]string, error) { - items, found := queryVars[varKey] +// getStringListFromCSVQueryParameter obtains a list of strings from the provided query parameter, +// by parsing the CSV value. Up to maxNumItems values are allowed. +func getStringListFromCSVQueryParameter(queryVars url.Values, varKey string, maxNumItems int) (items []string, err error) { + q, found := queryVars[varKey] if !found { return []string{}, nil } + r := csv.NewReader(strings.NewReader(q[0])) + items, err = r.Read() if len(items) > maxNumItems { return []string{}, errs.ErrTooManyQueryParameters } @@ -172,7 +177,7 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques } // get list of option IDs that we want to get - ids, err := getStringListFromQueryParameter(r.URL.Query(), "ids", maxIDs) + ids, err := getStringListFromCSVQueryParameter(r.URL.Query(), "ids", maxIDs) if err != nil { logData["query_params"] = r.URL.RawQuery handleDimensionsErr(ctx, w, "failed to obtain list of IDs from request query paramters", err, logData) diff --git a/api/dimensions_test.go b/api/dimensions_test.go index 7ad6fa4c..1267b6aa 100644 --- a/api/dimensions_test.go +++ b/api/dimensions_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sort" "testing" errs "github.com/ONSdigital/dp-dataset-api/apierrors" @@ -174,6 +175,26 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { Offset: offset, }, nil }, + GetDimensionOptionsFromIDsFunc: func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { + ret := &models.DimensionOptionResults{TotalCount: 5} + sort.Strings(ids) + for _, id := range ids { + switch id { + case "op1": + ret.Items = append(ret.Items, models.PublicDimensionOption{Option: "op1"}) + case "op2": + ret.Items = append(ret.Items, models.PublicDimensionOption{Option: "op2"}) + case "op3": + ret.Items = append(ret.Items, models.PublicDimensionOption{Option: "op3"}) + case "op4": + ret.Items = append(ret.Items, models.PublicDimensionOption{Option: "op4"}) + case "op5": + ret.Items = append(ret.Items, models.PublicDimensionOption{Option: "op5"}) + } + } + ret.Count = len(ret.Items) + return ret, nil + }, } // permissions mocks @@ -299,6 +320,54 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { validateCalls() }) }) + + Convey("When a valid dimension and list of existing IDs is provided", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?ids=op1,op3", nil) + w := call(r) + + Convey("Then the call succeeds with 200 OK code, expected body and calls", func() { + expectedResponse := models.DimensionOptionResults{ + Items: []models.PublicDimensionOption{ + {Option: "op1", Links: expectedLinks}, + {Option: "op3", Links: expectedLinks}, + }, + Count: 2, + Offset: 0, + Limit: 0, + TotalCount: 5, + } + So(w.Code, ShouldEqual, http.StatusOK) + validateBody(w.Body.Bytes(), expectedResponse) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionOptionsFromIDsCalls()), ShouldEqual, 1) + }) + }) + + Convey("When a valid offset, limit and dimension and list of existing IDs are provided", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?ids=op1,op3&offset=0&limit=1", nil) + w := call(r) + + Convey("Then the call succeeds with 200 OK code, the list of IDs take precedence (offset and limit are ignored), and the expected body and calls are performed", func() { + expectedResponse := models.DimensionOptionResults{ + Items: []models.PublicDimensionOption{ + {Option: "op1", Links: expectedLinks}, + {Option: "op3", Links: expectedLinks}, + }, + Count: 2, + Offset: 0, + Limit: 0, + TotalCount: 5, + } + So(w.Code, ShouldEqual, http.StatusOK) + validateBody(w.Body.Bytes(), expectedResponse) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionOptionsFromIDsCalls()), ShouldEqual, 1) + }) + }) }) } From 6722fda714dd749c7d8140b385fc251f907c07b9 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Tue, 22 Dec 2020 10:14:00 +0100 Subject: [PATCH 5/8] improved getQueryParamListValues and tests --- api/dimensions.go | 31 ++++++++++------ api/dimensions_test.go | 80 ++++++++++++++++++++++++++++++++---------- 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/api/dimensions.go b/api/dimensions.go index ed9a2964..29c276b6 100644 --- a/api/dimensions.go +++ b/api/dimensions.go @@ -2,7 +2,6 @@ package api import ( "context" - "encoding/csv" "encoding/json" "fmt" "net/http" @@ -17,7 +16,12 @@ import ( "github.com/gorilla/mux" ) -const maxIDs = 1000 +const maxIDs = 200 + +// MaxIDs returns the maximum number of IDs acceptable in a list +var MaxIDs = func() int { + return maxIDs +} func (api *DatasetAPI) getDimensions(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -145,17 +149,22 @@ func getPositiveIntQueryParameter(queryVars url.Values, varKey string, defaultVa return val, nil } -// getStringListFromCSVQueryParameter obtains a list of strings from the provided query parameter, -// by parsing the CSV value. Up to maxNumItems values are allowed. -func getStringListFromCSVQueryParameter(queryVars url.Values, varKey string, maxNumItems int) (items []string, err error) { - q, found := queryVars[varKey] +// getQueryParamListValues obtains a list of strings from the provided queryVars, +// by parsing all values with key 'varKey' and splitting the values by commas, if they contain commas. +// Up to maxNumItems values are allowed in total. +func getQueryParamListValues(queryVars url.Values, varKey string, maxNumItems int) (items []string, err error) { + // get query paramters values for the provided key + values, found := queryVars[varKey] if !found { return []string{}, nil } - r := csv.NewReader(strings.NewReader(q[0])) - items, err = r.Read() - if len(items) > maxNumItems { - return []string{}, errs.ErrTooManyQueryParameters + + // each value may contain a simple value or a list of values, in a comma-separated format + for _, value := range values { + items = append(items, strings.Split(value, ",")...) + if len(items) > maxNumItems { + return []string{}, errs.ErrTooManyQueryParameters + } } return items, nil } @@ -177,7 +186,7 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques } // get list of option IDs that we want to get - ids, err := getStringListFromCSVQueryParameter(r.URL.Query(), "ids", maxIDs) + ids, err := getQueryParamListValues(r.URL.Query(), "id", MaxIDs()) if err != nil { logData["query_params"] = r.URL.RawQuery handleDimensionsErr(ctx, w, "failed to obtain list of IDs from request query paramters", err, logData) diff --git a/api/dimensions_test.go b/api/dimensions_test.go index 1267b6aa..e87167bc 100644 --- a/api/dimensions_test.go +++ b/api/dimensions_test.go @@ -144,7 +144,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { // testing DataStore with 5 dimension options mockedDataStore := &storetest.StorerMock{ GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { - return &models.Version{State: models.AssociatedState}, nil + return &models.Version{State: models.AssociatedState, ID: "v1"}, nil }, GetDimensionOptionsFunc: func(version *models.Version, dimensions string, offset, limit int) (*models.DimensionOptionResults, error) { if offset > 4 { @@ -210,11 +210,22 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } // func to validate expected calls - validateCalls := func() { + validateCalls := func(expectedIDs *[]string) { So(datasetPermissions.Required.Calls, ShouldEqual, 1) So(permissions.Required.Calls, ShouldEqual, 0) So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) - So(len(mockedDataStore.GetDimensionOptionsCalls()), ShouldEqual, 1) + if expectedIDs == nil { + So(mockedDataStore.GetDimensionOptionsCalls(), ShouldHaveLength, 1) + So(mockedDataStore.GetDimensionOptionsFromIDsCalls(), ShouldHaveLength, 0) + So(mockedDataStore.GetDimensionOptionsCalls()[0].Dimension, ShouldEqual, "age") + So(mockedDataStore.GetDimensionOptionsCalls()[0].Version.ID, ShouldEqual, "v1") + } else { + So(mockedDataStore.GetDimensionOptionsCalls(), ShouldHaveLength, 0) + So(mockedDataStore.GetDimensionOptionsFromIDsCalls(), ShouldHaveLength, 1) + So(mockedDataStore.GetDimensionOptionsFromIDsCalls()[0].Dimension, ShouldEqual, "age") + So(mockedDataStore.GetDimensionOptionsFromIDsCalls()[0].Version.ID, ShouldEqual, "v1") + So(mockedDataStore.GetDimensionOptionsFromIDsCalls()[0].Ids, ShouldResemble, *expectedIDs) + } } // expected Links structure for the requested dataset version @@ -250,7 +261,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - validateCalls() + validateCalls(nil) }) }) @@ -271,7 +282,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - validateCalls() + validateCalls(nil) }) }) @@ -293,7 +304,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - validateCalls() + validateCalls(nil) }) }) @@ -317,12 +328,12 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - validateCalls() + validateCalls(nil) }) }) - Convey("When a valid dimension and list of existing IDs is provided", func() { - r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?ids=op1,op3", nil) + Convey("When a valid dimension and list of existing IDs is provided in more than one parameter, in comma-separated format", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?id=op1,op3&id=op5", nil) w := call(r) Convey("Then the call succeeds with 200 OK code, expected body and calls", func() { @@ -330,23 +341,21 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { Items: []models.PublicDimensionOption{ {Option: "op1", Links: expectedLinks}, {Option: "op3", Links: expectedLinks}, + {Option: "op5", Links: expectedLinks}, }, - Count: 2, + Count: 3, Offset: 0, Limit: 0, TotalCount: 5, } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - So(datasetPermissions.Required.Calls, ShouldEqual, 1) - So(permissions.Required.Calls, ShouldEqual, 0) - So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) - So(len(mockedDataStore.GetDimensionOptionsFromIDsCalls()), ShouldEqual, 1) + validateCalls(&[]string{"op1", "op3", "op5"}) }) }) Convey("When a valid offset, limit and dimension and list of existing IDs are provided", func() { - r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?ids=op1,op3&offset=0&limit=1", nil) + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?id=op1,op3&offset=0&limit=1", nil) w := call(r) Convey("Then the call succeeds with 200 OK code, the list of IDs take precedence (offset and limit are ignored), and the expected body and calls are performed", func() { @@ -362,10 +371,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { } So(w.Code, ShouldEqual, http.StatusOK) validateBody(w.Body.Bytes(), expectedResponse) - So(datasetPermissions.Required.Calls, ShouldEqual, 1) - So(permissions.Required.Calls, ShouldEqual, 0) - So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) - So(len(mockedDataStore.GetDimensionOptionsFromIDsCalls()), ShouldEqual, 1) + validateCalls(&[]string{"op1", "op3"}) }) }) }) @@ -408,6 +414,17 @@ func TestGetDimensionOptionsReturnsErrors(t *testing.T) { So(datasetPermissions.Required.Calls, ShouldEqual, 1) So(permissions.Required.Calls, ShouldEqual, 0) }) + + Convey("Then providing more IDs than the maximum allowed results in 400 BadRequest response", func() { + MaxIDs = func() int { return 5 } + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?id=id1,id2,id3&id=id4,id5,id6", nil) + w := call(r) + + So(w.Code, ShouldEqual, http.StatusBadRequest) + So(w.Body.String(), ShouldContainSubstring, errs.ErrTooManyQueryParameters.Error()) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + }) }) Convey("When the version doesn't exist in a request for dimension options, then return not found", t, func() { @@ -456,6 +473,31 @@ func TestGetDimensionOptionsReturnsErrors(t *testing.T) { So(len(mockedDataStore.GetDimensionOptionsCalls()), ShouldEqual, 1) }) + Convey("When an internal error causes failure to retrieve dimension options from IDs, then return internal server error", t, func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?id=id1", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { + return &models.Version{State: models.AssociatedState}, nil + }, + GetDimensionOptionsFromIDsFunc: func(version *models.Version, dimension string, ids []string) (*models.DimensionOptionResults, error) { + return nil, errs.ErrInternalServer + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + api.Router.ServeHTTP(w, r) + + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(w.Body.String(), ShouldContainSubstring, errs.ErrInternalServer.Error()) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionOptionsFromIDsCalls()), ShouldEqual, 1) + }) + Convey("When the version has an invalid state return internal server error", t, func() { r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options", nil) w := httptest.NewRecorder() From cf1e92815e376ab2cd27048e498b35f8c3d48302 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Tue, 22 Dec 2020 10:26:23 +0100 Subject: [PATCH 6/8] described 'id' query paramters in swagger --- go.mod | 2 -- go.sum | 5 ----- swagger.yaml | 9 ++++++++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 527a70f9..ad31c946 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,4 @@ require ( github.com/satori/go.uuid v1.2.0 github.com/smartystreets/goconvey v1.6.4 gopkg.in/avro.v0 v0.0.0-20171217001914-a730b5802183 // indirect - gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect - gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index 38a9b3cc..30df9138 100644 --- a/go.sum +++ b/go.sum @@ -33,7 +33,6 @@ github.com/ONSdigital/go-ns v0.0.0-20191104121206-f144c4ec2e58/go.mod h1:iWos35i github.com/ONSdigital/go-ns v0.0.0-20200205115900-a11716f93bad/go.mod h1:uHT6LaUlRbJsJRrIlN31t+QLUB80tAbk6ZR9sfoHL8Y= github.com/ONSdigital/go-ns v0.0.0-20200511161740-afc39066ee62 h1:DyGIxcRIEEHGGb/FuvrWoYXvpbR0WDbnvEn9kBF4tvM= github.com/ONSdigital/go-ns v0.0.0-20200511161740-afc39066ee62/go.mod h1:uHT6LaUlRbJsJRrIlN31t+QLUB80tAbk6ZR9sfoHL8Y= -github.com/ONSdigital/go-ns v0.0.0-20200902154605-290c8b5ba5eb h1:JQyVnHu+gr8NL+QTd2Dt+/03WRs21UWoU3HqOmTKnJE= github.com/ONSdigital/golang-neo4j-bolt-driver v0.0.0-20190228153339-da534111531d h1:Z0FsB7q0SG3tG4O/WGv0hh1MyxScyZ5JWjECEgVCIzM= github.com/ONSdigital/golang-neo4j-bolt-driver v0.0.0-20190228153339-da534111531d/go.mod h1:75Sxr59AMz2RiPskqSymLFxdeaIEhnkNaJE5lonMS3M= github.com/ONSdigital/graphson v0.0.0-20190718134034-c13ceacd109d h1:yrCtEGlohmA3OnXtke0nOOp/m9O83orpSnTGOfYOw1Q= @@ -192,10 +191,6 @@ gopkg.in/jcmturner/gokrb5.v7 v7.5.0 h1:a9tsXlIDD9SKxotJMK3niV7rPZAJeX2aD/0yg3qlI gopkg.in/jcmturner/gokrb5.v7 v7.5.0/go.mod h1:l8VISx+WGYp+Fp7KRbsiUuXTTOnxIc3Tuvyavf11/WM= gopkg.in/jcmturner/rpc.v1 v1.1.0 h1:QHIUxTX1ISuAv9dD2wJ9HWQVuWDX/Zc0PfeC2tjc4rU= gopkg.in/jcmturner/rpc.v1 v1.1.0/go.mod h1:YIdkC4XfD6GXbzje11McwsDuOlZQSb9W4vfLvuNnlv8= -gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 h1:VpOs+IwYnYBaFnrNAeB8UUWtL3vEUnzSCL1nVjPhqrw= -gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= -gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= -gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200601152816-913338de1bd2 h1:VEmvx0P+GVTgkNu2EdTN988YCZPcD3lo9AoczZpucwc= gopkg.in/yaml.v3 v3.0.0-20200601152816-913338de1bd2/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/swagger.yaml b/swagger.yaml index f8c8cbb6..aeb6547e 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -172,6 +172,12 @@ parameters: in: query required: false type: integer + ids: + name: id + description: "List of ids, as comma separated values and/or as multiple query parameters with the same key (e.g. 'id=op1,op2&id=op3'). It defines the IDs that we want to retrieve. If provided, it takes precedence over offset and limit." + in: query + required: false + type: string securityDefinitions: FlorenceAPIKey: name: florence-token @@ -451,7 +457,7 @@ paths: tags: - "Public" summary: "Get a list of options from a dimension" - description: "Get a list of options which appear in this dimension and dataset. By default all options are returned, but a subset can be requested by providing offset and limit query paramters." + description: "Get a list of options which appear in this dimension and dataset. By default all options are returned, but a subset can be requested by providing offset and limit query parameters, or by providing the list of opion IDs that we want, only the IDs that can be found will be returned." parameters: - $ref: '#/parameters/dimension' - $ref: '#/parameters/edition' @@ -459,6 +465,7 @@ paths: - $ref: '#/parameters/version' - $ref: '#/parameters/limit' - $ref: '#/parameters/offset' + - $ref: '#/parameters/ids' responses: 200: description: "Json object containing all options for a dimension" From 293c8072c02d91e174132582b714c723c8ecb261 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Tue, 22 Dec 2020 10:44:02 +0100 Subject: [PATCH 7/8] fixed race condition in test --- api/dimensions_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/dimensions_test.go b/api/dimensions_test.go index e87167bc..e724b259 100644 --- a/api/dimensions_test.go +++ b/api/dimensions_test.go @@ -381,6 +381,8 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) { func TestGetDimensionOptionsReturnsErrors(t *testing.T) { t.Parallel() + MaxIDs = func() int { return 5 } + Convey("Given a set of mocked dependencies", t, func() { // permissions mocks @@ -416,7 +418,6 @@ func TestGetDimensionOptionsReturnsErrors(t *testing.T) { }) Convey("Then providing more IDs than the maximum allowed results in 400 BadRequest response", func() { - MaxIDs = func() int { return 5 } r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions/age/options?id=id1,id2,id3&id=id4,id5,id6", nil) w := call(r) From 327a1f241bd693655fb7164919621f21b13516df Mon Sep 17 00:00:00 2001 From: David Subiros Date: Wed, 23 Dec 2020 15:28:47 +0100 Subject: [PATCH 8/8] fixed typos --- api/dimensions.go | 8 ++++---- swagger.yaml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/dimensions.go b/api/dimensions.go index 29c276b6..2c6f832f 100644 --- a/api/dimensions.go +++ b/api/dimensions.go @@ -153,7 +153,7 @@ func getPositiveIntQueryParameter(queryVars url.Values, varKey string, defaultVa // by parsing all values with key 'varKey' and splitting the values by commas, if they contain commas. // Up to maxNumItems values are allowed in total. func getQueryParamListValues(queryVars url.Values, varKey string, maxNumItems int) (items []string, err error) { - // get query paramters values for the provided key + // get query parameters values for the provided key values, found := queryVars[varKey] if !found { return []string{}, nil @@ -189,7 +189,7 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques ids, err := getQueryParamListValues(r.URL.Query(), "id", MaxIDs()) if err != nil { logData["query_params"] = r.URL.RawQuery - handleDimensionsErr(ctx, w, "failed to obtain list of IDs from request query paramters", err, logData) + handleDimensionsErr(ctx, w, "failed to obtain list of IDs from request query parameters", err, logData) return } @@ -200,7 +200,7 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques limit, err = getPositiveIntQueryParameter(r.URL.Query(), "limit", api.defaultLimit) if err != nil { logData["query_params"] = r.URL.RawQuery - handleDimensionsErr(ctx, w, "failed to obtain limit from request query paramters", err, logData) + handleDimensionsErr(ctx, w, "failed to obtain limit from request query parameters", err, logData) return } @@ -208,7 +208,7 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques offset, err = getPositiveIntQueryParameter(r.URL.Query(), "offset", api.defaultOffset) if err != nil { logData["query_params"] = r.URL.RawQuery - handleDimensionsErr(ctx, w, "failed to obtain offset from request query paramters", err, logData) + handleDimensionsErr(ctx, w, "failed to obtain offset from request query parameters", err, logData) return } } diff --git a/swagger.yaml b/swagger.yaml index aeb6547e..a60a7c15 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -457,7 +457,7 @@ paths: tags: - "Public" summary: "Get a list of options from a dimension" - description: "Get a list of options which appear in this dimension and dataset. By default all options are returned, but a subset can be requested by providing offset and limit query parameters, or by providing the list of opion IDs that we want, only the IDs that can be found will be returned." + description: "Get a list of options which appear in this dimension and dataset. By default all options are returned, but a subset can be requested by providing offset and limit query parameters, or by providing the list of option IDs, only the IDs that are found will be returned." parameters: - $ref: '#/parameters/dimension' - $ref: '#/parameters/edition'