From 3ae34d47b462ee6008cfa8c67dc66001f2894303 Mon Sep 17 00:00:00 2001
From: "vito.he" <hxmhlt@163.com>
Date: Sat, 22 Jun 2019 11:47:01 +0800
Subject: [PATCH] Mod: based on AlexStock's review

---
 common/config/environment.go                  | 11 +++++-----
 common/constant/default.go                    |  4 +++-
 common/extension/config_center_factory.go     |  1 -
 config/base_config.go                         |  6 ++++--
 config/config_loader.go                       |  6 ++++--
 config_center/configuration_parser_test.go    |  4 +++-
 .../dynamic_configuration_factory.go          |  4 +++-
 config_center/mock_dynamic_config.go          |  9 +++++----
 registry/directory/directory.go               |  8 ++++----
 registry/directory/directory_test.go          |  8 ++++----
 registry/zookeeper/listener.go                |  2 +-
 remoting/listener.go                          |  6 +++---
 remoting/zookeeper/client_test.go             | 17 ----------------
 remoting/zookeeper/listener.go                | 20 +++++++++----------
 14 files changed, 50 insertions(+), 56 deletions(-)

diff --git a/common/config/environment.go b/common/config/environment.go
index 7d2f4e598..998f0beef 100644
--- a/common/config/environment.go
+++ b/common/config/environment.go
@@ -34,9 +34,10 @@ type Environment struct {
 	externalConfigMap sync.Map
 }
 
-var instance *Environment
-
-var once sync.Once
+var (
+	instance *Environment
+	once     sync.Once
+)
 
 func GetEnvInstance() *Environment {
 	once.Do(func() {
@@ -82,9 +83,9 @@ func (conf *InmemoryConfiguration) GetProperty(key string) (bool, string) {
 	v, ok := conf.store.Load(key)
 	if ok {
 		return true, v.(string)
-	} else {
-		return false, ""
 	}
+	return false, ""
+
 }
 
 func (conf *InmemoryConfiguration) GetSubProperty(subKey string) map[string]struct{} {
diff --git a/common/constant/default.go b/common/constant/default.go
index 9b49fb05b..d2c622618 100644
--- a/common/constant/default.go
+++ b/common/constant/default.go
@@ -17,7 +17,9 @@
 
 package constant
 
-const DUBBO = "dubbo"
+const (
+	DUBBO = "dubbo"
+)
 const (
 	DEFAULT_WEIGHT = 100     //
 	DEFAULT_WARMUP = 10 * 60 // in java here is 10*60*1000 because of System.currentTimeMillis() is measured in milliseconds & in go time.Unix() is second
diff --git a/common/extension/config_center_factory.go b/common/extension/config_center_factory.go
index b66f40b11..82e0ef6eb 100644
--- a/common/extension/config_center_factory.go
+++ b/common/extension/config_center_factory.go
@@ -34,5 +34,4 @@ func GetConfigCenterFactory(name string) config_center.DynamicConfigurationFacto
 		panic("config center for " + name + " is not existing, make sure you have import the package.")
 	}
 	return configCenterFactories[name]()
-
 }
diff --git a/config/base_config.go b/config/base_config.go
index f15eada0e..19acea2fd 100644
--- a/config/base_config.go
+++ b/config/base_config.go
@@ -78,8 +78,10 @@ func (c *BaseConfig) prepareEnvironment() error {
 }
 
 func getKeyPrefix(val reflect.Value, id reflect.Value) string {
-	var prefix string
-	var idStr string
+	var (
+		prefix string
+		idStr  string
+	)
 	if id.Kind() == reflect.String {
 		idStr = id.Interface().(string)
 	}
diff --git a/config/config_loader.go b/config/config_loader.go
index 927697d40..86260c867 100644
--- a/config/config_loader.go
+++ b/config/config_loader.go
@@ -58,8 +58,10 @@ func init() {
 
 // Dubbo Init
 func Load() {
-	var refMap map[string]*ReferenceConfig
-	var srvMap map[string]*ServiceConfig
+	var (
+		refMap map[string]*ReferenceConfig
+		srvMap map[string]*ServiceConfig
+	)
 
 	// reference config
 	if consumerConfig == nil {
diff --git a/config_center/configuration_parser_test.go b/config_center/configuration_parser_test.go
index b97bc7e8d..3c84fd70b 100644
--- a/config_center/configuration_parser_test.go
+++ b/config_center/configuration_parser_test.go
@@ -1,9 +1,11 @@
 package config_center
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
 )
+import (
+	"github.com/stretchr/testify/assert"
+)
 
 func TestDefaultConfigurationParser_Parser(t *testing.T) {
 	parser := &DefaultConfigurationParser{}
diff --git a/config_center/dynamic_configuration_factory.go b/config_center/dynamic_configuration_factory.go
index 95f074e28..0720896fb 100644
--- a/config_center/dynamic_configuration_factory.go
+++ b/config_center/dynamic_configuration_factory.go
@@ -17,7 +17,9 @@
 
 package config_center
 
-import "github.com/apache/dubbo-go/common"
+import (
+	"github.com/apache/dubbo-go/common"
+)
 
 type DynamicConfigurationFactory interface {
 	GetDynamicConfiguration(*common.URL) (DynamicConfiguration, error)
diff --git a/config_center/mock_dynamic_config.go b/config_center/mock_dynamic_config.go
index 610817e18..a6c7267a4 100644
--- a/config_center/mock_dynamic_config.go
+++ b/config_center/mock_dynamic_config.go
@@ -25,11 +25,12 @@ import (
 	"github.com/apache/dubbo-go/remoting"
 )
 
-type MockDynamicConfigurationFactory struct {
-}
+type MockDynamicConfigurationFactory struct{}
 
-var once sync.Once
-var dynamicConfiguration *mockDynamicConfiguration
+var (
+	once                 sync.Once
+	dynamicConfiguration *mockDynamicConfiguration
+)
 
 func (f *MockDynamicConfigurationFactory) GetDynamicConfiguration(url *common.URL) (DynamicConfiguration, error) {
 	var err error
diff --git a/registry/directory/directory.go b/registry/directory/directory.go
index a4a926315..11687f82e 100644
--- a/registry/directory/directory.go
+++ b/registry/directory/directory.go
@@ -131,11 +131,11 @@ func (dir *registryDirectory) update(res *registry.ServiceEvent) {
 func (dir *registryDirectory) refreshInvokers(res *registry.ServiceEvent) {
 
 	switch res.Action {
-	case remoting.Add:
-		//dir.cacheService.Add(res.Path, dir.serviceTTL)
+	case remoting.EventTypeAdd:
+		//dir.cacheService.EventTypeAdd(res.Path, dir.serviceTTL)
 		dir.cacheInvoker(res.Service)
-	case remoting.Del:
-		//dir.cacheService.Del(res.Path, dir.serviceTTL)
+	case remoting.EventTypeDel:
+		//dir.cacheService.EventTypeDel(res.Path, dir.serviceTTL)
 		dir.uncacheInvoker(res.Service)
 		logger.Infof("selector delete service url{%s}", res.Service)
 	default:
diff --git a/registry/directory/directory_test.go b/registry/directory/directory_test.go
index a40452756..f31165d0a 100644
--- a/registry/directory/directory_test.go
+++ b/registry/directory/directory_test.go
@@ -51,7 +51,7 @@ func TestSubscribe_Delete(t *testing.T) {
 	registryDirectory, mockRegistry := normalRegistryDir()
 	time.Sleep(1e9)
 	assert.Len(t, registryDirectory.cacheInvokers, 3)
-	mockRegistry.MockEvent(&registry.ServiceEvent{Action: remoting.Del, Service: *common.NewURLWithOptions(common.WithPath("TEST0"), common.WithProtocol("dubbo"))})
+	mockRegistry.MockEvent(&registry.ServiceEvent{Action: remoting.EventTypeDel, Service: *common.NewURLWithOptions(common.WithPath("TEST0"), common.WithProtocol("dubbo"))})
 	time.Sleep(1e9)
 	assert.Len(t, registryDirectory.cacheInvokers, 2)
 }
@@ -81,7 +81,7 @@ func TestSubscribe_Group(t *testing.T) {
 	urlmap.Set(constant.GROUP_KEY, "group1")
 	urlmap.Set(constant.CLUSTER_KEY, "failover") //to test merge url
 	for i := 0; i < 3; i++ {
-		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.Add, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"),
+		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.EventTypeAdd, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"),
 			common.WithParams(urlmap))})
 	}
 	//for group2
@@ -89,7 +89,7 @@ func TestSubscribe_Group(t *testing.T) {
 	urlmap2.Set(constant.GROUP_KEY, "group2")
 	urlmap2.Set(constant.CLUSTER_KEY, "failover") //to test merge url
 	for i := 0; i < 3; i++ {
-		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.Add, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"),
+		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.EventTypeAdd, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"),
 			common.WithParams(urlmap2))})
 	}
 
@@ -129,7 +129,7 @@ func normalRegistryDir() (*registryDirectory, *registry.MockRegistry) {
 
 	go registryDirectory.Subscribe(*common.NewURLWithOptions(common.WithPath("testservice")))
 	for i := 0; i < 3; i++ {
-		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.Add, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"))})
+		mockRegistry.(*registry.MockRegistry).MockEvent(&registry.ServiceEvent{Action: remoting.EventTypeAdd, Service: *common.NewURLWithOptions(common.WithPath("TEST"+strconv.FormatInt(int64(i), 10)), common.WithProtocol("dubbo"))})
 	}
 	return registryDirectory, mockRegistry.(*registry.MockRegistry)
 }
diff --git a/registry/zookeeper/listener.go b/registry/zookeeper/listener.go
index 99e4559b7..67f203706 100644
--- a/registry/zookeeper/listener.go
+++ b/registry/zookeeper/listener.go
@@ -89,7 +89,7 @@ func (l *RegistryConfigurationListener) Next() (*registry.ServiceEvent, error) {
 
 		case e := <-l.events:
 			logger.Debugf("got zk event %s", e)
-			if e.ConfigType == remoting.Del && !l.valid() {
+			if e.ConfigType == remoting.EventTypeDel && !l.valid() {
 				logger.Warnf("update @result{%s}. But its connection to registry is invalid", e.Value)
 				continue
 			}
diff --git a/remoting/listener.go b/remoting/listener.go
index 1bf5b5284..da30f6989 100644
--- a/remoting/listener.go
+++ b/remoting/listener.go
@@ -44,9 +44,9 @@ func (c ConfigChangeEvent) String() string {
 type EventType int
 
 const (
-	Add = iota
-	Del
-	Mod
+	EventTypeAdd = iota
+	EventTypeDel
+	EvnetTypeUpdate
 )
 
 var serviceEventTypeStrings = [...]string{
diff --git a/remoting/zookeeper/client_test.go b/remoting/zookeeper/client_test.go
index 4a71ebd61..f1bd0c2cb 100644
--- a/remoting/zookeeper/client_test.go
+++ b/remoting/zookeeper/client_test.go
@@ -48,23 +48,6 @@ func verifyEventStateOrder(t *testing.T, c <-chan zk.Event, expectedStates []zk.
 	}
 }
 
-func verifyEventOrder(t *testing.T, c <-chan zk.Event, expectedEvent []zk.EventType, source string) {
-	for _, e := range expectedEvent {
-		for {
-			event, ok := <-c
-			if !ok {
-				t.Fatalf("unexpected channel close for %s", source)
-			}
-
-			if event.Type != e {
-				t.Fatalf("mismatched state order from %s, expected %v, received %v", source, event, event.Type)
-			}
-
-			break
-		}
-	}
-}
-
 //func Test_newZookeeperClient(t *testing.T) {
 //	ts, err := zk.StartTestCluster(1, nil, nil)
 //	if err != nil {
diff --git a/remoting/zookeeper/listener.go b/remoting/zookeeper/listener.go
index 089092d23..af668a1aa 100644
--- a/remoting/zookeeper/listener.go
+++ b/remoting/zookeeper/listener.go
@@ -69,14 +69,14 @@ func (l *ZkEventListener) ListenServiceNodeEvent(zkPath string, listener ...remo
 				logger.Warnf("zk.ExistW(key{%s}) = event{EventNodeDataChanged}", zkPath)
 				if len(listener) > 0 {
 					content, _, _ := l.client.Conn.Get(zkEvent.Path)
-					listener[0].DataChange(remoting.Event{Path: zkEvent.Path, Action: remoting.Mod, Content: string(content)})
+					listener[0].DataChange(remoting.Event{Path: zkEvent.Path, Action: remoting.EvnetTypeUpdate, Content: string(content)})
 				}
 
 			case zk.EventNodeCreated:
 				logger.Warnf("zk.ExistW(key{%s}) = event{EventNodeCreated}", zkPath)
 				if len(listener) > 0 {
 					content, _, _ := l.client.Conn.Get(zkEvent.Path)
-					listener[0].DataChange(remoting.Event{Path: zkEvent.Path, Action: remoting.Add, Content: string(content)})
+					listener[0].DataChange(remoting.Event{Path: zkEvent.Path, Action: remoting.EventTypeAdd, Content: string(content)})
 				}
 			case zk.EventNotWatching:
 				logger.Warnf("zk.ExistW(key{%s}) = event{EventNotWatching}", zkPath)
@@ -125,7 +125,7 @@ func (l *ZkEventListener) handleZkNodeEvent(zkPath string, children []string, li
 			logger.Errorf("Get new node path {%v} 's content error,message is  {%v}", newNode, perrors.WithStack(err))
 		}
 
-		if !listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.Add, Content: string(content)}) {
+		if !listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.EventTypeAdd, Content: string(content)}) {
 			continue
 		}
 		// listen l service node
@@ -133,7 +133,7 @@ func (l *ZkEventListener) handleZkNodeEvent(zkPath string, children []string, li
 			logger.Infof("delete zkNode{%s}", node)
 			if l.ListenServiceNodeEvent(node, listener) {
 				logger.Infof("delete content{%s}", n)
-				listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.Del})
+				listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.EventTypeDel})
 			}
 			logger.Warnf("listenSelf(zk path{%s}) goroutine exit now", zkPath)
 		}(newNode)
@@ -153,7 +153,7 @@ func (l *ZkEventListener) handleZkNodeEvent(zkPath string, children []string, li
 			logger.Errorf("NewURL(i{%s}) = error{%v}", n, perrors.WithStack(err))
 			continue
 		}
-		listener.DataChange(remoting.Event{Path: oldNode, Action: remoting.Del})
+		listener.DataChange(remoting.Event{Path: oldNode, Action: remoting.EventTypeDel})
 	}
 }
 
@@ -212,13 +212,13 @@ func (l *ZkEventListener) listenDirEvent(zkPath string, listener remoting.DataLi
 				logger.Errorf("Get new node path {%v} 's content error,message is  {%v}", dubboPath, perrors.WithStack(err))
 			}
 			logger.Infof("Get children!{%s}", dubboPath)
-			if !listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.Add, Content: string(content)}) {
+			if !listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.EventTypeAdd, Content: string(content)}) {
 				continue
 			}
 			logger.Infof("listen dubbo service key{%s}", dubboPath)
 			go func(zkPath string) {
 				if l.ListenServiceNodeEvent(dubboPath) {
-					listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.Del})
+					listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.EventTypeDel})
 				}
 				logger.Warnf("listenSelf(zk path{%s}) goroutine exit now", zkPath)
 			}(dubboPath)
@@ -246,7 +246,7 @@ func (l *ZkEventListener) listenDirEvent(zkPath string, listener remoting.DataLi
 
 //
 //func (l *ZkEventListener) listenFileEvent(zkPath string, listener remoting.DataListener) {
-//	l.wg.Add(1)
+//	l.wg.EventTypeAdd(1)
 //	defer l.wg.Done()
 //
 //	var (
@@ -347,13 +347,13 @@ func (l *ZkEventListener) ListenServiceEvent(zkPath string, listener remoting.Da
 		if err != nil {
 			logger.Errorf("Get new node path {%v} 's content error,message is  {%v}", dubboPath, perrors.WithStack(err))
 		}
-		if !listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.Add, Content: string(content)}) {
+		if !listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.EventTypeAdd, Content: string(content)}) {
 			continue
 		}
 		logger.Infof("listen dubbo service key{%s}", dubboPath)
 		go func(zkPath string) {
 			if l.ListenServiceNodeEvent(dubboPath) {
-				listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.Del})
+				listener.DataChange(remoting.Event{Path: dubboPath, Action: remoting.EventTypeDel})
 			}
 			logger.Warnf("listenSelf(zk path{%s}) goroutine exit now", zkPath)
 		}(dubboPath)
-- 
GitLab