Skip to content

Commit

Permalink
refactor(api): clean project pointer useless usages (#5010)
Browse files Browse the repository at this point in the history
  • Loading branch information
richardlt committed Mar 2, 2020
1 parent 2b4848d commit d82d5e3
Show file tree
Hide file tree
Showing 100 changed files with 1,076 additions and 1,084 deletions.
8 changes: 4 additions & 4 deletions engine/api/application.go
Expand Up @@ -208,7 +208,7 @@ func (api *API) getApplicationVCSInfosHandler() service.Handler {
return service.WriteJSON(w, resp, http.StatusOK)
}

vcsServer := repositoriesmanager.GetProjectVCSServer(proj, app.VCSServer)
vcsServer := repositoriesmanager.GetProjectVCSServer(*proj, app.VCSServer)
client, erra := repositoriesmanager.AuthorizedClient(ctx, api.mustDB(), api.Cache, projectKey, vcsServer)
if erra != nil {
return sdk.WrapError(sdk.ErrNoReposManagerClientAuth, "getApplicationVCSInfosHandler> Cannot get client got %s %s : %s", projectKey, app.VCSServer, erra)
Expand Down Expand Up @@ -269,7 +269,7 @@ func (api *API) addApplicationHandler() service.Handler {

defer tx.Rollback() // nolint

if err := application.Insert(tx, api.Cache, proj, &app); err != nil {
if err := application.Insert(tx, api.Cache, *proj, &app); err != nil {
return sdk.WrapError(err, "Cannot insert pipeline")
}

Expand Down Expand Up @@ -359,7 +359,7 @@ func (api *API) cloneApplicationHandler() service.Handler {
}
defer tx.Rollback() // nolint

if err := cloneApplication(ctx, tx, api.Cache, proj, &newApp, appToClone); err != nil {
if err := cloneApplication(ctx, tx, api.Cache, *proj, &newApp, appToClone); err != nil {
return sdk.WrapError(err, "Cannot insert new application %s", newApp.Name)
}

Expand All @@ -372,7 +372,7 @@ func (api *API) cloneApplicationHandler() service.Handler {
}

// cloneApplication Clone an application with all her dependencies: pipelines, permissions, triggers
func cloneApplication(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj *sdk.Project, newApp *sdk.Application, appToClone *sdk.Application) error {
func cloneApplication(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj sdk.Project, newApp *sdk.Application, appToClone *sdk.Application) error {
// Create Application
if err := application.Insert(db, store, proj, newApp); err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions engine/api/application/application_importer.go
Expand Up @@ -13,7 +13,7 @@ import (
)

//Import is able to create a new application and all its components
func Import(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj *sdk.Project, app *sdk.Application, repomanager string, u sdk.Identifiable, msgChan chan<- sdk.Message) error {
func Import(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj sdk.Project, app *sdk.Application, repomanager string, u sdk.Identifiable, msgChan chan<- sdk.Message) error {
doUpdate, erre := Exists(db, proj.Key, app.Name)
if erre != nil {
return sdk.WrapError(erre, "application.Import> Unable to check if application exists")
Expand Down Expand Up @@ -64,7 +64,7 @@ func Import(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj *s
}
}

if err := importVariables(db, store, proj, app, u, msgChan); err != nil {
if err := importVariables(db, store, app, u, msgChan); err != nil {
return err
}

Expand Down Expand Up @@ -110,7 +110,7 @@ func Import(ctx context.Context, db gorp.SqlExecutor, store cache.Store, proj *s
}

//importVariables is able to create variable on an existing application
func importVariables(db gorp.SqlExecutor, store cache.Store, proj *sdk.Project, app *sdk.Application, u sdk.Identifiable, msgChan chan<- sdk.Message) error {
func importVariables(db gorp.SqlExecutor, store cache.Store, app *sdk.Application, u sdk.Identifiable, msgChan chan<- sdk.Message) error {
for _, newVar := range app.Variable {
var errCreate error
switch newVar.Type {
Expand Down
2 changes: 1 addition & 1 deletion engine/api/application/application_parser.go
Expand Up @@ -21,7 +21,7 @@ type ImportOptions struct {
}

// ParseAndImport parse an exportentities.Application and insert or update the application in database
func ParseAndImport(ctx context.Context, db gorp.SqlExecutor, cache cache.Store, proj *sdk.Project, eapp *exportentities.Application, opts ImportOptions, decryptFunc keys.DecryptFunc, u sdk.Identifiable) (*sdk.Application, []sdk.Message, error) {
func ParseAndImport(ctx context.Context, db gorp.SqlExecutor, cache cache.Store, proj sdk.Project, eapp *exportentities.Application, opts ImportOptions, decryptFunc keys.DecryptFunc, u sdk.Identifiable) (*sdk.Application, []sdk.Message, error) {
log.Info(ctx, "ParseAndImport>> Import application %s in project %s (force=%v)", eapp.Name, proj.Key, opts.Force)
msgList := []sdk.Message{}

Expand Down
4 changes: 2 additions & 2 deletions engine/api/application/dao.go
Expand Up @@ -15,7 +15,7 @@ import (

const appRows = `
application.id,
application.name,
application.name,
application.project_id,
application.repo_fullname,
application.repositories_manager_id,
Expand Down Expand Up @@ -150,7 +150,7 @@ func unwrap(db gorp.SqlExecutor, store cache.Store, opts []LoadOptionFunc, dbApp
}

// Insert add an application id database
func Insert(db gorp.SqlExecutor, store cache.Store, proj *sdk.Project, app *sdk.Application) error {
func Insert(db gorp.SqlExecutor, store cache.Store, proj sdk.Project, app *sdk.Application) error {
if err := app.IsValid(); err != nil {
return sdk.WrapError(err, "application is not valid")
}
Expand Down
27 changes: 14 additions & 13 deletions engine/api/application/dao_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ovh/cds/engine/api/application"
"github.com/ovh/cds/engine/api/bootstrap"
Expand All @@ -27,7 +28,7 @@ func TestLoadByNameAsAdmin(t *testing.T) {
Name: "my-app",
}

test.NoError(t, application.Insert(db, cache, proj, &app))
test.NoError(t, application.Insert(db, cache, *proj, &app))

actual, err := application.LoadByName(db, cache, key, "my-app")
test.NoError(t, err)
Expand All @@ -46,7 +47,7 @@ func TestLoadByNameAsUser(t *testing.T) {
Name: "my-app",
}

test.NoError(t, application.Insert(db, cache, proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app))

_, _ = assets.InsertLambdaUser(t, db, &proj.ProjectGroups[0].Group)

Expand All @@ -67,10 +68,10 @@ func TestLoadByIDAsAdmin(t *testing.T) {
Name: "my-app",
}

test.NoError(t, application.Insert(db, cache, proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app))

actual, err := application.LoadByID(db, cache, app.ID)
test.NoError(t, err)
require.NoError(t, err)

assert.Equal(t, app.Name, actual.Name)
assert.Equal(t, proj.ID, actual.ProjectID)
Expand All @@ -87,7 +88,7 @@ func TestLoadByIDAsUser(t *testing.T) {
Name: "my-app",
}

test.NoError(t, application.Insert(db, cache, proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app))

_, _ = assets.InsertLambdaUser(t, db, &proj.ProjectGroups[0].Group)

Expand Down Expand Up @@ -118,11 +119,11 @@ func TestLoadAllAsAdmin(t *testing.T) {
},
}

test.NoError(t, application.Insert(db, cache, proj, &app))
test.NoError(t, application.Insert(db, cache, proj, &app2))
require.NoError(t, application.Insert(db, cache, *proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app2))

actual, err := application.LoadAll(db, cache, proj.Key)
test.NoError(t, err)
require.NoError(t, err)

assert.Equal(t, 2, len(actual))

Expand All @@ -145,8 +146,8 @@ func TestLoadAllAsUser(t *testing.T) {
Name: "my-app2",
}

test.NoError(t, application.Insert(db, cache, proj, &app))
test.NoError(t, application.Insert(db, cache, proj, &app2))
require.NoError(t, application.Insert(db, cache, *proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app2))

_, _ = assets.InsertLambdaUser(t, db, &proj.ProjectGroups[0].Group)

Expand All @@ -167,15 +168,15 @@ func TestLoadByWorkflowID(t *testing.T) {
ProjectKey: proj.Key,
ProjectID: proj.ID,
}
test.NoError(t, application.Insert(db, cache, proj, &app))
require.NoError(t, application.Insert(db, cache, *proj, &app))

pip := sdk.Pipeline{
ProjectID: proj.ID,
ProjectKey: proj.Key,
Name: "pip1",
}

test.NoError(t, pipeline.InsertPipeline(db, cache, proj, &pip))
require.NoError(t, pipeline.InsertPipeline(db, &pip))

w := sdk.Workflow{
Name: "test_1",
Expand All @@ -196,7 +197,7 @@ func TestLoadByWorkflowID(t *testing.T) {

proj, _ = project.LoadByID(db, cache, proj.ID, project.LoadOptions.WithApplications, project.LoadOptions.WithPipelines, project.LoadOptions.WithEnvironments, project.LoadOptions.WithGroups)

test.NoError(t, workflow.Insert(context.TODO(), db, cache, &w, proj))
require.NoError(t, workflow.Insert(context.TODO(), db, cache, *proj, &w))

actuals, err := application.LoadByWorkflowID(db, w.ID)
assert.NoError(t, err)
Expand Down
8 changes: 4 additions & 4 deletions engine/api/application_deployment_test.go
Expand Up @@ -30,7 +30,7 @@ func Test_getApplicationDeploymentStrategiesConfigHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
test.NoError(t, application.Insert(api.mustDB(), api.Cache, proj, app))
test.NoError(t, application.Insert(api.mustDB(), api.Cache, *proj, app))

vars := map[string]string{
"permProjectKey": proj.Key,
Expand All @@ -57,7 +57,7 @@ func Test_postApplicationDeploymentStrategyConfigHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
test.NoError(t, application.Insert(api.mustDB(), api.Cache, proj, app))
test.NoError(t, application.Insert(api.mustDB(), api.Cache, *proj, app))

pf := sdk.IntegrationModel{
Name: "test-deploy-post-2" + pkey,
Expand Down Expand Up @@ -180,7 +180,7 @@ func Test_postApplicationDeploymentStrategyConfigHandler_InsertTwoDifferentInteg
app := &sdk.Application{
Name: sdk.RandomString(10),
}
test.NoError(t, application.Insert(api.mustDB(), api.Cache, proj, app))
test.NoError(t, application.Insert(api.mustDB(), api.Cache, *proj, app))

pf := sdk.IntegrationModel{
Name: "test-deploy-TwoDifferentIntegrations-2" + pkey,
Expand Down Expand Up @@ -310,7 +310,7 @@ func Test_postApplicationDeploymentStrategyConfigHandlerAsProvider(t *testing.T)
app := &sdk.Application{
Name: sdk.RandomString(10),
}
test.NoError(t, application.Insert(api.mustDB(), api.Cache, proj, app))
test.NoError(t, application.Insert(api.mustDB(), api.Cache, *proj, app))

pf := sdk.IntegrationModel{
Name: "test-deploy-3" + pkey,
Expand Down
2 changes: 1 addition & 1 deletion engine/api/application_export_test.go
Expand Up @@ -23,7 +23,7 @@ func Test_getApplicationExportHandler(t *testing.T) {
app := &sdk.Application{
Name: appName,
}
if err := application.Insert(db, api.Cache, proj, app); err != nil {
if err := application.Insert(db, api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down
15 changes: 7 additions & 8 deletions engine/api/application_import.go
Expand Up @@ -25,15 +25,14 @@ func (api *API) postApplicationImportHandler() service.Handler {
key := vars[permProjectKey]
force := FormBool(r, "force")

//Load project
proj, errp := project.Load(api.mustDB(), api.Cache, key, project.LoadOptions.WithGroups, project.LoadOptions.WithIntegrations)
if errp != nil {
return sdk.WrapError(errp, "postApplicationImportHandler>> Unable load project")
proj, err := project.Load(api.mustDB(), api.Cache, key, project.LoadOptions.WithGroups, project.LoadOptions.WithIntegrations)
if err != nil {
return sdk.WrapError(err, "unable load project")
}

body, errr := ioutil.ReadAll(r.Body)
if errr != nil {
return sdk.NewError(sdk.ErrWrongRequest, errr)
body, err := ioutil.ReadAll(r.Body)
if err != nil {
return sdk.NewError(sdk.ErrWrongRequest, err)
}
defer r.Body.Close()

Expand Down Expand Up @@ -63,7 +62,7 @@ func (api *API) postApplicationImportHandler() service.Handler {
}
defer tx.Rollback() // nolint

newApp, msgList, globalError := application.ParseAndImport(ctx, tx, api.Cache, proj, eapp, application.ImportOptions{Force: force}, project.DecryptWithBuiltinKey, getAPIConsumer(ctx))
newApp, msgList, globalError := application.ParseAndImport(ctx, tx, api.Cache, *proj, eapp, application.ImportOptions{Force: force}, project.DecryptWithBuiltinKey, getAPIConsumer(ctx))
msgListString := translate(r, msgList)
if globalError != nil {
globalError = sdk.WrapError(globalError, "Unable to import application %s", eapp.Name)
Expand Down
14 changes: 7 additions & 7 deletions engine/api/application_import_test.go
Expand Up @@ -102,7 +102,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecrets(t *testi
app := &sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, app))
test.NoError(t, application.Insert(db, api.Cache, *proj, app))

k := &sdk.ApplicationKey{
Name: "app-mykey",
Expand Down Expand Up @@ -211,7 +211,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecretsAndReImpo
app := &sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, app))
test.NoError(t, application.Insert(db, api.Cache, *proj, app))

k := &sdk.ApplicationKey{
Name: "app-mykey",
Expand Down Expand Up @@ -375,7 +375,7 @@ func Test_postApplicationImportHandler_NewAppFromYAMLWithKeysAndSecretsAndReImpo
app := &sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, app))
test.NoError(t, application.Insert(db, api.Cache, *proj, app))

// create password, pgp and ssh keys
k1 := &sdk.ApplicationKey{
Expand Down Expand Up @@ -579,7 +579,7 @@ func Test_postApplicationImportHandler_ExistingAppFromYAMLWithoutForce(t *testin
app := sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, &app))
test.NoError(t, application.Insert(db, api.Cache, *proj, &app))

//Prepare request
vars := map[string]string{
Expand Down Expand Up @@ -614,7 +614,7 @@ func Test_postApplicationImportHandler_ExistingAppFromYAMLInheritPermissions(t *
app := sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, &app))
test.NoError(t, application.Insert(db, api.Cache, *proj, &app))

//Prepare request
vars := map[string]string{
Expand Down Expand Up @@ -675,7 +675,7 @@ func Test_postApplicationImportHandler_ExistingAppWithDeploymentStrategy(t *test
app := sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, &app))
test.NoError(t, application.Insert(db, api.Cache, *proj, &app))

test.NoError(t, application.SetDeploymentStrategy(db, proj.ID, app.ID, pf.ID, pp.Name, sdk.IntegrationConfig{
"token": sdk.IntegrationConfigValue{
Expand Down Expand Up @@ -787,7 +787,7 @@ func Test_postApplicationImportHandler_DontOverrideDeploymentPasswordIfNotGiven(
app := sdk.Application{
Name: "myNewApp",
}
test.NoError(t, application.Insert(db, api.Cache, proj, &app))
test.NoError(t, application.Insert(db, api.Cache, *proj, &app))

test.NoError(t, application.SetDeploymentStrategy(db, proj.ID, app.ID, pf.ID, pp.Name, sdk.IntegrationConfig{
"token": sdk.IntegrationConfigValue{
Expand Down
6 changes: 3 additions & 3 deletions engine/api/application_key_test.go
Expand Up @@ -31,7 +31,7 @@ func Test_getKeysInApplicationHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
if err := application.Insert(api.mustDB(), api.Cache, proj, app); err != nil {
if err := application.Insert(api.mustDB(), api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -90,7 +90,7 @@ func Test_deleteKeyInApplicationHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
if err := application.Insert(api.mustDB(), api.Cache, proj, app); err != nil {
if err := application.Insert(api.mustDB(), api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -143,7 +143,7 @@ func Test_addKeyInApplicationHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
if err := application.Insert(api.mustDB(), api.Cache, proj, app); err != nil {
if err := application.Insert(api.mustDB(), api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down
7 changes: 4 additions & 3 deletions engine/api/application_test.go
Expand Up @@ -4,11 +4,12 @@ import (
"context"
"encoding/json"
"fmt"
"gopkg.in/yaml.v2"
"net/http"
"net/http/httptest"
"testing"

"gopkg.in/yaml.v2"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -49,7 +50,7 @@ func Test_postApplicationMetadataHandler_AsProvider(t *testing.T) {
"a1": "a1",
},
}
if err := application.Insert(api.mustDB(), api.Cache, proj, app); err != nil {
if err := application.Insert(api.mustDB(), api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -94,7 +95,7 @@ vcs_ssh_key: proj-blabla
`
var eapp = new(exportentities.Application)
assert.NoError(t, yaml.Unmarshal([]byte(appS), eapp))
app, _, globalError := application.ParseAndImport(context.Background(), db, api.Cache, p, eapp, application.ImportOptions{Force: true}, nil, u)
app, _, globalError := application.ParseAndImport(context.Background(), db, api.Cache, *p, eapp, application.ImportOptions{Force: true}, nil, u)
assert.NoError(t, globalError)

app.FromRepository = "myrepository"
Expand Down
2 changes: 1 addition & 1 deletion engine/api/application_variable_test.go
Expand Up @@ -28,7 +28,7 @@ func Test_getVariableAuditInApplicationHandler(t *testing.T) {
app := &sdk.Application{
Name: sdk.RandomString(10),
}
if err := application.Insert(api.mustDB(), api.Cache, proj, app); err != nil {
if err := application.Insert(api.mustDB(), api.Cache, *proj, app); err != nil {
t.Fatal(err)
}

Expand Down

0 comments on commit d82d5e3

Please sign in to comment.