From 21c5284d6d3a95ecff835c4c763cf64e9cbc3626 Mon Sep 17 00:00:00 2001 From: Harshil Sharma Date: Mon, 10 May 2021 20:45:27 +0530 Subject: [PATCH] #GH-31 Configured Golangci-Lint and fixed errors (#381) * Moved golangci-lint config to config file * Fixed all golangci lint errors * Not allowing telemetry shutdown error to prevent store from closing * Remved golangci-lint presets --- Makefile | 2 +- server/.golangci.yml | 59 +++++++++++++++++++++++++++++++++++++ server/server/enterprise.go | 9 ++++-- server/server/server.go | 58 +++++++++++++++++++++++------------- 4 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 server/.golangci.yml diff --git a/Makefile b/Makefile index 51be48a90..4fe871fcc 100644 --- a/Makefile +++ b/Makefile @@ -113,7 +113,7 @@ server-lint: echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \ exit 1; \ fi; \ - cd server; golangci-lint run -p format -p unused -p complexity -p bugs -p performance -E asciicheck -E depguard -E dogsled -E dupl -E funlen -E gochecknoglobals -E gochecknoinits -E goconst -E gocritic -E godot -E godox -E goerr113 -E goheader -E golint -E gomnd -E gomodguard -E goprintffuncname -E gosimple -E interfacer -E lll -E misspell -E nlreturn -E nolintlint -E stylecheck -E unconvert -E whitespace -E wsl --skip-dirs services/store/sqlstore/migrations/ ./... + cd server; golangci-lint run ./server/... server-test: cd server; go test -v ./... diff --git a/server/.golangci.yml b/server/.golangci.yml new file mode 100644 index 000000000..46e8ce4c0 --- /dev/null +++ b/server/.golangci.yml @@ -0,0 +1,59 @@ +run: + timeout: 5m + modules-download-mode: readonly + skip-dirs: + - services/store/sqlstore/migrations/ + +linters-settings: + gofmt: + simplify: true + govet: + check-shadowing: true + enable-all: true + disable: + - fieldalignment + +linters: + disable-all: true + + enable: + - gofmt + - goimports + - deadcode + - ineffassign + - structcheck + - varcheck + - unparam + - errcheck + - govet + - bodyclose + - durationcheck + - errorlint + - exhaustive + - exportloopref + - gosec + - makezero + - staticcheck + - prealloc + - asciicheck + - depguard + - dogsled + - dupl + - gochecknoinits + - goconst + - gocritic + - godot + - goerr113 + - goheader + - golint + - gomnd + - gomodguard + - goprintffuncname + - gosimple + - lll + - misspell + - nolintlint + - stylecheck + - unconvert + - whitespace + - gocyclo diff --git a/server/server/enterprise.go b/server/server/enterprise.go index 2b42247fd..10e989b82 100644 --- a/server/server/enterprise.go +++ b/server/server/enterprise.go @@ -4,8 +4,13 @@ import ( "github.com/mattermost/focalboard/server/einterfaces" ) -var mattermostAuth func(einterfaces.MattermostAuthParameters, einterfaces.MattermostAuthStore) einterfaces.MattermostAuth +var mattermostAuth func( + einterfaces.MattermostAuthParameters, + einterfaces.MattermostAuthStore, +) einterfaces.MattermostAuth -func RegisterMattermostAuth(f func(einterfaces.MattermostAuthParameters, einterfaces.MattermostAuthStore) einterfaces.MattermostAuth) { +func RegisterMattermostAuth( + f func(einterfaces.MattermostAuthParameters, einterfaces.MattermostAuthStore) einterfaces.MattermostAuth, +) { mattermostAuth = f } diff --git a/server/server/server.go b/server/server/server.go index 91df46b93..4b0b43b2f 100644 --- a/server/server/server.go +++ b/server/server/server.go @@ -9,10 +9,11 @@ import ( "syscall" "time" + "go.uber.org/zap" + "github.com/google/uuid" "github.com/gorilla/mux" "github.com/pkg/errors" - "go.uber.org/zap" "github.com/mattermost/focalboard/server/api" "github.com/mattermost/focalboard/server/app" @@ -31,6 +32,13 @@ import ( "github.com/mattermost/mattermost-server/v5/utils" ) +const ( + cleanupSessionTaskFrequency = 10 * time.Minute + + //nolint:gomnd + minSessionExpiryTime = int64(60 * 60 * 24 * 31) // 31 days +) + type Server struct { config *config.Configuration wsServer *ws.Server @@ -53,20 +61,21 @@ func New(cfg *config.Configuration, singleUserToken string) (*Server, error) { return nil, err } - store, err := sqlstore.New(cfg.DBType, cfg.DBConfigString, cfg.DBTablePrefix) + db, err := sqlstore.New(cfg.DBType, cfg.DBConfigString, cfg.DBTablePrefix) if err != nil { log.Print("Unable to start the database", err) return nil, err } - auth := auth.New(cfg, store) + authenticator := auth.New(cfg, db) - wsServer := ws.NewServer(auth, singleUserToken) + wsServer := ws.NewServer(authenticator, singleUserToken) filesBackendSettings := filesstore.FileBackendSettings{} filesBackendSettings.DriverName = "local" filesBackendSettings.Directory = cfg.FilesPath filesBackend, appErr := filesstore.NewFileBackend(filesBackendSettings) + if appErr != nil { log.Print("Unable to initialize the files storage") @@ -75,31 +84,34 @@ func New(cfg *config.Configuration, singleUserToken string) (*Server, error) { webhookClient := webhook.NewClient(cfg) - appBuilder := func() *app.App { return app.New(cfg, store, auth, wsServer, filesBackend, webhookClient) } - api := api.NewAPI(appBuilder, singleUserToken, cfg.AuthMode) + appBuilder := func() *app.App { return app.New(cfg, db, authenticator, wsServer, filesBackend, webhookClient) } + focalboardAPI := api.NewAPI(appBuilder, singleUserToken, cfg.AuthMode) // Local router for admin APIs localRouter := mux.NewRouter() - api.RegisterAdminRoutes(localRouter) + focalboardAPI.RegisterAdminRoutes(localRouter) // Init workspace - appBuilder().GetRootWorkspace() + if _, err = appBuilder().GetRootWorkspace(); err != nil { + log.Print("Unable to get root workspace", err) + return nil, err + } webServer := web.NewServer(cfg.WebPath, cfg.ServerRoot, cfg.Port, cfg.UseSSL, cfg.LocalOnly) webServer.AddRoutes(wsServer) - webServer.AddRoutes(api) + webServer.AddRoutes(focalboardAPI) // Init telemetry - settings, err := store.GetSystemSettings() + settings, err := db.GetSystemSettings() if err != nil { return nil, err } telemetryID := settings["TelemetryID"] + if len(telemetryID) == 0 { telemetryID = uuid.New().String() - err := store.SetSystemSetting("TelemetryID", uuid.New().String()) - if err != nil { + if err = db.SetSystemSetting("TelemetryID", uuid.New().String()); err != nil { return nil, err } } @@ -156,12 +168,12 @@ func New(cfg *config.Configuration, singleUserToken string) (*Server, error) { config: cfg, wsServer: wsServer, webServer: webServer, - store: store, + store: db, filesBackend: filesBackend, telemetry: telemetryService, logger: logger, localRouter: localRouter, - api: api, + api: focalboardAPI, appBuilder: appBuilder, } @@ -182,14 +194,15 @@ func (s *Server) Start() error { } s.cleanUpSessionsTask = scheduler.CreateRecurringTask("cleanUpSessions", func() { - secondsAgo := int64(60 * 60 * 24 * 31) + secondsAgo := minSessionExpiryTime if secondsAgo < s.config.SessionExpireTime { secondsAgo = s.config.SessionExpireTime } + if err := s.store.CleanUpSessions(secondsAgo); err != nil { s.logger.Error("Unable to clean up the sessions", zap.Error(err)) } - }, 10*time.Minute) + }, cleanupSessionTaskFrequency) if s.config.Telemetry { firstRun := utils.MillisFromTime(time.Now()) @@ -210,7 +223,9 @@ func (s *Server) Shutdown() error { s.cleanUpSessionsTask.Cancel() } - s.telemetry.Shutdown() + if err := s.telemetry.Shutdown(); err != nil { + s.logger.Warn("Error occurred when shutting down telemetry", zap.Error(err)) + } defer s.logger.Info("Server.Shutdown") @@ -230,7 +245,10 @@ func (s *Server) startLocalModeServer() error { } // TODO: Close and delete socket file on shutdown - syscall.Unlink(s.config.LocalModeSocketLocation) + if err := syscall.Unlink(s.config.LocalModeSocketLocation); err != nil { + log.Print("Unable to unlink socket.", err) + return err + } socket := s.config.LocalModeSocketLocation unixListener, err := net.Listen("unix", socket) @@ -244,7 +262,7 @@ func (s *Server) startLocalModeServer() error { go func() { log.Println("Starting unix socket server") err = s.localModeServer.Serve(unixListener) - if err != nil && err != http.ErrServerClosed { + if err != nil && !errors.Is(err, http.ErrServerClosed) { log.Printf("Error starting unix socket server: %v", err) } }() @@ -254,7 +272,7 @@ func (s *Server) startLocalModeServer() error { func (s *Server) stopLocalModeServer() { if s.localModeServer != nil { - s.localModeServer.Close() + _ = s.localModeServer.Close() s.localModeServer = nil } }