From e36a5e9ad640699dbb33f756eeb34236dc7ad401 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Tue, 26 Jan 2021 16:01:37 +0000 Subject: [PATCH 1/2] encode id query parameters when requesting options to dataset api --- api/filter_dimension_options_test.go | 53 ++++++++++++++++++++++++++++ api/filters.go | 6 +++- models/models.go | 10 ++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/api/filter_dimension_options_test.go b/api/filter_dimension_options_test.go index 333a7a9a..91b156f2 100644 --- a/api/filter_dimension_options_test.go +++ b/api/filter_dimension_options_test.go @@ -56,6 +56,59 @@ func TestSuccessfulAddFilterBlueprintDimensionOption(t *testing.T) { So(*datasetAPIMock.GetOptionsBatchProcessCalls()[0].OptionIDs, ShouldResemble, []string{"33"}) }) }) + + // Convey("Given that a dimension option with special characters is successfully added to a filter", t, func() { + // r, err := http.NewRequest("POST", "http://localhost:22100/filters/12345678/dimensions/age/options/90+", nil) + // So(err, ShouldBeNil) + + // w := httptest.NewRecorder() + + // datasetAPIMock := mocks.NewDatasetAPI().Mock + // datasetAPIMock.GetOptionsBatchProcessFunc = func(ctx context.Context, userAuthToken, serviceAuthToken, collectionID, id, edition, version, dimension string, optionIDs *[]string, processBatch dataset.OptionsBatchProcessor, batchSize int, maxWorkers int) (err error) { + // opts := dataset.Options{ + // Items: []dataset.Option{ + // { + // Option: "90%2B", + // Label: "90 or more", + // }, + // }, + // Count: 1, + // Offset: 0, + // Limit: 0, + // TotalCount: 1, + // } + // _, err = processBatch(opts) + // return err + // } + + // dataStoreMock := mocks.NewDataStore().Mock + // dataStoreMock.GetFilterFunc = func(filerID string) (*models.Filter, error) { + // return &models.Filter{ + // Dimensions: []models.Dimension{ + // { + // Name: "age", + // Options: []string{"90+"}, + // }, + // }, + // Dataset: &models.Dataset{ + // Version: 1, + // }, + // }, nil + // } + + // api := Setup(cfg(), mux.NewRouter(), dataStoreMock, &mocks.FilterJob{}, datasetAPIMock, previewMock) + // api.router.ServeHTTP(w, r) + + // Convey("Then a 201 Created status code is returned", func() { + // So(w.Code, ShouldEqual, http.StatusCreated) + // }) + + // Convey("And the dimension and options are efficiently validated with dataset API, with the IDs correctly escaped", func() { + // So(datasetAPIMock.GetVersionDimensionsCalls(), ShouldHaveLength, 1) + // So(datasetAPIMock.GetOptionsBatchProcessCalls(), ShouldHaveLength, 1) + // So(*datasetAPIMock.GetOptionsBatchProcessCalls()[0].OptionIDs, ShouldResemble, []string{`90%2B`}) + // }) + // }) } func TestFailedToAddFilterBlueprintDimensionOption(t *testing.T) { diff --git a/api/filters.go b/api/filters.go index 4380ad2c..48e73ed0 100644 --- a/api/filters.go +++ b/api/filters.go @@ -479,6 +479,10 @@ func (api *FilterAPI) getDimensionOptionsBatchProcess(ctx context.Context, dimen return nil } + // get encoded IDs so that they can be used as query paramters + encodedIDs := dimension.EncodedOptions() + + // validate the options with Dataset API, in batches err := api.datasetAPI.GetOptionsBatchProcess(ctx, getUserAuthToken(ctx), api.serviceAuthToken, @@ -487,7 +491,7 @@ func (api *FilterAPI) getDimensionOptionsBatchProcess(ctx context.Context, dimen dataset.Edition, strconv.Itoa(dataset.Version), dimension.Name, - &dimension.Options, + &encodedIDs, processBatch, api.maxDatasetOptions, api.BatchMaxWorkers) diff --git a/models/models.go b/models/models.go index a8e8e514..a767e4fa 100644 --- a/models/models.go +++ b/models/models.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/ioutil" + "net/url" "time" "github.com/ONSdigital/dp-api-clients-go/dataset" @@ -75,6 +76,15 @@ type Dimension struct { Options []string `bson:"options" json:"options"` } +// EncodedOptions returns the list of options for this dimension after escaping the values for URL query paramters +func (d *Dimension) EncodedOptions() []string { + encodedIDs := make([]string, len(d.Options)) + for i, op := range d.Options { + encodedIDs[i] = url.QueryEscape(op) + } + return encodedIDs +} + // PublicDimension represents information about a single dimension as served by /dimensions and /dimensions/ type PublicDimension struct { Name string `bson:"name" json:"name"` From 635709c6d21f4433b23c7403f6eeddb2b7833093 Mon Sep 17 00:00:00 2001 From: David Subiros Date: Tue, 26 Jan 2021 17:30:07 +0000 Subject: [PATCH 2/2] updated version of dp-api-clients-go --- api/filter_dimension_options_test.go | 53 ---------------------------- go.mod | 2 +- go.sum | 4 +-- 3 files changed, 3 insertions(+), 56 deletions(-) diff --git a/api/filter_dimension_options_test.go b/api/filter_dimension_options_test.go index 91b156f2..333a7a9a 100644 --- a/api/filter_dimension_options_test.go +++ b/api/filter_dimension_options_test.go @@ -56,59 +56,6 @@ func TestSuccessfulAddFilterBlueprintDimensionOption(t *testing.T) { So(*datasetAPIMock.GetOptionsBatchProcessCalls()[0].OptionIDs, ShouldResemble, []string{"33"}) }) }) - - // Convey("Given that a dimension option with special characters is successfully added to a filter", t, func() { - // r, err := http.NewRequest("POST", "http://localhost:22100/filters/12345678/dimensions/age/options/90+", nil) - // So(err, ShouldBeNil) - - // w := httptest.NewRecorder() - - // datasetAPIMock := mocks.NewDatasetAPI().Mock - // datasetAPIMock.GetOptionsBatchProcessFunc = func(ctx context.Context, userAuthToken, serviceAuthToken, collectionID, id, edition, version, dimension string, optionIDs *[]string, processBatch dataset.OptionsBatchProcessor, batchSize int, maxWorkers int) (err error) { - // opts := dataset.Options{ - // Items: []dataset.Option{ - // { - // Option: "90%2B", - // Label: "90 or more", - // }, - // }, - // Count: 1, - // Offset: 0, - // Limit: 0, - // TotalCount: 1, - // } - // _, err = processBatch(opts) - // return err - // } - - // dataStoreMock := mocks.NewDataStore().Mock - // dataStoreMock.GetFilterFunc = func(filerID string) (*models.Filter, error) { - // return &models.Filter{ - // Dimensions: []models.Dimension{ - // { - // Name: "age", - // Options: []string{"90+"}, - // }, - // }, - // Dataset: &models.Dataset{ - // Version: 1, - // }, - // }, nil - // } - - // api := Setup(cfg(), mux.NewRouter(), dataStoreMock, &mocks.FilterJob{}, datasetAPIMock, previewMock) - // api.router.ServeHTTP(w, r) - - // Convey("Then a 201 Created status code is returned", func() { - // So(w.Code, ShouldEqual, http.StatusCreated) - // }) - - // Convey("And the dimension and options are efficiently validated with dataset API, with the IDs correctly escaped", func() { - // So(datasetAPIMock.GetVersionDimensionsCalls(), ShouldHaveLength, 1) - // So(datasetAPIMock.GetOptionsBatchProcessCalls(), ShouldHaveLength, 1) - // So(*datasetAPIMock.GetOptionsBatchProcessCalls()[0].OptionIDs, ShouldResemble, []string{`90%2B`}) - // }) - // }) } func TestFailedToAddFilterBlueprintDimensionOption(t *testing.T) { diff --git a/go.mod b/go.mod index 864aac10..2f08af7d 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/ONSdigital/dp-filter-api go 1.15 require ( - github.com/ONSdigital/dp-api-clients-go v1.32.6 + github.com/ONSdigital/dp-api-clients-go v1.32.10 github.com/ONSdigital/dp-graph/v2 v2.3.0 github.com/ONSdigital/dp-healthcheck v1.0.5 github.com/ONSdigital/dp-kafka/v2 v2.1.2 diff --git a/go.sum b/go.sum index 3ae24149..c6f64616 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ github.com/ONSdigital/dp-api-clients-go v1.1.0/go.mod h1:9lqor0I7caCnRWr04gU/r7x5dqxgoODob8L48q+cE4E= github.com/ONSdigital/dp-api-clients-go v1.28.0/go.mod h1:iyJy6uRL4B6OYOJA0XMr5UHt6+Q8XmN9uwmURO+9Oj4= -github.com/ONSdigital/dp-api-clients-go v1.32.6 h1:jLAhlSAFx8iV9P9K5TBEkzQMnl3gxAT5IzvyKSE3tQ0= -github.com/ONSdigital/dp-api-clients-go v1.32.6/go.mod h1:0pUK3MN1v7DTjq0JSAD+DqbsZ8AVTodrXSXgJecg9Pw= +github.com/ONSdigital/dp-api-clients-go v1.32.10 h1:oH3+9PHPn4EFr/m4tEdLi/yW1W6vu5t+gNE8qPzr0lM= +github.com/ONSdigital/dp-api-clients-go v1.32.10/go.mod h1:0pUK3MN1v7DTjq0JSAD+DqbsZ8AVTodrXSXgJecg9Pw= github.com/ONSdigital/dp-frontend-models v1.1.0/go.mod h1:TT96P7Mi69N3Tc/jFNdbjiwG4GAaMjP26HLotFQ6BPw= github.com/ONSdigital/dp-graph/v2 v2.3.0 h1:xK9qImVbh86l04aAUeurjB7d8mwn27eacP+5gpvPLO8= github.com/ONSdigital/dp-graph/v2 v2.3.0/go.mod h1:K4LIhFcyxB8g7nUG5I5I8x6QVf89x82dCEFBbE0mmaQ=