This is an automated email from the ASF dual-hosted git repository.
AlexStocks pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new e1031b70d Feat/revision canonical metadata (#3370)
e1031b70d is described below
commit e1031b70dba9d1ac0407376f6b7d442ef31f79c9
Author: Yuqi Qiao <[email protected]>
AuthorDate: Fri Jun 12 09:39:29 2026 +0800
Feat/revision canonical metadata (#3370)
* feat(metadata): add canonical revision calculation for MetadataInfo
Add three methods to replace CRC32-based revision with MD5 over
deterministic ServiceInfo serialization, aligning with Java dubbo's
MetadataInfo.calAndGetRevision().
- ServiceInfo.toDescString(): deterministic string representation
in format (name|group|version|protocol|port|path|params|methods),
with sorted params/methods keys for stable output.
- CalRevision(app, services): computes MD5 hex digest from app name
concatenated with sorted ServiceInfo.toDescString() outputs.
Returns 0 for empty services (matches Java EMPTY_REVISION).
- MetadataInfo.CalAndGetRevision(): updates info.Revision in-place.
This replaces the coarse app+path+version+port+method CRC32 approach
in resolveRevision, ensuring group/protocol/params changes are captured
and revision is strongly bound to the serialized MetadataInfo content.
* refactor(revision): replace CRC32 revision with canonical MD5-based
calculation
Refactor resolveRevision to derive revision from canonical ServiceInfo
objects rather than raw URLs, aligning with Java dubbo's
MetadataInfo.calAndGetRevision().
Changes in service_revision_customizer.go:
- resolveRevision now converts URLs to ServiceInfo via
NewServiceInfoWithURL,
groups by MatchKey, and delegates to info.CalRevision (MD5 over sorted
toDescString) instead of CRC32-summing app+path+version+port+method.
- Customizers now source URLs from instance.GetServiceMetadata() rather
than the global GetMetadataService(), so each instance's revision is
scoped to its own MetadataInfo. This fixes cross-test state leakage.
Changes in service_revision_customizer_test.go (new, 10 cases):
- Group / protocol / params / methods / version change triggers revision
change
- Deterministic: same input always produces same revision
- Empty URL list returns 0
- Ordering-independent: URL insertion order and param key order do not
affect revision
- Non-IncludeKeys params are ignored
* fix: remove useless assertion leftover from debugging
* fix(metadata): replace MD5 with SHA-512 in revision calculation
Switch the hash algorithm used in CalRevision from MD5 (32 hex chars)
to SHA-512 (128 hex chars) for stronger collision resistance.
* fix(metadata): exclude EnvironmentKey from ServiceInfo IncludeKeys
Environment is instance-level routing metadata (per the consumer-side
comment in service_instances_changed_listener_impl.go). The consumer
always overrides it from the current instance metadata, regardless of
what is cached by revision. Including it in IncludeKeys caused revision
to change between instances with different environment tags, producing
spurious revision updates with no routing impact.
* chore:delete useless comments
* chore: go fmt
* refactor(metadata): use incremental hash and add CalAndGetRevision test
* refactor(metadata): remove unused CalAndGetRevision method
No callers in production code. Go's MetadataInfo.Revision is set
externally by the registry/customizer path, unlike Java where
calAndGetRevision() manages the revision internally. CalRevision
remains as the public API for revision calculation.
* chore: retrigger CI
---
metadata/info/metadata_info.go | 90 +++++++++++-
metadata/info/metadata_info_test.go | 6 +-
.../customizer/service_revision_customizer.go | 42 ++----
.../customizer/service_revision_customizer_test.go | 152 +++++++++++++++++++++
4 files changed, 257 insertions(+), 33 deletions(-)
diff --git a/metadata/info/metadata_info.go b/metadata/info/metadata_info.go
index 1dd4ffc10..4caed3bfe 100644
--- a/metadata/info/metadata_info.go
+++ b/metadata/info/metadata_info.go
@@ -18,7 +18,10 @@
package info
import (
+ "crypto/sha512"
+ "fmt"
"net/url"
+ "sort"
"strconv"
"strings"
)
@@ -52,7 +55,6 @@ var IncludeKeys = gxset.NewSet(
constant.VersionKey,
constant.WarmupKey,
constant.WeightKey,
- constant.EnvironmentKey,
constant.ReleaseKey)
// MetadataInfo the metadata information of instance
@@ -283,3 +285,89 @@ func (si *ServiceInfo) GetServiceKey() string {
si.ServiceKey = common.ServiceKey(si.Name, si.Group, si.Version)
return si.ServiceKey
}
+
+// toDescString returns a deterministic string representation of ServiceInfo
+// for revision calculation. Aligned with Java dubbo
ServiceInfo.toDescString().
+//
+// Format: name|group|version|protocol|port|path|params|methods
+//
+// Empty fields use "" as placeholder to keep separator count stable.
+// Params are sorted by key alphabetically, joined as k=v&k=v.
+// The "methods" key is excluded from params and appended separately.
+// Methods are sorted alphabetically and comma-joined.
+// No escaping is performed on param values (aligned with Java behavior).
+func (si *ServiceInfo) toDescString() string {
+ var b strings.Builder
+
+ b.WriteString(si.Name)
+ b.WriteByte('|')
+ b.WriteString(si.Group)
+ b.WriteByte('|')
+ b.WriteString(si.Version)
+ b.WriteByte('|')
+ b.WriteString(si.Protocol)
+ b.WriteByte('|')
+ b.WriteString(strconv.Itoa(si.Port))
+ b.WriteByte('|')
+ b.WriteString(si.Path)
+ b.WriteByte('|')
+
+ // params: sorted keys, exclude methods key
+ keys := make([]string, 0, len(si.Params))
+ for k := range si.Params {
+ if k == constant.MethodsKey {
+ continue
+ }
+ keys = append(keys, k)
+ }
+ sort.Strings(keys)
+ for i, k := range keys {
+ if i > 0 {
+ b.WriteByte('&')
+ }
+ b.WriteString(k)
+ b.WriteByte('=')
+ b.WriteString(si.Params[k])
+ }
+
+ b.WriteByte('|')
+
+ // methods: sorted alphabetically, comma-joined
+ if methodsStr, ok := si.Params[constant.MethodsKey]; ok &&
len(methodsStr) > 0 {
+ methods := strings.Split(methodsStr, ",")
+ sort.Strings(methods)
+ for i, m := range methods {
+ if i > 0 {
+ b.WriteByte(',')
+ }
+ b.WriteString(m)
+ }
+ }
+
+ return b.String()
+}
+
+// CalRevision calculates a deterministic revision string from canonical
ServiceInfo objects.
+// Returns "0" if services is empty (aligned with Java EMPTY_REVISION).
+// Services are sorted by matchKey before serialization to ensure
deterministic output.
+// The revision is a SHA-512 hex digest of: app + sorted toDescString of each
ServiceInfo.
+func CalRevision(app string, services map[string]*ServiceInfo) string {
+ if len(services) == 0 {
+ return "0"
+ }
+
+ // collect and sort matchKeys for deterministic iteration
+ matchKeys := make([]string, 0, len(services))
+ for mk := range services {
+ matchKeys = append(matchKeys, mk)
+ }
+ sort.Strings(matchKeys)
+
+ h := sha512.New()
+ h.Write([]byte(app))
+ for _, mk := range matchKeys {
+ h.Write([]byte(services[mk].toDescString()))
+ }
+
+ return fmt.Sprintf("%x", h.Sum(nil))
+}
diff --git a/metadata/info/metadata_info_test.go
b/metadata/info/metadata_info_test.go
index f9a6d9a09..7af7dc73c 100644
--- a/metadata/info/metadata_info_test.go
+++ b/metadata/info/metadata_info_test.go
@@ -164,7 +164,7 @@ func TestServiceInfoGetParams(t *testing.T) {
assert.Equal(t, []string{"random"}, service.GetParams()["loadbalance"])
}
-func TestServiceInfoGetParamsIncludesEnvironment(t *testing.T) {
+func TestServiceInfoExcludesInstanceLevelParams(t *testing.T) {
serviceURL, err :=
common.NewURL("tri://127.0.0.1:20000/org.apache.dubbo.samples.proto.GreetService",
common.WithInterface("org.apache.dubbo.samples.proto.GreetService"),
common.WithParamsValue(constant.EnvironmentKey, "pre"),
@@ -174,7 +174,9 @@ func TestServiceInfoGetParamsIncludesEnvironment(t
*testing.T) {
service := NewServiceInfoWithURL(serviceURL)
- assert.Equal(t, []string{"pre"},
service.GetParams()[constant.EnvironmentKey])
+ // Environment is instance-level metadata, not service-level.
+ // It should NOT appear in ServiceInfo.Params and thus not affect
revision.
+ assert.Empty(t, service.GetParams()[constant.EnvironmentKey])
}
func TestServiceInfoGetMatchKey(t *testing.T) {
diff --git
a/registry/servicediscovery/customizer/service_revision_customizer.go
b/registry/servicediscovery/customizer/service_revision_customizer.go
index e3b220da7..ca1d41f1e 100644
--- a/registry/servicediscovery/customizer/service_revision_customizer.go
+++ b/registry/servicediscovery/customizer/service_revision_customizer.go
@@ -17,12 +17,6 @@
package customizer
-import (
- "fmt"
- "hash/crc32"
- "sort"
-)
-
import (
"github.com/dubbogo/gost/log/logger"
)
@@ -32,6 +26,7 @@ import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/common/extension"
"dubbo.apache.org/dubbo-go/v3/metadata"
+ "dubbo.apache.org/dubbo-go/v3/metadata/info"
"dubbo.apache.org/dubbo-go/v3/registry"
)
@@ -54,8 +49,6 @@ func (e *exportedServicesRevisionMetadataCustomizer)
GetPriority() int {
func (e *exportedServicesRevisionMetadataCustomizer) Customize(instance
registry.ServiceInstance) {
registryId := instance.GetMetadata()[constant.RegistryIdKey]
if len(registryId) == 0 {
- // revision will be "0" (no services found for empty key),
which causes OnEvent to skip
- // this instance entirely — ensure RegistryIdKey is set before
customizers run.
logger.Errorf("[Registry][ServiceDiscovery] instance has no
registryId in metadata; " +
"exported revision will be \"0\" and this instance will
be invisible to consumers")
}
@@ -82,8 +75,6 @@ func (e *subscribedServicesRevisionMetadataCustomizer)
GetPriority() int {
func (e *subscribedServicesRevisionMetadataCustomizer) Customize(instance
registry.ServiceInstance) {
registryId := instance.GetMetadata()[constant.RegistryIdKey]
if len(registryId) == 0 {
- // revision will be "0" (no subscriptions found for empty key),
which causes OnEvent to skip
- // this instance entirely — ensure RegistryIdKey is set before
customizers run.
logger.Errorf("[Registry][ServiceDiscovery] instance has no
registryId in metadata; " +
"subscribed revision will be \"0\" and this instance
will be invisible to consumers")
}
@@ -99,33 +90,24 @@ func (e *subscribedServicesRevisionMetadataCustomizer)
Customize(instance regist
instance.GetMetadata()[constant.SubscribedServicesRevisionPropertyName]
= revision
}
-// resolveRevision provides the actual pattern to calculate the revision.
-// please refer to dubbo-java's method,
org.apache.dubbo.metadata.Metadata#calAndGetRevision
+// resolveRevision calculates a deterministic revision from the given URLs.
+// It converts URLs to canonical ServiceInfo objects and delegates to
info.CalRevision,
+// aligning with Java dubbo's MetadataInfo.calAndGetRevision().
func resolveRevision(urls []*common.URL) string {
if len(urls) == 0 {
return "0"
}
- candidates := make([]string, 0, len(urls))
+ // build canonical ServiceInfo map from URLs, keyed by MatchKey
+ services := make(map[string]*info.ServiceInfo, len(urls))
+ app := ""
for _, u := range urls {
- desc := u.GetParam(constant.ApplicationKey, "") + u.Path +
u.GetParam(constant.VersionKey, "") + u.Port
-
- if len(u.Methods) == 0 {
- candidates = append(candidates, desc)
- } else {
- for _, m := range u.Methods {
- // methods are part of candidates
- candidates = append(candidates,
desc+constant.KeySeparator+m)
- }
+ si := info.NewServiceInfoWithURL(u)
+ services[si.GetMatchKey()] = si
+ if app == "" {
+ app = u.GetParam(constant.ApplicationKey, "")
}
-
}
- sort.Strings(candidates)
- // it's nearly impossible to be overflow
- res := uint64(0)
- for _, c := range candidates {
- res += uint64(crc32.ChecksumIEEE([]byte(c)))
- }
- return fmt.Sprint(res)
+ return info.CalRevision(app, services)
}
diff --git
a/registry/servicediscovery/customizer/service_revision_customizer_test.go
b/registry/servicediscovery/customizer/service_revision_customizer_test.go
index 8ecaa763e..14705255e 100644
--- a/registry/servicediscovery/customizer/service_revision_customizer_test.go
+++ b/registry/servicediscovery/customizer/service_revision_customizer_test.go
@@ -32,6 +32,158 @@ import (
"dubbo.apache.org/dubbo-go/v3/registry"
)
+// helper to create a URL with common service discovery fields
+func newTestURL(protocol string, port string, path string, application string,
group string, version string, methods []string, extraParams map[string]string)
*common.URL {
+ opts := []common.Option{
+ common.WithProtocol(protocol),
+ common.WithPort(port),
+ common.WithPath(path),
+ common.WithParamsValue(constant.ApplicationKey, application),
+ common.WithParamsValue(constant.GroupKey, group),
+ common.WithParamsValue(constant.VersionKey, version),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ }
+ if len(methods) > 0 {
+ opts = append(opts, common.WithMethods(methods))
+ }
+ for k, v := range extraParams {
+ opts = append(opts, common.WithParamsValue(k, v))
+ }
+ u, _ := common.NewURL(protocol+"://127.0.0.1:"+port+"/"+path, opts...)
+ return u
+}
+
+// 1. group change → revision changes
+func TestRevisionChangesOnGroupChange(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"}, nil)
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupB", "1.0.0", []string{"sayHello"}, nil)
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.NotEmpty(t, r1)
+ assert.NotEmpty(t, r2)
+ assert.NotEqual(t, r1, r2, "revision should change when group changes")
+}
+
+// 2. protocol change → revision changes
+func TestRevisionChangesOnProtocolChange(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"}, nil)
+ u2 := newTestURL("tri", "20880", "com.example.TestService", "test-app",
"groupA", "1.0.0", []string{"sayHello"}, nil)
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.NotEmpty(t, r1)
+ assert.NotEmpty(t, r2)
+ assert.NotEqual(t, r1, r2, "revision should change when protocol
changes")
+}
+
+// 3. params change (timeout, loadbalance) → revision changes
+func TestRevisionChangesOnParamsChange(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.TimeoutKey: "3000",
constant.LoadbalanceKey: "random"})
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.TimeoutKey: "5000",
constant.LoadbalanceKey: "roundrobin"})
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.NotEmpty(t, r1)
+ assert.NotEmpty(t, r2)
+ assert.NotEqual(t, r1, r2, "revision should change when params
(timeout/loadbalance) change")
+}
+
+// 4. methods change → revision changes
+func TestRevisionChangesOnMethodChange(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"}, nil)
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello", "sayGoodbye"}, nil)
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.NotEmpty(t, r1)
+ assert.NotEmpty(t, r2)
+ assert.NotEqual(t, r1, r2, "revision should change when methods change")
+}
+
+// 5. version change → revision changes
+func TestRevisionChangesOnVersionChange(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"}, nil)
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "2.0.0", []string{"sayHello"}, nil)
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.NotEmpty(t, r1)
+ assert.NotEmpty(t, r2)
+ assert.NotEqual(t, r1, r2, "revision should change when version
changes")
+}
+
+// 6. same input → same revision (deterministic)
+func TestRevisionStable(t *testing.T) {
+ u := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello", "sayGoodbye"},
+ map[string]string{constant.TimeoutKey: "3000"})
+
+ r1 := resolveRevision([]*common.URL{u})
+ r2 := resolveRevision([]*common.URL{u})
+ r3 := resolveRevision([]*common.URL{u})
+
+ assert.Equal(t, r1, r2, "same input should produce same revision")
+ assert.Equal(t, r2, r3, "same input should produce same revision")
+}
+
+// 7. empty URL list → "0"
+func TestRevisionEmptyServices(t *testing.T) {
+ r := resolveRevision(nil)
+ assert.Equal(t, "0", r)
+
+ r = resolveRevision([]*common.URL{})
+ assert.Equal(t, "0", r)
+}
+
+// 8. different insertion order → same revision
+func TestRevisionOrderingIndependent(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"}, nil)
+ u2 := newTestURL("dubbo", "20881", "com.example.AnotherService",
"test-app", "groupA", "1.0.0", []string{"getUser"}, nil)
+
+ r1 := resolveRevision([]*common.URL{u1, u2})
+ r2 := resolveRevision([]*common.URL{u2, u1})
+
+ assert.Equal(t, r1, r2, "revision should be the same regardless of URL
order")
+}
+
+// 9. params key-value ordering → same revision
+func TestRevisionParamsOrderStable(t *testing.T) {
+ // Params in different order within URL — NewURL handles param
insertion,
+ // so we build two URLs that end up with the same effective params.
+ // The key test is that ServiceInfo.toDescString() sorts params by key.
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.TimeoutKey: "3000",
constant.ClusterKey: "failover"})
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.ClusterKey: "failover",
constant.TimeoutKey: "3000"})
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ assert.Equal(t, r1, r2, "revision should be the same regardless of
param key-value ordering")
+}
+
+// 10. non-IncludeKeys params → revision unchanged
+func TestRevisionIgnoresNonIncludeKeys(t *testing.T) {
+ u1 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.TimeoutKey: "3000"})
+ // add a param NOT in IncludeKeys (e.g., a custom arbitrary param)
+ u2 := newTestURL("dubbo", "20880", "com.example.TestService",
"test-app", "groupA", "1.0.0", []string{"sayHello"},
+ map[string]string{constant.TimeoutKey: "3000",
"custom.arbitrary.key": "someValue"})
+
+ r1 := resolveRevision([]*common.URL{u1})
+ r2 := resolveRevision([]*common.URL{u2})
+
+ // Note: custom.arbitrary.key is NOT in IncludeKeys, so it should be
filtered by NewServiceInfoWithURL
+ assert.Equal(t, r1, r2, "revision should ignore params not in
IncludeKeys")
+}
+
// TestExportedRevisionIsRegistryScoped verifies that when two registries
export different
// service sets, their instances get different revision values — not a merged
cross-registry one.
func TestExportedRevisionIsRegistryScoped(t *testing.T) {
