Merge pull request #519 from flycash/remove-url-lock

remove lock from url

common/url.go

@@ -25,7 +25,6 @@ import (
 	"net/url"
 	"strconv"
 	"strings"
-	"sync"
 )
 
 import (
@@ -79,13 +78,15 @@ type baseUrl struct {
 	Location     string // ip+port
 	Ip           string
 	Port         string
-	//url.Values is not safe map, add to avoid concurrent map read and map write error
-	paramsLock   sync.RWMutex
 	params       url.Values
 	PrimitiveURL string
 }
 
-// URL ...
+// URL is not thread-safe.
+// we fail to define this struct to be immutable object.
+// but, those method which will update the URL, including SetParam, SetParams
+// are only allowed to be invoked in creating URL instance
+// Please keep in mind that this struct is immutable after it has been created and initialized.
 type URL struct {
 	baseUrl
 	Path     string // like  /com.ikurento.dubbo.UserProvider3
@@ -301,9 +302,7 @@ func (c URL) String() string {
 			"%s://%s:%s@%s:%s%s?",
 			c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
 	}
-	c.paramsLock.RLock()
 	buf.WriteString(c.params.Encode())
-	c.paramsLock.RUnlock()
 	return buf.String()
 }
 
@@ -381,24 +380,24 @@ func (c URL) Service() string {
 	return ""
 }
 
-// AddParam ...
+// AddParam will add the key-value pair
+// Not thread-safe
+// think twice before using it.
 func (c *URL) AddParam(key string, value string) {
-	c.paramsLock.Lock()
 	c.params.Add(key, value)
-	c.paramsLock.Unlock()
 }
 
-// SetParam ...
+// SetParam will put the key-value pair into url
+// it's not thread safe.
+// think twice before you want to use this method
+// usually it should only be invoked when you want to initialized an url
 func (c *URL) SetParam(key string, value string) {
-	c.paramsLock.Lock()
 	c.params.Set(key, value)
-	c.paramsLock.Unlock()
 }
 
-// RangeParams ...
+// RangeParams will iterate the params
+// it's not thread-safe
 func (c *URL) RangeParams(f func(key, value string) bool) {
-	c.paramsLock.RLock()
-	defer c.paramsLock.RUnlock()
 	for k, v := range c.params {
 		if !f(k, v[0]) {
 			break
@@ -408,8 +407,6 @@ func (c *URL) RangeParams(f func(key, value string) bool) {
 
 // GetParam ...
 func (c URL) GetParam(s string, d string) string {
-	c.paramsLock.RLock()
-	defer c.paramsLock.RUnlock()
 	r := c.params.Get(s)
 	if len(r) == 0 {
 		r = d
@@ -424,8 +421,6 @@ func (c URL) GetParams() url.Values {
 
 // GetParamAndDecoded ...
 func (c URL) GetParamAndDecoded(key string) (string, error) {
-	c.paramsLock.RLock()
-	defer c.paramsLock.RUnlock()
 	ruleDec, err := base64.URLEncoding.DecodeString(c.GetParam(key, ""))
 	value := string(ruleDec)
 	return value, err
@@ -502,17 +497,10 @@ func (c URL) GetMethodParamBool(method string, key string, d bool) bool {
 	return r
 }
 
-// RemoveParams ...
-func (c *URL) RemoveParams(set *gxset.HashSet) {
-	c.paramsLock.Lock()
-	defer c.paramsLock.Unlock()
-	for k := range set.Items {
-		s := k.(string)
-		delete(c.params, s)
-	}
-}
-
-// SetParams ...
+// SetParams will put all key-value pair into url.
+// 1. if there already has same key, the value will be override
+// 2. it's not thread safe
+// 3. think twice when you want to invoke this method
 func (c *URL) SetParams(m url.Values) {
 	for k := range m {
 		c.SetParam(k, m.Get(k))
@@ -563,7 +551,13 @@ func (c URL) ToMap() map[string]string {
 //  in this function we should merge the reference local url config into the service url from registry.
 // TODO configuration merge, in the future , the configuration center's config should merge too.
 
-// MergeUrl ...
+// MergeUrl will merge those two url
+// the result is based on serviceUrl, and the key which si only contained in referenceUrl
+// will be added into result.
+// for example, if serviceUrl contains params (a1->v1, b1->v2) and referenceUrl contains params(a2->v3, b1 -> v4)
+// the params of result will be (a1->v1, b1->v2, a2->v3).
+// You should notice that the value of b1 is v2, not v4.
+// due to URL is not thread-safe, so this method is not thread-safe
 func MergeUrl(serviceUrl *URL, referenceUrl *URL) *URL {
 	mergedUrl := serviceUrl.Clone()
 
@@ -593,7 +587,7 @@ func MergeUrl(serviceUrl *URL, referenceUrl *URL) *URL {
 	return mergedUrl
 }
 
-// Clone ...
+// Clone will copy the url
 func (c *URL) Clone() *URL {
 	newUrl := &URL{}
 	copier.Copy(newUrl, c)
@@ -605,6 +599,19 @@ func (c *URL) Clone() *URL {
 	return newUrl
 }
 
+func (c *URL) CloneExceptParams(excludeParams *gxset.HashSet) *URL {
+	newUrl := &URL{}
+	copier.Copy(newUrl, c)
+	newUrl.params = url.Values{}
+	c.RangeParams(func(key, value string) bool {
+		if !excludeParams.Contains(key) {
+			newUrl.SetParam(key, value)
+		}
+		return true
+	})
+	return newUrl
+}
+
 // Copy url based on the reserved parameters' keys.
 func (c *URL) CloneWithParams(reserveParams []string) *URL {
 	params := url.Values{}

config_center/configurator/override.go

@@ -72,7 +72,7 @@ func (c *overrideConfigurator) Configure(url *common.URL) {
 	}
 }
 
-//translate from java, compatible rules in java
+// configureIfMatch translate from java, compatible rules in java
 func (c *overrideConfigurator) configureIfMatch(host string, url *common.URL) {
 	if constant.ANYHOST_VALUE == c.configuratorUrl.Ip || host == c.configuratorUrl.Ip {
 		providers := c.configuratorUrl.GetParam(constant.OVERRIDE_PROVIDERS_KEY, "")
@@ -105,8 +105,7 @@ func (c *overrideConfigurator) configureIfMatch(host string, url *common.URL) {
 				if returnUrl {
 					return
 				}
-				configUrl := c.configuratorUrl.Clone()
-				configUrl.RemoveParams(conditionKeys)
+				configUrl := c.configuratorUrl.CloneExceptParams(conditionKeys)
 				url.SetParams(configUrl.GetParams())
 			}
 		}

registry/protocol/protocol.go

@@ -70,10 +70,8 @@ func init() {
 }
 
 func getCacheKey(url *common.URL) string {
-	newUrl := url.Clone()
 	delKeys := gxset.NewSet("dynamic", "enabled")
-	newUrl.RemoveParams(delKeys)
-	return newUrl.String()
+	return url.CloneExceptParams(delKeys).String()
 }
 
 func newRegistryProtocol() *registryProtocol {
@@ -104,15 +102,13 @@ func getUrlToRegistry(providerUrl *common.URL, registryUrl *common.URL) *common.
 func filterHideKey(url *common.URL) *common.URL {
 
 	// be careful params maps in url is map type
-	cloneURL := url.Clone()
 	removeSet := gxset.NewSet()
-	for k, _ := range cloneURL.GetParams() {
+	for k, _ := range url.GetParams() {
 		if strings.HasPrefix(k, ".") {
 			removeSet.Add(k)
 		}
 	}
-	cloneURL.RemoveParams(removeSet)
-	return cloneURL
+	return url.CloneExceptParams(removeSet)
 }
 
 func (proto *registryProtocol) initConfigurationListeners() {