Feat/revision canonical metadata

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))
+}

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) {

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)
 }

registry/servicediscovery/customizer/service_revision_customizer_test.go

@@ -32,6 +32,158 @@
 	"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) {