Copilot commented on code in PR #3488:
URL: https://github.com/apache/dubbo-go/pull/3488#discussion_r3577998750
##########
common/url.go:
##########
@@ -50,7 +50,6 @@ const (
CONFIGURATOR
ROUTER
PROVIDER
Review Comment:
Removing the exported `PROTOCOL` constant is a breaking change for
downstream users that referenced `common.PROTOCOL` as the canonical key for
protocol in URL maps/params. Since this PR is primarily a perf refactor,
consider keeping `PROTOCOL` as a deprecated alias to `constant.ProtocolKey` to
preserve compatibility.
##########
common/url.go:
##########
@@ -1163,26 +1195,91 @@ func IsEquals(left *URL, right *URL, excludes
...string) bool {
return false
}
- leftMap := left.ToMap()
- rightMap := right.ToMap()
- for _, exclude := range excludes {
- delete(leftMap, exclude)
- delete(rightMap, exclude)
+ excluded := newKeySet(excludes)
+ return equalScalars(left, right, excluded) &&
+ equalLocation(left.Location, right.Location, excluded) &&
+ equalParams(left, right, excluded)
+}
+
+// equalScalars compares the non-param scalar fields of two URLs, skipping any
excluded key.
+func equalScalars(left, right *URL, excluded keySet) bool {
+ scalars := []struct {
+ key string
+ left, right string
+ }{
+ {constant.ProtocolKey, left.Protocol, right.Protocol},
+ {constant.UsernameKey, left.Username, right.Username},
+ {constant.PasswordKey, left.Password, right.Password},
+ {constant.PathKey, left.Path, right.Path},
}
+ for _, s := range scalars {
+ if s.left != s.right && !excluded.contains(s.key) {
+ return false
+ }
+ }
+ return true
+}
- if len(leftMap) != len(rightMap) {
+// equalLocation compares two "host:port" locations, skipping host and/or port
when excluded.
+func equalLocation(left, right string, excluded keySet) bool {
+ if left == right {
+ return true
+ }
+
+ hostExcluded := excluded.contains(constant.HostKey)
+ portExcluded := excluded.contains(constant.PortKey)
+ if hostExcluded && portExcluded {
+ return true
+ }
+
+ leftHost, leftPort := parseLocation(left)
+ rightHost, rightPort := parseLocation(right)
+ if !hostExcluded && leftHost != rightHost {
+ return false
+ }
+ if !portExcluded && leftPort != rightPort {
return false
}
+ return true
+}
- for lk, lv := range leftMap {
- if rv, ok := rightMap[lk]; !ok {
- return false
- } else if lv != rv {
+// equalParams compares the params of two URLs, skipping excluded keys. It
avoids
+// materializing both maps by snapshotting only the left side and streaming
the right.
+func equalParams(left, right *URL, excluded keySet) bool {
+ leftParams := make(map[string]string)
+ left.RangeParams(func(key, value string) bool {
+ if !excluded.contains(key) {
+ leftParams[key] = value
+ }
+ return true
+ })
+
+ rightCount := 0
+ matched := true
+ right.RangeParams(func(key, value string) bool {
+ if excluded.contains(key) {
+ return true
+ }
+ rightCount++
+ if lv, ok := leftParams[key]; !ok || lv != value {
+ matched = false
return false
}
- }
+ return true
+ })
- return true
+ return matched && rightCount == len(leftParams)
+}
+
+// parseLocation splits "host:port" into host and port
+func parseLocation(location string) (host, port string) {
+ if location == "" {
+ return "", "0"
+ }
+ if idx := strings.IndexByte(location, ':'); idx >= 0 {
+ return location[:idx], location[idx+1:]
+ }
+ return location, "0"
}
Review Comment:
`parseLocation` currently returns everything after the first ':' as the
port. This changes `ToMap()["port"]` and `IsEquals` semantics for locations
that contain additional ':' (e.g., registry address lists like
"127.0.0.1:2181,127.0.0.1:2182"), where the previous behavior only took the
substring between the first and second ':' (matching `strings.Split(location,
":")[1]`). This violates the stated goal of preserving existing host/port
behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
