Merge branch 'develop' into release/1.25.0

.gitignore

@@ -2,4 +2,5 @@
 debug
 *.tar.gz
 build
-.idea
+.idea
+.vscode

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 (

api/dimensions.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 
 	errs "github.com/ONSdigital/dp-dataset-api/apierrors"
 	"github.com/ONSdigital/dp-dataset-api/models"
@@ -15,6 +16,13 @@ import (
 	"github.com/gorilla/mux"
 )
 
+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()
 	vars := mux.Vars(r)
@@ -141,6 +149,26 @@ func getPositiveIntQueryParameter(queryVars url.Values, varKey string, defaultVa
 	return val, nil
 }
 
+// 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 parameters values for the provided key
+	values, found := queryVars[varKey]
+	if !found {
+		return []string{}, nil
+	}
+
+	// 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
+}
+
 func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	vars := mux.Vars(r)
@@ -157,20 +185,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 := getQueryParamListValues(r.URL.Query(), "id", 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 parameters", 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 parameters", 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 parameters", err, logData)
+			return
+		}
 	}
 
 	// ger version for provided dataset, edition and versionID
@@ -187,11 +227,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

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"
@@ -143,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 {
@@ -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
@@ -189,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
@@ -229,7 +261,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) {
 				}
 				So(w.Code, ShouldEqual, http.StatusOK)
 				validateBody(w.Body.Bytes(), expectedResponse)
-				validateCalls()
+				validateCalls(nil)
 			})
 		})
 
@@ -250,7 +282,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) {
 				}
 				So(w.Code, ShouldEqual, http.StatusOK)
 				validateBody(w.Body.Bytes(), expectedResponse)
-				validateCalls()
+				validateCalls(nil)
 			})
 		})
 
@@ -272,7 +304,7 @@ func TestGetDimensionOptionsReturnsOk(t *testing.T) {
 				}
 				So(w.Code, ShouldEqual, http.StatusOK)
 				validateBody(w.Body.Bytes(), expectedResponse)
-				validateCalls()
+				validateCalls(nil)
 			})
 		})
 
@@ -296,7 +328,50 @@ 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 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() {
+				expectedResponse := models.DimensionOptionResults{
+					Items: []models.PublicDimensionOption{
+						{Option: "op1", Links: expectedLinks},
+						{Option: "op3", Links: expectedLinks},
+						{Option: "op5", Links: expectedLinks},
+					},
+					Count:      3,
+					Offset:     0,
+					Limit:      0,
+					TotalCount: 5,
+				}
+				So(w.Code, ShouldEqual, http.StatusOK)
+				validateBody(w.Body.Bytes(), expectedResponse)
+				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?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() {
+				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)
+				validateCalls(&[]string{"op1", "op3"})
 			})
 		})
 	})
@@ -306,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
@@ -339,6 +416,16 @@ 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() {
+			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() {
@@ -387,6 +474,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()

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,

config/config.go

@@ -68,7 +68,7 @@ func Get() (*Configuration, error) {
 			BindAddr:   "localhost:27017",
 			Collection: "datasets",
 			Database:   "datasets",
-			Limit:      0,
+			Limit:      100,
 			Offset:     0,
 		},
 	}

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)