diff --git a/application/application_db_calls_bench_test.go b/application/application_db_calls_bench_test.go index 826837c392..4c209a6e12 100644 --- a/application/application_db_calls_bench_test.go +++ b/application/application_db_calls_bench_test.go @@ -5,6 +5,7 @@ import ( "testing" _ "github.com/lib/pq" + "github.com/stretchr/testify/require" "github.com/fabric8-services/fabric8-wit/application" "github.com/fabric8-services/fabric8-wit/gormapplication" @@ -125,7 +126,8 @@ func (s *BenchDbOperations) BenchmarkGormSelectSpaceNameRaw() { defer result.Close() for result.Next() { var wit string - result.Scan(&wit) + err := result.Scan(&wit) + require.NoError(s.B(), err) names = append(names, wit) } } @@ -195,11 +197,12 @@ func (s *BenchDbOperations) BenchmarkGormSelectSpaceRaw() { defer result.Close() for result.Next() { var sp space.Space - result.Scan( + err := result.Scan( &sp.Version, &sp.Name, &sp.Description, &sp.OwnerID) + require.NoError(s.B(), err) sps = append(sps, sp) } } diff --git a/application/transaction.go b/application/transaction.go index 5a974e15a4..d54eea639f 100644 --- a/application/transaction.go +++ b/application/transaction.go @@ -1,6 +1,7 @@ package application import ( + "context" "runtime/debug" "time" @@ -46,15 +47,27 @@ func Transactional(db DB, todo func(f Application) error) error { case err := <-errorChan: if err != nil { log.Debug(nil, map[string]interface{}{"error": err}, "Rolling back the transaction...") - tx.Rollback() + errRollback := tx.Rollback() + if errRollback != nil { + log.Error(context.Background(), map[string]interface{}{ + "errRollback": errors.WithStack(errRollback), + "err": errors.WithStack(err), + }, "failed to rollback transaction: %+v", errRollback) + } log.Error(nil, map[string]interface{}{ "err": err, }, "database transaction failed!") return errors.WithStack(err) } - tx.Commit() - log.Debug(nil, nil, "Commit the transaction!") + log.Debug(nil, nil, "Committing the transaction!") + errCommit := tx.Commit() + if errCommit != nil { + log.Error(context.Background(), map[string]interface{}{ + "errCommit": errors.WithStack(errCommit), + }, "failed to commit transaction: %+v", errCommit) + } + return nil case <-txTimeout: log.Debug(nil, nil, "Rolling back the transaction...") diff --git a/application/transaction_bench_test.go b/application/transaction_bench_test.go index dad0704a56..1e9187f5ce 100644 --- a/application/transaction_bench_test.go +++ b/application/transaction_bench_test.go @@ -1,7 +1,6 @@ package application_test import ( - "database/sql" "testing" _ "github.com/lib/pq" @@ -17,7 +16,6 @@ type BenchTransactional struct { gormbench.DBBenchSuite repo space.Repository appDB application.DB - dbPq *sql.DB } func BenchmarkRunTransactional(b *testing.B) { diff --git a/codebase/analytics-gemini/client_blackbox_test.go b/codebase/analytics-gemini/client_blackbox_test.go index 2d88cc95e3..f4d8cae0fe 100644 --- a/codebase/analytics-gemini/client_blackbox_test.go +++ b/codebase/analytics-gemini/client_blackbox_test.go @@ -28,7 +28,10 @@ func TestRegister(t *testing.T) { testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"), ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{ Transport: r.Transport, @@ -50,7 +53,10 @@ func TestRegister(t *testing.T) { "../../test/data/gemini-scan/register-401", ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{ Transport: r.Transport, @@ -102,7 +108,10 @@ func TestRegister(t *testing.T) { testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"), ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{ Transport: r.Transport, @@ -128,7 +137,10 @@ func TestRegister(t *testing.T) { testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"), ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{ Transport: r.Transport, @@ -165,7 +177,10 @@ func TestDeRegister(t *testing.T) { "../../test/data/gemini-scan/deregister-200", ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{Transport: r.Transport} cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false) @@ -187,7 +202,10 @@ func TestDeRegister(t *testing.T) { "../../test/data/gemini-scan/deregister-call-gemini-codebases-200", ) require.NoError(t, err) - defer recordCodebases.Stop() + defer func() { + err := recordCodebases.Stop() + require.NoError(t, err) + }() codebaseClient := &http.Client{Transport: recordCodebases.Transport} recordGemini, err := testrecorder.New( @@ -195,7 +213,10 @@ func TestDeRegister(t *testing.T) { testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"), ) require.NoError(t, err) - defer recordGemini.Stop() + defer func() { + err := recordGemini.Stop() + require.NoError(t, err) + }() geminiClient := &http.Client{Transport: recordGemini.Transport} cli := gemini.NewScanRepoClient(geminiURL, geminiClient, codebaseURL, codebaseClient, false) @@ -242,7 +263,10 @@ func TestDeRegister(t *testing.T) { "../../test/data/gemini-scan/deregister-400", ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{Transport: r.Transport} cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false) @@ -263,7 +287,10 @@ func TestDeRegister(t *testing.T) { "../../test/data/gemini-scan/deregister-bad-output", ) require.NoError(t, err) - defer r.Stop() + defer func() { + err := r.Stop() + require.NoError(t, err) + }() httpClient := &http.Client{Transport: r.Transport} cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false) diff --git a/codebase/che/client.go b/codebase/che/client.go index 1de5f59e58..911a3b49d0 100644 --- a/codebase/che/client.go +++ b/codebase/che/client.go @@ -448,7 +448,7 @@ func (w WorkspaceResponse) GetHrefByRelOfWorkspaceLink(rel string) string { // Following const define commonly used WorkspaceLink rel const ( - IdeUrlRel = "ide url" + IdeURLRel = "ide url" SelfLinkRel = "self link" ) diff --git a/codebase/codebase.go b/codebase/codebase.go index a9fb9cd9e5..9168bffd76 100644 --- a/codebase/codebase.go +++ b/codebase/codebase.go @@ -283,7 +283,10 @@ func (m *GormCodebaseRepository) List(ctx context.Context, spaceID uuid.UUID, st for rows.Next() { value := Codebase{} - db.ScanRows(rows, &value) + err := db.ScanRows(rows, &value) + if err != nil { + return nil, 0, errs.Wrapf(err, "failed to scan codebase rows") + } if first { first = false if err = rows.Scan(columnValues...); err != nil { @@ -300,10 +303,14 @@ func (m *GormCodebaseRepository) List(ctx context.Context, spaceID uuid.UUID, st rows2, err := orgDB.Rows() defer closeable.Close(ctx, rows2) if err != nil { - return nil, 0, err + return nil, 0, errs.WithStack(err) } rows2.Next() // count(*) will always return a row - rows2.Scan(&count) + err = rows2.Scan(&count) + if err != nil { + return nil, 0, errs.WithStack(err) + } + } return result, count, nil } @@ -384,7 +391,10 @@ func (m *GormCodebaseRepository) SearchByURL(ctx context.Context, url string, st first := true for rows.Next() { value := Codebase{} - db.ScanRows(rows, &value) + err := db.ScanRows(rows, &value) + if err != nil { + return nil, 0, errors.NewInternalError(ctx, err) + } if first { first = false if err = rows.Scan(columnValues...); err != nil { diff --git a/comment/comment_repository.go b/comment/comment_repository.go index 9cb34cedd1..ebb07c1140 100644 --- a/comment/comment_repository.go +++ b/comment/comment_repository.go @@ -175,7 +175,10 @@ func (m *GormCommentRepository) List(ctx context.Context, parentID uuid.UUID, st for rows.Next() { value := &Comment{} - db.ScanRows(rows, value) + err := db.ScanRows(rows, value) + if err != nil { + return nil, 0, errors.NewInternalError(ctx, err) + } if first { first = false if err = rows.Scan(columnValues...); err != nil { @@ -191,10 +194,13 @@ func (m *GormCommentRepository) List(ctx context.Context, parentID uuid.UUID, st rows2, err := orgDB.Rows() defer closeable.Close(ctx, rows2) if err != nil { - return nil, 0, err + return nil, 0, errs.WithStack(err) } rows2.Next() // count(*) will always return a row - rows2.Scan(&count) + err = rows2.Scan(&count) + if err != nil { + return nil, 0, errors.NewInternalError(ctx, err) + } } return result, count, nil } diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index e1637b5567..ca06ce1790 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -56,7 +56,8 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() { // parent comment fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1)) parentComment := newComment(uuid.NewV4(), "Test A", rendering.SystemMarkupMarkdown) - s.repo.Create(s.Ctx, parentComment, fxt.Identities[0].ID) + err := s.repo.Create(s.Ctx, parentComment, fxt.Identities[0].ID) + require.NoError(s.T(), err) // child comments childComment := newComment(uuid.NewV4(), "Test Child A", rendering.SystemMarkupMarkdown) childComment.ParentCommentID = id.NullUUID{ @@ -64,8 +65,9 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() { Valid: true, } // when - s.repo.Create(s.Ctx, childComment, fxt.Identities[0].ID) + err = s.repo.Create(s.Ctx, childComment, fxt.Identities[0].ID) // then + require.NoError(s.T(), err) require.NotNil(s.T(), childComment.ID, "Comment was not created, ID nil") require.NotNil(s.T(), childComment.CreatedAt, "Comment was not created") require.False(s.T(), childComment.CreatedAt.After(time.Now()), "Comment was not created, CreatedAt after Now()") @@ -74,7 +76,7 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() { // now retrieving the stored child comment again and see if the parent reference was stored var resultComment *comment.Comment // when - resultComment, err := s.repo.Load(s.Ctx, childComment.ID) + resultComment, err = s.repo.Load(s.Ctx, childComment.ID) // then require.NoError(s.T(), err) require.NotNil(s.T(), resultComment.ParentCommentID, "Parent comment id was not set, ID nil") @@ -86,8 +88,9 @@ func (s *TestCommentRepository) TestCreateCommentWithMarkup() { fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1)) comment := newComment(uuid.NewV4(), "Test A", rendering.SystemMarkupMarkdown) // when - s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID) + err := s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID) // then + require.NoError(s.T(), err) assert.NotNil(s.T(), comment.ID, "Comment was not created, ID nil") require.NotNil(s.T(), comment.CreatedAt, "Comment was not created?") assert.False(s.T(), comment.CreatedAt.After(time.Now()), "Comment was not created, CreatedAt after Now()?") @@ -98,8 +101,9 @@ func (s *TestCommentRepository) TestCreateCommentWithoutMarkup() { fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1)) comment := newComment(uuid.NewV4(), "Test A", "") // when - s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID) + err := s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID) // then + require.NoError(s.T(), err) assert.NotNil(s.T(), comment.ID, "Comment was not created, ID nil") require.NotNil(s.T(), comment.CreatedAt, "Comment was not created?") assert.False(s.T(), comment.CreatedAt.After(time.Now()), "CreatedAt after Now()?") @@ -116,7 +120,8 @@ func (s *TestCommentRepository) TestSaveCommentWithMarkup() { // when comment.Body = "Test AB" comment.Markup = rendering.SystemMarkupMarkdown - s.repo.Save(s.Ctx, comment, comment.Creator) + err := s.repo.Save(s.Ctx, comment, comment.Creator) + require.NoError(s.T(), err) offset := 0 limit := 1 comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit) @@ -137,7 +142,8 @@ func (s *TestCommentRepository) TestSaveCommentWithoutMarkup() { // when comment.Body = "Test AB" comment.Markup = "" - s.repo.Save(s.Ctx, comment, comment.Creator) + err := s.repo.Save(s.Ctx, comment, comment.Creator) + require.NoError(s.T(), err) offset := 0 limit := 1 comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit) diff --git a/configuration/configuration_blackbox_test.go b/configuration/configuration_blackbox_test.go index 0fe464af88..ef3ef36250 100644 --- a/configuration/configuration_blackbox_test.go +++ b/configuration/configuration_blackbox_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "strconv" - "strings" "testing" "net/http" @@ -13,7 +12,6 @@ import ( "github.com/fabric8-services/fabric8-wit/configuration" "github.com/fabric8-services/fabric8-wit/resource" - "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -183,30 +181,6 @@ func TestGetMaxHeaderSizeSetByEnvVaribaleOK(t *testing.T) { assert.Equal(t, envValue, viperValue) } -func generateEnvKey(yamlKey string) string { - return "F8_" + strings.ToUpper(strings.Replace(yamlKey, ".", "_", -1)) -} - -func checkGetKeycloakEndpointSetByEnvVaribaleOK(t *testing.T, envName string, getEndpoint func(req *http.Request) (string, error)) { - envValue := uuid.NewV4().String() - env := os.Getenv(envName) - defer func() { - os.Setenv(envName, env) - resetConfiguration(defaultValuesConfigFilePath) - }() - - os.Setenv(envName, envValue) - resetConfiguration(defaultValuesConfigFilePath) - - url, err := getEndpoint(reqLong) - require.NoError(t, err) - require.Equal(t, envValue, url) - - url, err = getEndpoint(reqShort) - require.NoError(t, err) - require.Equal(t, envValue, url) -} - func TestGetDeploymentsTimeoutDefault(t *testing.T) { resource.Require(t, resource.UnitTest) viperValue := config.GetDeploymentsHTTPTimeoutSeconds() diff --git a/controller/codebase.go b/controller/codebase.go index 9c98ec30dd..5a42a23e21 100644 --- a/controller/codebase.go +++ b/controller/codebase.go @@ -179,7 +179,7 @@ func (c *CodebaseController) ListWorkspaces(ctx *app.ListWorkspacesCodebaseConte codebaseRelatedLink := rest.AbsoluteURL(ctx.Request, fmt.Sprintf(app.CodebaseHref(cb.ID))) openLink := rest.AbsoluteURL(ctx.Request, fmt.Sprintf(app.CodebaseHref(cb.ID)+"/open/%v", workspace.Config.Name)) - ideLink := workspace.GetHrefByRelOfWorkspaceLink(che.IdeUrlRel) + ideLink := workspace.GetHrefByRelOfWorkspaceLink(che.IdeURLRel) selfLink := workspace.GetHrefByRelOfWorkspaceLink(che.SelfLinkRel) existingWorkspaces = append(existingWorkspaces, &app.Workspace{ @@ -385,7 +385,7 @@ func (c *CodebaseController) Create(ctx *app.CreateCodebaseContext) error { if err != nil { return jsonapi.JSONErrorResponse(ctx, err) } - ideURL := workspaceResp.GetHrefByRelOfWorkspaceLink(che.IdeUrlRel) + ideURL := workspaceResp.GetHrefByRelOfWorkspaceLink(che.IdeURLRel) resp := &app.WorkspaceOpen{ Links: &app.WorkspaceOpenLinks{ Open: &ideURL, @@ -442,7 +442,7 @@ func (c *CodebaseController) Open(ctx *app.OpenCodebaseContext) error { return jsonapi.JSONErrorResponse(ctx, goa.ErrInternal(err.Error())) } - ideURL := workspaceResp.GetHrefByRelOfWorkspaceLink(che.IdeUrlRel) + ideURL := workspaceResp.GetHrefByRelOfWorkspaceLink(che.IdeURLRel) resp := &app.WorkspaceOpen{ Links: &app.WorkspaceOpenLinks{ Open: &ideURL, diff --git a/controller/codebase_whitebox_test.go b/controller/codebase_whitebox_test.go index edacf725b3..0aaeb4c5b1 100644 --- a/controller/codebase_whitebox_test.go +++ b/controller/codebase_whitebox_test.go @@ -31,7 +31,7 @@ func TestWorkspaceLinks(t *testing.T) { Status: "RUNNING", Links: []che.WorkspaceLink{workspaceIdeLink, workspaceSelfLink}} - ideLink := workspaceResponse.GetHrefByRelOfWorkspaceLink(che.IdeUrlRel) + ideLink := workspaceResponse.GetHrefByRelOfWorkspaceLink(che.IdeURLRel) selfLink := workspaceResponse.GetHrefByRelOfWorkspaceLink(che.SelfLinkRel) assert.Equal(t, hrefIde, ideLink) diff --git a/controller/comments_blackbox_test.go b/controller/comments_blackbox_test.go index e93dab962c..41510949dc 100644 --- a/controller/comments_blackbox_test.go +++ b/controller/comments_blackbox_test.go @@ -396,6 +396,7 @@ func (s *CommentsSuite) TestNonCollaboratorCanNotDelete() { c := s.createWorkItemComment(*testIdentity, *wi.Data.ID, "body", &plaintextMarkup, nil) testIdentity2, err := testsupport.CreateTestIdentity(s.DB, testsupport.CreateRandomValidTestName("TestNonCollaboraterCanNotDelete-"), "TestWI") + require.NoError(s.T(), err) svcNotAuthorized := testsupport.ServiceAsSpaceUser("Collaborators-Service", *testIdentity2, &TestSpaceAuthzService{*testIdentity, ""}) commentsCtrlNotAuthorized := NewCommentsController(svcNotAuthorized, s.GormDB, s.Configuration) diff --git a/controller/login_test.go b/controller/login_test.go index afbce8ac35..5bb1f309d3 100644 --- a/controller/login_test.go +++ b/controller/login_test.go @@ -90,6 +90,6 @@ func validateToken(t *testing.T, token *app.AuthToken, controler *LoginControlle type TestLoginService struct{} -func (t TestLoginService) CreateOrUpdateKeycloakUser(accessToken string, ctx context.Context) (*account.Identity, *account.User, error) { +func (t TestLoginService) CreateOrUpdateKeycloakUser(ctx context.Context, accessToken string) (*account.Identity, *account.User, error) { return nil, nil, nil } diff --git a/controller/space.go b/controller/space.go index 7d5c085c5f..ccc98ec1e3 100644 --- a/controller/space.go +++ b/controller/space.go @@ -206,7 +206,7 @@ func (c *SpaceController) Delete(ctx *app.DeleteSpaceContext) error { } // delete all the codebases associated with this space - err = deleteCodebases(c.CodebaseClient, config, ctx.Context, spaceID) + err = deleteCodebases(ctx.Context, c.CodebaseClient, config, spaceID) if err != nil { log.Error(ctx, map[string]interface{}{ "space_id": spaceID, @@ -219,7 +219,7 @@ func (c *SpaceController) Delete(ctx *app.DeleteSpaceContext) error { // now delete the OpenShift resources associated with this space on an // OpenShift cluster, unless otherwise specified if ctx.SkipCluster == nil || !*ctx.SkipCluster { - err = deleteOpenShiftResource(c.DeploymentsClient, config, ctx.Context, spaceID) + err = deleteOpenShiftResource(ctx.Context, c.DeploymentsClient, config, spaceID) if err != nil { log.Error(ctx, map[string]interface{}{ "space_id": spaceID, @@ -261,9 +261,9 @@ func (c *SpaceController) Delete(ctx *app.DeleteSpaceContext) error { // deleteCodebases deletes all the codebases that are associated with this space func deleteCodebases( + ctx context.Context, httpClient *http.Client, config *configuration.Registry, - ctx context.Context, spaceID goauuid.UUID) error { u, err := url.Parse(config.GetCodebaseServiceURL()) @@ -349,9 +349,9 @@ func deleteCodebases( // OpenShift online cluster corresponding to the given spaceID // TODO: fix all the errors, return appropriate errors func deleteOpenShiftResource( + ctx context.Context, httpClient *http.Client, config *configuration.Registry, - ctx context.Context, spaceID goauuid.UUID) error { u, err := url.Parse(config.GetDeploymentsServiceURL()) @@ -636,7 +636,7 @@ func ConvertSpaceToModel(appSpace app.Space) space.Space { // conversion from internal to API type SpaceConvertFunc func(*http.Request, *space.Space, *app.Space) error -// IncludeBacklog returns a SpaceConvertFunc that includes the a link to the backlog +// IncludeBacklogTotalCount returns a SpaceConvertFunc that includes the a link to the backlog // along with the total count of items in the backlog of the current space func IncludeBacklogTotalCount(ctx context.Context, db application.DB) SpaceConvertFunc { return func(req *http.Request, modelSpace *space.Space, appSpace *app.Space) error { diff --git a/controller/space_test.go b/controller/space_test.go index 7d9ad21cee..001bfe3878 100644 --- a/controller/space_test.go +++ b/controller/space_test.go @@ -32,7 +32,7 @@ func TestDeleteOpenShiftResource(t *testing.T) { } // when - err = deleteOpenShiftResource(client, config, ctx, spaceID) + err = deleteOpenShiftResource(ctx, client, config, spaceID) // then require.NoError(t, err) }) @@ -60,7 +60,7 @@ func TestDeleteOpenShiftResource(t *testing.T) { } // when - err = deleteOpenShiftResource(client, config, ctx, spaceID) + err = deleteOpenShiftResource(ctx, client, config, spaceID) // then require.Error(t, err) }) @@ -82,6 +82,6 @@ func TestDeleteCodebases(t *testing.T) { Transport: r.Transport, } - err = deleteCodebases(client, config, ctx, spaceID) + err = deleteCodebases(ctx, client, config, spaceID) require.NoError(t, err) } diff --git a/controller/work_item_link.go b/controller/work_item_link.go index aadc4912f3..df619e49c6 100644 --- a/controller/work_item_link.go +++ b/controller/work_item_link.go @@ -293,7 +293,7 @@ func (c *WorkItemLinkController) checkWorkItemCreatorOrSpaceOwner(ctx context.Co return true, nil, nil } space, err := appl.Spaces().Load(ctx, wi.SpaceID) - return currentIdentityID == space.OwnerID, &wi.SpaceID, nil + return currentIdentityID == space.OwnerID, &wi.SpaceID, err } type deleteWorkItemLinkFuncs interface { diff --git a/controller/work_item_link_blackbox_test.go b/controller/work_item_link_blackbox_test.go index cac98e15fd..a2ab247ec7 100644 --- a/controller/work_item_link_blackbox_test.go +++ b/controller/work_item_link_blackbox_test.go @@ -43,21 +43,6 @@ func (s *workItemLinkSuite) SetupTest() { s.testDir = filepath.Join("test-files", "work_item_link") } -// CreateWorkItemLinkCategory creates a work item link category -func newCreateWorkItemLinkCategoryPayload(name string) *app.CreateWorkItemLinkCategoryPayload { - description := "This work item link category is managed by an admin user." - // Use the goa generated code to create a work item link category - return &app.CreateWorkItemLinkCategoryPayload{ - Data: &app.WorkItemLinkCategoryData{ - Type: link.EndpointWorkItemLinkCategories, - Attributes: &app.WorkItemLinkCategoryAttributes{ - Name: &name, - Description: &description, - }, - }, - } -} - // createWorkItemLinkType creates a workitem link type func createWorkItemLinkType(t *testing.T, db application.DB, name string, categoryID, spaceTemplateID uuid.UUID) *link.WorkItemLinkType { description := "Specify that one bug blocks another one." @@ -87,21 +72,6 @@ func newCreateWorkItemLinkPayload(sourceID, targetID, linkTypeID uuid.UUID) *app } } -// newUpdateWorkItemLinkPayload returns the payload to update a work item link -func newUpdateWorkItemLinkPayload(linkID, sourceID, targetID, linkTypeID uuid.UUID) *app.UpdateWorkItemLinkPayload { - lt := link.WorkItemLink{ - ID: linkID, - SourceID: sourceID, - TargetID: targetID, - LinkTypeID: linkTypeID, - } - payload := ConvertLinkFromModel(&http.Request{Host: "api.service.domain.org"}, lt) - // The create payload is required during creation. Simply copy data over. - return &app.UpdateWorkItemLinkPayload{ - Data: payload.Data, - } -} - func (s *workItemLinkSuite) SecuredController(identity account.Identity) (*goa.Service, *WorkItemLinkController) { svc := testsupport.ServiceAsUser("WorkItemLink-Service", identity) return svc, NewWorkItemLinkController(svc, s.GormDB, s.Configuration) diff --git a/controller/work_item_link_type_blackbox_test.go b/controller/work_item_link_type_blackbox_test.go index 5345d994ca..24c8f6fdf3 100644 --- a/controller/work_item_link_type_blackbox_test.go +++ b/controller/work_item_link_type_blackbox_test.go @@ -1,15 +1,12 @@ package controller_test import ( - "context" - "net/http" "path/filepath" "testing" "time" "github.com/fabric8-services/fabric8-wit/app" "github.com/fabric8-services/fabric8-wit/app/test" - "github.com/fabric8-services/fabric8-wit/application" . "github.com/fabric8-services/fabric8-wit/controller" "github.com/fabric8-services/fabric8-wit/gormtestsupport" "github.com/fabric8-services/fabric8-wit/resource" @@ -47,26 +44,6 @@ func (s *workItemLinkTypeSuite) UnSecuredController() (*goa.Service, *WorkItemLi return svc, NewWorkItemLinkTypeController(svc, s.GormDB, s.Configuration) } -func createWorkItemLinkTypeInRepo(t *testing.T, db application.DB, ctx context.Context, payload *app.CreateWorkItemLinkTypePayload) *app.WorkItemLinkTypeSingle { - appLinkType := app.WorkItemLinkTypeSingle{ - Data: payload.Data, - } - modelLinkType, err := ConvertWorkItemLinkTypeToModel(appLinkType) - require.NoError(t, err) - var appLinkTypeResult app.WorkItemLinkTypeSingle - err = application.Transactional(db, func(appl application.Application) error { - createdModelLinkType, err := appl.WorkItemLinkTypes().Create(ctx, *modelLinkType) - if err != nil { - return err - } - r := &http.Request{Host: "domain.io"} - appLinkTypeResult = ConvertWorkItemLinkTypeFromModel(r, *createdModelLinkType) - return nil - }) - require.NoError(t, err) - return &appLinkTypeResult -} - func (s *workItemLinkTypeSuite) TestShow() { // given fxt := tf.NewTestFixture(s.T(), s.DB, tf.WorkItemLinkTypes(1)) diff --git a/controller/workitem.go b/controller/workitem.go index 644aaad804..e6c71280d2 100644 --- a/controller/workitem.go +++ b/controller/workitem.go @@ -529,6 +529,9 @@ func setupCodebase(appl application.Application, cb *codebase.Content, spaceID u //TODO: Think of making stackID dynamic value (from analyzer) } existingCB, err := appl.Codebases().LoadByRepo(context.Background(), spaceID, cb.Repository) + if err != nil { + return errs.WithStack(err) + } if existingCB != nil { cb.CodebaseID = existingCB.ID.String() return nil diff --git a/design/filters.go b/design/filters.go index f0a2164f43..7a2d2403b2 100644 --- a/design/filters.go +++ b/design/filters.go @@ -40,7 +40,7 @@ var filterList = JSONList( pagingLinks, // pagingLinks would eventually remain nil. meta) // again, this being a pointer gets auto-assigned nil. -var filterSingle = JSONSingle( +var _ = JSONSingle( "filter", "Holds filter information", filter, nil) diff --git a/design/work_item_board.go b/design/work_item_board.go index 55592224d3..d34e374e5e 100644 --- a/design/work_item_board.go +++ b/design/work_item_board.go @@ -27,7 +27,7 @@ var workItemBoardLinks = a.Type("WorkItemBoardLinks", func() { a.Required("self") }) -var workItemBoardColumnData = a.Type("WorkItemBoardColumnData", func() { +var _ = a.Type("WorkItemBoardColumnData", func() { a.Description(`a column represents a vertical lane in a board`) a.Attribute("type", d.String, "The type string of the work item board column", func() { a.Enum("boardcolumns") diff --git a/design/work_item_event.go b/design/work_item_event.go index a1fce957e1..d66d832558 100644 --- a/design/work_item_event.go +++ b/design/work_item_event.go @@ -55,7 +55,7 @@ var eventList = JSONList( nil, ) -var eventSingle = JSONSingle( +var _ = JSONSingle( "Event", "Holds a single Event", event, nil) diff --git a/design/work_item_link.go b/design/work_item_link.go index e44d0dd558..f5669bad77 100644 --- a/design/work_item_link.go +++ b/design/work_item_link.go @@ -19,12 +19,6 @@ var createWorkItemLinkPayload = a.Type("CreateWorkItemLinkPayload", func() { a.Required("data") }) -// updateWorkItemLinkPayload defines the structure of work item link payload in JSONAPI format during update -var updateWorkItemLinkPayload = a.Type("UpdateWorkItemLinkPayload", func() { - a.Attribute("data", workItemLinkData) - a.Required("data") -}) - // workItemLinkListMeta holds meta information for a work item link array response var workItemLinkListMeta = a.Type("WorkItemLinkListMeta", func() { a.Attribute("totalCount", d.Integer, func() { diff --git a/design/work_item_link_category.go b/design/work_item_link_category.go index bb177cdfce..a4b58413b1 100644 --- a/design/work_item_link_category.go +++ b/design/work_item_link_category.go @@ -13,18 +13,6 @@ var workItemLinkCategoryLinks = a.Type("WorkItemLinkCategoryLinks", func() { a.Required("self") }) -// createWorkItemLinkCategoryPayload defines the structure of work item link category payload in JSONAPI format during creation -var createWorkItemLinkCategoryPayload = a.Type("CreateWorkItemLinkCategoryPayload", func() { - a.Attribute("data", workItemLinkCategoryData) - a.Required("data") -}) - -// updateWorkItemLinkCategoryPayload defines the structure of work item link category payload in JSONAPI format during update -var updateWorkItemLinkCategoryPayload = a.Type("UpdateWorkItemLinkCategoryPayload", func() { - a.Attribute("data", workItemLinkCategoryData) - a.Required("data") -}) - // workItemLinkCategoryListMeta holds meta information for a work item link category array response var workItemLinkCategoryListMeta = a.Type("WorkItemLinkCategoryListMeta", func() { a.Attribute("totalCount", d.Integer, func() { diff --git a/design/work_item_link_type.go b/design/work_item_link_type.go index 8e7fdfe4d3..11c4c56b4c 100644 --- a/design/work_item_link_type.go +++ b/design/work_item_link_type.go @@ -5,18 +5,6 @@ import ( a "github.com/goadesign/goa/design/apidsl" ) -// createWorkItemLinkTypePayload defines the structure of work item link type payload in JSONAPI format during creation -var createWorkItemLinkTypePayload = a.Type("CreateWorkItemLinkTypePayload", func() { - a.Attribute("data", workItemLinkTypeData) - a.Required("data") -}) - -// updateWorkItemLinkTypePayload defines the structure of work item link type payload in JSONAPI format during update -var updateWorkItemLinkTypePayload = a.Type("UpdateWorkItemLinkTypePayload", func() { - a.Attribute("data", workItemLinkTypeData) - a.Required("data") -}) - // workItemLinkTypeListMeta holds meta information for a work item link type array response var workItemLinkTypeListMeta = a.Type("WorkItemLinkTypeListMeta", func() { a.Attribute("totalCount", d.Integer, func() { @@ -83,7 +71,7 @@ See also http://jsonapi.org/format/#document-resource-object-relationships`) }) // relationWorkItemType is the JSONAPI store for the work item type relationship objects -var relationWorkItemType = a.Type("RelationWorkItemType", func() { +var _ = a.Type("RelationWorkItemType", func() { a.Attribute("data", relationWorkItemTypeData) a.Attribute("links", genericLinks) }) diff --git a/goasupport/conditional_request/generator.go b/goasupport/conditional_request/generator.go index cfea4b523f..ae97e5a647 100644 --- a/goasupport/conditional_request/generator.go +++ b/goasupport/conditional_request/generator.go @@ -5,11 +5,11 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/goadesign/goa/design" "github.com/goadesign/goa/goagen/codegen" + errs "github.com/pkg/errors" ) // Generate adds method to support conditional queries @@ -22,11 +22,13 @@ func Generate() ([]string, error) { set.String("design", "", "") // Consume design argument so Parse doesn't complain set.StringVar(&ver, "version", "", "") set.StringVar(&outDir, "out", "", "") - set.Parse(os.Args[2:]) + if err := set.Parse(os.Args[2:]); err != nil { + return nil, errs.WithStack(err) + } // First check compatibility if err := codegen.CheckVersion(ver); err != nil { - return nil, err + return nil, errs.WithStack(err) } return WriteNames(design.Design, outDir) @@ -125,8 +127,8 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { var contexts []RequestContext var entities []Entity - api.IterateResources(func(res *design.ResourceDefinition) error { - res.IterateActions(func(act *design.ActionDefinition) error { + err := api.IterateResources(func(res *design.ResourceDefinition) error { + return res.IterateActions(func(act *design.ActionDefinition) error { name := fmt.Sprintf("%v%vContext", codegen.Goify(act.Name, true), codegen.Goify(res.Name, true)) // look-up headers for conditional request support if act.Headers != nil { @@ -185,9 +187,12 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { } return nil }) - return nil }) + if err != nil { + panic(err) + } + ctxFile := filepath.Join(outDir, "conditional_requests.go") ctxWr, err := codegen.SourceFileFor(ctxFile) if err != nil { @@ -211,7 +216,9 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { for alias, pkg := range packageAliases { imports = append(imports, codegen.NewImport(alias, pkg)) } - ctxWr.WriteHeader(title, "app", imports) + if err := ctxWr.WriteHeader(title, "app", imports); err != nil { + return nil, err + } if err := ctxWr.ExecuteTemplate("constants", constants, nil, nil); err != nil { return nil, err } diff --git a/goasupport/forward_requestid_test.go b/goasupport/forward_requestid_test.go index 84370f960e..27a121c0c7 100644 --- a/goasupport/forward_requestid_test.go +++ b/goasupport/forward_requestid_test.go @@ -2,6 +2,7 @@ package goasupport import ( "context" + "io" "net/http" "testing" @@ -12,17 +13,19 @@ import ( "github.com/goadesign/goa/middleware" "github.com/goadesign/goa/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestForwardRequest(t *testing.T) { - reqID := uuid.NewV4().String() - ctx := context.Background() service := goa.New("test") rw := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/goo", nil) req.Header.Set(middleware.RequestIDHeader, reqID) + service.Context = goa.NewContext(nil, rw, req, nil) + service.Encoder.Register(func(w io.Writer) goa.Encoder { return goa.NewJSONEncoder(w) }, "*/*") + service.Decoder.Register(func(r io.Reader) goa.Decoder { return goa.NewJSONDecoder(r) }, "*/*") var newCtx context.Context h := func(ctx context.Context, rw http.ResponseWriter, req *http.Request) error { @@ -30,7 +33,8 @@ func TestForwardRequest(t *testing.T) { return service.Send(ctx, 200, "ok") } rg := middleware.RequestID()(h) - rg(ctx, rw, req) + err := rg(service.Context, rw, req) + require.NoError(t, err) assert.Equal(t, middleware.ContextRequestID(newCtx), reqID) diff --git a/goasupport/helper_function/generator.go b/goasupport/helper_function/generator.go index 019b78dc18..80cad569a6 100644 --- a/goasupport/helper_function/generator.go +++ b/goasupport/helper_function/generator.go @@ -8,6 +8,7 @@ import ( "github.com/goadesign/goa/design" "github.com/goadesign/goa/goagen/codegen" + errs "github.com/pkg/errors" ) // Generate adds method to support conditional queries @@ -20,7 +21,9 @@ func Generate() ([]string, error) { set.String("design", "", "") // Consume design argument so Parse doesn't complain set.StringVar(&ver, "version", "", "") set.StringVar(&outDir, "out", "", "") - set.Parse(os.Args[2:]) + if err := set.Parse(os.Args[2:]); err != nil { + return nil, errs.WithStack(err) + } // First check compatibility if err := codegen.CheckVersion(ver); err != nil { return nil, err @@ -40,7 +43,9 @@ func writeFunctions(api *design.APIDefinition, outDir string) ([]string, error) codegen.NewImport("uuid", "github.com/satori/go.uuid"), codegen.NewImport("", "github.com/fabric8-services/fabric8-wit/ptr"), } - ctxWr.WriteHeader(title, "app", imports) + if err := ctxWr.WriteHeader(title, "app", imports); err != nil { + return nil, errs.WithStack(err) + } if err := ctxWr.ExecuteTemplate("newSpaceRelation", newSpaceRelation, nil, nil); err != nil { return nil, err } diff --git a/goasupport/jsonapi_errors_stringer/generator.go b/goasupport/jsonapi_errors_stringer/generator.go index 3b80f6456f..c5987a3f34 100644 --- a/goasupport/jsonapi_errors_stringer/generator.go +++ b/goasupport/jsonapi_errors_stringer/generator.go @@ -8,6 +8,7 @@ import ( "github.com/goadesign/goa/design" "github.com/goadesign/goa/goagen/codegen" + errs "github.com/pkg/errors" ) // Generate adds method to support conditional queries @@ -20,7 +21,9 @@ func Generate() ([]string, error) { set.String("design", "", "") // Consume design argument so Parse doesn't complain set.StringVar(&ver, "version", "", "") set.StringVar(&outDir, "out", "", "") - set.Parse(os.Args[2:]) + if err := set.Parse(os.Args[2:]); err != nil { + return nil, errs.WithStack(err) + } // First check compatibility if err := codegen.CheckVersion(ver); err != nil { return nil, err @@ -40,7 +43,9 @@ func writeFunctions(api *design.APIDefinition, outDir string) ([]string, error) codegen.SimpleImport("fmt"), codegen.SimpleImport("github.com/davecgh/go-spew/spew"), } - ctxWr.WriteHeader(title, "app", imports) + if err := ctxWr.WriteHeader(title, "app", imports); err != nil { + return nil, errs.WithStack(err) + } if err := ctxWr.ExecuteTemplate("jsonAPIErrorsStringer", jsonAPIErrorsStringer, nil, nil); err != nil { return nil, err } diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index 9bbda6586b..f3d805a337 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -82,7 +82,8 @@ func (s *TestIterationRepository) TestCreate() { EndAt: &end, } // when - repo.Create(context.Background(), &i) + err := repo.Create(context.Background(), &i) + require.NoError(t, err) parentPath := append(i.Path, i.ID) require.NotNil(t, parentPath) i2 := iteration.Iteration{ @@ -92,8 +93,9 @@ func (s *TestIterationRepository) TestCreate() { EndAt: &end, Path: parentPath, } - repo.Create(context.Background(), &i2) + err = repo.Create(context.Background(), &i2) // then + require.NoError(t, err) i2L, err := repo.Load(context.Background(), i2.ID) require.NoError(t, err) assert.NotEmpty(t, i2.Path) @@ -211,7 +213,8 @@ func (s *TestIterationRepository) TestLoad() { EndAt: &end, } // when - repo.Create(context.Background(), &i) + err := repo.Create(context.Background(), &i) + require.NoError(t, err) i2 := iteration.Iteration{ Name: name2, @@ -220,8 +223,9 @@ func (s *TestIterationRepository) TestLoad() { EndAt: &end, } i2.MakeChildOf(i) - repo.Create(context.Background(), &i2) + err = repo.Create(context.Background(), &i2) // then + require.NoError(t, err) res, err := repo.Root(context.Background(), fxt.Spaces[0].ID) require.NoError(t, err) assert.Equal(t, i.Name, res.Name) @@ -247,8 +251,7 @@ func (s *TestIterationRepository) TestLoad() { // then require.NoError(t, err) assert.Len(t, its, 3) - var mustHaveIDs = make(map[uuid.UUID]struct{}, 3) - mustHaveIDs = map[uuid.UUID]struct{}{ + mustHaveIDs := map[uuid.UUID]struct{}{ fxt.Iterations[0].ID: {}, fxt.Iterations[1].ID: {}, fxt.Iterations[2].ID: {}, @@ -263,7 +266,6 @@ func (s *TestIterationRepository) TestLoad() { // then require.NoError(t, err) assert.Len(t, its, 1) - mustHaveIDs = make(map[uuid.UUID]struct{}, 1) mustHaveIDs = map[uuid.UUID]struct{}{ fxt.Iterations[3].ID: {}, } diff --git a/jsonapi/error_handler.go b/jsonapi/error_handler.go index e27a04d93f..f11cafb595 100644 --- a/jsonapi/error_handler.go +++ b/jsonapi/error_handler.go @@ -20,6 +20,12 @@ const ( ErrorMediaIdentifier = "application/vnd.api+json" ) +type contextKey int + +const ( + reqIDKey contextKey = 1 +) + func shortID() string { b := make([]byte, 6) io.ReadFull(rand.Reader, b) @@ -40,9 +46,8 @@ func ErrorHandler(service *goa.Service, verbose bool) goa.Middleware { return nil } cause := errs.Cause(e) - status := http.StatusInternalServerError var respBody interface{} - respBody, status = ErrorToJSONAPIErrors(ctx, e) + respBody, status := ErrorToJSONAPIErrors(ctx, e) rw.Header().Set("Content-Type", ErrorMediaIdentifier) if err, ok := cause.(goa.ServiceError); ok { status = err.ResponseStatus() @@ -54,12 +59,10 @@ func ErrorHandler(service *goa.Service, verbose bool) goa.Middleware { //rw.Header().Set("Content-Type", "text/plain") } if status >= 500 && status < 600 { - //reqID := ctx.Value(reqIDKey) - reqID := ctx.Value(1) // TODO remove this hack + reqID := ctx.Value(reqIDKey) if reqID == nil { reqID = shortID() - //ctx = context.WithValue(ctx, reqIDKey, reqID) - ctx = context.WithValue(ctx, 1, reqID) // TODO remove this hack + ctx = context.WithValue(ctx, reqIDKey, reqID) } log.Error(ctx, map[string]interface{}{ "msg": respBody, diff --git a/kubernetes/deployments_kubeclient.go b/kubernetes/deployments_kubeclient.go index 682b871fe9..679d1ee0e2 100644 --- a/kubernetes/deployments_kubeclient.go +++ b/kubernetes/deployments_kubeclient.go @@ -1504,7 +1504,7 @@ func (kc *kubeClient) getPodStatus(pods []*v1.Pod) ([][]string, int) { podStatus := make(map[string]int) podTotal := 0 for _, pod := range pods { - statusKey := podUnknown + var statusKey string if pod.Status.Phase == v1.PodFailed { // Failed pods are not included, see web console: // https://github.com/openshift/origin-web-console/blob/v3.7.0/app/scripts/directives/podDonut.js#L32 diff --git a/log/log_request.go b/log/log_request.go index 71446ec4da..7d398d9ce0 100644 --- a/log/log_request.go +++ b/log/log_request.go @@ -112,7 +112,9 @@ func LogRequest(verbose bool) goa.Middleware { // Do not use as a reliable way to get unique IDs, instead use for things like logging. func shortID() string { b := make([]byte, 6) - io.ReadFull(rand.Reader, b) + if _, err := io.ReadFull(rand.Reader, b); err != nil && err == io.ErrUnexpectedEOF { + panic(err) + } return base64.StdEncoding.EncodeToString(b) } diff --git a/login/service.go b/login/service.go index e43d8b63ab..a0f7bf20e8 100644 --- a/login/service.go +++ b/login/service.go @@ -41,11 +41,11 @@ type KeycloakOAuthProvider struct { // KeycloakOAuthService represents keycloak OAuth service interface type KeycloakOAuthService interface { - CreateOrUpdateKeycloakUser(accessToken string, ctx context.Context) (*account.Identity, *account.User, error) + CreateOrUpdateKeycloakUser(ctx context.Context, accessToken string) (*account.Identity, *account.User, error) } // CreateOrUpdateKeycloakUser creates a user and a keycloak identity. If the user and identity already exist then update them. -func (keycloak *KeycloakOAuthProvider) CreateOrUpdateKeycloakUser(accessToken string, ctx context.Context) (*account.Identity, *account.User, error) { +func (keycloak *KeycloakOAuthProvider) CreateOrUpdateKeycloakUser(ctx context.Context, accessToken string) (*account.Identity, *account.User, error) { var identity *account.Identity var user *account.User @@ -171,7 +171,9 @@ func generateGravatarURL(email string) (string, error) { return "", errs.WithStack(err) } hash := md5.New() - hash.Write([]byte(email)) + if _, err := hash.Write([]byte(email)); err != nil { + return "", errs.WithStack(err) + } grURL.Path += fmt.Sprintf("%v", hex.EncodeToString(hash.Sum(nil))) + ".jpg" // We can use our own default image if there is no gravatar available for this email diff --git a/metric/recorder_test.go b/metric/recorder_test.go index cb0322a2b0..fea9dffad7 100644 --- a/metric/recorder_test.go +++ b/metric/recorder_test.go @@ -11,11 +11,11 @@ import ( "github.com/goadesign/goa" dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( dummyCtrl = "DummyController" - testCtrl = "TestController" postMethod = "POST" getMethod = "GET" ) @@ -63,7 +63,8 @@ func TestReqDurationMetric(t *testing.T) { // validate reqMetric, _ := reqDuration.GetMetricWithLabelValues(post, dummy, "2xx") m := &dto.Metric{} - reqMetric.Write(m) + err := reqMetric.Write(m) + require.NoError(t, err) checkHistogram(t, m, uint64(len(reqTimes)), expectedBound, expectedCnt) } @@ -85,7 +86,8 @@ func TestResSizeMetric(t *testing.T) { // validate reqMetric, _ := resSize.GetMetricWithLabelValues(get, dummy, "2xx") m := &dto.Metric{} - reqMetric.Write(m) + err := reqMetric.Write(m) + require.NoError(t, err) checkHistogram(t, m, uint64(len(resSizes)), expectedBound, expectedCnt) } @@ -107,7 +109,8 @@ func TestReqSizeMetric(t *testing.T) { // validate reqMetric, _ := reqSize.GetMetricWithLabelValues(post, dummy, "2xx") m := &dto.Metric{} - reqMetric.Write(m) + err := reqMetric.Write(m) + require.NoError(t, err) checkHistogram(t, m, uint64(len(reqSizes)), expectedBound, expectedCnt) } diff --git a/models/transaction.go b/models/transaction.go index c16fe38697..ff85fa8276 100644 --- a/models/transaction.go +++ b/models/transaction.go @@ -1,6 +1,9 @@ package models import ( + "context" + + "github.com/fabric8-services/fabric8-wit/log" "github.com/jinzhu/gorm" errs "github.com/pkg/errors" ) @@ -12,9 +15,15 @@ func Transactional(db *gorm.DB, todo func(tx *gorm.DB) error) error { return tx.Error } if err := todo(tx); err != nil { - tx.Rollback() + tx := tx.Rollback() + if tx.Error != nil { + log.Error(context.Background(), map[string]interface{}{ + "errRollback": errs.WithStack(tx.Error), + "err": errs.WithStack(err), + }, "failed to rollback transaction: %+v", tx.Error) + } return errs.WithStack(err) } - tx.Commit() + tx = tx.Commit() return tx.Error } diff --git a/remoteworkitem/tracker_repository_test.go b/remoteworkitem/tracker_repository_test.go index a607150f35..1ae6af9107 100644 --- a/remoteworkitem/tracker_repository_test.go +++ b/remoteworkitem/tracker_repository_test.go @@ -100,7 +100,7 @@ func (test *TestTrackerRepository) TestTrackerSave() { URL: "random1", Type: remoteworkitem.ProviderJira, } - tracker4, err = test.repo.Save(context.Background(), tracker4) + _, err = test.repo.Save(context.Background(), tracker4) assert.IsType(t, errors.NotFoundError{}, err) tracker5 := &remoteworkitem.Tracker{ diff --git a/rest/url.go b/rest/url.go index be7621003d..81fa8f38af 100644 --- a/rest/url.go +++ b/rest/url.go @@ -43,12 +43,16 @@ func ReplaceDomainPrefix(host string, replaceBy string) (string, error) { // ReadBody reads body from a ReadCloser and returns it as a string func ReadBody(body io.ReadCloser) string { buf := new(bytes.Buffer) - buf.ReadFrom(body) + if _, err := buf.ReadFrom(body); err != nil { + panic(err) + } return buf.String() } // CloseResponse reads the body and close the response. To be used to prevent file descriptor leaks. func CloseResponse(response *http.Response) { - ioutil.ReadAll(response.Body) + if _, err := ioutil.ReadAll(response.Body); err != nil { + panic(err) + } response.Body.Close() } diff --git a/search/search_repository.go b/search/search_repository.go index fe45bdaead..e958a4eb79 100644 --- a/search/search_repository.go +++ b/search/search_repository.go @@ -56,10 +56,6 @@ func NewGormSearchRepository(db *gorm.DB) *GormSearchRepository { return &GormSearchRepository{db, workitem.NewWorkItemTypeRepository(db)} } -func generateSearchQuery(q string) (string, error) { - return q, nil -} - //searchKeyword defines how a decomposed raw search query will look like type searchKeyword struct { workItemTypes []uuid.UUID @@ -770,7 +766,9 @@ func (r *GormSearchRepository) listItemsFromDB(ctx context.Context, criteria cri return nil, 0, errs.WithStack(err) } rows2.Next() // count(*) will always return a row - rows2.Scan(&count) + if err := rows2.Scan(&count); err != nil { + return nil, 0, errs.WithStack(err) + } } return result, count, nil } diff --git a/space/authz/authz_test.go b/space/authz/authz_test.go index 05fcedd1d1..5e6c70f8ff 100644 --- a/space/authz/authz_test.go +++ b/space/authz/authz_test.go @@ -9,7 +9,6 @@ import ( "net/http" "testing" - "github.com/fabric8-services/fabric8-wit/auth" witerrors "github.com/fabric8-services/fabric8-wit/errors" "github.com/fabric8-services/fabric8-wit/resource" "github.com/fabric8-services/fabric8-wit/rest" @@ -34,7 +33,6 @@ type TestAuthzSuite struct { testsuite.UnitTestSuite authzService *AuthzRoleService doer *testsupport.DummyHttpDoer - authzConfig *auth.ServiceConfiguration } func (s *TestAuthzSuite) SetupSuite() { diff --git a/spacetemplate/importer/repository_blackbox_test.go b/spacetemplate/importer/repository_blackbox_test.go index 5a121ffe1d..db04b9c386 100644 --- a/spacetemplate/importer/repository_blackbox_test.go +++ b/spacetemplate/importer/repository_blackbox_test.go @@ -15,7 +15,6 @@ import ( "github.com/fabric8-services/fabric8-wit/workitem" "github.com/fabric8-services/fabric8-wit/workitem/link" uuid "github.com/satori/go.uuid" - "github.com/sergi/go-diff/diffmatchpatch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -47,12 +46,6 @@ func (s *repoSuite) SetupSuite() { s.wibRepo = workitem.NewBoardRepository(s.DB) } -func diff(expectedStr, actualStr string) string { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(expectedStr, actualStr, false) - return dmp.DiffPrettyText(diffs) -} - func (s *repoSuite) TestImport() { // given spaceTemplateID := uuid.NewV4() diff --git a/spacetemplate/repository_blackbox_test.go b/spacetemplate/repository_blackbox_test.go index 981dc46189..07e97a1065 100644 --- a/spacetemplate/repository_blackbox_test.go +++ b/spacetemplate/repository_blackbox_test.go @@ -11,7 +11,6 @@ import ( "github.com/fabric8-services/fabric8-wit/workitem" "github.com/fabric8-services/fabric8-wit/workitem/link" uuid "github.com/satori/go.uuid" - "github.com/sergi/go-diff/diffmatchpatch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -40,12 +39,6 @@ func (s *repoSuite) SetupSuite() { s.witgRepo = workitem.NewWorkItemTypeGroupRepository(s.DB) } -func diff(expectedStr, actualStr string) string { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(expectedStr, actualStr, false) - return dmp.DiffPrettyText(diffs) -} - func (s *repoSuite) TestExists() { resource.Require(s.T(), resource.Database) diff --git a/workitem/field_definition_blackbox_test.go b/workitem/field_definition_blackbox_test.go index b582c0e126..0eb266db22 100644 --- a/workitem/field_definition_blackbox_test.go +++ b/workitem/field_definition_blackbox_test.go @@ -20,7 +20,8 @@ func testFieldDefinitionMarshalUnmarshal(t *testing.T, def workitem.FieldDefinit t.Logf("bytes are %s", string(bytes)) unmarshalled := workitem.FieldDefinition{} - json.Unmarshal(bytes, &unmarshalled) + err = json.Unmarshal(bytes, &unmarshalled) + require.NoError(t, err) if !reflect.DeepEqual(def, unmarshalled) { t.Errorf("field should be %v, but is %v", def, unmarshalled) diff --git a/workitem/link/link_repository.go b/workitem/link/link_repository.go index 24feb62ac1..bd9140c9b0 100644 --- a/workitem/link/link_repository.go +++ b/workitem/link/link_repository.go @@ -195,7 +195,9 @@ func (r *GormWorkItemLinkRepository) acquireLock(spaceID uuid.UUID) error { // effectively limits us to just 32 bit hashes. Once we allow cross-space // linking we need to revisit the locking. h := fnv.New32() - h.Write([]byte("links-" + spaceID.String())) + if _, err := h.Write([]byte("links-" + spaceID.String())); err != nil { + return errs.WithStack(err) + } key := h.Sum32() db := r.db.Exec("SELECT pg_advisory_xact_lock($1)", key) @@ -349,8 +351,7 @@ func (r *GormWorkItemLinkRepository) Delete(ctx context.Context, linkID uuid.UUI if err := r.acquireLock(src.SpaceID); err != nil { return errs.Wrap(err, "failed to acquire lock during link deletion") } - r.deleteLink(ctx, lnk, suppressorID) - return nil + return r.deleteLink(ctx, lnk, suppressorID) } // DeleteRelatedLinks deletes all links in which the source or target equals the @@ -376,7 +377,9 @@ func (r *GormWorkItemLinkRepository) DeleteRelatedLinks(ctx context.Context, wiI } locked = true } - r.deleteLink(ctx, workitemLink, suppressorID) + if err := r.deleteLink(ctx, workitemLink, suppressorID); err != nil { + return errs.WithStack(err) + } } return nil } diff --git a/workitem/workitem_repository_blackbox_test.go b/workitem/workitem_repository_blackbox_test.go index b5365bd3ce..aca7235560 100644 --- a/workitem/workitem_repository_blackbox_test.go +++ b/workitem/workitem_repository_blackbox_test.go @@ -709,7 +709,8 @@ func (s *workItemRepoBlackBoxTest) TestList() { t.Run("by updated descending", func(t *testing.T) { // when for _, v := range []int{3, 2, 0, 1, 6, 5, 4} { - s.repo.Save(context.Background(), fxt.WorkItems[v].SpaceID, *fxt.WorkItems[v], fxt.Identities[0].ID) + _, _, err := s.repo.Save(context.Background(), fxt.WorkItems[v].SpaceID, *fxt.WorkItems[v], fxt.Identities[0].ID) + require.NoError(t, err) } exp, _ := query.Parse(ptr.String(`{"system.state": "open"}`)) sort, _ := workitem.ParseSortWorkItemsBy(ptr.String("-updated")) diff --git a/workitem/workitemtype.go b/workitem/workitemtype.go index 2541be987c..8213f841ec 100644 --- a/workitem/workitemtype.go +++ b/workitem/workitemtype.go @@ -151,22 +151,6 @@ func (wit WorkItemType) TableName() string { var _ convert.Equaler = WorkItemType{} var _ convert.Equaler = (*WorkItemType)(nil) -// returns true if the left hand and right hand side string -// pointers either both point to nil or reference the same -// content; otherwise false is returned. -func strPtrIsNilOrContentIsEqual(l, r *string) bool { - if l == nil && r != nil { - return false - } - if l != nil && r == nil { - return false - } - if l == nil && r == nil { - return true - } - return *l == *r -} - // Equal returns true if two WorkItemType objects are equal; otherwise false is returned. func (wit WorkItemType) Equal(u convert.Equaler) bool { other, ok := u.(WorkItemType) diff --git a/workitem/workitemtype_blackbox_test.go b/workitem/workitemtype_blackbox_test.go index d708bd2b15..7bd363cc92 100644 --- a/workitem/workitemtype_blackbox_test.go +++ b/workitem/workitemtype_blackbox_test.go @@ -46,7 +46,8 @@ func TestJsonMarshalListType(t *testing.T) { } var parsedWIT workitem.WorkItemType - json.Unmarshal(bytes, &parsedWIT) + err = json.Unmarshal(bytes, &parsedWIT) + require.NoError(t, err) if !expectedWIT.Equal(parsedWIT) { t.Errorf("Unmarshalled work item type: \n %v \n has not the same type as \"normal\" workitem type: \n %v \n", parsedWIT, expectedWIT) @@ -79,7 +80,8 @@ func TestMarshalEnumType(t *testing.T) { } var parsedWIT workitem.WorkItemType - json.Unmarshal(bytes, &parsedWIT) + err = json.Unmarshal(bytes, &parsedWIT) + require.NoError(t, err) if !expectedWIT.Equal(parsedWIT) { t.Errorf("Unmarshalled work item type: \n %v \n has not the same type as \"normal\" workitem type: \n %v \n", parsedWIT, expectedWIT) diff --git a/workitem/workitemtype_repository_bench_test.go b/workitem/workitemtype_repository_bench_test.go index 3a7a279c4b..1f7d40da48 100644 --- a/workitem/workitemtype_repository_bench_test.go +++ b/workitem/workitemtype_repository_bench_test.go @@ -9,6 +9,7 @@ import ( testsupport "github.com/fabric8-services/fabric8-wit/test" tf "github.com/fabric8-services/fabric8-wit/test/testfixture" "github.com/fabric8-services/fabric8-wit/workitem" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -134,7 +135,8 @@ func (r *BenchWorkItemTypeRepository) BenchmarkListRawScan() { defer result.Close() for result.Next() { wit := workitem.WorkItemType{} - result.Scan(&wit) + err := result.Scan(&wit) + require.NoError(r.B(), err) rows = append(rows, wit) } }