Refactor config methods for internal use (cadence-workflow#4448)

- Export config validate() and fillDefaults() methods for internal use
- Move logic that will change persistence config values to pesistenceConfig.FillDefaults()

common/config/cluster.go

@@ -80,7 +80,8 @@ type (
 	}
 )
 
-func (m *ClusterGroupMetadata) validate() error {
+// Validate validates ClusterGroupMetadata
+func (m *ClusterGroupMetadata) Validate() error {
 	if m == nil {
 		return errors.New("ClusterGroupMetadata cannot be empty")
 	}
@@ -142,7 +143,8 @@ func (m *ClusterGroupMetadata) validate() error {
 	return errs
 }
 
-func (m *ClusterGroupMetadata) fillDefaults() {
+// FillDefaults populates default values for unspecified fields
+func (m *ClusterGroupMetadata) FillDefaults() {
 	if m == nil {
 		return
 	}

common/config/cluster_test.go

@@ -34,7 +34,7 @@ func TestClusterGroupMetadataDefaults(t *testing.T) {
 		},
 	}
 
-	config.fillDefaults()
+	config.FillDefaults()
 
 	assert.Equal(t, "active", config.PrimaryClusterName)
 	assert.Equal(t, "cadence-frontend", config.ClusterGroup["active"].RPCName)
@@ -164,7 +164,7 @@ func TestClusterGroupMetadataValidate(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.msg, func(t *testing.T) {
-			err := tt.config.validate()
+			err := tt.config.Validate()
 			if tt.err == "" {
 				assert.NoError(t, err)
 			} else {

common/config/config.go

@@ -29,7 +29,6 @@ import (
 	"github.com/uber-go/tally/prometheus"
 	"github.com/uber/ringpop-go/discovery"
 
-	"github.com/uber/cadence/common"
 	"github.com/uber/cadence/common/dynamicconfig"
 	c "github.com/uber/cadence/common/dynamicconfig/configstore/config"
 )
@@ -445,7 +444,7 @@ func (c *Config) validate() error {
 	if err := c.Persistence.Validate(); err != nil {
 		return err
 	}
-	if err := c.ClusterGroupMetadata.validate(); err != nil {
+	if err := c.ClusterGroupMetadata.Validate(); err != nil {
 		return err
 	}
 	if err := c.Archival.Validate(&c.DomainDefaults.Archival); err != nil {
@@ -456,28 +455,15 @@ func (c *Config) validate() error {
 }
 
 func (c *Config) fillDefaults() {
-	// filling default encodingType/decodingTypes for SQL persistence
-	for k, store := range c.Persistence.DataStores {
-		if store.SQL != nil {
-			if store.SQL.EncodingType == "" {
-				store.SQL.EncodingType = string(common.EncodingTypeThriftRW)
-			}
-			if len(store.SQL.DecodingTypes) == 0 {
-				store.SQL.DecodingTypes = []string{
-					string(common.EncodingTypeThriftRW),
-				}
-			}
-			c.Persistence.DataStores[k] = store
-		}
-	}
+	c.Persistence.FillDefaults()
 
 	// TODO: remove this after 0.23 and mention a breaking change in config.
 	if c.ClusterGroupMetadata == nil && c.ClusterMetadata != nil {
 		c.ClusterGroupMetadata = c.ClusterMetadata
 		log.Println("[WARN] clusterMetadata config is deprecated. Please replace it with clusterGroupMetadata.")
 	}
 
-	c.ClusterGroupMetadata.fillDefaults()
+	c.ClusterGroupMetadata.FillDefaults()
 
 	// filling publicClient with current cluster's RPC address if empty
 	if c.PublicClient.HostPort == "" && c.ClusterGroupMetadata != nil {

common/config/persistence.go

@@ -20,7 +20,11 @@
 
 package config
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/uber/cadence/common"
+)
 
 const (
 	// StoreTypeSQL refers to sql based storage as persistence store
@@ -37,6 +41,36 @@ func (c *Persistence) DefaultStoreType() string {
 	return StoreTypeCassandra
 }
 
+// FillDefaults populates default values for unspecified fields in persistence config
+func (c *Persistence) FillDefaults() {
+	for k, store := range c.DataStores {
+		if store.Cassandra != nil && store.NoSQL == nil {
+			// for backward-compatibility
+			store.NoSQL = store.Cassandra
+			store.NoSQL.PluginName = "cassandra"
+		}
+
+		if store.SQL != nil {
+			// filling default encodingType/decodingTypes for SQL persistence
+			if store.SQL.EncodingType == "" {
+				store.SQL.EncodingType = string(common.EncodingTypeThriftRW)
+			}
+			if len(store.SQL.DecodingTypes) == 0 {
+				store.SQL.DecodingTypes = []string{
+					string(common.EncodingTypeThriftRW),
+				}
+			}
+
+			if store.SQL.NumShards == 0 {
+				store.SQL.NumShards = 1
+			}
+		}
+
+		// write changes back to DataStores, as ds is a value object
+		c.DataStores[k] = store
+	}
+}
+
 // Validate validates the persistence config
 func (c *Persistence) Validate() error {
 	dbStoreKeys := []string{c.DefaultStore}
@@ -45,7 +79,7 @@ func (c *Persistence) Validate() error {
 		dbStoreKeys = append(dbStoreKeys, c.VisibilityStore)
 	} else {
 		if _, ok := c.DataStores[c.AdvancedVisibilityStore]; !ok {
-			return fmt.Errorf(" Must provide one of VisibilityStore and AdvancedVisibilityStore")
+			return fmt.Errorf("must provide one of VisibilityStore and AdvancedVisibilityStore")
 		}
 	}
 
@@ -54,26 +88,17 @@ func (c *Persistence) Validate() error {
 		if !ok {
 			return fmt.Errorf("persistence config: missing config for datastore %v", st)
 		}
-		if ds.Cassandra != nil {
-			if ds.NoSQL != nil {
-				return fmt.Errorf("persistence config: datastore %v: only one of Cassandra or NoSQL can be specified", st)
-			}
-			// for backward-compatibility
-			ds.NoSQL = ds.Cassandra
-			ds.NoSQL.PluginName = "cassandra"
+		if ds.Cassandra != nil && ds.NoSQL != nil && ds.Cassandra != ds.NoSQL {
+			return fmt.Errorf("persistence config: datastore %v: only one of Cassandra or NoSQL can be specified", st)
 		}
 		if ds.SQL == nil && ds.NoSQL == nil {
 			return fmt.Errorf("persistence config: datastore %v: must provide config for one of SQL or NoSQL stores", st)
 		}
 		if ds.SQL != nil && ds.NoSQL != nil {
 			return fmt.Errorf("persistence config: datastore %v: only one of SQL or NoSQL can be specified", st)
 		}
-		if ds.SQL != nil && ds.SQL.NumShards == 0 {
-			ds.SQL.NumShards = 1
-		}
-		// write changes back to DataStores, as ds is a value object
-		c.DataStores[st] = ds
 	}
+
 	return nil
 }