diff --git a/metadata/info/metadata_info.go b/metadata/info/metadata_info.go index 1dd4ffc108..4caed3bfe2 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 f9a6d9a098..7af7dc73cf 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 e3b220da72..ca1d41f1e6 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 8ecaa763ef..14705255e1 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) {