Copilot commented on code in PR #3370:
URL: https://github.com/apache/dubbo-go/pull/3370#discussion_r3367022950


##########
metadata/info/metadata_info.go:
##########
@@ -174,6 +176,15 @@ func (info *MetadataInfo) ReplaceExportedServices(urls 
[]*common.URL) {
        }
 }
 
+// CalAndGetRevision calculates and updates the revision for this MetadataInfo.
+// The revision is derived from the canonical ServiceInfo representation,
+// ensuring strong binding between revision and serialized metadata content.
+// Aligned with Java dubbo MetadataInfo.calAndGetRevision().
+func (info *MetadataInfo) CalAndGetRevision() string {

Review Comment:
   CalAndGetRevision is a new public method (on MetadataInfo) but there isn't a 
unit test asserting that it both (1) updates info.Revision in-place and (2) 
matches CalRevision() for the same Services/App input. Adding a focused test 
would help prevent accidental divergence during future refactors of revision 
logic.



##########
metadata/info/metadata_info.go:
##########
@@ -283,3 +294,90 @@ 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)
+
+       var b strings.Builder
+       b.WriteString(app)
+       for _, mk := range matchKeys {
+               b.WriteString(services[mk].toDescString())
+       }
+
+       sum := sha512.Sum512([]byte(b.String()))
+       return fmt.Sprintf("%x", sum)

Review Comment:
   CalRevision currently concatenates the full canonical metadata into a single 
in-memory string and then hashes it. For large applications (many services / 
many params), this does extra allocations proportional to total metadata size; 
you can hash incrementally instead (same output, lower memory).



-- 
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]

Reply via email to