From a4c44376819d41c093ed8ef2a2aaae959a6e0ebf Mon Sep 17 00:00:00 2001 From: Andy Librian Date: Tue, 7 Jan 2020 23:00:56 +0700 Subject: [PATCH] Golint fix config (#13544) --- .golangci.yml | 2 +- config/client.go | 8 ++--- config/client_test.go | 8 ++--- .../config_generator/generator/generator.go | 1 + config/database_test.go | 14 ++++---- config/file.go | 1 + config/memory.go | 32 +++++++++---------- config/migrate.go | 1 + config/utils_test.go | 4 +-- 9 files changed, 37 insertions(+), 34 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3b44f5f19f..2770dea477 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -39,4 +39,4 @@ issues: - linters: # ignore golint error for a lot of packages for now - golint - path: "api4|app|cmd|config|einterface|enterprise|imports|jobs|manualtesting|migrations|mlog|model|testlib|services|store|utils|web|wsapi|plugin/api.go|plugin/context.go|plugin/client.go|plugin/client_rpc.go|plugin/environment.go|plugin/health_check.go|plugin/hooks.go|plugin/supervisor.go|plugin/valid.go" + path: "api4|app|cmd|einterface|enterprise|imports|jobs|manualtesting|migrations|mlog|model|testlib|services|store|utils|web|wsapi|plugin/api.go|plugin/context.go|plugin/client.go|plugin/client_rpc.go|plugin/environment.go|plugin/health_check.go|plugin/hooks.go|plugin/supervisor.go|plugin/valid.go" diff --git a/config/client.go b/config/client.go index 3b677a099e..1b93f31c51 100644 --- a/config/client.go +++ b/config/client.go @@ -12,8 +12,8 @@ import ( ) // GenerateClientConfig renders the given configuration for a client. -func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.License) map[string]string { - props := GenerateLimitedClientConfig(c, diagnosticId, license) +func GenerateClientConfig(c *model.Config, diagnosticID string, license *model.License) map[string]string { + props := GenerateLimitedClientConfig(c, diagnosticID, license) props["SiteURL"] = strings.TrimRight(*c.ServiceSettings.SiteURL, "/") props["EnableUserDeactivation"] = strconv.FormatBool(*c.TeamSettings.EnableUserDeactivation) @@ -198,7 +198,7 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L } // GenerateLimitedClientConfig renders the given configuration for an untrusted client. -func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license *model.License) map[string]string { +func GenerateLimitedClientConfig(c *model.Config, diagnosticID string, license *model.License) map[string]string { props := make(map[string]string) props["Version"] = model.CurrentVersion @@ -250,7 +250,7 @@ func GenerateLimitedClientConfig(c *model.Config, diagnosticId string, license * props["AndroidAppDownloadLink"] = *c.NativeAppSettings.AndroidAppDownloadLink props["IosAppDownloadLink"] = *c.NativeAppSettings.IosAppDownloadLink - props["DiagnosticId"] = diagnosticId + props["DiagnosticId"] = diagnosticID props["DiagnosticsEnabled"] = strconv.FormatBool(*c.LogSettings.EnableDiagnostics) props["HasImageProxy"] = strconv.FormatBool(*c.ImageProxySettings.Enable) diff --git a/config/client_test.go b/config/client_test.go index 6a35036c06..238e7b2fca 100644 --- a/config/client_test.go +++ b/config/client_test.go @@ -18,7 +18,7 @@ func TestGetClientConfig(t *testing.T) { testCases := []struct { description string config *model.Config - diagnosticId string + diagnosticID string license *model.License expectedFields map[string]string }{ @@ -177,7 +177,7 @@ func TestGetClientConfig(t *testing.T) { testCase.license.Features.SetDefaults() } - configMap := config.GenerateClientConfig(testCase.config, testCase.diagnosticId, testCase.license) + configMap := config.GenerateClientConfig(testCase.config, testCase.diagnosticID, testCase.license) for expectedField, expectedValue := range testCase.expectedFields { actualValue, ok := configMap[expectedField] if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { @@ -193,7 +193,7 @@ func TestGetLimitedClientConfig(t *testing.T) { testCases := []struct { description string config *model.Config - diagnosticId string + diagnosticID string license *model.License expectedFields map[string]string }{ @@ -235,7 +235,7 @@ func TestGetLimitedClientConfig(t *testing.T) { testCase.license.Features.SetDefaults() } - configMap := config.GenerateLimitedClientConfig(testCase.config, testCase.diagnosticId, testCase.license) + configMap := config.GenerateLimitedClientConfig(testCase.config, testCase.diagnosticID, testCase.license) for expectedField, expectedValue := range testCase.expectedFields { actualValue, ok := configMap[expectedField] if assert.True(t, ok, fmt.Sprintf("config does not contain %v", expectedField)) { diff --git a/config/config_generator/generator/generator.go b/config/config_generator/generator/generator.go index 4fea9d004b..9a531311eb 100644 --- a/config/config_generator/generator/generator.go +++ b/config/config_generator/generator/generator.go @@ -10,6 +10,7 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) +// GenerateDefaultConfig writes default config to outputFile. func GenerateDefaultConfig(outputFile *os.File) error { defaultCfg := &model.Config{} defaultCfg.SetDefaults() diff --git a/config/database_test.go b/config/database_test.go index 0b5c7b5a3d..bb8e38817b 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -61,7 +61,7 @@ func getActualDatabaseConfig(t *testing.T) (string, *model.Config) { t.Helper() var actual struct { - Id string `db:"Id"` + ID string `db:"Id"` Value []byte `db:"Value"` } db := sqlx.NewDb(mainHelper.GetSqlSupplier().GetMaster().Db, *mainHelper.GetSqlSettings().DriverName) @@ -71,7 +71,7 @@ func getActualDatabaseConfig(t *testing.T) (string, *model.Config) { actualCfg, _, err := config.UnmarshalConfig(bytes.NewReader(actual.Value), false) require.Nil(t, err) - return actual.Id, actualCfg + return actual.ID, actualCfg } // assertDatabaseEqualsConfig verifies the active in-database configuration equals the given config. @@ -399,12 +399,12 @@ func TestDatabaseStoreSet(t *testing.T) { _, err = ds.Set(ds.Get()) require.NoError(t, err) - beforeId, _ := getActualDatabaseConfig(t) + beforeID, _ := getActualDatabaseConfig(t) _, err = ds.Set(ds.Get()) require.NoError(t, err) - afterId, _ := getActualDatabaseConfig(t) - assert.Equal(t, beforeId, afterId, "new record should not have been written") + afterID, _ := getActualDatabaseConfig(t) + assert.Equal(t, beforeID, afterID, "new record should not have been written") }) t.Run("read-only ignored", func(t *testing.T) { @@ -489,7 +489,7 @@ func TestDatabaseStoreSet(t *testing.T) { }) t.Run("listeners notified", func(t *testing.T) { - activeId, tearDown := setupConfigDatabase(t, emptyConfig, nil) + activeID, tearDown := setupConfigDatabase(t, emptyConfig, nil) defer tearDown() ds, err := config.NewDatabaseStore(fmt.Sprintf("%s://%s", *sqlSettings.DriverName, *sqlSettings.DataSource)) @@ -511,7 +511,7 @@ func TestDatabaseStoreSet(t *testing.T) { assert.Equal(t, oldCfg, retCfg) id, _ := getActualDatabaseConfig(t) - assert.NotEqual(t, activeId, id, "new record should have been written") + assert.NotEqual(t, activeID, id, "new record should have been written") require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written") }) diff --git a/config/file.go b/config/file.go index 7ef396ce58..73ff3cb6f5 100644 --- a/config/file.go +++ b/config/file.go @@ -19,6 +19,7 @@ import ( ) var ( + // ErrReadOnlyConfiguration is returned when an attempt to modify a read-only configuration is made. ErrReadOnlyConfiguration = errors.New("configuration is read-only") ) diff --git a/config/memory.go b/config/memory.go index ceaf23f251..639a933bdf 100644 --- a/config/memory.go +++ b/config/memory.go @@ -13,8 +13,8 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) -// memoryStore implements the Store interface. It is meant primarily for testing. -type memoryStore struct { +// MemoryStore implements the Store interface. It is meant primarily for testing. +type MemoryStore struct { commonStore allowEnvironmentOverrides bool @@ -31,13 +31,13 @@ type MemoryStoreOptions struct { InitialFiles map[string][]byte } -// NewMemoryStore creates a new memoryStore instance with default options. -func NewMemoryStore() (*memoryStore, error) { +// NewMemoryStore creates a new MemoryStore instance with default options. +func NewMemoryStore() (*MemoryStore, error) { return NewMemoryStoreWithOptions(&MemoryStoreOptions{}) } -// NewMemoryStoreWithOptions creates a new memoryStore instance. -func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*memoryStore, error) { +// NewMemoryStoreWithOptions creates a new MemoryStore instance. +func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*MemoryStore, error) { savedConfig := options.InitialConfig if savedConfig == nil { savedConfig = &model.Config{} @@ -49,7 +49,7 @@ func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*memoryStore, error initialFiles = make(map[string][]byte) } - ms := &memoryStore{ + ms := &MemoryStore{ allowEnvironmentOverrides: !options.IgnoreEnvironmentOverrides, validate: !options.SkipValidation, files: initialFiles, @@ -67,7 +67,7 @@ func NewMemoryStoreWithOptions(options *MemoryStoreOptions) (*memoryStore, error } // Set replaces the current configuration in its entirety. -func (ms *memoryStore) Set(newCfg *model.Config) (*model.Config, error) { +func (ms *MemoryStore) Set(newCfg *model.Config) (*model.Config, error) { validate := ms.commonStore.validate if !ms.validate { validate = nil @@ -77,14 +77,14 @@ func (ms *memoryStore) Set(newCfg *model.Config) (*model.Config, error) { } // persist copies the active config to the saved config. -func (ms *memoryStore) persist(cfg *model.Config) error { +func (ms *MemoryStore) persist(cfg *model.Config) error { ms.savedConfig = cfg.Clone() return nil } // Load applies environment overrides to the default config as if a re-load had occurred. -func (ms *memoryStore) Load() (err error) { +func (ms *MemoryStore) Load() (err error) { var cfgBytes []byte cfgBytes, err = marshalConfig(ms.savedConfig) if err != nil { @@ -102,7 +102,7 @@ func (ms *memoryStore) Load() (err error) { } // GetFile fetches the contents of a previously persisted configuration file. -func (ms *memoryStore) GetFile(name string) ([]byte, error) { +func (ms *MemoryStore) GetFile(name string) ([]byte, error) { ms.configLock.RLock() defer ms.configLock.RUnlock() @@ -115,7 +115,7 @@ func (ms *memoryStore) GetFile(name string) ([]byte, error) { } // SetFile sets or replaces the contents of a configuration file. -func (ms *memoryStore) SetFile(name string, data []byte) error { +func (ms *MemoryStore) SetFile(name string, data []byte) error { ms.configLock.Lock() defer ms.configLock.Unlock() @@ -125,7 +125,7 @@ func (ms *memoryStore) SetFile(name string, data []byte) error { } // HasFile returns true if the given file was previously persisted. -func (ms *memoryStore) HasFile(name string) (bool, error) { +func (ms *MemoryStore) HasFile(name string) (bool, error) { ms.configLock.RLock() defer ms.configLock.RUnlock() @@ -134,7 +134,7 @@ func (ms *memoryStore) HasFile(name string) (bool, error) { } // RemoveFile removes a previously persisted configuration file. -func (ms *memoryStore) RemoveFile(name string) error { +func (ms *MemoryStore) RemoveFile(name string) error { ms.configLock.Lock() defer ms.configLock.Unlock() @@ -144,11 +144,11 @@ func (ms *memoryStore) RemoveFile(name string) error { } // String returns a hard-coded description, as there is no backing store. -func (ms *memoryStore) String() string { +func (ms *MemoryStore) String() string { return "memory://" } // Close does nothing for a memory store. -func (ms *memoryStore) Close() error { +func (ms *MemoryStore) Close() error { return nil } diff --git a/config/migrate.go b/config/migrate.go index de3888223c..a300cebb7f 100644 --- a/config/migrate.go +++ b/config/migrate.go @@ -5,6 +5,7 @@ package config import "github.com/pkg/errors" +// Migrate migrates SAML keys and certificates from one store to another given their data source names. func Migrate(from, to string) error { source, err := NewStore(from, false) if err != nil { diff --git a/config/utils_test.go b/config/utils_test.go index 676f94dca8..4d883102e0 100644 --- a/config/utils_test.go +++ b/config/utils_test.go @@ -53,9 +53,9 @@ func TestDesanitize(t *testing.T) { target.SqlSettings.DataSourceReplicas = append(target.SqlSettings.DataSourceReplicas, "old_replica0") target.SqlSettings.DataSourceSearchReplicas = append(target.SqlSettings.DataSourceReplicas, "old_search_replica0") - actual_clone := actual.Clone() + actualClone := actual.Clone() desanitize(actual, target) - assert.Equal(t, actual_clone, actual, "actual should not have been changed") + assert.Equal(t, actualClone, actual, "actual should not have been changed") // Verify the settings that should have been left untouched in target assert.True(t, *target.LdapSettings.Enable, "LdapSettings.Enable should not have changed")