updated validation for dataset type ,dataset nomis url and added rele…

…vant tests

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)

api/dataset_test.go

@@ -33,10 +33,9 @@ const (
 )
 
 var (
-	datasetPayload = `{"contacts":[{"email":"[email protected]","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":"[email protected]","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":"[email protected]","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":"[email protected]","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":"[email protected]","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":"[email protected]","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":"[email protected]","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":"[email protected]","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\"}"

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{

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 {