From 9835e47e2d044a9aec8bef797a2de47da6c22640 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:37:57 +0200 Subject: [PATCH 01/77] golint: context.Context should be the first parameter of a function --- controller/login_test.go | 2 +- controller/space.go | 6 +++--- controller/space_test.go | 4 ++-- login/service.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) 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..42cc4c741b 100644 --- a/controller/space.go +++ b/controller/space.go @@ -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()) diff --git a/controller/space_test.go b/controller/space_test.go index 7d9ad21cee..ab00df67d6 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) }) diff --git a/login/service.go b/login/service.go index e43d8b63ab..555cae00ca 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 From 89c261bb00de018e08611d5bd7c69832fb595ddc Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:44:25 +0200 Subject: [PATCH 02/77] structcheck: dbPq is unused --- application/transaction_bench_test.go | 2 -- 1 file changed, 2 deletions(-) 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) { From caf76163acca89f7b88bd319bcfe5d3a3c652123 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:45:08 +0200 Subject: [PATCH 03/77] structcheck: authzConfig is unused --- space/authz/authz_test.go | 2 -- 1 file changed, 2 deletions(-) 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() { From 2ebb838ba9363665b1569139c3dd1dccca06ae18 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:49:27 +0200 Subject: [PATCH 04/77] varcheck: updateWorkItemLinkPayload is unused --- design/work_item_link.go | 6 ------ 1 file changed, 6 deletions(-) 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() { From 4a778524c0134495b37d1b51debb131df504daa9 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:50:11 +0200 Subject: [PATCH 05/77] varcheck: createWorkItemLinkTypePayload is unused --- design/work_item_link_type.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/design/work_item_link_type.go b/design/work_item_link_type.go index 8e7fdfe4d3..224e981cc0 100644 --- a/design/work_item_link_type.go +++ b/design/work_item_link_type.go @@ -5,12 +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) From 72f03911b937e69f4f9b2296178393f729c5a179 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:51:36 +0200 Subject: [PATCH 06/77] varcheck: eventSingle is unused --- design/work_item_event.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From a9a2ff94b9686902ee1a91f48c65344fe4ea0549 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:52:16 +0200 Subject: [PATCH 07/77] varcheck: testCtrl is unused --- metric/recorder_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/metric/recorder_test.go b/metric/recorder_test.go index cb0322a2b0..2b03d42146 100644 --- a/metric/recorder_test.go +++ b/metric/recorder_test.go @@ -15,7 +15,6 @@ import ( var ( dummyCtrl = "DummyController" - testCtrl = "TestController" postMethod = "POST" getMethod = "GET" ) From 85ff2295604ee577b67984f00fe398abe263dbaf Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:52:57 +0200 Subject: [PATCH 08/77] varcheck: workItemBoardColumnData is unused --- design/work_item_board.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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") From ff90c8375726259981c786bed752a14bf855e8aa Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:54:53 +0200 Subject: [PATCH 09/77] varcheck: relationWorkItemType is unused --- design/work_item_link_type.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design/work_item_link_type.go b/design/work_item_link_type.go index 224e981cc0..a0f3f26deb 100644 --- a/design/work_item_link_type.go +++ b/design/work_item_link_type.go @@ -77,7 +77,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) }) From 4528e0fce2a0f9c3683d3d5b19f95faaf7019d50 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:55:24 +0200 Subject: [PATCH 10/77] varcheck: filterSingle is unused --- design/filters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From b12f31433b41e36c490fb82801f801f3a2d2d9f8 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 10:56:07 +0200 Subject: [PATCH 11/77] varcheck: updateWorkItemLinkCategoryPayload is unused --- design/work_item_link_category.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/design/work_item_link_category.go b/design/work_item_link_category.go index bb177cdfce..bc97b017b6 100644 --- a/design/work_item_link_category.go +++ b/design/work_item_link_category.go @@ -19,8 +19,8 @@ var createWorkItemLinkCategoryPayload = a.Type("CreateWorkItemLinkCategoryPayloa a.Required("data") }) -// updateWorkItemLinkCategoryPayload defines the structure of work item link category payload in JSONAPI format during update -var updateWorkItemLinkCategoryPayload = a.Type("UpdateWorkItemLinkCategoryPayload", func() { +// UpdateWorkItemLinkCategoryPayload defines the structure of work item link category payload in JSONAPI format during update +var _ = a.Type("UpdateWorkItemLinkCategoryPayload", func() { a.Attribute("data", workItemLinkCategoryData) a.Required("data") }) From ab03635efa97ff31de300c4df3566f588c445bd8 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:00:03 +0200 Subject: [PATCH 12/77] remove unused newUpdateWorkItemLinkPayload and newUpdateWorkItemLinkPayload --- controller/work_item_link_blackbox_test.go | 30 ---------------------- 1 file changed, 30 deletions(-) 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) From 82875418bc77305529f4b703bde9937fd2173bae Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:01:04 +0200 Subject: [PATCH 13/77] varcheck: updateWorkItemLinkTypePayload is unused --- design/work_item_link_type.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/design/work_item_link_type.go b/design/work_item_link_type.go index a0f3f26deb..11c4c56b4c 100644 --- a/design/work_item_link_type.go +++ b/design/work_item_link_type.go @@ -5,12 +5,6 @@ import ( a "github.com/goadesign/goa/design/apidsl" ) -// 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() { From 95ff06af4666fbd0c0c9f2788cbd4a53c5c33034 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:04:09 +0200 Subject: [PATCH 14/77] deadcode: generateSearchQuery is unused --- search/search_repository.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/search/search_repository.go b/search/search_repository.go index fe45bdaead..8ed6b415d9 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 From fa9804140a34726fcae888e93d7cbef57f098ee5 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:04:41 +0200 Subject: [PATCH 15/77] deadcode: strPtrIsNilOrContentIsEqual is unused --- workitem/workitemtype.go | 16 ---------------- 1 file changed, 16 deletions(-) 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) From c31cbe811b13bc958f152d9e26e922456fa174ae Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:05:44 +0200 Subject: [PATCH 16/77] deadcode: generateEnvKey is unused --- configuration/configuration_blackbox_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/configuration/configuration_blackbox_test.go b/configuration/configuration_blackbox_test.go index 0fe464af88..cfa204f9ca 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" @@ -183,10 +182,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) From 7e6ff5b1d414385ea83de2a01af12a80c251d79d Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:06:14 +0200 Subject: [PATCH 17/77] deadcode: checkGetKeycloakEndpointSetByEnvVaribaleOK is unused --- configuration/configuration_blackbox_test.go | 21 -------------------- 1 file changed, 21 deletions(-) diff --git a/configuration/configuration_blackbox_test.go b/configuration/configuration_blackbox_test.go index cfa204f9ca..ef3ef36250 100644 --- a/configuration/configuration_blackbox_test.go +++ b/configuration/configuration_blackbox_test.go @@ -12,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" ) @@ -182,26 +181,6 @@ func TestGetMaxHeaderSizeSetByEnvVaribaleOK(t *testing.T) { assert.Equal(t, envValue, viperValue) } -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() From 5d1d207cf68798cc496ceddc4679ce9071f9465d Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:07:05 +0200 Subject: [PATCH 18/77] deadcode: diff is unused --- spacetemplate/repository_blackbox_test.go | 7 ------- 1 file changed, 7 deletions(-) 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) From 8847ed3dc4e325e52b1e578f3eccd9f0a5358254 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:07:26 +0200 Subject: [PATCH 19/77] deadcode: diff is unused --- spacetemplate/importer/repository_blackbox_test.go | 7 ------- 1 file changed, 7 deletions(-) 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() From 27a1303f939d684f4bc01b4b922838c22c94bfd7 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:09:03 +0200 Subject: [PATCH 20/77] remove unused CreateWorkItemLinkCategoryPayload and UpdateWorkItemLinkCategoryPayload --- design/work_item_link_category.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/design/work_item_link_category.go b/design/work_item_link_category.go index bc97b017b6..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 _ = 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() { From 6780e2d19efdb20552fc2f387c19b74bd745a25c Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:10:01 +0200 Subject: [PATCH 21/77] fixup --- controller/space.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/space.go b/controller/space.go index 42cc4c741b..a2f6ee4181 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, From ab99cd773035977c0ce832cd77ceaea329f948d6 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:14:36 +0200 Subject: [PATCH 22/77] fix comment to IncludeBacklogTotalCount --- controller/space.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/space.go b/controller/space.go index a2f6ee4181..ccc98ec1e3 100644 --- a/controller/space.go +++ b/controller/space.go @@ -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 { From 1cde51364f1f9cd1935175059d4922819ff341b7 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:16:04 +0200 Subject: [PATCH 23/77] errcheck: Error return value of result.Scan is not checked --- application/application_db_calls_bench_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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) } } From b843890121b78b21a90f582d484e601e0b731aba Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:19:48 +0200 Subject: [PATCH 24/77] errcheck: Error return value of tx.Rollback/tx.Commit is not checked --- application/transaction.go | 19 ++++++++++++++++--- models/transaction.go | 11 ++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) 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/models/transaction.go b/models/transaction.go index c16fe38697..b7097e78ab 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,7 +15,13 @@ 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", errRollback) + } return errs.WithStack(err) } tx.Commit() From 2002f59778b38bee9198aeceea65928be31898f6 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:42:02 +0200 Subject: [PATCH 25/77] errcheck: Error return value of r.Stop is not checked --- .../analytics-gemini/client_blackbox_test.go | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/codebase/analytics-gemini/client_blackbox_test.go b/codebase/analytics-gemini/client_blackbox_test.go index 2d88cc95e3..549697671d 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) @@ -242,7 +257,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 +281,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) From 10e08e3d5d0f2f8042d8999ab65beeda417347bc Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:45:18 +0200 Subject: [PATCH 26/77] errcheck: Error return value of recordCodebases.Stop is not checked --- codebase/analytics-gemini/client_blackbox_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/codebase/analytics-gemini/client_blackbox_test.go b/codebase/analytics-gemini/client_blackbox_test.go index 549697671d..f4d8cae0fe 100644 --- a/codebase/analytics-gemini/client_blackbox_test.go +++ b/codebase/analytics-gemini/client_blackbox_test.go @@ -202,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( @@ -210,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) From ee099354be9cb2b9d5f8bdd28c653d1a3dadecb6 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:47:00 +0200 Subject: [PATCH 27/77] errcheck: Error return value of db.ScanRows is not checked --- codebase/codebase.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/codebase/codebase.go b/codebase/codebase.go index a9fb9cd9e5..1a193a1092 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 { From 13f1616c27b9ed3aae5fdb9d53843d57ca53b497 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:48:04 +0200 Subject: [PATCH 28/77] errcheck: Error return value of rows2.Scan is not checked --- codebase/codebase.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/codebase/codebase.go b/codebase/codebase.go index 1a193a1092..c82372f562 100644 --- a/codebase/codebase.go +++ b/codebase/codebase.go @@ -303,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 } From 3fe312b92f3b76fc845eb87964a6d87c72debcba Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:50:07 +0200 Subject: [PATCH 29/77] errcheck: Error return value of db.ScanRows is not checked --- codebase/codebase.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/codebase/codebase.go b/codebase/codebase.go index c82372f562..9168bffd76 100644 --- a/codebase/codebase.go +++ b/codebase/codebase.go @@ -391,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 { From b5c92c0366dea3fd4932123716b72c878033c171 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:51:06 +0200 Subject: [PATCH 30/77] errcheck: Error return value of db.ScanRows is not checked --- comment/comment_repository.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository.go b/comment/comment_repository.go index 9cb34cedd1..fe4a5ffc27 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 { From 962371469d14503a27ce16d8f61526e3d1fecced Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:51:47 +0200 Subject: [PATCH 31/77] errcheck: Error return value of rows2.Scan is not checked --- comment/comment_repository.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/comment/comment_repository.go b/comment/comment_repository.go index fe4a5ffc27..ebb07c1140 100644 --- a/comment/comment_repository.go +++ b/comment/comment_repository.go @@ -194,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 } From dd194e45d36423a70f5ff369780d4ca06acf0ad7 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:52:50 +0200 Subject: [PATCH 32/77] errcheck: Error return value of s.repo.Create is not checked --- comment/comment_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index e1637b5567..1f8f868f03 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{ From e1835c68589e512cb0d9253fa9283db9eee0cd79 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:53:42 +0200 Subject: [PATCH 33/77] errcheck: Error return value of s.repo.Create is not checked --- comment/comment_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index 1f8f868f03..197e3fb7d2 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -65,7 +65,8 @@ 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) + require.NoError(s.T(), err) // then require.NotNil(s.T(), childComment.ID, "Comment was not created, ID nil") require.NotNil(s.T(), childComment.CreatedAt, "Comment was not created") From 7d942fcc1ea1724feb1b98b8db25e38d81f0c4e3 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:54:12 +0200 Subject: [PATCH 34/77] errcheck: Error return value of s.repo.Create is not checked --- comment/comment_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index 197e3fb7d2..0ac69b5e6d 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -88,7 +88,8 @@ 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) + require.NoError(s.T(), err) // then assert.NotNil(s.T(), comment.ID, "Comment was not created, ID nil") require.NotNil(s.T(), comment.CreatedAt, "Comment was not created?") From 292713d92b0bd0ce67f3b994613acb6df8c31a82 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:55:02 +0200 Subject: [PATCH 35/77] errcheck: Error return value of s.repo.Create is not checked --- comment/comment_repository_blackbox_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index 0ac69b5e6d..b7e6bf980c 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -66,8 +66,8 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() { } // when err = s.repo.Create(s.Ctx, childComment, fxt.Identities[0].ID) - require.NoError(s.T(), err) // 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()") @@ -89,8 +89,8 @@ func (s *TestCommentRepository) TestCreateCommentWithMarkup() { comment := newComment(uuid.NewV4(), "Test A", rendering.SystemMarkupMarkdown) // when err := s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID) - require.NoError(s.T(), err) // 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()?") @@ -101,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()?") From e2485625c76a70b6d6149b9474df1ead13f9db62 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:55:51 +0200 Subject: [PATCH 36/77] errcheck: Error return value of s.repo.Save is not checked --- comment/comment_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index b7e6bf980c..3c010bc1bf 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -120,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(sT(), err) offset := 0 limit := 1 comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit) From f89798ed18c2f185d9f33c7c91af5ac50c4fb810 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:56:56 +0200 Subject: [PATCH 37/77] errcheck: Error return value of s.repo.Save is not checked --- comment/comment_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index 3c010bc1bf..74fe7bde57 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -142,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) From 141ce7a21685c3435f81160ab6b350b8c7d7f212 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 11:59:04 +0200 Subject: [PATCH 38/77] errcheck: Error return value of set.Parse is not checked --- goasupport/conditional_request/generator.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/goasupport/conditional_request/generator.go b/goasupport/conditional_request/generator.go index cfea4b523f..178ae918c5 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) From 1d3a8a91cdd54bdeb8c4997b53b282833dfd79e6 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:00:24 +0200 Subject: [PATCH 39/77] errcheck: Error return value of api.IterateResources is not checked --- goasupport/conditional_request/generator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/goasupport/conditional_request/generator.go b/goasupport/conditional_request/generator.go index 178ae918c5..2d7eced4de 100644 --- a/goasupport/conditional_request/generator.go +++ b/goasupport/conditional_request/generator.go @@ -127,7 +127,7 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { var contexts []RequestContext var entities []Entity - api.IterateResources(func(res *design.ResourceDefinition) error { + err := api.IterateResources(func(res *design.ResourceDefinition) error { 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 @@ -190,6 +190,10 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { return nil }) + if err != nil { + panic(err) + } + ctxFile := filepath.Join(outDir, "conditional_requests.go") ctxWr, err := codegen.SourceFileFor(ctxFile) if err != nil { From 4bd2cdad11d784e0de2dccb12ce2d3b47e422e4b Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:09:45 +0200 Subject: [PATCH 40/77] errcheck: Error return value of res.IterateActions is not checked --- goasupport/conditional_request/generator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/goasupport/conditional_request/generator.go b/goasupport/conditional_request/generator.go index 2d7eced4de..7734eac5ee 100644 --- a/goasupport/conditional_request/generator.go +++ b/goasupport/conditional_request/generator.go @@ -128,7 +128,7 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { var entities []Entity err := api.IterateResources(func(res *design.ResourceDefinition) error { - res.IterateActions(func(act *design.ActionDefinition) 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 { @@ -187,7 +187,6 @@ func WriteNames(api *design.APIDefinition, outDir string) ([]string, error) { } return nil }) - return nil }) if err != nil { From f5f10078ad9a1c560c63d3509c473a734dba46fd Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:11:03 +0200 Subject: [PATCH 41/77] errcheck: Error return value of ctxWr.WriteHeader is not checked --- goasupport/conditional_request/generator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/goasupport/conditional_request/generator.go b/goasupport/conditional_request/generator.go index 7734eac5ee..ae97e5a647 100644 --- a/goasupport/conditional_request/generator.go +++ b/goasupport/conditional_request/generator.go @@ -216,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 } From 2c181d8eff6198d76d17f69f0d41d39f91754974 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:12:05 +0200 Subject: [PATCH 42/77] errcheck: Error return value of rg is not checked --- goasupport/forward_requestid_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/goasupport/forward_requestid_test.go b/goasupport/forward_requestid_test.go index 84370f960e..877a11298f 100644 --- a/goasupport/forward_requestid_test.go +++ b/goasupport/forward_requestid_test.go @@ -12,6 +12,7 @@ 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) { @@ -30,7 +31,8 @@ func TestForwardRequest(t *testing.T) { return service.Send(ctx, 200, "ok") } rg := middleware.RequestID()(h) - rg(ctx, rw, req) + err := rg(ctx, rw, req) + require.NoError(t, err) assert.Equal(t, middleware.ContextRequestID(newCtx), reqID) From 5cb9c5c299b5707ea3de1712a705b122b208b101 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:13:09 +0200 Subject: [PATCH 43/77] errcheck: Error return value of set.Parse is not checked --- goasupport/helper_function/generator.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/goasupport/helper_function/generator.go b/goasupport/helper_function/generator.go index 019b78dc18..ab7db1ffb6 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 From da2892bc05f766206c2444a8a950bafec2706d60 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:13:58 +0200 Subject: [PATCH 44/77] errcheck: Error return value of ctxWr.WriteHeader is not checked --- goasupport/helper_function/generator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/goasupport/helper_function/generator.go b/goasupport/helper_function/generator.go index ab7db1ffb6..80cad569a6 100644 --- a/goasupport/helper_function/generator.go +++ b/goasupport/helper_function/generator.go @@ -43,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 } From c30e173b5059cadb64f2d856ac92069dc8839174 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:15:03 +0200 Subject: [PATCH 45/77] errcheck: Error return value of set.Parse is not checked --- goasupport/jsonapi_errors_stringer/generator.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/goasupport/jsonapi_errors_stringer/generator.go b/goasupport/jsonapi_errors_stringer/generator.go index 3b80f6456f..538db5df14 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 From 9703b370c457c962f044bdae3060f54e53bc2dd2 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:16:32 +0200 Subject: [PATCH 46/77] errcheck: Error return value of ctxWr.WriteHeader is not checked --- goasupport/jsonapi_errors_stringer/generator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/goasupport/jsonapi_errors_stringer/generator.go b/goasupport/jsonapi_errors_stringer/generator.go index 538db5df14..c5987a3f34 100644 --- a/goasupport/jsonapi_errors_stringer/generator.go +++ b/goasupport/jsonapi_errors_stringer/generator.go @@ -43,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 } From 3c7fd05c2bb9e05ea8a760eb274703a392b6e2f7 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:17:37 +0200 Subject: [PATCH 47/77] errcheck: Error return value of repo.Create is not checked --- iteration/iteration_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index 9bbda6586b..ba18bc8791 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{ From 1ea28b77fd3f059a749c14450f86217bd9827991 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:18:20 +0200 Subject: [PATCH 48/77] errcheck: Error return value of repo.Create is not checked --- iteration/iteration_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index ba18bc8791..80f2cde882 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -93,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) @@ -212,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, @@ -221,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) From 2dd1e2df099897221c0e1b51d13a0e04863a37d8 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:20:59 +0200 Subject: [PATCH 49/77] errcheck: Error return value of io.ReadFull is not checked --- log/log_request.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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) } From 7ea4f4a6daa2f594c7e7180ca4a8a11161ab7404 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:22:14 +0200 Subject: [PATCH 50/77] errcheck: Error return value of hash.Write is not checked --- login/service.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/login/service.go b/login/service.go index 555cae00ca..a0f7bf20e8 100644 --- a/login/service.go +++ b/login/service.go @@ -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 From 6e592eb41f82e4f97a5c2c3be468feda8559d294 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:22:57 +0200 Subject: [PATCH 51/77] errcheck: Error return value of reqMetric.Write is not checked --- metric/recorder_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/metric/recorder_test.go b/metric/recorder_test.go index 2b03d42146..fea9dffad7 100644 --- a/metric/recorder_test.go +++ b/metric/recorder_test.go @@ -11,6 +11,7 @@ import ( "github.com/goadesign/goa" dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -62,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) } @@ -84,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) } @@ -106,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) } From e1f842f54f53c45cd97eedb71f4312dba8eb4271 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:25:22 +0200 Subject: [PATCH 52/77] errcheck: Error return value of buf.ReadFrom is not checked --- rest/url.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rest/url.go b/rest/url.go index be7621003d..12c2c20942 100644 --- a/rest/url.go +++ b/rest/url.go @@ -43,7 +43,9 @@ 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() } From b1aa15619622e73154375c58ec1c46f69437d720 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:25:56 +0200 Subject: [PATCH 53/77] errcheck: Error return value of ioutil.ReadAll is not checked --- rest/url.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rest/url.go b/rest/url.go index 12c2c20942..81fa8f38af 100644 --- a/rest/url.go +++ b/rest/url.go @@ -51,6 +51,8 @@ func ReadBody(body io.ReadCloser) 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() } From 6314978984be3e9fd845a5c5400c9d97b9263469 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:27:07 +0200 Subject: [PATCH 54/77] errcheck: Error return value of rows2.Scan is not checked --- search/search_repository.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/search/search_repository.go b/search/search_repository.go index 8ed6b415d9..e958a4eb79 100644 --- a/search/search_repository.go +++ b/search/search_repository.go @@ -766,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 } From efd06ec75ada74710fde0d1d21622e36ab7de326 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:28:04 +0200 Subject: [PATCH 55/77] errcheck: Error return value of json.Unmarshal is not checked --- workitem/field_definition_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workitem/field_definition_blackbox_test.go b/workitem/field_definition_blackbox_test.go index b582c0e126..07c9f0cdc9 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) From f7bfe06e3d5231a3baf110900e5ee951adeb75e4 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:29:18 +0200 Subject: [PATCH 56/77] errcheck: Error return value of h.Write is not checked --- workitem/link/link_repository.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/workitem/link/link_repository.go b/workitem/link/link_repository.go index 24feb62ac1..1e99f48dfb 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) From 4e457be3c279be731407c811a43855920703df6d Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:29:56 +0200 Subject: [PATCH 57/77] errcheck: Error return value of r.deleteLink is not checked --- workitem/link/link_repository.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/workitem/link/link_repository.go b/workitem/link/link_repository.go index 1e99f48dfb..e17e8ba656 100644 --- a/workitem/link/link_repository.go +++ b/workitem/link/link_repository.go @@ -351,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 From 9dc9435e16b66a98111a93f5f08980445f548f68 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:30:54 +0200 Subject: [PATCH 58/77] errcheck: Error return value of r.deleteLink is not checked --- workitem/link/link_repository.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/workitem/link/link_repository.go b/workitem/link/link_repository.go index e17e8ba656..bd9140c9b0 100644 --- a/workitem/link/link_repository.go +++ b/workitem/link/link_repository.go @@ -377,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 } From dde21f7d132b11a787d69d5be1646d02da3b1b21 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:32:15 +0200 Subject: [PATCH 59/77] errcheck: Error return value of s.repo.Save is not checked --- workitem/workitem_repository_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workitem/workitem_repository_blackbox_test.go b/workitem/workitem_repository_blackbox_test.go index b5365bd3ce..3cee9ab858 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")) From 68b4917ead4942df1041055428a0fd93a00c8ef3 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:32:53 +0200 Subject: [PATCH 60/77] errcheck: Error return value of json.Unmarshal is not checked --- workitem/workitemtype_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workitem/workitemtype_blackbox_test.go b/workitem/workitemtype_blackbox_test.go index d708bd2b15..71739f8c93 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) From 241aff9a873a92d549e98fba501c9e2015e58c5d Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:33:23 +0200 Subject: [PATCH 61/77] errcheck: Error return value of json.Unmarshal is not checked --- workitem/workitemtype_blackbox_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workitem/workitemtype_blackbox_test.go b/workitem/workitemtype_blackbox_test.go index 71739f8c93..6ec04164c1 100644 --- a/workitem/workitemtype_blackbox_test.go +++ b/workitem/workitemtype_blackbox_test.go @@ -80,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) From 0b57704f0fca0dc8e306449af781d9bdbdbfaf11 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:34:19 +0200 Subject: [PATCH 62/77] errcheck: Error return value of result.Scan is not checked --- workitem/workitemtype_repository_bench_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workitem/workitemtype_repository_bench_test.go b/workitem/workitemtype_repository_bench_test.go index 3a7a279c4b..7a133c42b3 100644 --- a/workitem/workitemtype_repository_bench_test.go +++ b/workitem/workitemtype_repository_bench_test.go @@ -134,7 +134,8 @@ func (r *BenchWorkItemTypeRepository) BenchmarkListRawScan() { defer result.Close() for result.Next() { wit := workitem.WorkItemType{} - result.Scan(&wit) + err := result.Scan(&wit) + require.NoError(s.B(), err) rows = append(rows, wit) } } From 2239ca7cec742338e7d043ff86bba53279b31f5e Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:36:02 +0200 Subject: [PATCH 63/77] fixup controller/space_test.go:85:23: cannot use client (type *http.Client) as type "context".Context in argument to deleteCodebases: *http.Client does not implement "context".Context (missing Deadline method) have http.deadline() time.Time want Deadline() (time.Time, bool) controller/space_test.go:85:23: cannot use config (type *configuration.Registry) as type *http.Client in argument to deleteCodebases controller/space_test.go:85:23: cannot use ctx (type "context".Context) as type *configuration.Registry in argument to deleteCodebases --- controller/space_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/space_test.go b/controller/space_test.go index ab00df67d6..001bfe3878 100644 --- a/controller/space_test.go +++ b/controller/space_test.go @@ -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) } From 4f0b0a73e804cde1f18d2423cc5af8f641381745 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:37:37 +0200 Subject: [PATCH 64/77] ineffassign: ineffectual assignment to err --- controller/work_item_link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { From 4354587455ad6077f182f75df27d4546333d538c Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:38:42 +0200 Subject: [PATCH 65/77] ineffassign: ineffectual assignment to err --- controller/workitem.go | 3 +++ 1 file changed, 3 insertions(+) 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 From c1c5f721bddc1242a8ec52daf8e277d06d5f5f74 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:39:18 +0200 Subject: [PATCH 66/77] ineffassign: ineffectual assignment to err --- controller/comments_blackbox_test.go | 1 + 1 file changed, 1 insertion(+) 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) From d465c96f3ac2e7c00139af5e5e3f9b2322927efe Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:40:32 +0200 Subject: [PATCH 67/77] ineffassign: ineffectual assignment to mustHaveIDs --- iteration/iteration_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index 80f2cde882..6e1490c034 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -251,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: {}, From a381903d2ad03c643584734fe1bf075ad1db43d0 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:40:51 +0200 Subject: [PATCH 68/77] ineffassign: ineffectual assignment to mustHaveIDs --- iteration/iteration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/iteration/iteration_test.go b/iteration/iteration_test.go index 6e1490c034..41b18070da 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -266,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: {}, } From cd3a09afe603dc146022a6aab359e2a5ce9230d4 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:42:33 +0200 Subject: [PATCH 69/77] ineffassign: ineffectual assignment to status --- jsonapi/error_handler.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jsonapi/error_handler.go b/jsonapi/error_handler.go index e27a04d93f..5549543221 100644 --- a/jsonapi/error_handler.go +++ b/jsonapi/error_handler.go @@ -40,9 +40,7 @@ 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() From 91731e3dbbf93c62d1d7daf66910d1d71a34bd66 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:43:53 +0200 Subject: [PATCH 70/77] ineffassign: ineffectual assignment to statusKey --- kubernetes/deployments_kubeclient.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From ee69987421c7548645911b896ff4339942217956 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:45:02 +0200 Subject: [PATCH 71/77] ineffassign: ineffectual assignment to tracker4 --- remoteworkitem/tracker_repository_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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{ From ca79b5bd9a01168e3398557513a949b2f23375ee Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:49:01 +0200 Subject: [PATCH 72/77] golint: codebase/che/client.go:451:2: const IdeUrlRel should be IdeURLRel --- codebase/che/client.go | 2 +- controller/codebase.go | 6 +++--- controller/codebase_whitebox_test.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) 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/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) From d9f6ec580a77bd61718df017b3a2bf19458bb218 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 12:52:46 +0200 Subject: [PATCH 73/77] lint: IGNORE jsonapi/error_handler.go:61:12: should not use basic type untyped int as key in context.WithValue --- jsonapi/error_handler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/jsonapi/error_handler.go b/jsonapi/error_handler.go index 5549543221..c6247ec388 100644 --- a/jsonapi/error_handler.go +++ b/jsonapi/error_handler.go @@ -57,6 +57,7 @@ func ErrorHandler(service *goa.Service, verbose bool) goa.Middleware { if reqID == nil { reqID = shortID() //ctx = context.WithValue(ctx, reqIDKey, reqID) + // nolint ctx = context.WithValue(ctx, 1, reqID) // TODO remove this hack } log.Error(ctx, map[string]interface{}{ From 47c6a6d7b8dcf55b511234b00d28978c7eade737 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Wed, 26 Sep 2018 14:02:35 +0200 Subject: [PATCH 74/77] fixup --- models/transaction.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/transaction.go b/models/transaction.go index b7097e78ab..ff85fa8276 100644 --- a/models/transaction.go +++ b/models/transaction.go @@ -20,10 +20,10 @@ func Transactional(db *gorm.DB, todo func(tx *gorm.DB) error) error { log.Error(context.Background(), map[string]interface{}{ "errRollback": errs.WithStack(tx.Error), "err": errs.WithStack(err), - }, "failed to rollback transaction: %+v", errRollback) + }, "failed to rollback transaction: %+v", tx.Error) } return errs.WithStack(err) } - tx.Commit() + tx = tx.Commit() return tx.Error } From 2ad3d2d881906215d8f33583549b6d96b05476cc Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Thu, 27 Sep 2018 10:19:21 +0200 Subject: [PATCH 75/77] fixup: make TestForwardRequest work without errors --- goasupport/forward_requestid_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/goasupport/forward_requestid_test.go b/goasupport/forward_requestid_test.go index 877a11298f..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" @@ -16,14 +17,15 @@ import ( ) 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 { @@ -31,7 +33,7 @@ func TestForwardRequest(t *testing.T) { return service.Send(ctx, 200, "ok") } rg := middleware.RequestID()(h) - err := rg(ctx, rw, req) + err := rg(service.Context, rw, req) require.NoError(t, err) assert.Equal(t, middleware.ContextRequestID(newCtx), reqID) From 47faa4ab308f758188e5b2ba44e954ee4ba02c39 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Thu, 27 Sep 2018 10:24:29 +0200 Subject: [PATCH 76/77] fixup and use non-simply-typed value for context.WithValue() --- jsonapi/error_handler.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/jsonapi/error_handler.go b/jsonapi/error_handler.go index c6247ec388..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,6 +46,7 @@ func ErrorHandler(service *goa.Service, verbose bool) goa.Middleware { return nil } cause := errs.Cause(e) + var respBody interface{} respBody, status := ErrorToJSONAPIErrors(ctx, e) rw.Header().Set("Content-Type", ErrorMediaIdentifier) if err, ok := cause.(goa.ServiceError); ok { @@ -52,13 +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) - // nolint - ctx = context.WithValue(ctx, 1, reqID) // TODO remove this hack + ctx = context.WithValue(ctx, reqIDKey, reqID) } log.Error(ctx, map[string]interface{}{ "msg": respBody, From f1f44810c62ca650a1e8e9c2284703f6a761ac65 Mon Sep 17 00:00:00 2001 From: Konrad Kleine Date: Thu, 27 Sep 2018 10:28:30 +0200 Subject: [PATCH 77/77] fixup --- comment/comment_repository_blackbox_test.go | 4 ++-- .../work_item_link_type_blackbox_test.go | 23 ------------------- iteration/iteration_test.go | 4 ++-- workitem/field_definition_blackbox_test.go | 2 +- workitem/workitem_repository_blackbox_test.go | 2 +- workitem/workitemtype_blackbox_test.go | 4 ++-- .../workitemtype_repository_bench_test.go | 3 ++- 7 files changed, 10 insertions(+), 32 deletions(-) diff --git a/comment/comment_repository_blackbox_test.go b/comment/comment_repository_blackbox_test.go index 74fe7bde57..ca06ce1790 100644 --- a/comment/comment_repository_blackbox_test.go +++ b/comment/comment_repository_blackbox_test.go @@ -76,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") @@ -121,7 +121,7 @@ func (s *TestCommentRepository) TestSaveCommentWithMarkup() { comment.Body = "Test AB" comment.Markup = rendering.SystemMarkupMarkdown err := s.repo.Save(s.Ctx, comment, comment.Creator) - require.NoError(sT(), err) + require.NoError(s.T(), err) offset := 0 limit := 1 comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit) 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/iteration/iteration_test.go b/iteration/iteration_test.go index 41b18070da..f3d805a337 100644 --- a/iteration/iteration_test.go +++ b/iteration/iteration_test.go @@ -93,7 +93,7 @@ func (s *TestIterationRepository) TestCreate() { EndAt: &end, Path: parentPath, } - err := repo.Create(context.Background(), &i2) + err = repo.Create(context.Background(), &i2) // then require.NoError(t, err) i2L, err := repo.Load(context.Background(), i2.ID) @@ -223,7 +223,7 @@ func (s *TestIterationRepository) TestLoad() { EndAt: &end, } i2.MakeChildOf(i) - err := 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) diff --git a/workitem/field_definition_blackbox_test.go b/workitem/field_definition_blackbox_test.go index 07c9f0cdc9..0eb266db22 100644 --- a/workitem/field_definition_blackbox_test.go +++ b/workitem/field_definition_blackbox_test.go @@ -20,7 +20,7 @@ func testFieldDefinitionMarshalUnmarshal(t *testing.T, def workitem.FieldDefinit t.Logf("bytes are %s", string(bytes)) unmarshalled := workitem.FieldDefinition{} - err := json.Unmarshal(bytes, &unmarshalled) + err = json.Unmarshal(bytes, &unmarshalled) require.NoError(t, err) if !reflect.DeepEqual(def, unmarshalled) { diff --git a/workitem/workitem_repository_blackbox_test.go b/workitem/workitem_repository_blackbox_test.go index 3cee9ab858..aca7235560 100644 --- a/workitem/workitem_repository_blackbox_test.go +++ b/workitem/workitem_repository_blackbox_test.go @@ -709,7 +709,7 @@ 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} { - _, err := 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"}`)) diff --git a/workitem/workitemtype_blackbox_test.go b/workitem/workitemtype_blackbox_test.go index 6ec04164c1..7bd363cc92 100644 --- a/workitem/workitemtype_blackbox_test.go +++ b/workitem/workitemtype_blackbox_test.go @@ -46,7 +46,7 @@ func TestJsonMarshalListType(t *testing.T) { } var parsedWIT workitem.WorkItemType - err := json.Unmarshal(bytes, &parsedWIT) + err = json.Unmarshal(bytes, &parsedWIT) require.NoError(t, err) if !expectedWIT.Equal(parsedWIT) { @@ -80,7 +80,7 @@ func TestMarshalEnumType(t *testing.T) { } var parsedWIT workitem.WorkItemType - err := json.Unmarshal(bytes, &parsedWIT) + err = json.Unmarshal(bytes, &parsedWIT) require.NoError(t, err) if !expectedWIT.Equal(parsedWIT) { diff --git a/workitem/workitemtype_repository_bench_test.go b/workitem/workitemtype_repository_bench_test.go index 7a133c42b3..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" ) @@ -135,7 +136,7 @@ func (r *BenchWorkItemTypeRepository) BenchmarkListRawScan() { for result.Next() { wit := workitem.WorkItemType{} err := result.Scan(&wit) - require.NoError(s.B(), err) + require.NoError(r.B(), err) rows = append(rows, wit) } }