diff --git a/api/dataset.go b/api/dataset.go index 66724d85..dca9a44b 100644 --- a/api/dataset.go +++ b/api/dataset.go @@ -29,6 +29,8 @@ var ( // errors that should return a 400 status datasetsBadRequest = map[error]bool{ errs.ErrAddUpdateDatasetBadRequest: true, + errs.ErrTypeMismatch: true, + errs.ErrDatasetTypeInvalid: true, } // errors that should return a 404 status @@ -175,6 +177,19 @@ func (api *DatasetAPI) addDataset(w http.ResponseWriter, r *http.Request) { return nil, errs.ErrAddUpdateDatasetBadRequest } + dataType, err := models.ValidateDatasetType(ctx, dataset.Type) + if err != nil { + log.Event(ctx, "addDataset endpoint: error Invalid dataset type", log.INFO, logData) + return nil, err + } + + datasetType, err := models.ValidateNomisURL(ctx, dataType.String(), dataset.NomisReferenceURL) + if err != nil { + log.Event(ctx, "addDataset endpoint: error dataset.Type mismatch", log.INFO, logData) + return nil, err + } + + dataset.Type = datasetType dataset.State = models.CreatedState dataset.ID = datasetID @@ -194,7 +209,7 @@ func (api *DatasetAPI) addDataset(w http.ResponseWriter, r *http.Request) { dataset.Links.LatestVersion = nil dataset.LastUpdated = time.Now() - + datasetDoc := &models.DatasetUpdate{ ID: datasetID, Next: dataset, @@ -251,6 +266,14 @@ func (api *DatasetAPI) putDataset(w http.ResponseWriter, r *http.Request) { return err } + dataset.Type = currentDataset.Next.Type + + _, err = models.ValidateNomisURL(ctx, dataset.Type, dataset.NomisReferenceURL) + if err != nil { + log.Event(ctx, "putDataset endpoint: error dataset.Type mismatch", log.INFO, log.Error(err)) + return err + } + if dataset.State == models.PublishedState { if err := api.publishDataset(ctx, currentDataset, nil); err != nil { log.Event(ctx, "putDataset endpoint: failed to update dataset document to published", log.ERROR, log.Error(err), data) diff --git a/api/dataset_test.go b/api/dataset_test.go index d7925ef6..08f96205 100644 --- a/api/dataset_test.go +++ b/api/dataset_test.go @@ -33,10 +33,9 @@ const ( ) var ( - datasetPayload = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","periodicity":"yearly","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"}}` - - urlBuilder = url.NewBuilder("localhost:20000") - mu sync.Mutex + datasetPayload = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"type":"nomis","nomis_reference_url":"https://www.nomis.co.uk"}` + urlBuilder = url.NewBuilder("localhost:20000") + mu sync.Mutex ) func getAuthorisationHandlerMock() *mocks.AuthHandlerMock { @@ -397,6 +396,110 @@ func TestPostDatasetReturnsError(t *testing.T) { So(err, ShouldEqual, io.EOF) }) }) + + Convey("When the request has an filterable datatype and nomis url it should return type mismatch error", t, func() { + var b string + b = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"type":"filterable","nomis_reference_url":"https://www.nomis.co.uk"}` + + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123123", bytes.NewBufferString(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + UpsertDatasetFunc: func(string, *models.DatasetUpdate) error { + return nil + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + api.Router.ServeHTTP(w, r) + + So(w.Body.String(), ShouldResemble, "type mismatch\n") + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(w.Code, ShouldEqual, http.StatusBadRequest) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0) + + Convey("then the request body has been drained", func() { + _, err = r.Body.Read(make([]byte, 1)) + So(err, ShouldEqual, io.EOF) + }) + }) + + Convey("When the request has an invalid datatype it should return invalid type errorq", t, func() { + var b string + b = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"type":"nomis_filterable"}` + + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + UpsertDatasetFunc: func(string, *models.DatasetUpdate) error { + return nil + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + api.Router.ServeHTTP(w, r) + + So(w.Code, ShouldEqual, http.StatusBadRequest) + So(w.Body.String(), ShouldResemble, "invalid dataset type\n") + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + + Convey("then the request body has been drained", func() { + _, err = r.Body.Read(make([]byte, 1)) + So(err, ShouldEqual, io.EOF) + }) + }) + + Convey("When the request body has an empty type field it should create a dataset with type defaulted to filterable", t, func() { + var b string + b = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"type":""}` + res := `{"id":"123123","next":{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","id":"123123","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"},"editions":{"href":"http://localhost:22000/datasets/123123/editions"},"self":{"href":"http://localhost:22000/datasets/123123"}},"next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department"},"state":"created","theme":"population","title":"CensusEthnicity","type":"filterable"}}` + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123123", bytes.NewBufferString(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + UpsertDatasetFunc: func(string, *models.DatasetUpdate) error { + return nil + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + mockedDataStore.UpsertDataset("123123", &models.DatasetUpdate{Next: &models.Dataset{}}) + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + api.Router.ServeHTTP(w, r) + + So(w.Code, ShouldEqual, http.StatusCreated) + So(w.Body.String(), ShouldContainSubstring, res) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 2) + + Convey("then the request body has been drained", func() { + _, err = r.Body.Read(make([]byte, 1)) + So(err, ShouldEqual, io.EOF) + }) + }) + } func TestPutDatasetReturnsSuccessfully(t *testing.T) { @@ -410,7 +513,7 @@ func TestPutDatasetReturnsSuccessfully(t *testing.T) { w := httptest.NewRecorder() mockedDataStore := &storetest.StorerMock{ GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { - return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + return &models.DatasetUpdate{Next: &models.Dataset{Type: "nomis"}}, nil }, UpdateDatasetFunc: func(context.Context, string, *models.Dataset, string) error { return nil @@ -556,6 +659,83 @@ func TestPutDatasetReturnsError(t *testing.T) { }) }) + Convey("When updating the dataset nomis_reference_url and the stored dataset type is not nomis return bad request", t, func() { + var b string + b = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"nomis_reference_url":"https://www.nomis.co.uk"}` + + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{Type: "filterable"}}, nil + }, + UpdateDatasetFunc: func(context.Context, string, *models.Dataset, string) error { + return nil + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + + api.Router.ServeHTTP(w, r) + So(w.Code, ShouldEqual, http.StatusBadRequest) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + So(w.Body.String(), ShouldResemble, errs.ErrTypeMismatch.Error()+"\n") + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + Convey("then the request body has been drained", func() { + _, err = r.Body.Read(make([]byte, 1)) + So(err, ShouldEqual, io.EOF) + }) + }) + + Convey("When update dataset type has a value of filterable and stored dataset type is nomis return status ok", t, func() { + // Dataset type field cannot be updated and hence is ignored in any updates to the dataset + + var b string + b = `{"contacts":[{"email":"testing@hotmail.com","name":"John Cox","telephone":"01623 456789"}],"description":"census","links":{"access_rights":{"href":"http://ons.gov.uk/accessrights"}},"title":"CensusEthnicity","theme":"population","state":"completed","next_release":"2016-04-04","publisher":{"name":"The office of national statistics","type":"government department","url":"https://www.ons.gov.uk/"},"type":"filterable","nomis_reference_url":"https://www.nomis.co.uk"}` + + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{Type: "nomis"}}, nil + }, + UpdateDatasetFunc: func(context.Context, string, *models.Dataset, string) error { + return nil + }, + } + + datasetPermissions := getAuthorisationHandlerMock() + permissions := getAuthorisationHandlerMock() + + api := GetAPIWithMocks(mockedDataStore, &mocks.DownloadsGeneratorMock{}, datasetPermissions, permissions) + + api.Router.ServeHTTP(w, r) + So(w.Code, ShouldEqual, http.StatusOK) + So(datasetPermissions.Required.Calls, ShouldEqual, 1) + So(permissions.Required.Calls, ShouldEqual, 0) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 1) + + Convey("then the request body has been drained", func() { + _, err = r.Body.Read(make([]byte, 1)) + So(err, ShouldEqual, io.EOF) + }) + }) + Convey("When the request is not authorised to update dataset return status unauthorised", t, func() { var b string b = "{\"edition\":\"2017\",\"state\":\"created\",\"license\":\"ONS\",\"release_date\":\"2017-04-04\",\"version\":\"1\"}" diff --git a/apierrors/errors.go b/apierrors/errors.go index 03e53acd..20bd81d1 100644 --- a/apierrors/errors.go +++ b/apierrors/errors.go @@ -7,6 +7,8 @@ import ( // A list of error messages for Dataset API var ( ErrAddDatasetAlreadyExists = errors.New("forbidden - dataset already exists") + ErrDatasetTypeInvalid = errors.New("invalid dataset type") + ErrTypeMismatch = errors.New("type mismatch") ErrAddUpdateDatasetBadRequest = errors.New("failed to parse json body") ErrConflictUpdatingInstance = errors.New("conflict updating instance resource") ErrDatasetNotFound = errors.New("dataset not found") @@ -63,6 +65,8 @@ var ( ErrMissingParameters: true, ErrUnableToParseJSON: true, ErrUnableToReadMessage: true, + ErrTypeMismatch: true, + ErrDatasetTypeInvalid: true, } ConflictRequestMap = map[error]bool{ diff --git a/models/dataset.go b/models/dataset.go index fa94c33e..b6a758b2 100644 --- a/models/dataset.go +++ b/models/dataset.go @@ -22,24 +22,27 @@ type DatasetType int // possible dataset types const ( - FILTERABLE DatasetType = iota - NOMIS + Filterable DatasetType = iota + Nomis + Invalid ) -var datasetTypes = []string{"filterable", "nomis"} +var datasetTypes = []string{"filterable", "nomis", "invalid"} func (dt DatasetType) String() string { return datasetTypes[dt] } -func getDatasetType(datasetType string) DatasetType { +func GetDatasetType(datasetType string) (DatasetType, error) { switch datasetType { case "filterable": - return FILTERABLE + return Filterable, nil case "nomis": - return NOMIS + return Nomis, nil + case "": + return Filterable, nil default: - return FILTERABLE + return Invalid, errs.ErrDatasetTypeInvalid } } @@ -269,7 +272,7 @@ func CreateDataset(reader io.Reader) (*Dataset, error) { if err != nil { return nil, errs.ErrUnableToParseJSON } - dataset.Type = getDatasetType(dataset.Type).String() + return &dataset, nil } @@ -452,6 +455,26 @@ func ValidateDataset(ctx context.Context, dataset *Dataset) error { return nil } +//ValidateDatasetType checks the dataset.type field has valid type +func ValidateDatasetType(ctx context.Context, datasetType string) (*DatasetType, error) { + dataType, err := GetDatasetType(datasetType) + if err != nil { + log.Event(ctx, "error Invalid dataset type", log.ERROR, log.Error(errs.ErrDatasetTypeInvalid)) + return nil, errs.ErrDatasetTypeInvalid + } + return &dataType, nil +} + +//ValidateNomisURL checks for the nomis type when the dataset has nomis URL +func ValidateNomisURL(ctx context.Context, datasetType string, nomisURL string) (string, error) { + + if nomisURL != "" && datasetType != Nomis.String() { + log.Event(ctx, "error Type mismatch", log.ERROR, log.Error(errs.ErrDatasetTypeInvalid)) + return "", errs.ErrTypeMismatch + } + return datasetType, nil +} + // ValidateVersion checks the content of the version structure func ValidateVersion(version *Version) error { diff --git a/models/dataset_test.go b/models/dataset_test.go index bf9de7ce..3acc17af 100644 --- a/models/dataset_test.go +++ b/models/dataset_test.go @@ -23,10 +23,43 @@ func createDataset() Dataset { var testContext = context.Background() func TestGetDatasetType(t *testing.T) { - Convey("Given the dataset type to be nomis", t, func() { - Convey("Then the values should be set to the expected dataset type", func() { - result := getDatasetType("") - So(result, ShouldEqual, FILTERABLE) + Convey("Given the dataset type", t, func() { + Convey("When the type is empty", func() { + Convey("Then it should default to filterable", func() { + result, err := GetDatasetType("") + So(result, ShouldEqual, Filterable) + So(err, ShouldBeNil) + }) + }) + + Convey("When the type is invalid", func() { + Convey("Then an error should be returned", func() { + result, err := GetDatasetType("abcdefg") + So(result, ShouldEqual, Invalid) + So(err, ShouldResemble, errs.ErrDatasetTypeInvalid) + }) + }) + }) +} + +func TestValidateDatasetType(t *testing.T) { + Convey("Given a dataset type return an error ", t, func() { + Convey("When the request has invalid dataset type ", func() { + Convey("Then should return type invalid error", func() { + dt, err := ValidateDatasetType(testContext, "abc123") + So(dt, ShouldBeNil) + So(err, ShouldResemble, errs.ErrDatasetTypeInvalid) + }) + }) + }) +} +func TestValidateNomisURL(t *testing.T) { + Convey("Given a nomis URL return an error ", t, func() { + Convey("When the request has filterable type and a nomis url ", func() { + Convey("Then should return type mismatch", func() { + _, err := ValidateNomisURL(testContext, "filterable", "www.nomisweb.co.uk") + So(err, ShouldResemble, errs.ErrTypeMismatch) + }) }) }) } diff --git a/swagger.yaml b/swagger.yaml index 41ecfe14..81e63cd6 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -83,7 +83,12 @@ parameters: in: body required: true schema: - $ref: '#/definitions/Dataset' + allOf: + - type: object + properties: + type: + $ref: '#/definitions/Type' + - $ref: "#/definitions/Dataset" new_edition: name: new_edition description: "A new edition of the dataset" @@ -122,6 +127,13 @@ parameters: description: "A comma separated list of state values to filter on (e.g. ‘completed,edition-confirmed’)" in: query type: string + update_dataset: + name: dataset + description: "An update to a dataset" + in: body + required: true + schema: + $ref: "#/definitions/Dataset" update_dimension: name: dimension description: "A dimension object to update for a given instance" @@ -227,12 +239,10 @@ paths: description: "Update the metadata for the next release of the dataset. The dataset contains all high level information, for additional details see editions or versions of a dataset." parameters: - $ref: '#/parameters/id' - - $ref: '#/parameters/new_dataset' + - $ref: '#/parameters/update_dataset' responses: 200: description: "A json object for a single Dataset" - schema: - $ref: '#/definitions/NewDatasetResponse' 400: description: "Bad Request due to invalid json in the request body" 401: @@ -946,6 +956,8 @@ definitions: example: "DE3BC0B6-D6C4-4E20-917E-95D7EA8C91DC" readOnly: true type: string + type: + $ref: "#/definitions/Type" - $ref: "#/definitions/Dataset" Dataset: description: "The dataset" @@ -992,6 +1004,10 @@ definitions: next_release: description: "The next release date for a dataset" type: string + nomis_ref_url: + description: "The NOMIS reference url for the dataset" + type: string + example: "https://www.nomisweb.co.uk/census/2011/ks106ew" publications: description: "A list of publications for a dataset" type: array @@ -1052,15 +1068,12 @@ definitions: uri: description: "The uri to the location of this resource on the web" type: string - type: - description: "The type for a dataset" - type: string - enum: [filterable, nomis] - example: "filterable" - nomis_ref_url: - description: "The NOMIS reference url for the dataset" - type: string - example: "https://www.nomisweb.co.uk/census/2011/ks106ew" + + Type: + description: "The type for a dataset" + type: string + enum: [filterable, nomis] + default: "filterable" Dimension: description: "A single dimension within a dataset" type: object @@ -1617,6 +1630,8 @@ definitions: properties: collection_id: $ref: '#/definitions/CollectionID' + type: + $ref: '#/definitions/Type' - $ref: "#/definitions/Dataset" next: allOf: @@ -1624,6 +1639,8 @@ definitions: properties: collection_id: $ref: '#/definitions/CollectionID' + type: + $ref: '#/definitions/Type' - $ref: "#/definitions/Dataset" NewInstance: description: "A model for the request and response body for creating a new instance" @@ -1809,6 +1826,28 @@ definitions: frequency: description: "The time frequency the version of the dataset covers for the period of time between start_date and end_date" type: string + UpdateDatasetResponse: + description: "A model for the response body when creating a new dataset" + type: object + properties: + id: + description: "An unique id for a dataset" + example: "DE3BC0B6-D6C4-4E20-917E-95D7EA8C91DC" + type: string + current: + allOf: + - type: object + properties: + collection_id: + $ref: '#/definitions/CollectionID' + - $ref: "#/definitions/Dataset" + next: + allOf: + - type: object + properties: + collection_id: + $ref: '#/definitions/CollectionID' + - $ref: "#/definitions/Dataset" UpdateDimensionOptionRequest: description: "A cached dimension. (Only used by the Private API)" type: object