Skip to content

Commit

Permalink
Fix full file download not generating
Browse files Browse the repository at this point in the history 
The full file download is not generating due to a mismatch between the
dataset type expected by the dataset API and the type stored in the
recipes.

Also attempt to surface the errors that are currently getting swallowed
by the catch all internal error message.
  • Loading branch information
@janderson2
janderson2 committed Dec 9, 2021
1 parent 356bfc2 commit de87669
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 31 deletions.
7 changes: 4 additions & 3 deletions api/versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,11 @@ func (api *DatasetAPI) publishVersion(ctx context.Context, currentDataset *model
}
generator, ok := api.downloadGenerators[t]
if !ok {
fmt.Printf("we are here: %s", t)
return fmt.Errorf("no downloader available for type %s", t)
}
// Send Kafka message. The generator which is used depends on the type defined in VersionDoc.
if err := generator.Generate(ctx, versionDetails.datasetID, versionUpdate.ID, versionDetails.edition, versionDetails.version); err != nil{
if err := generator.Generate(ctx, versionDetails.datasetID, versionUpdate.ID, versionDetails.edition, versionDetails.version); err != nil {
data["instance_id"] = versionUpdate.ID
data["state"] = versionUpdate.State
data["type"] = t.String()
Expand Down Expand Up @@ -528,7 +529,7 @@ func (api *DatasetAPI) associateVersion(ctx context.Context, currentVersion, ver
return fmt.Errorf("no downloader available for type %s", t.String())
}
// ToDo outsidecoder - Pass the right mock and get Generate to fail by returning an error.New("Error message")
if err := generator.Generate(ctx, versionDetails.datasetID, versionDoc.ID, versionDetails.edition, versionDetails.version); err != nil{
if err := generator.Generate(ctx, versionDetails.datasetID, versionDoc.ID, versionDetails.edition, versionDetails.version); err != nil {
data["instance_id"] = versionDoc.ID
data["state"] = versionDoc.State
log.Error(ctx, "putVersion endpoint: error while attempting to generate full dataset version downloads on version association", err, data)
Expand Down Expand Up @@ -672,7 +673,7 @@ func handleVersionAPIErr(ctx context.Context, err error, w http.ResponseWriter,
case strings.HasPrefix(err.Error(), "invalid version requested"):
status = http.StatusBadRequest
default:
err = errs.ErrInternalServer
err = fmt.Errorf("%s: %s", errs.ErrInternalServer.Error(), err.Error())
status = http.StatusInternalServerError
}

Expand Down
208 changes: 181 additions & 27 deletions api/versions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
datasetPermissions := getAuthorisationHandlerMock()
permissions := getAuthorisationHandlerMock()

Convey("put version with CMD type", func() {
Convey("put version with filterable type", func() {

mockedDataStore := &storetest.StorerMock{
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
Expand All @@ -602,7 +602,51 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
},
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{
Type: models.Filterable.String(),
Type: "filterable",
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
return nil
},
UpdateDatasetWithAssociationFunc: func(string, string, *models.Version) error {
return nil
},
}

api := GetAPIWithCMDMocks(mockedDataStore, generatorMock, datasetPermissions, permissions)
api.Router.ServeHTTP(w, r)

So(w.Code, ShouldEqual, http.StatusOK)
So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 2)
So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpdateVersionCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpdateDatasetWithAssociationCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpsertEditionCalls()), ShouldEqual, 0)
So(len(mockedDataStore.SetInstanceIsPublishedCalls()), ShouldEqual, 0)
So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0)
So(len(generatorMock.GenerateCalls()), ShouldEqual, 1)

Convey("then the request body has been drained", func() {
_, err := r.Body.Read(make([]byte, 1))
So(err, ShouldEqual, io.EOF)
})
})

Convey("put version with v4 type", func() {

mockedDataStore := &storetest.StorerMock{
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
return &models.DatasetUpdate{}, nil
},
CheckEditionExistsFunc: func(string, string, string) error {
return nil
},
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{
Type: "v4",
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
Expand Down Expand Up @@ -637,25 +681,25 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {

Convey("put version with Cantabular type", func() {

mockedDataStore := &storetest.StorerMock{
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
return &models.DatasetUpdate{}, nil
},
CheckEditionExistsFunc: func(string, string, string) error {
return nil
},
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{
Type: models.CantabularTable.String(),
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
return nil
},
UpdateDatasetWithAssociationFunc: func(string, string, *models.Version) error {
return nil
},
}
mockedDataStore := &storetest.StorerMock{
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
return &models.DatasetUpdate{}, nil
},
CheckEditionExistsFunc: func(string, string, string) error {
return nil
},
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{
Type: models.CantabularTable.String(),
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
return nil
},
UpdateDatasetWithAssociationFunc: func(string, string, *models.Version) error {
return nil
},
}

api := GetAPIWithCantabularMocks(mockedDataStore, generatorMock, datasetPermissions, permissions)
api.Router.ServeHTTP(w, r)
Expand Down Expand Up @@ -765,7 +809,7 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
datasetPermissions := getAuthorisationHandlerMock()
permissions := getAuthorisationHandlerMock()

Convey("And the datatype is CMD", func(){
Convey("And the datatype is CMD", func() {
mockedDataStore := &storetest.StorerMock{
CheckEditionExistsFunc: func(string, string, string) error {
return nil
Expand Down Expand Up @@ -802,7 +846,7 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
},
},
State: models.EditionConfirmedState,
Type: models.Filterable.String(),
Type: models.Filterable.String(),
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
Expand Down Expand Up @@ -870,7 +914,6 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
})
})


Convey("And the datatype is Cantabular", func() {

mockedDataStore := &storetest.StorerMock{
Expand Down Expand Up @@ -909,7 +952,7 @@ func TestPutVersionReturnsSuccessfully(t *testing.T) {
},
},
State: models.EditionConfirmedState,
Type: models.CantabularTable.String(),
Type: models.CantabularTable.String(),
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
Expand Down Expand Up @@ -1174,13 +1217,14 @@ func TestPutVersionGenerateDownloadsError(t *testing.T) {
})
})
}

//v.Type Cant and one for Associated

func TestPutEmptyVersion(t *testing.T) {
getVersionAssociatedModel := func(datasetType models.DatasetType) models.Version {
var v models.Version
err := json.Unmarshal([]byte(versionAssociatedPayload), &v) //
So(err, ShouldBeNil) //
So(err, ShouldBeNil) //
v.Type = datasetType.String()
v.State = models.AssociatedState //
return v
Expand Down Expand Up @@ -1748,6 +1792,116 @@ func TestPutVersionReturnsError(t *testing.T) {
})
})

Convey("When the request body is invalid return status bad request", t, func() {
generatorMock := &mocks.DownloadsGeneratorMock{
GenerateFunc: func(context.Context, string, string, string, string) error {
return nil
},
}

b := `{"instance_id":"a1b2c3","edition":"2017","license":"ONS","release_date":"2017-04-04","state":"associated"}`
r := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123/editions/2017/versions/1", bytes.NewBufferString(b))

w := httptest.NewRecorder()
mockedDataStore := &storetest.StorerMock{
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{State: "associated"}, nil
},
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
return &models.DatasetUpdate{}, nil
},
CheckEditionExistsFunc: func(string, string, string) error {
return nil
},
UpdateVersionFunc: func(string, *models.Version) error {
return nil
},
}

datasetPermissions := getAuthorisationHandlerMock()
permissions := getAuthorisationHandlerMock()
api := GetAPIWithCMDMocks(mockedDataStore, generatorMock, datasetPermissions, permissions)
api.Router.ServeHTTP(w, r)

So(w.Code, ShouldEqual, http.StatusBadRequest)
So(w.Body.String(), ShouldEqual, "missing collection_id for association between version and a collection\n")

So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 2)
So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpdateVersionCalls()), ShouldEqual, 0)
So(len(generatorMock.GenerateCalls()), ShouldEqual, 0)

Convey("then the request body has been drained", func() {
_, err := r.Body.Read(make([]byte, 1))
So(err, ShouldEqual, io.EOF)
})
})

Convey("Given a version with state of associated", t, func() {
generatorMock := &mocks.DownloadsGeneratorMock{
GenerateFunc: func(context.Context, string, string, string, string) error {
return nil
},
}

b := versionAssociatedPayload
r := createRequestWithAuth("PUT", "http://localhost:22000/datasets/123/editions/2017/versions/1", bytes.NewBufferString(b))

w := httptest.NewRecorder()

datasetPermissions := getAuthorisationHandlerMock()
permissions := getAuthorisationHandlerMock()

Convey("when the PUT version is called with an invalid type", func() {
mockedDataStore := &storetest.StorerMock{
GetDatasetFunc: func(datasetID string) (*models.DatasetUpdate, error) {
return &models.DatasetUpdate{}, nil
},
CheckEditionExistsFunc: func(string, string, string) error {
return nil
},
GetVersionFunc: func(string, string, int, string) (*models.Version, error) {
return &models.Version{
Type: "thisisinvalid",
}, nil
},
UpdateVersionFunc: func(string, *models.Version) error {
return nil
},
UpdateDatasetWithAssociationFunc: func(string, string, *models.Version) error {
return nil
},
}

api := GetAPIWithCMDMocks(mockedDataStore, generatorMock, datasetPermissions, permissions)
api.Router.ServeHTTP(w, r)

Convey("then an error is returned", func() {
So(w.Code, ShouldEqual, http.StatusInternalServerError)
So(w.Body.String(), ShouldEqual, "internal error: error getting type of version: invalid dataset type\n")
So(datasetPermissions.Required.Calls, ShouldEqual, 1)
So(permissions.Required.Calls, ShouldEqual, 0)
So(len(mockedDataStore.GetVersionCalls()), ShouldEqual, 2)
So(len(mockedDataStore.GetDatasetCalls()), ShouldEqual, 1)
So(len(mockedDataStore.CheckEditionExistsCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpdateVersionCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpdateDatasetWithAssociationCalls()), ShouldEqual, 1)
So(len(mockedDataStore.UpsertEditionCalls()), ShouldEqual, 0)
So(len(mockedDataStore.SetInstanceIsPublishedCalls()), ShouldEqual, 0)
So(len(mockedDataStore.UpsertDatasetCalls()), ShouldEqual, 0)
So(len(generatorMock.GenerateCalls()), ShouldEqual, 0)
})

Convey("and the request body has been drained", func() {
_, err := r.Body.Read(make([]byte, 1))
So(err, ShouldEqual, io.EOF)
})
})
})

Convey("When setting the instance node to published fails", t, func() {
generatorMock := &mocks.DownloadsGeneratorMock{
GenerateFunc: func(context.Context, string, string, string, string) error {
Expand Down Expand Up @@ -2490,5 +2644,5 @@ func TestDetachVersionReturnsError(t *testing.T) {

func assertInternalServerErr(w *httptest.ResponseRecorder) {
So(w.Code, ShouldEqual, http.StatusInternalServerError)
So(strings.TrimSpace(w.Body.String()), ShouldEqual, errs.ErrInternalServer.Error())
So(strings.TrimSpace(w.Body.String()), ShouldStartWith, errs.ErrInternalServer.Error())
}
2 changes: 1 addition & 1 deletion models/dataset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (dt DatasetType) String() string {
// GetDatasetType returns a dataset type for a given dataset
func GetDatasetType(datasetType string) (DatasetType, error) {
switch datasetType {
case "filterable", "":
case "filterable", "v4", "":
return Filterable, nil
case "nomis":
return Nomis, nil
Expand Down

0 comments on commit de87669

Please sign in to comment.