From d01607ebdb0cf50c9f018e4bb7009ecc7774edd3 Mon Sep 17 00:00:00 2001 From: Andre Urbani Date: Tue, 20 Jun 2023 11:34:19 +0100 Subject: [PATCH] fix linter issues --- api/.golangci.yml => .golangci.yml | 0 Makefile | 2 +- api/api.go | 7 ++--- api/api_test.go | 4 +-- api/metadata.go | 18 ++++++------ api/metadata_test.go | 2 -- config/config_test.go | 2 -- .../makerecp/createrecipe/createrecipe.go | 28 ++++++++----------- devstack/makerecp/main.go | 8 +----- features/steps/component.go | 10 ++++--- main.go | 2 +- main_test.go | 2 +- service/service.go | 1 - service/service_test.go | 17 +++-------- 14 files changed, 39 insertions(+), 64 deletions(-) rename api/.golangci.yml => .golangci.yml (100%) diff --git a/api/.golangci.yml b/.golangci.yml similarity index 100% rename from api/.golangci.yml rename to .golangci.yml diff --git a/Makefile b/Makefile index ec1a302..e686aba 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ audit: ## run nancy auditor .PHONY: lint lint: - go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3 + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2 golangci-lint run ./... .PHONY: build diff --git a/api/api.go b/api/api.go index 5497925..9714272 100644 --- a/api/api.go +++ b/api/api.go @@ -29,16 +29,15 @@ type CantabularMetadataExtractorAPI struct { } // Setup function sets up the api and returns an api -func Setup(ctx context.Context, +func Setup(_ context.Context, r *mux.Router, - config *config.Config, + cfg *config.Config, c CantMetaAPI, auth authorisation.Middleware) *CantabularMetadataExtractorAPI { - api := &CantabularMetadataExtractorAPI{ Router: r, CantMetaAPI: c, - Cfg: config, + Cfg: cfg, auth: auth, } diff --git a/api/api_test.go b/api/api_test.go index 42ab1c7..e2373dc 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -32,10 +32,10 @@ func TestSetup(t *testing.T) { t.Fail() } - api := api.Setup(ctx, r, cfg, c, authorisationMiddleware) + apiSetup := api.Setup(ctx, r, cfg, c, authorisationMiddleware) Convey("When created the following routes should have been added", func() { - So(hasRoute(api.Router, "/cantabular-metadata/dataset/{datasetID}/lang/{lang}", "GET"), ShouldBeTrue) + So(hasRoute(apiSetup.Router, "/cantabular-metadata/dataset/{datasetID}/lang/{lang}", "GET"), ShouldBeTrue) }) }) } diff --git a/api/metadata.go b/api/metadata.go index c2aa808..9bfb457 100644 --- a/api/metadata.go +++ b/api/metadata.go @@ -39,20 +39,20 @@ func (api *CantabularMetadataExtractorAPI) getMetadataHandler(w http.ResponseWri return } - json, err := json.Marshal(m) + jsonMarshall, err := json.Marshal(m) if err != nil { log.Error(ctx, err.Error(), err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - _, err = w.Write(json) + _, err = w.Write(jsonMarshall) if err != nil { log.Error(ctx, err.Error(), err) } } -func (api *CantabularMetadataExtractorAPI) GetMetadata(ctx context.Context, datasetID string, lang string) (*cantabular.MetadataQueryResult, error) { +func (api *CantabularMetadataExtractorAPI) GetMetadata(ctx context.Context, datasetID, lang string) (*cantabular.MetadataQueryResult, error) { mt, dimensions, err := api.GetMetadataTable(ctx, cantabular.MetadataTableQueryRequest{ Variables: []string{datasetID}, Lang: lang, @@ -83,20 +83,18 @@ func (api *CantabularMetadataExtractorAPI) GetMetadata(ctx context.Context, data return &cantabular.MetadataQueryResult{TableQueryResult: mt, DatasetQueryResult: md}, nil } -func (api *CantabularMetadataExtractorAPI) GetMetadataTable(ctx context.Context, req cantabular.MetadataTableQueryRequest) (*cantabular.MetadataTableQuery, []string, error) { - var dims []string +func (api *CantabularMetadataExtractorAPI) GetMetadataTable(_ context.Context, req cantabular.MetadataTableQueryRequest) (*cantabular.MetadataTableQuery, []string, error) { + var dims = []string{} mt, err := api.CantMetaAPI.MetadataTableQuery(context.Background(), req) if err != nil { return mt, dims, err } if len(mt.Service.Tables) == 0 { - return mt, dims, fmt.Errorf("%s : %w", "mt.Service.Tables", errUnexpectedResp) } if len(mt.Service.Tables[0].Vars) == 0 { - return mt, dims, fmt.Errorf("%s : %w", "mt.Service.Tables.Vars", errUnexpectedResp) } @@ -125,9 +123,9 @@ func OverrideMetadataTable(dims []string, mt *cantabular.MetadataTableQuery) err } substituted = 0 - for i, v := range mt.Service.Tables { - for j, c := range v.Vars { - if inSlice(string(c), validGeo) { + for i := range mt.Service.Tables { + for j := range mt.Service.Tables[i].Vars { + if inSlice(string(mt.Service.Tables[i].Vars[j]), validGeo) { mt.Service.Tables[i].Vars[j] = graphql.String(geoCodeOverride) substituted++ } diff --git a/api/metadata_test.go b/api/metadata_test.go index 1195463..1b53b5b 100644 --- a/api/metadata_test.go +++ b/api/metadata_test.go @@ -38,7 +38,6 @@ func TestGetMetadataTable(t *testing.T) { So(dims, ShouldResemble, expected) }) }) - } func TestOverrideMetadataTable(t *testing.T) { @@ -59,7 +58,6 @@ func TestOverrideMetadataTable(t *testing.T) { }) }) }) - } func getMT() (cantabular.MetadataTableQuery, error) { diff --git a/config/config_test.go b/config/config_test.go index 9304ca5..d1bf23f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -18,9 +18,7 @@ func TestConfig(t *testing.T) { Convey("Then cfg should be nil", func() { So(cfg, ShouldBeNil) }) - Convey("When the config values are retrieved", func() { - Convey("Then there should be no error returned, and values are as expected", func() { configuration, err = Get() // This Get() is only called once, when inside this function So(err, ShouldBeNil) diff --git a/devstack/makerecp/createrecipe/createrecipe.go b/devstack/makerecp/createrecipe/createrecipe.go index 2124559..aaf9d43 100644 --- a/devstack/makerecp/createrecipe/createrecipe.go +++ b/devstack/makerecp/createrecipe/createrecipe.go @@ -66,13 +66,13 @@ type CreateRecipe struct { ONSDataSetID string Dimensions []string Host string - ExtApiHost string + ExtAPIHost string ValidIDs []string UUID string } -func New(id, host, extApiHost string) *CreateRecipe { - var validIDs []string +func New(id, host, extAPIHost string) *CreateRecipe { + var validIDs = []string{} for k := range GetMap() { validIDs = append(validIDs, k) } @@ -80,7 +80,7 @@ func New(id, host, extApiHost string) *CreateRecipe { return &CreateRecipe{ ONSDataSetID: id, Host: host, - ExtApiHost: extApiHost, + ExtAPIHost: extAPIHost, Dimensions: GetMap()[id], ValidIDs: validIDs, UUID: uuidV4(), @@ -107,7 +107,6 @@ func (cr *CreateRecipe) GetMetaData() (TableFrag, error) { } func (cr *CreateRecipe) GetCodeLists() (cls CodeLists) { - for _, v := range cr.Dimensions { cl := CodeList{ Href: fmt.Sprintf("http://localhost:22400/code-lists/%s", v), @@ -131,7 +130,6 @@ func (cr *CreateRecipe) CheckID() bool { } func (cr *CreateRecipe) OKDimsInDS() bool { - query := `variables={}&query={ dataset(name:"UR") { variables(names:["%s"]) { @@ -148,7 +146,7 @@ dataset(name:"UR") { payload := url.PathEscape(fmt.Sprintf(query, strings.Join(cr.Dimensions, "\",\""))) - resp, err := http.Get(fmt.Sprintf("%s/graphql?%s", cr.ExtApiHost, payload)) + resp, err := http.Get(fmt.Sprintf("%s/graphql?%s", cr.ExtAPIHost, payload)) if err != nil { log.Print(err) } @@ -159,7 +157,6 @@ dataset(name:"UR") { } return !strings.Contains(string(body), "does not exist") - } // GetMap returns our definitions as the Golden Source of Truth (maybe) @@ -176,9 +173,9 @@ func GetMap() map[string][]string { "TS015": {"oa", "year_arrival_uk"}, "TS016": {"oa", "residence_length_6b"}, "TS021": {"oa", "ethnic_group_tb_20b"}, - //"TS024": {"ltla", "main_language_detailed"}, + // "TS024": {"ltla", "main_language_detailed"}, "TS027": {"oa", "national_identity_all"}, - //"TS028": {"oa", "national_identity_detailed"}, + // "TS028": {"oa", "national_identity_detailed"}, "TS029": {"oa", "english_proficiency"}, "TS030": {"oa", "religion_tb"}, "TS032": {"oa", "welsh_skills_all"}, @@ -187,16 +184,16 @@ func GetMap() map[string][]string { "TS035": {"oa", "welsh_skills_read"}, "TS036": {"oa", "welsh_skills_understand"}, "TS037": {"oa", "health_in_general"}, - //"TS038": {"oa", "disability"}, + // "TS038": {"oa", "disability"}, // "TS039": {"oa", "is_carer"}, "TS056": {"oa", "alternative_address_indicator"}, "TS058": {"oa", "workplace_travel_10a"}, - //"TS059": {"oa", "hours_per_week_worked"}, - //"TS060": {"msoa", "industry_current_88a"}, + // "TS059": {"oa", "hours_per_week_worked"}, + // "TS060": {"msoa", "industry_current_88a"}, "TS061": {"oa", "transport_to_workplace_12a"}, "TS062": {"oa", "ns_sec_10a"}, - //"TS063": {"oa", "occupation_current_10a"}, - //"TS064": {"msoa", "occupation_current_105a"}, + // "TS063": {"oa", "occupation_current_10a"}, + // "TS064": {"msoa", "occupation_current_105a"}, "TS065": {"oa", "has_ever_worked"}, "TS066": {"oa", "economic_activity_status_12a"}, "TS067": {"oa", "highest_qualification"}, @@ -226,7 +223,6 @@ func IsGeo(s string) bool { } return isGeo[s] - } func SplitVars(totalVars []string) (geoVar string, vars []string) { for _, v := range totalVars { diff --git a/devstack/makerecp/main.go b/devstack/makerecp/main.go index 7ec14e1..784bb7d 100644 --- a/devstack/makerecp/main.go +++ b/devstack/makerecp/main.go @@ -14,7 +14,6 @@ import ( ) func main() { - log.SetFlags(log.LstdFlags | log.Lshortfile) var id, host, extapihost, checkdims, alias, format string @@ -32,7 +31,6 @@ func main() { flag.Parse() if checkall { - for id := range createrecipe.GetMap() { fmt.Printf("Testing id=%s ", id) cr := createrecipe.New(id, host, extapihost) @@ -64,11 +62,9 @@ func main() { } fmt.Print("OK\n") - } os.Exit(0) - } cr := createrecipe.New(id, host, extapihost) @@ -83,9 +79,8 @@ func main() { cr.Dimensions = strings.Split(checkdims, ",") if !cr.OKDimsInDS() { log.Fatalf("dims '%#v' not fully present in '%s' dataset", cr.Dimensions, "UR") // XXX - } else { - fmt.Println("dims OK") } + fmt.Println("dims OK") os.Exit(0) } @@ -149,5 +144,4 @@ func main() { log.Fatal(err) } fmt.Println(string(u)) - } diff --git a/features/steps/component.go b/features/steps/component.go index 57e0e1c..34eda63 100644 --- a/features/steps/component.go +++ b/features/steps/component.go @@ -3,6 +3,7 @@ package steps import ( "context" "net/http" + "time" "github.com/ONSdigital/dp-authorisation/v2/authorisation" "github.com/ONSdigital/dp-cantabular-metadata-extractor-api/config" @@ -26,9 +27,10 @@ type Component struct { } func NewComponent() (*Component, error) { - c := &Component{ - HTTPServer: &http.Server{}, + HTTPServer: &http.Server{ + ReadHeaderTimeout: time.Duration(5) * time.Second, + }, errorChan: make(chan error), ServiceRunning: false, } @@ -78,7 +80,7 @@ func (c *Component) InitialiseService() (http.Handler, error) { return c.HTTPServer.Handler, nil } -func (f *Component) DoGetAuthorisationMiddleware(ctx context.Context, cfg *authorisation.Config) (authorisation.Middleware, error) { +func (c *Component) DoGetAuthorisationMiddleware(ctx context.Context, cfg *authorisation.Config) (authorisation.Middleware, error) { middleware, err := authorisation.NewMiddlewareFromConfig(ctx, cfg, cfg.JWTVerificationPublicKeys) if err != nil { return nil, err @@ -86,7 +88,7 @@ func (f *Component) DoGetAuthorisationMiddleware(ctx context.Context, cfg *autho return middleware, nil } -func (c *Component) DoGetHealthcheckOk(cfg *config.Config, buildTime string, gitCommit string, version string) (service.HealthChecker, error) { +func (c *Component) DoGetHealthcheckOk(_ *config.Config, _, _, _ string) (service.HealthChecker, error) { return &mock.HealthCheckerMock{ AddCheckFunc: func(name string, checker healthcheck.Checker) error { return nil }, StartFunc: func(ctx context.Context) {}, diff --git a/main.go b/main.go index 8eaac65..13b860b 100644 --- a/main.go +++ b/main.go @@ -28,7 +28,7 @@ func main() { ctx := context.Background() if err := run(ctx); err != nil { - log.Fatal(ctx, "fatal runtime error", err) + log.Error(ctx, "fatal runtime error", err) os.Exit(1) } } diff --git a/main_test.go b/main_test.go index 2cd6f9d..05ffcce 100644 --- a/main_test.go +++ b/main_test.go @@ -37,7 +37,7 @@ func (f *ComponentTest) InitializeScenario(godogCtx *godog.ScenarioContext) { component.RegisterSteps(godogCtx) } -func (f *ComponentTest) InitializeTestSuite(ctx *godog.TestSuiteContext) { +func (f *ComponentTest) InitializeTestSuite(_ *godog.TestSuiteContext) { } diff --git a/service/service.go b/service/service.go index 9d3059a..9ffa678 100644 --- a/service/service.go +++ b/service/service.go @@ -26,7 +26,6 @@ type Service struct { // Run the service func Run(ctx context.Context, cfg *config.Config, serviceList *ExternalServiceList, buildTime, gitCommit, version string, svcErrors chan error) (*Service, error) { - log.Info(ctx, "running service") log.Info(ctx, "using service configuration", log.Data{"config": cfg}) diff --git a/service/service_test.go b/service/service_test.go index 61be979..9d57098 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -42,9 +42,7 @@ var funcDoGetHTTPServerNil = func(bindAddr string, router http.Handler) service. } func TestRun(t *testing.T) { - Convey("Having a set of mocked dependencies", t, func() { - cfg, err := config.Get() So(err, ShouldBeNil) @@ -101,7 +99,6 @@ func TestRun(t *testing.T) { } Convey("Given that initialising healthcheck returns an error", func() { - // setup (run before each `Convey` at this scope / indentation): initMock := &serviceMock.InitialiserMock{ DoGetHTTPServerFunc: funcDoGetHTTPServerNil, @@ -124,7 +121,6 @@ func TestRun(t *testing.T) { }) Convey("Given that all dependencies are successfully initialised", func() { - // setup (run before each `Convey` at this scope / indentation): initMock := &serviceMock.InitialiserMock{ DoGetHTTPServerFunc: funcDoGetHTTPServer, @@ -159,12 +155,11 @@ func TestRun(t *testing.T) { }) Convey("Given that all dependencies are successfully initialised but the http server fails", func() { - // setup (run before each `Convey` at this scope / indentation): initMock := &serviceMock.InitialiserMock{ - DoGetHealthCheckFunc: funcDoGetHealthcheckOk, - DoGetHTTPServerFunc: funcDoGetFailingHTTPSerer, - DoGetHealthClientFunc: funcDoGetHealthClientOk, + DoGetHealthCheckFunc: funcDoGetHealthcheckOk, + DoGetHTTPServerFunc: funcDoGetFailingHTTPSerer, + DoGetHealthClientFunc: funcDoGetHealthClientOk, DoGetAuthorisationMiddlewareFunc: funcDoGetAuthOk, } svcErrors := make(chan error, 1) @@ -187,9 +182,7 @@ func TestRun(t *testing.T) { } func TestClose(t *testing.T) { - Convey("Having a correctly initialised service", t, func() { - cfg, err := config.Get() So(err, ShouldBeNil) @@ -223,7 +216,6 @@ func TestClose(t *testing.T) { } Convey("Closing the service results in all the dependencies being closed in the expected order", func() { - initMock := &serviceMock.InitialiserMock{ DoGetHTTPServerFunc: func(bindAddr string, router http.Handler) service.HTTPServer { return serverMock }, DoGetHealthCheckFunc: func(cfg *config.Config, buildTime string, gitCommit string, version string) (service.HealthChecker, error) { @@ -247,7 +239,6 @@ func TestClose(t *testing.T) { }) Convey("If services fail to stop, the Close operation tries to close all dependencies and returns an error", func() { - failingserverMock := &serviceMock.HTTPServerMock{ ListenAndServeFunc: func() error { return nil }, ShutdownFunc: func(ctx context.Context) error { @@ -293,7 +284,7 @@ func TestClose(t *testing.T) { Config: cfg, ServiceList: svcList, Server: timeoutServerMock, - HealthCheck: hcMock, + HealthCheck: hcMock, } err = svc.Close(context.Background())