From 62bdc06ad1f0006d15ab53fe3acf6c699a1e6ee1 Mon Sep 17 00:00:00 2001 From: cvictory Date: Sat, 7 Nov 2020 12:31:42 +0800 Subject: [PATCH 1/4] Impl: support extension of two urls comparison. --- cluster/router/chain/chain.go | 4 +- common/url_tool_extension.go | 66 ++++++++++++++++++++++++ common/url_tool_extension_test.go | 84 +++++++++++++++++++++++++++++++ registry/directory/directory.go | 3 +- 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 common/url_tool_extension.go create mode 100644 common/url_tool_extension_test.go diff --git a/cluster/router/chain/chain.go b/cluster/router/chain/chain.go index 952aedf92d..b4c7cea47b 100644 --- a/cluster/router/chain/chain.go +++ b/cluster/router/chain/chain.go @@ -288,7 +288,9 @@ func isInvokersChanged(left []protocol.Invoker, right []protocol.Invoker) bool { for _, r := range right { found := false for _, l := range left { - if common.IsEquals(l.GetUrl(), r.GetUrl(), constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY) { + lurl := l.GetUrl() + rurl := r.GetUrl() + if common.GetURLTool().CompareURLEqual(&lurl, &rurl, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY) { found = true break } diff --git a/common/url_tool_extension.go b/common/url_tool_extension.go new file mode 100644 index 0000000000..84899fca51 --- /dev/null +++ b/common/url_tool_extension.go @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package common + +import "sync" + +var urlTool URLTool +var lock sync.Mutex + +// Define some func for URL, such as comparison of URL. +// Your can define your own implements, and invoke SetURLTool for high priority. +type URLTool interface { + // The higher of the number, the higher of the priority. + Priority() uint8 + // comparison of two URL, excluding some params + CompareURLEqual(*URL, *URL, ...string) bool +} + +func SetURLTool(tool URLTool) { + lock.Lock() + defer lock.Unlock() + if urlTool == nil { + urlTool = tool + return + } + if urlTool.Priority() < tool.Priority() { + urlTool = tool + } +} +func GetURLTool() URLTool { + return urlTool +} + +// Config default urlTools. +func init() { + SetURLTool(defaultURLTool{}) +} + +type defaultURLTool struct { +} + +// default priority is 16. +func (defaultURLTool) Priority() uint8 { + //default is 16. + return 16 +} + +// default comparison implements +func (defaultURLTool) CompareURLEqual(l *URL, r *URL, execludeParam ...string) bool { + return IsEquals(*l, *r, execludeParam...) +} diff --git a/common/url_tool_extension_test.go b/common/url_tool_extension_test.go new file mode 100644 index 0000000000..8254c18aaa --- /dev/null +++ b/common/url_tool_extension_test.go @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package common + +import ( + "github.com/apache/dubbo-go/common/constant" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestDefaultURLTool(t *testing.T) { + url1, _ := NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000×tamp=1556509797245") + url2, _ := NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000×tamp=155650979798") + assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) +} + +func TestNewCustomURLTool(t *testing.T) { + url1, _ := NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000×tamp=1556509797245") + url2, _ := NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000×tamp=155650979798") + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + SetURLTool(customURLTool{}) + assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2)) + assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + + url1, _ = NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000") + url2, _ = NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + + "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + + "side=provider&timeout=3000") + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + SetURLTool(customURLTool{}) + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) +} + +// just for no timestamp, it depend on write data. +type customURLTool struct { +} + +func (customURLTool) Priority() uint8 { + //default is 16. + return 32 +} +func (customURLTool) CompareURLEqual(l *URL, r *URL, execludeParam ...string) bool { + return l.PrimitiveURL == r.PrimitiveURL +} diff --git a/registry/directory/directory.go b/registry/directory/directory.go index 6f9c4fc889..2d75fe71d2 100644 --- a/registry/directory/directory.go +++ b/registry/directory/directory.go @@ -303,7 +303,8 @@ func (dir *RegistryDirectory) cacheInvoker(url *common.URL) protocol.Invoker { } else { // if cached invoker has the same URL with the new URL, then no need to re-refer, and no need to destroy // the old invoker. - if common.IsEquals(*newUrl, cacheInvoker.(protocol.Invoker).GetUrl()) { + urlTmp := cacheInvoker.(protocol.Invoker).GetUrl() + if common.GetURLTool().CompareURLEqual(newUrl, &urlTmp) { return nil } From 83d057817891ccd1618ce931934bc7e8288a9878 Mon Sep 17 00:00:00 2001 From: cvictory Date: Sat, 7 Nov 2020 13:38:09 +0800 Subject: [PATCH 2/4] optimize get url with pointer type --- cluster/router/chain/chain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster/router/chain/chain.go b/cluster/router/chain/chain.go index b4c7cea47b..4c3cf8c2a7 100644 --- a/cluster/router/chain/chain.go +++ b/cluster/router/chain/chain.go @@ -287,9 +287,9 @@ func isInvokersChanged(left []protocol.Invoker, right []protocol.Invoker) bool { for _, r := range right { found := false + rurl := r.GetUrl() for _, l := range left { lurl := l.GetUrl() - rurl := r.GetUrl() if common.GetURLTool().CompareURLEqual(&lurl, &rurl, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY) { found = true break From f4ce65e5c2be1d1fd294cd20ac6be14fa9f0c3ba Mon Sep 17 00:00:00 2001 From: cvictory Date: Mon, 9 Nov 2020 14:50:18 +0800 Subject: [PATCH 3/4] Fix review Issue: rename the struct and remove some code. --- cluster/router/chain/chain.go | 2 +- common/url_tool_extension.go | 44 +++++++++---------------------- common/url_tool_extension_test.go | 22 ++++++++-------- registry/directory/directory.go | 2 +- 4 files changed, 25 insertions(+), 45 deletions(-) diff --git a/cluster/router/chain/chain.go b/cluster/router/chain/chain.go index 4c3cf8c2a7..3efed15868 100644 --- a/cluster/router/chain/chain.go +++ b/cluster/router/chain/chain.go @@ -290,7 +290,7 @@ func isInvokersChanged(left []protocol.Invoker, right []protocol.Invoker) bool { rurl := r.GetUrl() for _, l := range left { lurl := l.GetUrl() - if common.GetURLTool().CompareURLEqual(&lurl, &rurl, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY) { + if common.GetURLComparator().CompareURLEqual(&lurl, &rurl, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY) { found = true break } diff --git a/common/url_tool_extension.go b/common/url_tool_extension.go index 84899fca51..e446f7ee69 100644 --- a/common/url_tool_extension.go +++ b/common/url_tool_extension.go @@ -17,50 +17,30 @@ package common -import "sync" - -var urlTool URLTool -var lock sync.Mutex +var urlComparator URLComparator // Define some func for URL, such as comparison of URL. -// Your can define your own implements, and invoke SetURLTool for high priority. -type URLTool interface { - // The higher of the number, the higher of the priority. - Priority() uint8 - // comparison of two URL, excluding some params +// Your can define your own implements, and invoke SetURLComparator. +type URLComparator interface { CompareURLEqual(*URL, *URL, ...string) bool } -func SetURLTool(tool URLTool) { - lock.Lock() - defer lock.Unlock() - if urlTool == nil { - urlTool = tool - return - } - if urlTool.Priority() < tool.Priority() { - urlTool = tool - } +func SetURLComparator(comparator URLComparator) { + urlComparator = comparator } -func GetURLTool() URLTool { - return urlTool +func GetURLComparator() URLComparator { + return urlComparator } -// Config default urlTools. +// Config default defaultURLComparator. func init() { - SetURLTool(defaultURLTool{}) -} - -type defaultURLTool struct { + SetURLComparator(defaultURLComparator{}) } -// default priority is 16. -func (defaultURLTool) Priority() uint8 { - //default is 16. - return 16 +type defaultURLComparator struct { } // default comparison implements -func (defaultURLTool) CompareURLEqual(l *URL, r *URL, execludeParam ...string) bool { - return IsEquals(*l, *r, execludeParam...) +func (defaultURLComparator) CompareURLEqual(l *URL, r *URL, excludeParam ...string) bool { + return IsEquals(*l, *r, excludeParam...) } diff --git a/common/url_tool_extension_test.go b/common/url_tool_extension_test.go index 8254c18aaa..6a5a3cf9b2 100644 --- a/common/url_tool_extension_test.go +++ b/common/url_tool_extension_test.go @@ -34,8 +34,8 @@ func TestDefaultURLTool(t *testing.T) { "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + "side=provider&timeout=3000×tamp=155650979798") - assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2)) - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + assert.False(t, GetURLComparator().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) } func TestNewCustomURLTool(t *testing.T) { @@ -49,10 +49,10 @@ func TestNewCustomURLTool(t *testing.T) { "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + "side=provider&timeout=3000×tamp=155650979798") - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) - SetURLTool(customURLTool{}) - assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2)) - assert.False(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + SetURLComparator(customURLTool{}) + assert.False(t, GetURLComparator().CompareURLEqual(&url1, &url2)) + assert.False(t, GetURLComparator().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) url1, _ = NewURL("dubbo://127.0.0.1:20000/com.ikurento.user.UserProvider?anyhost=true&" + "application=BDTService&category=providers&default.timeout=10000&dubbo=dubbo-provider-golang-1.0.0&" + @@ -64,11 +64,11 @@ func TestNewCustomURLTool(t *testing.T) { "environment=dev&interface=com.ikurento.user.UserProvider&ip=192.168.56.1&methods=GetUser%2C&" + "module=dubbogo+user-info+server&org=ikurento.com&owner=ZX&pid=1447&revision=0.0.1&" + "side=provider&timeout=3000") - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2)) - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) - SetURLTool(customURLTool{}) - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2)) - assert.True(t, GetURLTool().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) + SetURLComparator(customURLTool{}) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2)) + assert.True(t, GetURLComparator().CompareURLEqual(&url1, &url2, constant.TIMESTAMP_KEY, constant.REMOTE_TIMESTAMP_KEY)) } // just for no timestamp, it depend on write data. diff --git a/registry/directory/directory.go b/registry/directory/directory.go index 2d75fe71d2..aaf699bdef 100644 --- a/registry/directory/directory.go +++ b/registry/directory/directory.go @@ -304,7 +304,7 @@ func (dir *RegistryDirectory) cacheInvoker(url *common.URL) protocol.Invoker { // if cached invoker has the same URL with the new URL, then no need to re-refer, and no need to destroy // the old invoker. urlTmp := cacheInvoker.(protocol.Invoker).GetUrl() - if common.GetURLTool().CompareURLEqual(newUrl, &urlTmp) { + if common.GetURLComparator().CompareURLEqual(newUrl, &urlTmp) { return nil } From 0d7baa5585a0e0a09c0e8ba3cff3ce9f5a70ab70 Mon Sep 17 00:00:00 2001 From: cvictory Date: Wed, 9 Dec 2020 14:30:22 +0800 Subject: [PATCH 4/4] add nil check for IsEquals in url.go --- common/url.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/url.go b/common/url.go index 3a558846e4..d93b634f09 100644 --- a/common/url.go +++ b/common/url.go @@ -713,6 +713,9 @@ func (c *URL) CloneWithParams(reserveParams []string) *URL { // IsEquals compares if two URLs equals with each other. Excludes are all parameter keys which should ignored. func IsEquals(left *URL, right *URL, excludes ...string) bool { + if (left == nil && right != nil) || (right == nil && left != nil) { + return false + } if left.Ip != right.Ip || left.Port != right.Port { return false }