From 75c33c71a19ac95dd53133dd7db2f3e32025d9b7 Mon Sep 17 00:00:00 2001 From: Sonja Happ Date: Wed, 11 Sep 2019 15:44:14 +0200 Subject: [PATCH] improve code coverage of simulation model testing - fix a bug in POST endpoint - remove unnecessary code from endpoint, validator, and method implementations --- .../simulationmodel_endpoints.go | 32 ++-- .../simulationmodel_methods.go | 6 +- .../simulationmodel/simulationmodel_test.go | 154 ++++++++++++++++-- .../simulationmodel_validators.go | 4 +- 4 files changed, 155 insertions(+), 41 deletions(-) diff --git a/routes/simulationmodel/simulationmodel_endpoints.go b/routes/simulationmodel/simulationmodel_endpoints.go index 44f3218..1fcf3d0 100644 --- a/routes/simulationmodel/simulationmodel_endpoints.go +++ b/routes/simulationmodel/simulationmodel_endpoints.go @@ -39,11 +39,10 @@ func getSimulationModels(c *gin.Context) { db := database.GetDB() var models []database.SimulationModel err := db.Order("ID asc").Model(so).Related(&models, "Models").Error - if helper.DBError(c, err) { - return + if !helper.DBError(c, err) { + c.JSON(http.StatusOK, gin.H{"models": models}) } - c.JSON(http.StatusOK, gin.H{"models": models}) } // addSimulationModel godoc @@ -79,19 +78,17 @@ func addSimulationModel(c *gin.Context) { newSimulationModel := req.createSimulationModel() // check access to the scenario - ok, _ := scenario.CheckPermissions(c, database.Create, "body", int(newSimulationModel.ScenarioID)) + ok, _ := scenario.CheckPermissions(c, database.Update, "body", int(newSimulationModel.ScenarioID)) if !ok { return } // add the new simulation model to the scenario err = newSimulationModel.addToScenario() - if err != nil { - helper.DBError(c, err) - return + if !helper.DBError(c, err) { + c.JSON(http.StatusOK, gin.H{"model": newSimulationModel.SimulationModel}) } - c.JSON(http.StatusOK, gin.H{"model": newSimulationModel.SimulationModel}) } // updateSimulationModel godoc @@ -129,21 +126,14 @@ func updateSimulationModel(c *gin.Context) { } // Create the updatedSimulationModel from oldSimulationModel - updatedSimulationModel, err := req.updatedSimulationModel(oldSimulationModel) - if err != nil { - helper.BadRequestError(c, err.Error()) - return - } + updatedSimulationModel := req.updatedSimulationModel(oldSimulationModel) // Finally, update the simulation model err = oldSimulationModel.Update(updatedSimulationModel) - if err != nil { - helper.DBError(c, err) - return + if !helper.DBError(c, err) { + c.JSON(http.StatusOK, gin.H{"model": updatedSimulationModel.SimulationModel}) } - c.JSON(http.StatusOK, gin.H{"model": updatedSimulationModel.SimulationModel}) - } // getSimulationModel godoc @@ -188,9 +178,7 @@ func deleteSimulationModel(c *gin.Context) { } err := m.delete() - if helper.DBError(c, err) { - return + if !helper.DBError(c, err) { + c.JSON(http.StatusOK, gin.H{"model": m.SimulationModel}) } - - c.JSON(http.StatusOK, gin.H{"model": m.SimulationModel}) } diff --git a/routes/simulationmodel/simulationmodel_methods.go b/routes/simulationmodel/simulationmodel_methods.go index 0512720..2fdfb78 100644 --- a/routes/simulationmodel/simulationmodel_methods.go +++ b/routes/simulationmodel/simulationmodel_methods.go @@ -56,6 +56,7 @@ func (m *SimulationModel) addToScenario() error { func (m *SimulationModel) Update(modifiedSimulationModel SimulationModel) error { db := database.GetDB() + // check if simulator has been updated if m.SimulatorID != modifiedSimulationModel.SimulatorID { // update simulator var s simulator.Simulator @@ -80,15 +81,10 @@ func (m *SimulationModel) Update(modifiedSimulationModel SimulationModel) error } } - if m.ScenarioID != modifiedSimulationModel.ScenarioID { - // TODO do we allow this case? - } - err := db.Model(m).Updates(map[string]interface{}{ "Name": modifiedSimulationModel.Name, "StartParameters": modifiedSimulationModel.StartParameters, "SimulatorID": modifiedSimulationModel.SimulatorID, - //"ScenarioID": modifiedSimulationModel.ScenarioID, }).Error return err diff --git a/routes/simulationmodel/simulationmodel_test.go b/routes/simulationmodel/simulationmodel_test.go index 74c329f..db85dae 100644 --- a/routes/simulationmodel/simulationmodel_test.go +++ b/routes/simulationmodel/simulationmodel_test.go @@ -59,6 +59,17 @@ func addScenarioAndSimulator() (scenarioID uint, simulatorID uint) { // Read newSimulator's ID from the response newSimulatorID, _ := helper.GetResponseID(resp) + // POST a second simulator to change to that simulator during testing + newSimulatorB := SimulatorRequest{ + UUID: database.SimulatorB.UUID, + Host: database.SimulatorB.Host, + Modeltype: database.SimulatorB.Modeltype, + State: database.SimulatorB.State, + Properties: database.SimulatorB.Properties, + } + _, resp, _ = helper.TestEndpoint(router, token, + "/api/simulators", "POST", helper.KeyModels{"simulator": newSimulatorB}) + // authenticate as normal user token, _ = helper.AuthenticateForTest(router, "/api/authenticate", "POST", helper.UserACredentials) @@ -75,6 +86,10 @@ func addScenarioAndSimulator() (scenarioID uint, simulatorID uint) { // Read newScenario's ID from the response newScenarioID, _ := helper.GetResponseID(resp) + // add the guest user to the new scenario + _, resp, _ = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) + return uint(newScenarioID), uint(newSimulatorID) } @@ -109,21 +124,45 @@ func TestAddSimulationModel(t *testing.T) { // using the respective endpoints of the API scenarioID, simulatorID := addScenarioAndSimulator() - // authenticate as normal user - token, err := helper.AuthenticateForTest(router, - "/api/authenticate", "POST", helper.UserACredentials) - assert.NoError(t, err) - - // test POST models/ $newSimulationModel newSimulationModel := SimulationModelRequest{ Name: database.SimulationModelA.Name, ScenarioID: scenarioID, SimulatorID: simulatorID, StartParameters: database.SimulationModelA.StartParameters, } + + // authenticate as normal userB who has no access to new scenario + token, err := helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserBCredentials) + assert.NoError(t, err) + + // try to POST with no access + // should result in unprocessable entity code, resp, err := helper.TestEndpoint(router, token, "/api/models", "POST", helper.KeyModels{"model": newSimulationModel}) assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // authenticate as normal user + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserACredentials) + assert.NoError(t, err) + + // try to POST non JSON body + code, resp, err = helper.TestEndpoint(router, token, + "/api/models", "POST", "this is not JSON") + assert.NoError(t, err) + assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) + + // authenticate as normal user + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserACredentials) + assert.NoError(t, err) + + // test POST models/ $newSimulationModel + code, resp, err = helper.TestEndpoint(router, token, + "/api/models", "POST", helper.KeyModels{"model": newSimulationModel}) + assert.NoError(t, err) assert.Equalf(t, 200, code, "Response body: \n%v\n", resp) // Compare POST's response with the newSimulationModel @@ -155,6 +194,18 @@ func TestAddSimulationModel(t *testing.T) { assert.NoError(t, err) assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + // authenticate as normal userB who has no access to new scenario + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserBCredentials) + assert.NoError(t, err) + + // Try to GET the newSimulationModel with no access + // Should result in unprocessable entity + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "GET", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + } func TestUpdateSimulationModel(t *testing.T) { @@ -194,6 +245,55 @@ func TestUpdateSimulationModel(t *testing.T) { StartParameters: database.SimulationModelB.StartParameters, } + // authenticate as normal userB who has no access to new scenario + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserBCredentials) + assert.NoError(t, err) + + // try to PUT with no access + // should result in unprocessable entity + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "PUT", helper.KeyModels{"model": updatedSimulationModel}) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // authenticate as guest user who has access to simulation model + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.GuestCredentials) + assert.NoError(t, err) + + // try to PUT as guest + // should NOT work and result in unprocessable entity + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "PUT", helper.KeyModels{"model": updatedSimulationModel}) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // authenticate as normal user + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserACredentials) + assert.NoError(t, err) + + // try to PUT a non JSON body + // should result in a bad request + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "PUT", "This is not JSON") + assert.NoError(t, err) + assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) + + // test PUT + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "PUT", helper.KeyModels{"model": updatedSimulationModel}) + assert.NoError(t, err) + assert.Equalf(t, 200, code, "Response body: \n%v\n", resp) + + // Compare PUT's response with the updatedSimulationModel + err = helper.CompareResponse(resp, helper.KeyModels{"model": updatedSimulationModel}) + assert.NoError(t, err) + + //Change simulator ID to use second simulator available in DB + updatedSimulationModel.SimulatorID = simulatorID + 1 + // test PUT again code, resp, err = helper.TestEndpoint(router, token, fmt.Sprintf("/api/models/%v", newSimulationModelID), "PUT", helper.KeyModels{"model": updatedSimulationModel}) assert.NoError(t, err) @@ -230,18 +330,19 @@ func TestDeleteSimulationModel(t *testing.T) { // using the respective endpoints of the API scenarioID, simulatorID := addScenarioAndSimulator() - // authenticate as normal user - token, err := helper.AuthenticateForTest(router, - "/api/authenticate", "POST", helper.UserACredentials) - assert.NoError(t, err) - - // test POST models/ $newSimulationModel newSimulationModel := SimulationModelRequest{ Name: database.SimulationModelA.Name, ScenarioID: scenarioID, SimulatorID: simulatorID, StartParameters: database.SimulationModelA.StartParameters, } + + // authenticate as normal user + token, err := helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserACredentials) + assert.NoError(t, err) + + // test POST models/ $newSimulationModel code, resp, err := helper.TestEndpoint(router, token, "/api/models", "POST", helper.KeyModels{"model": newSimulationModel}) assert.NoError(t, err) @@ -251,6 +352,23 @@ func TestDeleteSimulationModel(t *testing.T) { newSimulationModelID, err := helper.GetResponseID(resp) assert.NoError(t, err) + // authenticate as normal userB who has no access to new scenario + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserBCredentials) + assert.NoError(t, err) + + // try to DELETE with no access + // should result in unprocessable entity + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models/%v", newSimulationModelID), "DELETE", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + + // authenticate as normal user + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserACredentials) + assert.NoError(t, err) + // Count the number of all the simulation models returned for scenario initialNumber, err := helper.LengthOfResponse(router, token, fmt.Sprintf("/api/models?scenarioID=%v", scenarioID), "GET", nil) @@ -308,4 +426,16 @@ func TestGetAllSimulationModelsOfScenario(t *testing.T) { assert.Equal(t, 1, NumberOfSimulationModels) + // authenticate as normal userB who has no access to scenario + token, err = helper.AuthenticateForTest(router, + "/api/authenticate", "POST", helper.UserBCredentials) + assert.NoError(t, err) + + // try to get models without access + // should result in unprocessable entity + code, resp, err = helper.TestEndpoint(router, token, + fmt.Sprintf("/api/models?scenarioID=%v", scenarioID), "GET", nil) + assert.NoError(t, err) + assert.Equalf(t, 422, code, "Response body: \n%v\n", resp) + } diff --git a/routes/simulationmodel/simulationmodel_validators.go b/routes/simulationmodel/simulationmodel_validators.go index c800ccd..9c65fbb 100644 --- a/routes/simulationmodel/simulationmodel_validators.go +++ b/routes/simulationmodel/simulationmodel_validators.go @@ -53,7 +53,7 @@ func (r *addSimulationModelRequest) createSimulationModel() SimulationModel { return s } -func (r *updateSimulationModelRequest) updatedSimulationModel(oldSimulationModel SimulationModel) (SimulationModel, error) { +func (r *updateSimulationModelRequest) updatedSimulationModel(oldSimulationModel SimulationModel) SimulationModel { // Use the old SimulationModel as a basis for the updated Simulation model s := oldSimulationModel @@ -76,5 +76,5 @@ func (r *updateSimulationModelRequest) updatedSimulationModel(oldSimulationModel s.StartParameters = r.StartParameters } - return s, nil + return s }