diff --git a/api/api.go b/api/api.go index c9b8ac69..4807338a 100644 --- a/api/api.go +++ b/api/api.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "io/ioutil" "net/http" "time" @@ -18,12 +17,14 @@ import ( "github.com/ONSdigital/dp-dataset-api/url" "github.com/ONSdigital/go-ns/audit" "github.com/ONSdigital/go-ns/common" + "github.com/ONSdigital/go-ns/handlers/requestID" "github.com/ONSdigital/go-ns/healthcheck" "github.com/ONSdigital/go-ns/identity" "github.com/ONSdigital/go-ns/log" "github.com/ONSdigital/go-ns/server" "github.com/gorilla/mux" "github.com/justinas/alice" + "github.com/pkg/errors" ) var httpServer *server.Server @@ -38,19 +39,25 @@ const ( dimensionOptionDocType = "dimension-option" // audit actions - getDatasetsAction = "getDatasets" - getDatasetAction = "getDataset" - getEditionsAction = "getEditions" - getEditionAction = "getEdition" - getVersionsAction = "getVersions" - getVersionAction = "getVersion" + getDatasetsAction = "getDatasets" + getDatasetAction = "getDataset" + deleteDatasetAction = "deleteDataset" + addDatasetAction = "addDataset" + putDatasetAction = "putDataset" + getEditionsAction = "getEditions" + getEditionAction = "getEdition" + getVersionsAction = "getVersions" + getVersionAction = "getVersion" + getDimensionsAction = "getDimensions" + getMetadataAction = "getMetadata" // audit results actionAttempted = "attempted" actionSuccessful = "successful" actionUnsuccessful = "unsuccessful" - auditError = "error while attempting to record audit event, failing request" + auditError = "error while attempting to record audit event, failing request" + auditActionErr = "failed to audit action" ) // PublishCheck Checks if an version has been published @@ -183,11 +190,23 @@ func handleErrorType(docType string, err error, w http.ResponseWriter) { switch docType { default: - if err == errs.ErrDatasetNotFound || err == errs.ErrEditionNotFound || err == errs.ErrVersionNotFound || err == errs.ErrDimensionNodeNotFound || err == errs.ErrInstanceNotFound { + if err == errs.ErrEditionNotFound || err == errs.ErrVersionNotFound || err == errs.ErrDimensionNodeNotFound || err == errs.ErrInstanceNotFound { http.Error(w, err.Error(), http.StatusNotFound) } else { http.Error(w, err.Error(), http.StatusInternalServerError) } + case "dataset": + if err == errs.ErrDatasetNotFound { + http.Error(w, err.Error(), http.StatusNotFound) + } else if err == errs.ErrDeleteDatasetNotFound { + http.Error(w, err.Error(), http.StatusNoContent) + } else if err == errs.ErrDeletePublishedDatasetForbidden || err == errs.ErrAddDatasetAlreadyExists { + http.Error(w, err.Error(), http.StatusForbidden) + } else if err == errs.ErrAddUpdateDatasetBadRequest { + http.Error(w, err.Error(), http.StatusBadRequest) + } else { + http.Error(w, err.Error(), http.StatusInternalServerError) + } case "edition": if err == errs.ErrDatasetNotFound { http.Error(w, err.Error(), http.StatusNotFound) @@ -345,6 +364,45 @@ func handleAuditingFailure(w http.ResponseWriter, err error, logData log.Data) { http.Error(w, "internal server error", http.StatusInternalServerError) } +func auditActionFailure(ctx context.Context, auditedAction string, auditedResult string, err error, logData log.Data) { + if logData == nil { + logData = log.Data{} + } + + logData["auditAction"] = auditedAction + logData["auditResult"] = auditedResult + + logError(ctx, errors.WithMessage(err, auditActionErr), logData) +} + +func logError(ctx context.Context, err error, data log.Data) { + if data == nil { + data = log.Data{} + } + reqID := requestID.Get(ctx) + if user := common.User(ctx); user != "" { + data["user"] = user + } + if caller := common.Caller(ctx); caller != "" { + data["caller"] = caller + } + log.ErrorC(reqID, err, data) +} + +func logInfo(ctx context.Context, message string, data log.Data) { + if data == nil { + data = log.Data{} + } + reqID := requestID.Get(ctx) + if user := common.User(ctx); user != "" { + data["user"] = user + } + if caller := common.Caller(ctx); caller != "" { + data["caller"] = caller + } + log.InfoC(reqID, message, data) +} + // Close represents the graceful shutting down of the http server func Close(ctx context.Context) error { if err := httpServer.Shutdown(ctx); err != nil { diff --git a/api/dataset.go b/api/dataset.go index 2d501916..c9ea158a 100644 --- a/api/dataset.go +++ b/api/dataset.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "fmt" "net/http" @@ -11,244 +12,296 @@ import ( "github.com/ONSdigital/go-ns/common" "github.com/ONSdigital/go-ns/log" "github.com/gorilla/mux" + "github.com/pkg/errors" ) func (api *DatasetAPI) getDatasets(w http.ResponseWriter, r *http.Request) { - if err := api.auditor.Record(r.Context(), getDatasetsAction, actionAttempted, nil); err != nil { - handleAuditingFailure(w, err, nil) + ctx := r.Context() + if err := api.auditor.Record(ctx, getDatasetsAction, actionAttempted, nil); err != nil { + auditActionFailure(ctx, getDatasetsAction, actionAttempted, err, nil) + handleDatasetAPIErr(ctx, errs.ErrAuditActionAttemptedFailure, w, nil) return } - results, err := api.dataStore.Backend.GetDatasets() - if err != nil { - log.Error(err, nil) - if auditErr := api.auditor.Record(r.Context(), getDatasetsAction, actionUnsuccessful, nil); auditErr != nil { - handleAuditingFailure(w, auditErr, nil) - return - } - handleErrorType(datasetDocType, err, w) - return - } - - authorised, logData := api.authenticate(r, log.Data{}) - - var b []byte - if authorised { - - // User has valid authentication to get raw dataset document - datasets := &models.DatasetUpdateResults{} - datasets.Items = results - b, err = json.Marshal(datasets) + b, err := func() ([]byte, error) { + results, err := api.dataStore.Backend.GetDatasets() if err != nil { - log.ErrorC("failed to marshal dataset resource into bytes", err, nil) - handleErrorType(datasetDocType, err, w) - return + logError(ctx, errors.WithMessage(err, "api endpoint getDatasets datastore.GetDatasets returned an error"), nil) + return nil, err } - } else { - - // User is not authenticated and hence has only access to current sub document - datasets := &models.DatasetResults{} - datasets.Items = mapResults(results) + authorised, logData := api.authenticate(r, log.Data{}) + + var b []byte + if authorised { + + // User has valid authentication to get raw dataset document + datasets := &models.DatasetUpdateResults{} + datasets.Items = results + b, err = json.Marshal(datasets) + if err != nil { + logError(ctx, errors.WithMessage(err, "api endpoint getDatasets failed to marshal dataset resource into bytes"), logData) + return nil, err + } + } else { + + // User is not authenticated and hence has only access to current sub document + datasets := &models.DatasetResults{} + datasets.Items = mapResults(results) + + b, err = json.Marshal(datasets) + if err != nil { + logError(ctx, errors.WithMessage(err, "api endpoint getDatasets failed to marshal dataset resource into bytes"), logData) + return nil, err + } + } + return b, err + }() - b, err = json.Marshal(datasets) - if err != nil { - log.ErrorC("failed to marshal dataset resource into bytes", err, nil) - handleErrorType(datasetDocType, err, w) - return + if err != nil { + if auditErr := api.auditor.Record(ctx, getDatasetsAction, actionUnsuccessful, nil); auditErr != nil { + auditActionFailure(ctx, getDatasetsAction, actionUnsuccessful, auditErr, nil) } + handleDatasetAPIErr(ctx, err, w, nil) + return } - if auditErr := api.auditor.Record(r.Context(), getDatasetsAction, actionSuccessful, nil); auditErr != nil { - handleAuditingFailure(w, auditErr, logData) - return + if auditErr := api.auditor.Record(ctx, getDatasetsAction, actionSuccessful, nil); auditErr != nil { + auditActionFailure(ctx, getDatasetsAction, actionSuccessful, auditErr, nil) } setJSONContentType(w) _, err = w.Write(b) if err != nil { - log.Error(err, nil) + logError(ctx, errors.WithMessage(err, "api endpoint getDatasets error writing response body"), nil) http.Error(w, err.Error(), http.StatusInternalServerError) } - log.Debug("get all datasets", logData) + logInfo(ctx, "api endpoint getDatasets request successful", nil) } func (api *DatasetAPI) getDataset(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) id := vars["id"] logData := log.Data{"dataset_id": id} auditParams := common.Params{"dataset_id": id} - if auditErr := api.auditor.Record(r.Context(), getDatasetAction, actionAttempted, auditParams); auditErr != nil { - handleAuditingFailure(w, auditErr, logData) + if auditErr := api.auditor.Record(ctx, getDatasetAction, actionAttempted, auditParams); auditErr != nil { + auditActionFailure(ctx, getDatasetAction, actionAttempted, auditErr, logData) + handleDatasetAPIErr(ctx, errs.ErrInternalServer, w, logData) return } - dataset, err := api.dataStore.Backend.GetDataset(id) - if err != nil { - log.Error(err, logData) - if auditErr := api.auditor.Record(r.Context(), getDatasetAction, actionUnsuccessful, auditParams); auditErr != nil { - handleAuditingFailure(w, auditErr, logData) - return + b, err := func() ([]byte, error) { + dataset, err := api.dataStore.Backend.GetDataset(id) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDataset endpoint: dataStore.Backend.GetDataset returned an error"), logData) + return nil, err } - handleErrorType(datasetDocType, err, w) - return - } - - authorised, logData := api.authenticate(r, logData) - var b []byte - if !authorised { - // User is not authenticated and hence has only access to current sub document - if dataset.Current == nil { - log.Debug("published dataset not found", nil) - handleErrorType(datasetDocType, errs.ErrDatasetNotFound, w) - return + authorised, logData := api.authenticate(r, logData) + + var b []byte + if !authorised { + // User is not authenticated and hence has only access to current sub document + if dataset.Current == nil { + logInfo(ctx, "getDataste endpoint: published dataset not found", logData) + return nil, errs.ErrDatasetNotFound + } + + dataset.Current.ID = dataset.ID + b, err = json.Marshal(dataset.Current) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDataset endpoint: failed to marshal dataset current sub document resource into bytes"), logData) + return nil, err + } + } else { + // User has valid authentication to get raw dataset document + if dataset == nil { + logInfo(ctx, "getDataset endpoint: published or unpublished dataset not found", logData) + return nil, errs.ErrDatasetNotFound + } + b, err = json.Marshal(dataset) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDataset endpoint: failed to marshal dataset current sub document resource into bytes"), logData) + return nil, err + } } + return b, nil + }() - dataset.Current.ID = dataset.ID - b, err = json.Marshal(dataset.Current) - if err != nil { - log.ErrorC("failed to marshal dataset current sub document resource into bytes", err, logData) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - } else { - // User has valid authentication to get raw dataset document - if dataset == nil { - log.Debug("published or unpublished dataset not found", logData) - handleErrorType(datasetDocType, errs.ErrDatasetNotFound, w) - } - b, err = json.Marshal(dataset) - if err != nil { - log.ErrorC("failed to marshal dataset current sub document resource into bytes", err, logData) - http.Error(w, err.Error(), http.StatusInternalServerError) - return + if err != nil { + if auditErr := api.auditor.Record(ctx, getDatasetAction, actionUnsuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getDatasetAction, actionUnsuccessful, auditErr, logData) } + handleDatasetAPIErr(ctx, err, w, logData) + return } - if auditErr := api.auditor.Record(r.Context(), getDatasetAction, actionSuccessful, auditParams); auditErr != nil { - handleAuditingFailure(w, auditErr, logData) + if auditErr := api.auditor.Record(ctx, getDatasetAction, actionSuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getDatasetAction, actionSuccessful, auditErr, logData) + handleDatasetAPIErr(ctx, errs.ErrInternalServer, w, logData) return } setJSONContentType(w) _, err = w.Write(b) if err != nil { - log.Error(err, logData) + logError(ctx, errors.WithMessage(err, "getDataset endpoint: error writing bytes to response"), logData) http.Error(w, err.Error(), http.StatusInternalServerError) } - log.Debug("get dataset", logData) + logInfo(ctx, "getDataset endpoint: request successful", logData) } func (api *DatasetAPI) addDataset(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) datasetID := vars["id"] - // TODO Could just do an insert, if dataset already existed we would get a duplicate key error - // instead of reading then writing doc - _, err := api.dataStore.Backend.GetDataset(datasetID) - if err != nil { - if err != errs.ErrDatasetNotFound { - log.ErrorC("failed to check if dataset exists", err, log.Data{"dataset_id": datasetID}) - handleErrorType(datasetDocType, err, w) - return - } - } else { - err = fmt.Errorf("forbidden - dataset already exists") - log.ErrorC("unable to create a dataset that already exists", err, log.Data{"dataset_id": datasetID}) - http.Error(w, err.Error(), http.StatusForbidden) - return - } + logData := log.Data{"dataset_id": datasetID} + auditParams := common.Params{"dataset_id": datasetID} - dataset, err := models.CreateDataset(r.Body) - if err != nil { - log.ErrorC("failed to model dataset resource based on request", err, log.Data{"dataset_id": datasetID}) - http.Error(w, err.Error(), http.StatusBadRequest) + if err := api.auditor.Record(ctx, addDatasetAction, actionAttempted, auditParams); err != nil { + auditActionFailure(ctx, addDatasetAction, actionAttempted, err, logData) + handleDatasetAPIErr(ctx, errs.ErrInternalServer, w, logData) return } - defer r.Body.Close() - dataset.State = models.CreatedState - dataset.ID = datasetID + // TODO Could just do an insert, if dataset already existed we would get a duplicate key error instead of reading then writing doc + b, err := func() ([]byte, error) { + _, err := api.dataStore.Backend.GetDataset(datasetID) + if err != nil { + if err != errs.ErrDatasetNotFound { + logError(ctx, errors.WithMessage(err, "addDataset endpoint: error checking if dataset exists"), logData) + return nil, err + } + } else { + logError(ctx, errors.WithMessage(errs.ErrAddDatasetAlreadyExists, "addDataset endpoint: unable to create a dataset that already exists"), logData) + return nil, errs.ErrAddDatasetAlreadyExists + } - if dataset.Links == nil { - dataset.Links = &models.DatasetLinks{} - } + defer r.Body.Close() + dataset, err := models.CreateDataset(r.Body) + if err != nil { + logError(ctx, errors.WithMessage(err, "addDataset endpoint: failed to model dataset resource based on request"), logData) + return nil, errs.ErrAddUpdateDatasetBadRequest + } - dataset.Links.Editions = &models.LinkObject{ - HRef: fmt.Sprintf("%s/datasets/%s/editions", api.host, datasetID), - } + dataset.State = models.CreatedState + dataset.ID = datasetID - dataset.Links.Self = &models.LinkObject{ - HRef: fmt.Sprintf("%s/datasets/%s", api.host, datasetID), - } + if dataset.Links == nil { + dataset.Links = &models.DatasetLinks{} + } - dataset.LastUpdated = time.Now() + dataset.Links.Editions = &models.LinkObject{ + HRef: fmt.Sprintf("%s/datasets/%s/editions", api.host, datasetID), + } - datasetDoc := &models.DatasetUpdate{ - ID: datasetID, - Next: dataset, - } + dataset.Links.Self = &models.LinkObject{ + HRef: fmt.Sprintf("%s/datasets/%s", api.host, datasetID), + } - if err = api.dataStore.Backend.UpsertDataset(datasetID, datasetDoc); err != nil { - log.ErrorC("failed to insert dataset resource to datastore", err, log.Data{"new_dataset": datasetID}) - handleErrorType(datasetDocType, err, w) - return - } + dataset.LastUpdated = time.Now() + + datasetDoc := &models.DatasetUpdate{ + ID: datasetID, + Next: dataset, + } + + if err = api.dataStore.Backend.UpsertDataset(datasetID, datasetDoc); err != nil { + logData["new_dataset"] = datasetID + logError(ctx, errors.WithMessage(err, "addDataset endpoint: failed to insert dataset resource to datastore"), logData) + return nil, err + } + + b, err := json.Marshal(datasetDoc) + if err != nil { + logError(ctx, errors.WithMessage(err, "addDataset endpoint: failed to marshal dataset resource into bytes"), logData) + return nil, err + } + return b, nil + }() - b, err := json.Marshal(datasetDoc) if err != nil { - log.ErrorC("failed to marshal dataset resource into bytes", err, log.Data{"new_dataset": datasetID}) - w.WriteHeader(http.StatusInternalServerError) + if auditErr := api.auditor.Record(ctx, addDatasetAction, actionUnsuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, addDatasetAction, actionUnsuccessful, auditErr, logData) + } + handleDatasetAPIErr(ctx, err, w, logData) return } + if auditErr := api.auditor.Record(ctx, addDatasetAction, actionSuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, addDatasetAction, actionSuccessful, auditErr, logData) + } + setJSONContentType(w) w.WriteHeader(http.StatusCreated) _, err = w.Write(b) if err != nil { - log.Error(err, log.Data{"dataset_id": datasetID}) + logError(ctx, errors.WithMessage(err, "addDataset endpoint: error writing bytes to response"), logData) http.Error(w, err.Error(), http.StatusInternalServerError) } - log.Debug("upsert dataset", log.Data{"dataset_id": datasetID}) + logInfo(ctx, "addDataset endpoint: request completed successfully", logData) } func (api *DatasetAPI) putDataset(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) datasetID := vars["id"] + data := log.Data{"dataset_id": datasetID} + auditParams := common.Params{"dataset_id": datasetID} - dataset, err := models.CreateDataset(r.Body) - if err != nil { - log.ErrorC("failed to model dataset resource based on request", err, log.Data{"dataset_id": datasetID}) - http.Error(w, err.Error(), http.StatusBadRequest) + if err := api.auditor.Record(ctx, putDatasetAction, actionAttempted, auditParams); err != nil { + auditActionFailure(ctx, putDatasetAction, actionAttempted, err, data) + handleDatasetAPIErr(ctx, err, w, data) return } - defer r.Body.Close() - currentDataset, err := api.dataStore.Backend.GetDataset(datasetID) + err := func() error { + defer r.Body.Close() + + dataset, err := models.CreateDataset(r.Body) + if err != nil { + logError(ctx, errors.WithMessage(err, "putDataset endpoint: failed to model dataset resource based on request"), data) + return errs.ErrAddUpdateDatasetBadRequest + } + + currentDataset, err := api.dataStore.Backend.GetDataset(datasetID) + if err != nil { + logError(ctx, errors.WithMessage(err, "putDataset endpoint: datastore.getDataset returned an error"), data) + return err + } + + if dataset.State == models.PublishedState { + if err := api.publishDataset(currentDataset, nil); err != nil { + logError(ctx, errors.WithMessage(err, "putDataset endpoint: failed to update dataset document to published"), data) + return err + } + } else { + if err := api.dataStore.Backend.UpdateDataset(datasetID, dataset, currentDataset.Next.State); err != nil { + logError(ctx, errors.WithMessage(err, "putDataset endpoint: failed to update dataset resource"), data) + return err + } + } + return nil + }() + if err != nil { - log.ErrorC("failed to find dataset", err, log.Data{"dataset_id": datasetID}) - handleErrorType(datasetDocType, err, w) + if err := api.auditor.Record(ctx, putDatasetAction, actionUnsuccessful, auditParams); err != nil { + auditActionFailure(ctx, putDatasetAction, actionUnsuccessful, err, data) + } + + handleDatasetAPIErr(ctx, err, w, data) return } - if dataset.State == models.PublishedState { - if err := api.publishDataset(currentDataset, nil); err != nil { - log.ErrorC("failed to update dataset document to published", err, log.Data{"dataset_id": datasetID}) - handleErrorType(versionDocType, err, w) - return - } - } else { - if err := api.dataStore.Backend.UpdateDataset(datasetID, dataset, currentDataset.Next.State); err != nil { - log.ErrorC("failed to update dataset resource", err, log.Data{"dataset_id": datasetID}) - handleErrorType(datasetDocType, err, w) - return - } + if err := api.auditor.Record(ctx, putDatasetAction, actionSuccessful, auditParams); err != nil { + auditActionFailure(ctx, putDatasetAction, actionSuccessful, err, data) } setJSONContentType(w) w.WriteHeader(http.StatusOK) - log.Debug("update dataset", log.Data{"dataset_id": datasetID}) + logInfo(ctx, "putDataset endpoint: request successful", data) } func (api *DatasetAPI) publishDataset(currentDataset *models.DatasetUpdate, version *models.Version) error { @@ -283,36 +336,59 @@ func (api *DatasetAPI) publishDataset(currentDataset *models.DatasetUpdate, vers } func (api *DatasetAPI) deleteDataset(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) datasetID := vars["id"] + logData := log.Data{"dataset_id": datasetID} + auditParams := common.Params{"dataset_id": datasetID} - currentDataset, err := api.dataStore.Backend.GetDataset(datasetID) - if err == errs.ErrDatasetNotFound { - log.Debug("cannot delete dataset, it does not exist", log.Data{"dataset_id": datasetID}) - w.WriteHeader(http.StatusNoContent) // idempotent - return - } - if err != nil { - log.ErrorC("failed to run query for existing dataset", err, log.Data{"dataset_id": datasetID}) - handleErrorType(datasetDocType, err, w) + if err := api.auditor.Record(ctx, deleteDatasetAction, actionAttempted, auditParams); err != nil { + auditActionFailure(ctx, deleteDatasetAction, actionAttempted, err, logData) + handleDatasetAPIErr(ctx, err, w, logData) return } - if currentDataset.Current != nil && currentDataset.Current.State == models.PublishedState { - err = fmt.Errorf("forbidden - a published dataset cannot be deleted") - log.ErrorC("unable to delete a published dataset", err, log.Data{"dataset_id": datasetID}) - http.Error(w, err.Error(), http.StatusForbidden) - return - } + // attempt to delete the dataset. + err := func() error { + currentDataset, err := api.dataStore.Backend.GetDataset(datasetID) + + if err == errs.ErrDatasetNotFound { + log.Debug("cannot delete dataset, it does not exist", logData) + return errs.ErrDeleteDatasetNotFound + } + + if err != nil { + log.ErrorC("failed to run query for existing dataset", err, logData) + return err + } + + if currentDataset.Current != nil && currentDataset.Current.State == models.PublishedState { + log.ErrorC("unable to delete a published dataset", errs.ErrDeletePublishedDatasetForbidden, logData) + return errs.ErrDeletePublishedDatasetForbidden + } - if err := api.dataStore.Backend.DeleteDataset(datasetID); err != nil { - log.ErrorC("failed to delete dataset", err, log.Data{"dataset_id": datasetID}) - handleErrorType(datasetDocType, err, w) + if err := api.dataStore.Backend.DeleteDataset(datasetID); err != nil { + log.ErrorC("failed to delete dataset", err, logData) + return err + } + log.Debug("dataset deleted successfully", logData) + return nil + }() + + if err != nil { + if auditErr := api.auditor.Record(ctx, deleteDatasetAction, actionUnsuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, deleteDatasetAction, actionUnsuccessful, auditErr, logData) + } + handleDatasetAPIErr(ctx, err, w, logData) return } + if err := api.auditor.Record(ctx, deleteDatasetAction, actionSuccessful, auditParams); err != nil { + auditActionFailure(ctx, deleteDatasetAction, actionSuccessful, err, logData) + // fall through and return the origin status code as the action has been carried out at this point. + } w.WriteHeader(http.StatusNoContent) - log.Debug("delete dataset", log.Data{"dataset_id": datasetID}) + log.Debug("delete dataset", logData) } func mapResults(results []models.DatasetUpdate) []*models.Dataset { @@ -327,3 +403,29 @@ func mapResults(results []models.DatasetUpdate) []*models.Dataset { } return items } + +func handleDatasetAPIErr(ctx context.Context, err error, w http.ResponseWriter, data log.Data) { + if data == nil { + data = log.Data{} + } + + var status int + switch { + case err == errs.ErrDeletePublishedDatasetForbidden: + status = http.StatusForbidden + case err == errs.ErrAddDatasetAlreadyExists: + status = http.StatusForbidden + case err == errs.ErrDatasetNotFound: + status = http.StatusNotFound + case err == errs.ErrDeleteDatasetNotFound: + status = http.StatusNoContent + case err == errs.ErrAddUpdateDatasetBadRequest: + status = http.StatusBadRequest + default: + status = http.StatusInternalServerError + } + + data["responseStatus"] = status + logError(ctx, errors.WithMessage(err, "request unsuccessful"), data) + http.Error(w, err.Error(), status) +} diff --git a/api/dataset_test.go b/api/dataset_test.go index a72a9392..6aa7f2ff 100644 --- a/api/dataset_test.go +++ b/api/dataset_test.go @@ -3,10 +3,12 @@ package api import ( "bytes" "context" + "encoding/json" "errors" "io" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -152,7 +154,7 @@ func TestGetDatasetsReturnsErrorIfAuditAttemptFails(t *testing.T) { api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusInternalServerError) - So(w.Body.String(), ShouldEqual, internalServerErr) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, errInternal.Error()) recCalls := auditMock.RecordCalls() So(len(recCalls), ShouldEqual, 2) @@ -191,7 +193,7 @@ func TestGetDatasetsReturnsError(t *testing.T) { func TestGetDatasetsAuditActionSuccessfulError(t *testing.T) { t.Parallel() - Convey("when a successful request to get dataset fails to audit action successful then a 500 response is returned", t, func() { + Convey("when a successful request to get dataset fails to audit action successful then a 200 response is returned", t, func() { r := httptest.NewRequest("GET", "http://localhost:22000/datasets", nil) w := httptest.NewRecorder() mockedDataStore := &storetest.StorerMock{ @@ -217,8 +219,7 @@ func TestGetDatasetsAuditActionSuccessfulError(t *testing.T) { verifyAuditRecordCalls(recCalls[1], getDatasetsAction, actionSuccessful, nil) So(len(mockedDataStore.GetDatasetsCalls()), ShouldEqual, 1) - So(w.Code, ShouldEqual, http.StatusInternalServerError) - So(w.Body.String(), ShouldEqual, internalServerErr) + So(w.Code, ShouldEqual, http.StatusOK) }) } @@ -339,32 +340,34 @@ func TestGetDatasetReturnsError(t *testing.T) { } func TestGetDatasetAuditingErrors(t *testing.T) { - Convey("when auditing get dataset attempt action returns an error then a 500 response is returned", t, func() { - r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) - w := httptest.NewRecorder() - + Convey("given auditing attempted action returns an error", t, func() { auditMock := getMockAuditor() auditMock.RecordFunc = func(ctx context.Context, action string, result string, params common.Params) error { return errors.New("auditing error") } - mockDatastore := &storetest.StorerMock{} - api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) - api.router.ServeHTTP(w, r) + Convey("when get dataset is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) + w := httptest.NewRecorder() - recCalls := auditMock.RecordCalls() - So(len(recCalls), ShouldEqual, 1) - verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) + mockDatastore := &storetest.StorerMock{} + api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) - So(w.Code, ShouldEqual, http.StatusInternalServerError) - So(w.Body.String(), ShouldEqual, internalServerErr) - So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 0) - }) + api.router.ServeHTTP(w, r) - Convey("when auditing get dataset successful action returns an error then a 500 response is returned", t, func() { - r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) - w := httptest.NewRecorder() + Convey("then a 500 status is returned", func() { + recCalls := auditMock.RecordCalls() + So(len(recCalls), ShouldEqual, 1) + verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) + + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, errs.ErrInternalServer.Error()) + So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 0) + }) + }) + }) + Convey("given audit action successful returns an error", t, func() { auditMock := getMockAuditor() auditMock.RecordFunc = func(ctx context.Context, action string, result string, params common.Params) error { if action == getDatasetAction && result == actionSuccessful { @@ -373,35 +376,33 @@ func TestGetDatasetAuditingErrors(t *testing.T) { return nil } - mockDatastore := &storetest.StorerMock{ - GetDatasetFunc: func(id string) (*models.DatasetUpdate, error) { - return &models.DatasetUpdate{ID: "123", Current: &models.Dataset{ID: "123"}}, nil - }, - } - api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) - - api.router.ServeHTTP(w, r) + Convey("when get dataset is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) + w := httptest.NewRecorder() - recCalls := auditMock.RecordCalls() - So(len(recCalls), ShouldEqual, 2) - verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) - verifyAuditRecordCalls(recCalls[1], getDatasetAction, actionSuccessful, auditParams) + mockDatastore := &storetest.StorerMock{ + GetDatasetFunc: func(id string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{ID: "123", Current: &models.Dataset{ID: "123"}}, nil + }, + } + api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) - So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 1) - So(w.Code, ShouldEqual, http.StatusInternalServerError) - So(w.Body.String(), ShouldEqual, internalServerErr) - }) + api.router.ServeHTTP(w, r) - Convey("when get dataset errors and auditing action unsuccessful errors then a 500 response is returned", t, func() { - r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) - w := httptest.NewRecorder() + Convey("then a 500 status is returned", func() { + recCalls := auditMock.RecordCalls() + So(len(recCalls), ShouldEqual, 2) + verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) + verifyAuditRecordCalls(recCalls[1], getDatasetAction, actionSuccessful, auditParams) - mockDatastore := &storetest.StorerMock{ - GetDatasetFunc: func(id string) (*models.DatasetUpdate, error) { - return nil, errors.New("get dataset error") - }, - } + So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 1) + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, errs.ErrInternalServer.Error()) + }) + }) + }) + Convey("given auditing action unsuccessful returns an error", t, func() { auditMock := getMockAuditor() auditMock.RecordFunc = func(ctx context.Context, action string, result string, params common.Params) error { if action == getDatasetAction && result == actionUnsuccessful { @@ -409,19 +410,32 @@ func TestGetDatasetAuditingErrors(t *testing.T) { } return nil } - api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) - api.router.ServeHTTP(w, r) + Convey("when get dataset is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123-456", nil) + w := httptest.NewRecorder() - So(w.Code, ShouldEqual, http.StatusInternalServerError) - So(w.Body.String(), ShouldEqual, internalServerErr) + mockDatastore := &storetest.StorerMock{ + GetDatasetFunc: func(id string) (*models.DatasetUpdate, error) { + return nil, errors.New("get dataset error") + }, + } - recCalls := auditMock.RecordCalls() - So(len(recCalls), ShouldEqual, 2) - verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) - verifyAuditRecordCalls(recCalls[1], getDatasetAction, actionUnsuccessful, auditParams) + api := GetAPIWithMockedDatastore(mockDatastore, &mocks.DownloadsGeneratorMock{}, auditMock, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, "get dataset error") + + recCalls := auditMock.RecordCalls() + So(len(recCalls), ShouldEqual, 2) + verifyAuditRecordCalls(recCalls[0], getDatasetAction, actionAttempted, auditParams) + verifyAuditRecordCalls(recCalls[1], getDatasetAction, actionUnsuccessful, auditParams) - So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockDatastore.GetDatasetCalls()), ShouldEqual, 1) + }) + }) }) } @@ -562,6 +576,170 @@ func TestPostDatasetReturnsError(t *testing.T) { }) } +func TestPostDatasetAuditErrors(t *testing.T) { + ap := common.Params{"dataset_id": "123"} + + Convey("given audit action attempted returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == addDatasetAction && r == actionAttempted { + return errors.New("auditing error") + } + return nil + }) + + Convey("when add dataset is called", func() { + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString("{")) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{} + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, errs.ErrInternalServer.Error()) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 1) + verifyAuditRecordCalls(calls[0], addDatasetAction, actionAttempted, ap) + }) + }) + }) + + Convey("given audit action unsuccessful returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == addDatasetAction && r == actionUnsuccessful { + return errors.New("auditing error") + } + return nil + }) + + Convey("when datastore getdataset returns an error", func() { + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString("{")) + So(err, ShouldBeNil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errors.New("get dataset error") + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, "get dataset error") + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], addDatasetAction, actionAttempted, ap) + }) + }) + + Convey("when datastore getdataset returns an existing dataset", func() { + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString("{")) + So(err, ShouldBeNil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{}, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 403 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusForbidden) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, errs.ErrAddDatasetAlreadyExists.Error()) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], addDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], addDatasetAction, actionUnsuccessful, ap) + }) + }) + + Convey("when datastore upsertDataset returns error", func() { + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + UpsertDatasetFunc: func(ID string, datasetDoc *models.DatasetUpdate) error { + return errors.New("upsert datset error") + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(strings.TrimSpace(w.Body.String()), ShouldEqual, "upsert datset error") + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], addDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], addDatasetAction, actionUnsuccessful, ap) + }) + }) + }) + + Convey("given audit action successful returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == addDatasetAction && r == actionSuccessful { + return errors.New("auditing error") + } + return nil + }) + + Convey("when add dataset is successful", func() { + r, err := createRequestWithAuth("POST", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + UpsertDatasetFunc: func(ID string, datasetDoc *models.DatasetUpdate) error { + return nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 201 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusCreated) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], addDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], addDatasetAction, actionSuccessful, ap) + }) + }) + }) +} + func TestPutDatasetReturnsSuccessfully(t *testing.T) { t.Parallel() Convey("A successful request to put dataset returns 200 OK response", t, func() { @@ -585,12 +763,19 @@ func TestPutDatasetReturnsSuccessfully(t *testing.T) { } mockedDataStore.UpdateDataset("123", dataset, models.CreatedState) - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := createAuditor("", "") + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) + So(w.Code, ShouldEqual, http.StatusOK) So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 2) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, common.Params{"dataset_id": "123"}) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionSuccessful, common.Params{"dataset_id": "123"}) }) } @@ -613,14 +798,21 @@ func TestPutDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := createAuditor("", "") + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) + So(w.Code, ShouldEqual, http.StatusBadRequest) So(w.Body.String(), ShouldResemble, "Failed to parse json body\n") So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) So(len(mockedDataStore.UpdateVersionCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, common.Params{"dataset_id": "123"}) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, common.Params{"dataset_id": "123"}) }) Convey("When the api cannot connect to datastore return an internal server error", t, func() { @@ -645,7 +837,8 @@ func TestPutDatasetReturnsError(t *testing.T) { } mockedDataStore.UpdateDataset("123", dataset, models.CreatedState) - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := createAuditor("", "") + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusInternalServerError) @@ -653,6 +846,11 @@ func TestPutDatasetReturnsError(t *testing.T) { So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 2) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, common.Params{"dataset_id": "123"}) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, common.Params{"dataset_id": "123"}) }) Convey("When the dataset document cannot be found return status not found ", t, func() { @@ -672,7 +870,8 @@ func TestPutDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := createAuditor("", "") + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusNotFound) @@ -680,6 +879,11 @@ func TestPutDatasetReturnsError(t *testing.T) { So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, common.Params{"dataset_id": "123"}) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, common.Params{"dataset_id": "123"}) }) Convey("When the request is not authorised to update dataset return status not found", t, func() { @@ -698,7 +902,8 @@ func TestPutDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := createAuditor("", "") + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusUnauthorized) @@ -706,6 +911,201 @@ func TestPutDatasetReturnsError(t *testing.T) { So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + So(len(auditor.RecordCalls()), ShouldEqual, 0) + }) +} + +func TestPutDatasetAuditErrors(t *testing.T) { + ap := common.Params{"dataset_id": "123"} + + t.Parallel() + Convey("given audit action attempted returns an error", t, func() { + auditor := createAuditor(putDatasetAction, actionAttempted) + + Convey("when put dataset is called", func() { + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + }, + UpdateDatasetFunc: func(string, *models.Dataset, string) error { + return nil + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 1) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + }) + }) + }) + + Convey("given audit action successful returns an error", t, func() { + auditor := createAuditor(putDatasetAction, actionSuccessful) + + Convey("when a put dataset request is successful", func() { + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + }, + UpdateDatasetFunc: func(string, *models.Dataset, string) error { + return nil + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 200 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusOK) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionSuccessful, ap) + }) + }) + }) + + Convey("given audit action unsuccessful returns an error", t, func() { + auditor := createAuditor(putDatasetAction, actionUnsuccessful) + + Convey("when a put dataset request contains an invalid dataset body", func() { + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString("`zxcvbnm,./")) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + }, + UpdateDatasetFunc: func(string, *models.Dataset, string) error { + return nil + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 400 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusBadRequest) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, ap) + }) + }) + + Convey("when datastore.getDataset returns an error", func() { + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 400 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, ap) + }) + }) + + Convey("when the requested dataset has a published state", func() { + + var publishedDataset models.Dataset + json.Unmarshal([]byte(datasetPayload), &publishedDataset) + publishedDataset.State = models.PublishedState + b, _ := json.Marshal(publishedDataset) + + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBuffer(b)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + }, + UpsertDatasetFunc: func(ID string, datasetDoc *models.DatasetUpdate) error { + return errors.New("upsertDataset error") + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 400 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, ap) + }) + }) + + Convey("when datastore.UpdateDataset returns an error", func() { + r, err := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123", bytes.NewBufferString(datasetPayload)) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{}}, nil + }, + UpdateDatasetFunc: func(ID string, dataset *models.Dataset, currentState string) error { + return errors.New("update dataset error") + }, + } + + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 400 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], putDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], putDatasetAction, actionUnsuccessful, ap) + }) + }) }) } @@ -725,10 +1125,18 @@ func TestDeleteDatasetReturnsSuccessfully(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(w.Code, ShouldEqual, http.StatusNoContent) + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionSuccessful, ap) So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 1) }) @@ -750,12 +1158,20 @@ func TestDeleteDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) + So(w.Code, ShouldEqual, http.StatusForbidden) So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) }) Convey("When the api cannot connect to datastore return an internal server error", t, func() { @@ -773,7 +1189,8 @@ func TestDeleteDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusInternalServerError) @@ -781,6 +1198,13 @@ func TestDeleteDatasetReturnsError(t *testing.T) { So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 1) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) }) Convey("When the dataset document cannot be found return status not found ", t, func() { @@ -798,13 +1222,21 @@ func TestDeleteDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) - So(w.Code, ShouldEqual, http.StatusNoContent) + So(w.Code, ShouldEqual, http.StatusNoContent) So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) }) Convey("When the dataset document cannot be queried return status 500 ", t, func() { @@ -822,13 +1254,20 @@ func TestDeleteDatasetReturnsError(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) - So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(w.Code, ShouldEqual, http.StatusInternalServerError) So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) So(len(mockedDataStore.UpdateDatasetCalls()), ShouldEqual, 0) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) }) Convey("When the request is not authorised to delete the dataset return status not found", t, func() { @@ -839,15 +1278,232 @@ func TestDeleteDatasetReturnsError(t *testing.T) { w := httptest.NewRecorder() mockedDataStore := &storetest.StorerMock{} - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditorMock := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusUnauthorized) So(w.Body.String(), ShouldResemble, "unauthenticated request\n") - So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + So(len(auditorMock.RecordCalls()), ShouldEqual, 0) + }) +} + +func TestDeleteDatasetAuditActionAttemptedError(t *testing.T) { + t.Parallel() + Convey("given audit action attempted returns an error", t, func() { + auditorMock := &audit.AuditorServiceMock{ + RecordFunc: func(ctx context.Context, action string, result string, params common.Params) error { + return errors.New("audit error") + }, + } + + Convey("when delete dataset is called", func() { + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{} + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 1) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + }) + }) + }) +} + +func TestDeleteDatasetAuditActionUnsuccessfulError(t *testing.T) { + getAuditor := func() *audit.AuditorServiceMock { + return &audit.AuditorServiceMock{ + RecordFunc: func(ctx context.Context, action string, result string, params common.Params) error { + if deleteDatasetAction == action && result == actionUnsuccessful { + return errors.New("audit error") + } + return nil + }, + } + } + + Convey("given auditing action unsuccessful returns an errors", t, func() { + auditorMock := getAuditor() + + Convey("when attempting to delete a dataset that does not exist", func() { + + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 204 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNoContent) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + }) + }) + + Convey("when dataStore.Backend.GetDataset returns an error", func() { + auditorMock = getAuditor() + + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return nil, errors.New("dataStore.Backend.GetDataset error") + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + }) + }) + + Convey("when attempting to delete a published dataset", func() { + auditorMock = getAuditor() + + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Current: &models.Dataset{State: models.PublishedState}}, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 403 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusForbidden) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 0) + }) + }) + + Convey("when dataStore.Backend.DeleteDataset returns an error", func() { + auditorMock = getAuditor() + + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{State: models.CompletedState}}, nil + }, + DeleteDatasetFunc: func(ID string) error { + return errors.New("DeleteDatasetFunc error") + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 1) + }) + }) + }) +} + +func TestDeleteDatasetAuditActionSuccessfulError(t *testing.T) { + Convey("given audit action successful returns an error", t, func() { + r, err := createRequestWithAuth("DELETE", "http://localhost:22000/datasets/123", nil) + So(err, ShouldBeNil) + + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{Next: &models.Dataset{State: models.CreatedState}}, nil + }, + DeleteDatasetFunc: func(string) error { + return nil + }, + } + + auditorMock := &audit.AuditorServiceMock{ + RecordFunc: func(ctx context.Context, action string, result string, params common.Params) error { + if deleteDatasetAction == action && result == actionSuccessful { + return errors.New("audit error") + } + return nil + }, + } + Convey("when delete dataset is called", func() { + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditorMock, genericMockedObservationStore) + api.router.ServeHTTP(w, r) + + Convey("then a 204 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNoContent) + + calls := auditorMock.RecordCalls() + ap := common.Params{"dataset_id": "123"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], deleteDatasetAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], deleteDatasetAction, actionSuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.DeleteDatasetCalls()), ShouldEqual, 1) + }) + }) }) } @@ -858,3 +1514,22 @@ func getMockAuditor() *audit.AuditorServiceMock { }, } } + +func createAuditor(a string, r string) *audit.AuditorServiceMock { + return &audit.AuditorServiceMock{ + RecordFunc: func(ctx context.Context, action string, result string, params common.Params) error { + if action == a && result == r { + return errors.New("auditing error") + } + return nil + }, + } +} + +func getMockAuditorFunc(f func(action string, result string) error) *audit.AuditorServiceMock { + return &audit.AuditorServiceMock{ + RecordFunc: func(ctx context.Context, action string, result string, params common.Params) error { + return f(action, result) + }, + } +} diff --git a/api/dimensions.go b/api/dimensions.go index 122c07d8..6a0b3383 100644 --- a/api/dimensions.go +++ b/api/dimensions.go @@ -1,73 +1,98 @@ package api import ( + "context" "encoding/json" "fmt" "net/http" + errs "github.com/ONSdigital/dp-dataset-api/apierrors" "github.com/ONSdigital/dp-dataset-api/models" + "github.com/ONSdigital/go-ns/common" "github.com/ONSdigital/go-ns/log" "github.com/gorilla/mux" + "github.com/pkg/errors" "gopkg.in/mgo.v2/bson" ) func (api *DatasetAPI) getDimensions(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) datasetID := vars["id"] edition := vars["edition"] version := vars["version"] logData := log.Data{"dataset_id": datasetID, "edition": edition, "version": version} + auditParams := common.Params{"dataset_id": datasetID, "edition": edition, "version": version} - authorised, logData := api.authenticate(r, logData) - - var state string - if !authorised { - state = models.PublishedState - } - - versionDoc, err := api.dataStore.Backend.GetVersion(datasetID, edition, version, state) - if err != nil { - log.ErrorC("failed to get version", err, logData) - handleErrorType(dimensionDocType, err, w) + if err := api.auditor.Record(ctx, getDimensionsAction, actionAttempted, auditParams); err != nil { + auditActionFailure(ctx, getDimensionsAction, actionAttempted, err, logData) + handleDimensionsErr(ctx, w, err) return } - if err = models.CheckState("version", versionDoc.State); err != nil { - log.ErrorC("unpublished version has an invalid state", err, log.Data{"state": versionDoc.State}) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } + b, err := func() ([]byte, error) { + authorised, logData := api.authenticate(r, logData) - dimensions, err := api.dataStore.Backend.GetDimensions(datasetID, versionDoc.ID) - if err != nil { - log.ErrorC("failed to get version dimensions", err, logData) - handleErrorType(dimensionDocType, err, w) - return - } + var state string + if !authorised { + state = models.PublishedState + } - results, err := api.createListOfDimensions(versionDoc, dimensions) - if err != nil { - log.ErrorC("failed to convert bson to dimension", err, logData) - http.Error(w, err.Error(), http.StatusInternalServerError) - } + versionDoc, err := api.dataStore.Backend.GetVersion(datasetID, edition, version, state) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: datastore.getversion returned an error"), logData) + return nil, err + } - listOfDimensions := &models.DatasetDimensionResults{Items: results} + if err = models.CheckState("version", versionDoc.State); err != nil { + logData["state"] = versionDoc.State + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: unpublished version has an invalid state"), logData) + return nil, err + } + + dimensions, err := api.dataStore.Backend.GetDimensions(datasetID, versionDoc.ID) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: failed to get version dimensions"), logData) + return nil, err + } + + results, err := api.createListOfDimensions(versionDoc, dimensions) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: failed to convert bson to dimension"), logData) + return nil, err + } + + listOfDimensions := &models.DatasetDimensionResults{Items: results} + + b, err := json.Marshal(listOfDimensions) + if err != nil { + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: failed to marshal list of dimension resources into bytes"), logData) + return nil, err + } + return b, nil + }() - b, err := json.Marshal(listOfDimensions) if err != nil { - log.ErrorC("failed to marshal list of dimension resources into bytes", err, logData) - http.Error(w, err.Error(), http.StatusInternalServerError) + if auditErr := api.auditor.Record(ctx, getDimensionsAction, actionUnsuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getDimensionsAction, actionUnsuccessful, auditErr, logData) + } + handleDimensionsErr(ctx, w, err) + return + } + + if auditErr := api.auditor.Record(ctx, getDimensionsAction, actionSuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getDimensionsAction, actionSuccessful, auditErr, logData) + handleDimensionsErr(ctx, w, auditErr) return } setJSONContentType(w) _, err = w.Write(b) if err != nil { - log.Error(err, logData) + logError(ctx, errors.WithMessage(err, "getDimensions endpoint: error writing bytes to response"), logData) http.Error(w, err.Error(), http.StatusInternalServerError) } - - log.Debug("get dimensions", log.Data{"dataset_id": datasetID, "edition": edition, "version": version}) + logInfo(ctx, "getDimensions endpoint: request successful", logData) } func (api *DatasetAPI) createListOfDimensions(versionDoc *models.Version, dimensions []bson.M) ([]models.Dimension, error) { @@ -91,7 +116,8 @@ func (api *DatasetAPI) createListOfDimensions(versionDoc *models.Version, dimens dimension.Links.CodeList = opt.Links.CodeList dimension.Links.Options = models.LinkObject{ID: opt.Name, HRef: fmt.Sprintf("%s/datasets/%s/editions/%s/versions/%s/dimensions/%s/options", api.host, versionDoc.Links.Dataset.ID, versionDoc.Edition, versionDoc.Links.Version.ID, opt.Name)} - dimension.Links.Version = *versionDoc.Links.Self + dimension.Links.Version = models.LinkObject{HRef: fmt.Sprintf("%s/datasets/%s/editions/%s/versions/%s", + api.host, versionDoc.Links.Dataset.ID, versionDoc.Edition, versionDoc.Links.Version.ID)} // Add description to dimension from hash map dimension.Description = dimensionDescriptions[dimension.Name] @@ -151,6 +177,11 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques return } + for i := range results.Items { + results.Items[i].Links.Version.HRef = fmt.Sprintf("%s/datasets/%s/editions/%s/versions/%s", + api.host, datasetID, editionID, versionID) + } + b, err := json.Marshal(results) if err != nil { log.ErrorC("failed to marshal list of dimension option resources into bytes", err, logData) @@ -167,3 +198,23 @@ func (api *DatasetAPI) getDimensionOptions(w http.ResponseWriter, r *http.Reques log.Debug("get dimension options", logData) } + +func handleDimensionsErr(ctx context.Context, w http.ResponseWriter, err error) { + var responseStatus int + + switch { + case err == errs.ErrDatasetNotFound: + responseStatus = http.StatusNotFound + case err == errs.ErrEditionNotFound: + responseStatus = http.StatusNotFound + case err == errs.ErrVersionNotFound: + responseStatus = http.StatusNotFound + case err == errs.ErrDimensionsNotFound: + responseStatus = http.StatusNotFound + default: + responseStatus = http.StatusInternalServerError + } + + logError(ctx, errors.WithMessage(err, "request unsuccessful"), log.Data{"responseStatus": responseStatus}) + http.Error(w, err.Error(), responseStatus) +} diff --git a/api/dimensions_test.go b/api/dimensions_test.go index 22ae5e3f..0afc5d26 100644 --- a/api/dimensions_test.go +++ b/api/dimensions_test.go @@ -9,6 +9,7 @@ import ( "github.com/ONSdigital/dp-dataset-api/mocks" "github.com/ONSdigital/dp-dataset-api/models" "github.com/ONSdigital/dp-dataset-api/store/datastoretest" + "github.com/ONSdigital/go-ns/common" . "github.com/smartystreets/goconvey/convey" "gopkg.in/mgo.v2/bson" ) @@ -27,16 +28,33 @@ func TestGetDimensionsReturnsOk(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusOK) So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + ap := common.Params{ + "dataset_id": "123", + "edition": "2017", + "version": "1", + } + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionSuccessful, ap) }) } func TestGetDimensionsReturnsErrors(t *testing.T) { + ap := common.Params{ + "dataset_id": "123", + "edition": "2017", + "version": "1", + } + t.Parallel() Convey("When the api cannot connect to datastore to get dimension resource return an internal server error", t, func() { r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) @@ -47,7 +65,8 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusInternalServerError) @@ -55,6 +74,11 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) }) Convey("When the request contain an invalid version return not found", t, func() { @@ -66,7 +90,8 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusNotFound) @@ -74,6 +99,12 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) }) Convey("When there are no dimensions then return not found error", t, func() { @@ -88,7 +119,8 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusNotFound) @@ -96,6 +128,11 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) }) Convey("When the version has an invalid state return internal server error", t, func() { @@ -107,7 +144,8 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusInternalServerError) @@ -115,6 +153,149 @@ func TestGetDimensionsReturnsErrors(t *testing.T) { So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) + }) +} + +func TestGetDimensionsAuditingErrors(t *testing.T) { + t.Parallel() + ap := common.Params{"dataset_id": "123", "edition": "2017", "version": "1"} + + Convey("given audit action attempted returns an error", t, func() { + auditor := createAuditor(getDimensionsAction, actionAttempted) + + Convey("when get dimensions is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{} + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 0) + So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 1) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + }) + }) + }) + + Convey("given audit action successful returns an error", t, func() { + auditor := createAuditor(getDimensionsAction, actionSuccessful) + + Convey("when get dimensions is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { + return &models.Version{State: models.AssociatedState}, nil + }, + GetDimensionsFunc: func(datasetID, versionID string) ([]bson.M, error) { + return []bson.M{}, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionSuccessful, ap) + }) + }) + }) + + Convey("given audit action unsuccessful returns an error", t, func() { + auditor := createAuditor(getDimensionsAction, actionUnsuccessful) + + Convey("when datastore.getVersion returns an error", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { + return nil, errs.ErrVersionNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) + }) + }) + + Convey("when the version in not in a valid state", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { + return &models.Version{State: "BROKEN"}, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 0) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) + }) + }) + + Convey("when datastore.getDataset returns an error", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/dimensions", nil) + w := httptest.NewRecorder() + mockedDataStore := &storetest.StorerMock{ + GetVersionFunc: func(datasetID, edition, version, state string) (*models.Version, error) { + return &models.Version{State: models.AssociatedState}, nil + }, + GetDimensionsFunc: func(datasetID string, versionID string) ([]bson.M, error) { + return nil, errs.ErrDimensionsNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetDimensionsCalls()), ShouldEqual, 1) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getDimensionsAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getDimensionsAction, actionUnsuccessful, ap) + }) + }) }) } diff --git a/api/metadata.go b/api/metadata.go index dc9d06cb..f3b1f1fb 100644 --- a/api/metadata.go +++ b/api/metadata.go @@ -6,81 +6,118 @@ import ( errs "github.com/ONSdigital/dp-dataset-api/apierrors" "github.com/ONSdigital/dp-dataset-api/models" + "github.com/ONSdigital/go-ns/common" "github.com/ONSdigital/go-ns/log" "github.com/gorilla/mux" + "github.com/pkg/errors" ) func (api *DatasetAPI) getMetadata(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() vars := mux.Vars(r) datasetID := vars["id"] edition := vars["edition"] version := vars["version"] logData := log.Data{"dataset_id": datasetID, "edition": edition, "version": version} + auditParams := common.Params{"dataset_id": datasetID, "edition": edition, "version": version} - // get dataset document - datasetDoc, err := api.dataStore.Backend.GetDataset(datasetID) - if err != nil { - log.Error(err, logData) - handleErrorType(versionDocType, err, w) + if auditErr := api.auditor.Record(ctx, getMetadataAction, actionAttempted, auditParams); auditErr != nil { + auditActionFailure(ctx, getMetadataAction, actionAttempted, auditErr, logData) + handleMetadataErr(w, auditErr) return } - authorised, logData := api.authenticate(r, logData) + b, err := func() ([]byte, error) { + datasetDoc, err := api.dataStore.Backend.GetDataset(datasetID) + if err != nil { + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: get datastore.getDataset returned an error"), logData) + return nil, err + } + + authorised, logData := api.authenticate(r, logData) + var state string - var state string + // if request is authenticated then access resources of state other than published + if !authorised { + // Check for current sub document + if datasetDoc.Current == nil || datasetDoc.Current.State != models.PublishedState { + logData["dataset"] = datasetDoc.Current + logError(ctx, errors.New("getMetadata endpoint: caller not is authorised and dataset but currently unpublished"), logData) + return nil, errs.ErrDatasetNotFound + } - // if request is authenticated then access resources of state other than published - if !authorised { - // Check for current sub document - if datasetDoc.Current == nil || datasetDoc.Current.State != models.PublishedState { - log.ErrorC("found dataset but currently unpublished", errs.ErrDatasetNotFound, log.Data{"dataset_id": datasetID, "edition": edition, "version": version, "dataset": datasetDoc.Current}) - http.Error(w, errs.ErrDatasetNotFound.Error(), http.StatusNotFound) - return + state = datasetDoc.Current.State } - state = datasetDoc.Current.State - } + if err = api.dataStore.Backend.CheckEditionExists(datasetID, edition, state); err != nil { + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: failed to find edition for dataset"), logData) + return nil, err + } - if err = api.dataStore.Backend.CheckEditionExists(datasetID, edition, state); err != nil { - log.ErrorC("failed to find edition for dataset", err, logData) - handleErrorType(versionDocType, err, w) - return - } + versionDoc, err := api.dataStore.Backend.GetVersion(datasetID, edition, version, state) + if err != nil { + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: failed to find version for dataset edition"), logData) + return nil, errs.ErrMetadataVersionNotFound + } - versionDoc, err := api.dataStore.Backend.GetVersion(datasetID, edition, version, state) - if err != nil { - log.ErrorC("failed to find version for dataset edition", err, log.Data{"dataset_id": datasetID, "edition": edition, "version": version}) - handleErrorType(versionDocType, err, w) - return - } + if err = models.CheckState("version", versionDoc.State); err != nil { + logData["state"] = versionDoc.State + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: unpublished version has an invalid state"), logData) + return nil, err + } - if err = models.CheckState("version", versionDoc.State); err != nil { - log.ErrorC("unpublished version has an invalid state", err, log.Data{"state": versionDoc.State}) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } + var metaDataDoc *models.Metadata + // combine version and dataset metadata + if state != models.PublishedState { + metaDataDoc = models.CreateMetaDataDoc(datasetDoc.Next, versionDoc, api.urlBuilder) + } else { + metaDataDoc = models.CreateMetaDataDoc(datasetDoc.Current, versionDoc, api.urlBuilder) + } - var metaDataDoc *models.Metadata - // combine version and dataset metadata - if state != models.PublishedState { - metaDataDoc = models.CreateMetaDataDoc(datasetDoc.Next, versionDoc, api.urlBuilder) - } else { - metaDataDoc = models.CreateMetaDataDoc(datasetDoc.Current, versionDoc, api.urlBuilder) - } + b, err := json.Marshal(metaDataDoc) + if err != nil { + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: failed to marshal metadata resource into bytes"), logData) + return nil, err + } + return b, err + }() - b, err := json.Marshal(metaDataDoc) if err != nil { - log.ErrorC("failed to marshal metadata resource into bytes", err, logData) - http.Error(w, err.Error(), http.StatusInternalServerError) + if auditErr := api.auditor.Record(ctx, getMetadataAction, actionUnsuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getMetadataAction, actionUnsuccessful, auditErr, logData) + } + handleMetadataErr(w, err) + return + } + + if auditErr := api.auditor.Record(ctx, getMetadataAction, actionSuccessful, auditParams); auditErr != nil { + auditActionFailure(ctx, getMetadataAction, actionSuccessful, auditErr, logData) + handleMetadataErr(w, auditErr) return } setJSONContentType(w) _, err = w.Write(b) if err != nil { - log.Error(err, logData) + logError(ctx, errors.WithMessage(err, "getMetadata endpoint: failed to write bytes to response"), logData) http.Error(w, err.Error(), http.StatusInternalServerError) } + logInfo(ctx, "getMetadata endpoint: get metadata request successful", logData) +} + +func handleMetadataErr(w http.ResponseWriter, err error) { + var responseStatus int + + switch { + case err == errs.ErrEditionNotFound: + responseStatus = http.StatusNotFound + case err == errs.ErrMetadataVersionNotFound: + responseStatus = http.StatusNotFound + case err == errs.ErrDatasetNotFound: + responseStatus = http.StatusNotFound + default: + responseStatus = http.StatusInternalServerError + } - log.Debug("get metadata relevant to version", logData) + http.Error(w, err.Error(), responseStatus) } diff --git a/api/metadata_test.go b/api/metadata_test.go index c99ef428..dccac445 100644 --- a/api/metadata_test.go +++ b/api/metadata_test.go @@ -12,6 +12,8 @@ import ( "github.com/ONSdigital/dp-dataset-api/mocks" "github.com/ONSdigital/dp-dataset-api/models" "github.com/ONSdigital/dp-dataset-api/store/datastoretest" + "github.com/ONSdigital/go-ns/common" + "github.com/pkg/errors" . "github.com/smartystreets/goconvey/convey" ) @@ -37,7 +39,8 @@ func TestGetMetadataReturnsOk(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusOK) @@ -45,6 +48,12 @@ func TestGetMetadataReturnsOk(t *testing.T) { So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + calls := auditor.RecordCalls() + ap := common.Params{"dataset_id": "123", "edition": "2017", "version": "1"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionSuccessful, ap) + bytes, err := ioutil.ReadAll(w.Body) if err != nil { os.Exit(1) @@ -93,7 +102,8 @@ func TestGetMetadataReturnsOk(t *testing.T) { }, } - api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, getMockAuditor(), genericMockedObservationStore) + auditor := getMockAuditor() + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) api.router.ServeHTTP(w, r) So(w.Code, ShouldEqual, http.StatusOK) @@ -101,6 +111,12 @@ func TestGetMetadataReturnsOk(t *testing.T) { So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + calls := auditor.RecordCalls() + ap := common.Params{"dataset_id": "123", "edition": "2017", "version": "1"} + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionSuccessful, ap) + bytes, err := ioutil.ReadAll(w.Body) if err != nil { os.Exit(1) @@ -285,6 +301,246 @@ func TestGetMetadataReturnsError(t *testing.T) { }) } +func TestGetMetadataAuditingErrors(t *testing.T) { + ap := common.Params{"dataset_id": "123", "edition": "2017", "version": "1"} + + Convey("given auditing action attempted returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == getMetadataAction && r == actionAttempted { + return errors.New("audit error") + } + return nil + }) + + Convey("when get metadata is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{} + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 1) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 0) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 0) + }) + }) + }) + + Convey("given auditing action unsuccessful returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == getMetadataAction && r == actionUnsuccessful { + return errors.New("audit error") + } + return nil + }) + + Convey("when datastore getDataset returns dataset not found error", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return nil, errs.ErrDatasetNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 404 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 0) + }) + }) + + Convey("when dataset.current is empty", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return &models.DatasetUpdate{}, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 404 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 0) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 0) + }) + }) + + Convey("when dataset edition does not exist", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return createDatasetDoc(), nil + }, + CheckEditionExistsFunc: func(ID string, editionID string, state string) error { + return errs.ErrEditionNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 404 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 0) + }) + }) + + Convey("when dataset version does not exist", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return createDatasetDoc(), nil + }, + CheckEditionExistsFunc: func(ID string, editionID string, state string) error { + return nil + }, + GetVersionFunc: func(datasetID string, editionID string, version string, state string) (*models.Version, error) { + return nil, errs.ErrVersionNotFound + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusNotFound) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + }) + }) + + Convey("when version not published", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + versionDoc := createVersionDoc() + versionDoc.State = "not published" + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return createDatasetDoc(), nil + }, + CheckEditionExistsFunc: func(ID string, editionID string, state string) error { + return nil + }, + GetVersionFunc: func(datasetID string, editionID string, version string, state string) (*models.Version, error) { + return versionDoc, nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionUnsuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + }) + }) + }) + + Convey("given auditing action successful returns an error", t, func() { + auditor := getMockAuditorFunc(func(a string, r string) error { + if a == getMetadataAction && r == actionSuccessful { + return errors.New("audit error") + } + return nil + }) + + Convey("when get metadata is called", func() { + r := httptest.NewRequest("GET", "http://localhost:22000/datasets/123/editions/2017/versions/1/metadata", nil) + w := httptest.NewRecorder() + + mockedDataStore := &storetest.StorerMock{ + GetDatasetFunc: func(ID string) (*models.DatasetUpdate, error) { + return createDatasetDoc(), nil + }, + CheckEditionExistsFunc: func(ID string, editionID string, state string) error { + return nil + }, + GetVersionFunc: func(datasetID string, editionID string, version string, state string) (*models.Version, error) { + return createVersionDoc(), nil + }, + } + api := GetAPIWithMockedDatastore(mockedDataStore, &mocks.DownloadsGeneratorMock{}, auditor, genericMockedObservationStore) + + api.router.ServeHTTP(w, r) + + Convey("then a 500 status is returned", func() { + So(w.Code, ShouldEqual, http.StatusInternalServerError) + + calls := auditor.RecordCalls() + So(len(calls), ShouldEqual, 2) + verifyAuditRecordCalls(calls[0], getMetadataAction, actionAttempted, ap) + verifyAuditRecordCalls(calls[1], getMetadataAction, actionSuccessful, ap) + + So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1) + So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1) + So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 1) + }) + }) + + }) + +} + // createDatasetDoc returns a datasetUpdate doc containing minimal fields but // there is a clear difference between the current and next sub documents func createDatasetDoc() *models.DatasetUpdate { diff --git a/apierrors/errors.go b/apierrors/errors.go index 66094910..8a85b532 100644 --- a/apierrors/errors.go +++ b/apierrors/errors.go @@ -5,6 +5,8 @@ import "errors" // Error messages for Dataset API var ( ErrDatasetNotFound = errors.New("Dataset not found") + ErrAddDatasetAlreadyExists = errors.New("forbidden - dataset already exists") + ErrAddUpdateDatasetBadRequest = errors.New("Failed to parse json body") ErrEditionNotFound = errors.New("Edition not found") ErrVersionNotFound = errors.New("Version not found") ErrDimensionNodeNotFound = errors.New("Dimension node not found") @@ -20,4 +22,10 @@ var ( ErrIndexOutOfRange = errors.New("index out of range") ErrMissingVersionHeadersOrDimensions = errors.New("missing headers or dimensions or both from version doc") ErrTooManyWildcards = errors.New("only one wildcard (*) is allowed as a value in selected query parameters") + ErrDeletePublishedDatasetForbidden = errors.New("a published dataset cannot be deleted") + ErrDeleteDatasetNotFound = errors.New("dataset not found") + ErrAuditActionAttemptedFailure = errors.New("internal server error") + + // metadata endpoint errors + ErrMetadataVersionNotFound = errors.New("Version not found") ) diff --git a/swagger.yaml b/swagger.yaml index fd42aadf..10ce6f92 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -1,8 +1,11 @@ swagger: "2.0" info: - description: "An API used to query information about datasets published by the ONS." + description: "Used to find information about data published by the ONS. + `Datasets` are published in unique `versions`, which are categorized by `edition`. + Data in each version is broken down by `dimensions`, and a unique combination + of dimension `options` in a version can be used to retrieve `observation` level data." version: "1.0.0" - title: "Dataset API" + title: "Explore our data" license: name: "Open Government Licence v3.0" url: "http://www.nationalarchives.gov.uk/doc/open-government-licence/version/3/" @@ -69,11 +72,6 @@ parameters: required: true schema: $ref: '#/definitions/Instance' - limit: - name: limit - description: "The number of resources to be returned" - in: query - type: integer new_dataset: name: dataset description: "A new dataset" @@ -101,11 +99,6 @@ parameters: in: path required: true type: string - offset: - name: offset - description: "The first row of resources to retrieve, starting at 0. Use this parameter as a pagination mechanism along with the limit parameter" - in: query - type: integer newInstance: name: instance description: "An instance related to an import job" @@ -113,11 +106,6 @@ parameters: required: true schema: $ref: '#/definitions/NewInstance' - query: - name: q - description: "The query text to search datasets with" - in: query - type: string state: name: "state" description: "A comma separated list of state values to filter on (e.g. ‘completed,edition-confirmed’)" @@ -158,36 +146,12 @@ securityDefinitions: in: header type: apiKey paths: - /search/datasets: - get: - tags: - - "Public" - summary: "Search for datasets" - description: "Perform a search on all published datasets by the ONS" - parameters: - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' - - $ref: '#/parameters/query' - produces: - - "application/json" - responses: - 200: - description: "A json list containing datasets which have been published" - schema: - $ref: '#/definitions/Datasets' - 400: - description: "Body of the post request is invalid" - 500: - description: "Failed to process the request due to an internal error" /datasets: get: tags: - "Public" summary: "Get a list of datasets" description: "Returns a list of all datasets provided by the ONS that can be filtered using the filter API" - parameters: - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' produces: - "application/json" responses: @@ -278,12 +242,10 @@ paths: get: tags: - "Public" - summary: "Get a list of all editions from a dataset" - description: "Get a list of editions from a type of dataset" + summary: "Get a list of editions of a dataset" + description: "Get a list of editions of a type of dataset" parameters: - $ref: '#/parameters/id' - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' responses: 200: description: "A json list containing all editions for a dataset" @@ -319,13 +281,11 @@ paths: get: tags: - "Public" - summary: "Get a list of all versions from an edition of a dataset" + summary: "Get a list of versions of an edition" description: "Get a list of all versions for an edition of a dataset" parameters: - $ref: '#/parameters/edition' - $ref: '#/parameters/id' - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' responses: 200: description: "A json list containing all versions for a set type of dataset and edition" @@ -344,7 +304,7 @@ paths: put: tags: - "Private user" - summary: "Update a version for an edition of a dataset" + summary: "Update a version" description: "Update a version for an edition of a dataset, if the state is changed to associated or published, the parent documents(dataset and edition resources) will also be updated. A version can only be updated if the state is not published" parameters: - $ref: '#/parameters/id' @@ -372,8 +332,8 @@ paths: get: tags: - "Public" - summary: "Get a specific version and edition of a dataset" - description: "Get a specific version and edition of a dataset" + summary: "Get a version" + description: "Get a specific version of an edition of a dataset" parameters: - $ref: '#/parameters/edition' - $ref: '#/parameters/id' @@ -396,14 +356,12 @@ paths: get: tags: - "Public" - summary: "Get all dimensions from a dataset" + summary: "Get a list of dimensions from a dataset" description: "Get all dimensions which are used in the dataset" parameters: - $ref: '#/parameters/edition' - $ref: '#/parameters/id' - $ref: '#/parameters/version' - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' responses: 200: description: "A json list of dimensions" @@ -423,15 +381,13 @@ paths: get: tags: - "Public" - summary: "Get all options from a dimension" + summary: "Get a list of options from a dimension" description: "Get a list of all options which appear in this dimension and dataset" parameters: - $ref: '#/parameters/dimension' - $ref: '#/parameters/edition' - $ref: '#/parameters/id' - $ref: '#/parameters/version' - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' responses: 200: description: "Json object containing all options for a dimension" @@ -452,7 +408,7 @@ paths: get: tags: - "Public" - summary: "Get metadata relevant to a version" + summary: "Get metadata for a version" description: "Get all metadata relevant to a version" parameters: - $ref: '#/parameters/edition' @@ -476,8 +432,11 @@ paths: get: tags: - "Public" - summary: "Get a specific observation from version" - description: "Get observation from a version of the dataset by providing a single dimension option for each dimension" + summary: "Get specific observations" + description: "Get observations from a version of the dataset. By providing + a single option for each dimension, a single observation will be returned. + A wildcard (*) can be provided for one dimension, to retrieve a list of + observations." parameters: - $ref: '#/parameters/edition' - $ref: '#/parameters/id' @@ -507,11 +466,9 @@ paths: get: tags: - "Private user" - summary: "Get a list of instances" + summary: "Get instances" description: "Get a list of instances which has been paged" parameters: - - $ref: '#/parameters/limit' - - $ref: '#/parameters/offset' - $ref: '#/parameters/state' produces: - "application/json" @@ -557,7 +514,7 @@ paths: get: tags: - "Private user" - summary: "Get Information about an instance" + summary: "Get an instance" description: "Get the current state of an instance, this includes all events which have happened." parameters: - $ref: '#/parameters/instance_id' @@ -579,7 +536,7 @@ paths: put: tags: - "Private" - summary: "Update an instances properties" + summary: "Update an instance" description: "Update an instance by providing an unique id and a set of properties to over write" parameters: - $ref: '#/parameters/instance_id' @@ -605,7 +562,7 @@ paths: get: tags: - "Private user" - summary: "Get all dimensions from an instance" + summary: "Get a list of dimensions for an instance" description: "Get all dimensions from an instance" parameters: - $ref: '#/parameters/instance_id' @@ -633,7 +590,7 @@ paths: post: tags: - "Private" - summary: "Create a new dimension" + summary: "Create a dimension" description: "Create a new dimension which is related to an instance" parameters: - $ref: '#/parameters/instance_id' @@ -653,7 +610,7 @@ paths: put: tags: - "Private user" - summary: "Update dimension description and/or label" + summary: "Update dimension" description: "Update the label and/or description of a dimension within an instance, by providing dimension name and properties to over write" parameters: - $ref: '#/parameters/instance_id' @@ -678,7 +635,7 @@ paths: get: tags: - "Private user" - summary: "Get all unique options from a dimension" + summary: "Get a list of options for a dimension" description: "Get all unique options from a dimension" parameters: - $ref: '#/parameters/instance_id' @@ -740,7 +697,7 @@ paths: put: tags: - "Private" - summary: "Increment the observation inserted count" + summary: "Increment the inserted observation count" description: "This will add to the number already store in the api" parameters: - $ref: '#/parameters/instance_id' @@ -762,7 +719,7 @@ paths: put: tags: - "Private" - summary: "Update the state of an import task in an instance" + summary: "Update import tasks for an instance" description: "The instance import process involves multiple tasks. This endpoint updates the state of an import task." parameters: - $ref: '#/parameters/instance_id' @@ -784,7 +741,7 @@ paths: put: tags: - "Private" - summary: "Add an option to a dimension within the instance" + summary: "Add an option to an instance dimension" description: | Add a option to a dimension which has been found in the V4 data parameters: