From db02eb694d08c2691ec640eb031e158bc5038265 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Apr 2020 13:07:13 +0200 Subject: [PATCH] refactor: no more pmapifactory --- cmd/Desktop-Bridge/main.go | 9 +++--- internal/api/api.go | 3 +- internal/bridge/bridge.go | 6 ++-- internal/bridge/credentials/credentials.go | 4 +-- internal/frontend/cli/frontend.go | 3 +- internal/frontend/frontend.go | 3 +- internal/frontend/qt/frontend.go | 3 +- internal/frontend/qt/frontend_nogui.go | 3 +- internal/imap/imap.go | 4 +-- internal/smtp/smtp.go | 4 +-- pkg/config/config.go | 16 ++-------- pkg/config/logs.go | 7 ----- .../config}/pmapi_noprod.go | 17 ++++++++--- .../pmapifactory => pkg/config}/pmapi_prod.go | 29 +++++++++---------- pkg/connection/check_connection.go | 4 +-- pkg/keychain/keychain.go | 4 +-- pkg/listener/listener.go | 4 +-- pkg/logs/logs.go | 10 +++++++ pkg/pmapi/client.go | 3 -- pkg/pmapi/clientmanager.go | 5 ---- pkg/updates/updates.go | 4 +-- test/context/config.go | 1 - 22 files changed, 70 insertions(+), 76 deletions(-) rename {internal/pmapifactory => pkg/config}/pmapi_noprod.go (75%) rename {internal/pmapifactory => pkg/config}/pmapi_prod.go (54%) create mode 100644 pkg/logs/logs.go diff --git a/cmd/Desktop-Bridge/main.go b/cmd/Desktop-Bridge/main.go index 3ec567bc..5a37044c 100644 --- a/cmd/Desktop-Bridge/main.go +++ b/cmd/Desktop-Bridge/main.go @@ -51,12 +51,12 @@ import ( "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/internal/frontend" "github.com/ProtonMail/proton-bridge/internal/imap" - "github.com/ProtonMail/proton-bridge/internal/pmapifactory" "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/smtp" "github.com/ProtonMail/proton-bridge/pkg/args" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/updates" "github.com/allan-simon/go-singleinstance" @@ -87,7 +87,7 @@ var ( longVersion = Version + " (" + Revision + ")" //nolint[gochecknoglobals] buildVersion = longVersion + " " + BuildTime //nolint[gochecknoglobals] - log = config.GetLogEntry("main") //nolint[gochecknoglobals] + log = logs.GetLogEntry("main") //nolint[gochecknoglobals] // How many crashes in a row. numberOfCrashes = 0 //nolint[gochecknoglobals] @@ -274,9 +274,8 @@ func run(context *cli.Context) (contextError error) { // nolint[funlen] log.Error("Could not get credentials store: ", credentialsError) } - clientConfig := pmapifactory.GetClientConfig(cfg.GetAPIConfig()) - cm := pmapi.NewClientManager(clientConfig) - pmapifactory.SetClientRoundTripper(cm, clientConfig, eventListener) + cm := pmapi.NewClientManager(cfg.GetAPIConfig()) + cm.SetRoundTripper(cfg.GetRoundTripper(cm, eventListener)) bridgeInstance := bridge.New(cfg, pref, panicHandler, eventListener, Version, cm, credentialsStore) imapBackend := imap.NewIMAPBackend(panicHandler, eventListener, cfg, bridgeInstance) diff --git a/internal/api/api.go b/internal/api/api.go index 10c1fe79..d23fc379 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -31,11 +31,12 @@ import ( "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/ports" ) var ( - log = config.GetLogEntry("api") //nolint[gochecknoglobals] + log = logs.GetLogEntry("api") //nolint[gochecknoglobals] ) type apiServer struct { diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 6dc034ab..a81a5313 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -28,8 +28,8 @@ import ( "github.com/ProtonMail/proton-bridge/internal/metrics" "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/internal/store" - "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/hashicorp/go-multierror" "github.com/pkg/errors" @@ -37,8 +37,8 @@ import ( ) var ( - log = config.GetLogEntry("bridge") //nolint[gochecknoglobals] - isApplicationOutdated = false //nolint[gochecknoglobals] + log = logs.GetLogEntry("bridge") //nolint[gochecknoglobals] + isApplicationOutdated = false //nolint[gochecknoglobals] ) // Bridge is a struct handling users. diff --git a/internal/bridge/credentials/credentials.go b/internal/bridge/credentials/credentials.go index 802d4526..a1a6c0fd 100644 --- a/internal/bridge/credentials/credentials.go +++ b/internal/bridge/credentials/credentials.go @@ -27,14 +27,14 @@ import ( "fmt" "strings" - "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/sirupsen/logrus" ) const sep = "\x00" var ( - log = config.GetLogEntry("bridge") //nolint[gochecknoglobals] + log = logs.GetLogEntry("bridge") //nolint[gochecknoglobals] ErrWrongFormat = errors.New("backend/creds: malformed password") ) diff --git a/internal/frontend/cli/frontend.go b/internal/frontend/cli/frontend.go index d0e6c61f..c31c305a 100644 --- a/internal/frontend/cli/frontend.go +++ b/internal/frontend/cli/frontend.go @@ -24,12 +24,13 @@ import ( "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/abiosoft/ishell" ) var ( - log = config.GetLogEntry("frontend/cli") //nolint[gochecknoglobals] + log = logs.GetLogEntry("frontend/cli") //nolint[gochecknoglobals] ) type frontendCLI struct { diff --git a/internal/frontend/frontend.go b/internal/frontend/frontend.go index 96d45fa4..0a4d3a59 100644 --- a/internal/frontend/frontend.go +++ b/internal/frontend/frontend.go @@ -26,10 +26,11 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" ) var ( - log = config.GetLogEntry("frontend") // nolint[unused] + log = logs.GetLogEntry("frontend") // nolint[unused] ) // Frontend is an interface to be implemented by each frontend type (cli, gui, html). diff --git a/internal/frontend/qt/frontend.go b/internal/frontend/qt/frontend.go index 4b4ca5ab..f588eac0 100644 --- a/internal/frontend/qt/frontend.go +++ b/internal/frontend/qt/frontend.go @@ -43,6 +43,7 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/internal/preferences" "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/ports" "github.com/ProtonMail/proton-bridge/pkg/useragent" @@ -58,7 +59,7 @@ import ( "github.com/therecipe/qt/widgets" ) -var log = config.GetLogEntry("frontend-qt") +var log = logs.GetLogEntry("frontend-qt") var accountMutex = &sync.Mutex{} // API between Bridge and Qt. diff --git a/internal/frontend/qt/frontend_nogui.go b/internal/frontend/qt/frontend_nogui.go index 89e5dbc5..34aef3e7 100644 --- a/internal/frontend/qt/frontend_nogui.go +++ b/internal/frontend/qt/frontend_nogui.go @@ -26,9 +26,10 @@ import ( "github.com/ProtonMail/proton-bridge/internal/frontend/types" "github.com/ProtonMail/proton-bridge/pkg/config" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/logs" ) -var log = config.GetLogEntry("frontend-nogui") //nolint[gochecknoglobals] +var log = logs.GetLogEntry("frontend-nogui") //nolint[gochecknoglobals] type FrontendHeadless struct{} diff --git a/internal/imap/imap.go b/internal/imap/imap.go index b4736a7e..294e042a 100644 --- a/internal/imap/imap.go +++ b/internal/imap/imap.go @@ -17,7 +17,7 @@ package imap -import "github.com/ProtonMail/proton-bridge/pkg/config" +import "github.com/ProtonMail/proton-bridge/pkg/logs" const ( fetchMessagesWorkers = 5 // In how many workers to fetch message (group list on IMAP). @@ -31,5 +31,5 @@ const ( ) var ( - log = config.GetLogEntry("imap") //nolint[gochecknoglobals] + log = logs.GetLogEntry("imap") //nolint[gochecknoglobals] ) diff --git a/internal/smtp/smtp.go b/internal/smtp/smtp.go index 9ca0f416..8ca02d11 100644 --- a/internal/smtp/smtp.go +++ b/internal/smtp/smtp.go @@ -18,8 +18,8 @@ // Package smtp provides SMTP server of the Bridge. package smtp -import "github.com/ProtonMail/proton-bridge/pkg/config" +import "github.com/ProtonMail/proton-bridge/pkg/logs" var ( - log = config.GetLogEntry("smtp") //nolint[gochecknoglobals] + log = logs.GetLogEntry("smtp") //nolint[gochecknoglobals] ) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7968e88d..0de4a848 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -21,15 +21,14 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "github.com/ProtonMail/go-appdir" - "github.com/ProtonMail/proton-bridge/pkg/pmapi" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/hashicorp/go-multierror" ) var ( - log = GetLogEntry("config") //nolint[gochecknoglobals] + log = logs.GetLogEntry("config") //nolint[gochecknoglobals] ) type appDirProvider interface { @@ -45,7 +44,6 @@ type Config struct { cacheVersion string appDirs appDirProvider appDirsVersion appDirProvider - apiConfig *pmapi.ClientConfig } // New returns fully initialized config struct. @@ -67,11 +65,6 @@ func newConfig(appName, version, revision, cacheVersion string, appDirs, appDirs cacheVersion: cacheVersion, appDirs: appDirs, appDirsVersion: appDirsVersion, - apiConfig: &pmapi.ClientConfig{ - AppVersion: strings.Title(appName) + "_" + version, - ClientID: appName, - SentryDSN: "https://bacfb56338a7471a9fede610046afdda:ab437b0d13f54602a0f5feb684e6d319@api.protonmail.ch/reports/sentry/8", - }, } } @@ -229,11 +222,6 @@ func (c *Config) GetPreferencesPath() string { return filepath.Join(c.appDirsVersion.UserCache(), "prefs.json") } -// GetAPIConfig returns config for ProtonMail API. -func (c *Config) GetAPIConfig() *pmapi.ClientConfig { - return c.apiConfig -} - // GetDefaultAPIPort returns default Bridge local API port. func (c *Config) GetDefaultAPIPort() int { return 1042 diff --git a/pkg/config/logs.go b/pkg/config/logs.go index 9aa38e9e..1a554144 100644 --- a/pkg/config/logs.go +++ b/pkg/config/logs.go @@ -54,13 +54,6 @@ var logFile *os.File //nolint[gochecknoglobals] var logFileRgx = regexp.MustCompile("^v.*\\.log$") //nolint[gochecknoglobals] var logCrashRgx = regexp.MustCompile("^v.*_crash_.*\\.log$") //nolint[gochecknoglobals] -// GetLogEntry returns logrus.Entry with PID and `packageName`. -func GetLogEntry(packageName string) *logrus.Entry { - return logrus.WithFields(logrus.Fields{ - "pkg": packageName, - }) -} - // HandlePanic reports the crash to sentry or local file when sentry fails. func HandlePanic(cfg *Config, output string) { if !cfg.IsDevMode() { diff --git a/internal/pmapifactory/pmapi_noprod.go b/pkg/config/pmapi_noprod.go similarity index 75% rename from internal/pmapifactory/pmapi_noprod.go rename to pkg/config/pmapi_noprod.go index 2ed31155..653d1afb 100644 --- a/internal/pmapifactory/pmapi_noprod.go +++ b/pkg/config/pmapi_noprod.go @@ -17,18 +17,27 @@ // +build !pmapi_prod -// Package pmapifactory creates pmapi client instances. -package pmapifactory +package config import ( + "net/http" + "strings" + "github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/pmapi" ) -func GetClientConfig(clientConfig *pmapi.ClientConfig) *pmapi.ClientConfig { - return clientConfig +func (c *Config) GetAPIConfig() *pmapi.ClientConfig { + return &pmapi.ClientConfig{ + AppVersion: strings.Title(c.appName) + "_" + c.version, + ClientID: c.appName, + } } func SetClientRoundTripper(_ *pmapi.ClientManager, _ *pmapi.ClientConfig, _ listener.Listener) { // Use the default roundtripper; do nothing. } + +func (c *Config) GetRoundTripper(_ *pmapi.ClientManager, _ listener.Listener) http.RoundTripper { + return http.DefaultTransport +} diff --git a/internal/pmapifactory/pmapi_prod.go b/pkg/config/pmapi_prod.go similarity index 54% rename from internal/pmapifactory/pmapi_prod.go rename to pkg/config/pmapi_prod.go index 3b1c5dd2..46eefeb0 100644 --- a/internal/pmapifactory/pmapi_prod.go +++ b/pkg/config/pmapi_prod.go @@ -17,35 +17,34 @@ // +build pmapi_prod -// Package pmapifactory creates pmapi client instances. -package pmapifactory +package config import ( + "net/http" + "strings" "time" "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/pmapi" - "github.com/sirupsen/logrus" ) -func GetClientConfig(clientConfig *pmapi.ClientConfig) *pmapi.ClientConfig { - // We set additional timeouts/thresholds for the request as a whole: - clientConfig.Timeout = 10 * time.Minute // Overall request timeout (~25MB / 10 mins => ~40kB/s, should be reasonable). - clientConfig.FirstReadTimeout = 30 * time.Second // 30s to match 30s response header timeout. - clientConfig.MinSpeed = 1 << 13 // Enforce minimum download speed of 8kB/s. - - return clientConfig +func (c *Config) GetAPIConfig() *pmapi.ClientConfig { + return &pmapi.ClientConfig{ + AppVersion: strings.Title(c.appName) + "_" + c.version, + ClientID: c.appName, + Timeout: 10 * time.Minute, // Overall request timeout (~25MB / 10 mins => ~40kB/s, should be reasonable). + FirstReadTimeout: 30 * time.Second, // 30s to match 30s response header timeout. + MinSpeed: 1 << 13, // Enforce minimum download speed of 8kB/s. + } } -func SetClientRoundTripper(cm *pmapi.ClientManager, cfg *pmapi.ClientConfig, listener listener.Listener) { - logrus.Info("Setting ClientManager to create clients with key pinning") - - pin := pmapi.NewDialerWithPinning(cm, cfg.AppVersion) +func (c *Config) GetRoundTripper(cm *pmapi.ClientManager, listener listener.Listener) http.RoundTripper { + pin := pmapi.NewDialerWithPinning(cm, c.GetAPIConfig().AppVersion) pin.ReportCertIssueLocal = func() { listener.Emit(events.TLSCertIssue, "") } - cm.SetRoundTripper(pin.TransportWithPinning()) + return pin.TransportWithPinning() } diff --git a/pkg/connection/check_connection.go b/pkg/connection/check_connection.go index 1182649a..ec542dcd 100644 --- a/pkg/connection/check_connection.go +++ b/pkg/connection/check_connection.go @@ -22,7 +22,7 @@ import ( "fmt" "net/http" - "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/ProtonMail/proton-bridge/pkg/pmapi" ) @@ -30,7 +30,7 @@ import ( var ( ErrNoInternetConnection = errors.New("no internet connection") ErrCanNotReachAPI = errors.New("can not reach PM API") - log = config.GetLogEntry("connection") //nolint[gochecknoglobals] + log = logs.GetLogEntry("connection") //nolint[gochecknoglobals] ) // CheckInternetConnection does a check of API connection. It checks two of our endpoints in parallel. diff --git a/pkg/keychain/keychain.go b/pkg/keychain/keychain.go index 4c25c186..16a7f036 100644 --- a/pkg/keychain/keychain.go +++ b/pkg/keychain/keychain.go @@ -23,7 +23,7 @@ import ( "strings" "sync" - "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/docker/docker-credential-helpers/credentials" ) @@ -32,7 +32,7 @@ const ( ) var ( - log = config.GetLogEntry("bridgeUtils/keychain") //nolint[gochecknoglobals] + log = logs.GetLogEntry("bridgeUtils/keychain") //nolint[gochecknoglobals] ErrWrongKeychainURL = errors.New("wrong keychain base URL") ErrMacKeychainRebuild = errors.New("keychain error -25293") diff --git a/pkg/listener/listener.go b/pkg/listener/listener.go index deb79e6d..66eeef37 100644 --- a/pkg/listener/listener.go +++ b/pkg/listener/listener.go @@ -21,10 +21,10 @@ import ( "sync" "time" - "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" ) -var log = config.GetLogEntry("bridgeUtils/listener") //nolint[gochecknoglobals] +var log = logs.GetLogEntry("bridgeUtils/listener") //nolint[gochecknoglobals] // Listener has a list of channels watching for updates. type Listener interface { diff --git a/pkg/logs/logs.go b/pkg/logs/logs.go new file mode 100644 index 00000000..7dc10abc --- /dev/null +++ b/pkg/logs/logs.go @@ -0,0 +1,10 @@ +package logs + +import "github.com/sirupsen/logrus" + +// GetLogEntry returns logrus.Entry with PID and `packageName`. +func GetLogEntry(packageName string) *logrus.Entry { + return logrus.WithFields(logrus.Fields{ + "pkg": packageName, + }) +} diff --git a/pkg/pmapi/client.go b/pkg/pmapi/client.go index a5e5c57e..66751252 100644 --- a/pkg/pmapi/client.go +++ b/pkg/pmapi/client.go @@ -76,9 +76,6 @@ type ClientConfig struct { // The client ID. ClientID string - // The sentry DSN. - SentryDSN string - // Timeout specifies the timeout from request to getting response headers to our API. // Passed to http.Client, empty means no timeout. Timeout time.Duration diff --git a/pkg/pmapi/clientmanager.go b/pkg/pmapi/clientmanager.go index 1177c75f..a7d226a8 100644 --- a/pkg/pmapi/clientmanager.go +++ b/pkg/pmapi/clientmanager.go @@ -6,7 +6,6 @@ import ( "sync" "time" - "github.com/getsentry/raven-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -59,10 +58,6 @@ type tokenExpiration struct { // NewClientManager creates a new ClientMan which manages clients configured with the given client config. func NewClientManager(config *ClientConfig) (cm *ClientManager) { - if err := raven.SetDSN(config.SentryDSN); err != nil { - logrus.WithError(err).Error("Could not set up sentry DSN") - } - cm = &ClientManager{ config: config, roundTripper: http.DefaultTransport, diff --git a/pkg/updates/updates.go b/pkg/updates/updates.go index 852eca83..d7e74181 100644 --- a/pkg/updates/updates.go +++ b/pkg/updates/updates.go @@ -27,7 +27,7 @@ import ( "runtime" "strings" - "github.com/ProtonMail/proton-bridge/pkg/config" + "github.com/ProtonMail/proton-bridge/pkg/logs" "github.com/kardianos/osext" ) @@ -44,7 +44,7 @@ var ( ) var ( - log = config.GetLogEntry("bridgeUtils/updates") //nolint[gochecknoglobals] + log = logs.GetLogEntry("bridgeUtils/updates") //nolint[gochecknoglobals] installFileSuffix = map[string]string{ //nolint[gochecknoglobals] "darwin": ".dmg", diff --git a/test/context/config.go b/test/context/config.go index c932b47d..37324b8b 100644 --- a/test/context/config.go +++ b/test/context/config.go @@ -50,7 +50,6 @@ func (c *fakeConfig) GetAPIConfig() *pmapi.ClientConfig { return &pmapi.ClientConfig{ AppVersion: "Bridge_" + os.Getenv("VERSION"), ClientID: "bridge", - SentryDSN: "", } } func (c *fakeConfig) GetDBDir() string {