Alanxtl commented on code in PR #3711:
URL: https://github.com/apache/dubbo-go/pull/3711#discussion_r3903685061


##########
metadata/report/nacos/report.go:
##########
@@ -307,6 +308,31 @@ func (n *nacosMetadataReport) ListAppRevisions(application 
string) ([]report.App
        return result, nil
 }
 
+// PublishServiceDefinition stores one interface-level service definition,
+// implementing report.ServiceDefinitionPublisher.
+//
+// The dataId is byte-compatible with Java's MetadataIdentifier.getUniqueKey, 
and
+// the Nacos group is the fixed "dubbo" that Dubbo Admin's watcher searches —
+// deliberately not n.group, which serves the application-level metadata 
written
+// by PublishAppMetadata and defaults to DEFAULT_GROUP. See definition.DataID.
+//
+// PublishConfig overwrites in place, so republishing on every provider start 
is
+// idempotent: the key is derived only from the service identity, never from 
the
+// instance.
+func (n *nacosMetadataReport) PublishServiceDefinition(serviceInterface, 
version, group, application, definitionJSON string) error {
+       dataID := definition.DataID(serviceInterface, version, group, 
application)
+       if err := n.storeMetadata(vo.ConfigParam{
+               DataId:  dataID,
+               Group:   definition.MetadataGroup,
+               Content: definitionJSON,
+       }); err != nil {
+               return perrors.WithMessagef(err, "publishing service definition 
%s", dataID)
+       }
+       logger.Debugf("[Metadata][Nacos] published service definition, 
dataId=%s group=%s",
+               dataID, definition.MetadataGroup)
+       return nil
+}
+

Review Comment:
   ```go
   Group: definition.MetadataGroup // 固定为 "dubbo"
   ```
   
   该实现没有使用 `n.group`。因此配置 `metadata-report.group=foo` 时,应用级 metadata 在 `foo`,但 
service definition 仍发布到 `dubbo`。多个 Nacos group 之间还可能因为相同 DataID 相互覆盖,破坏隔离性。
   
   Java 的 Nacos metadata report 使用配置的 group,参见 
[[NacosMetadataReport.java](https://github.com/apache/dubbo/blob/3.3/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java)](https://github.com/apache/dubbo/blob/3.3/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java)。
   



##########
metadata/definition/definition.go:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Package definition builds and publishes interface-level service definitions.
+//
+// A service definition describes the RPC contract of a single exported service
+// interface: its methods, their signatures, and the structure of every type
+// reachable from those signatures. Dubbo Admin consumes these definitions to
+// render service documentation and to build generic-invocation request 
schemas.
+//
+// The JSON produced here is wire-compatible with Java's FullServiceDefinition
+// (dubbo-common/.../definition/model/), because Admin deserializes both into 
the
+// same ServiceProviderMetadata structure. Nothing in this package imports 
Admin
+// code — the compatibility contract is the JSON shape alone, pinned by the
+// golden tests in json_test.go.
+package definition
+
+// ServiceDefinition is the interface-level contract published for one exported
+// service. It mirrors Java's FullServiceDefinition.
+//
+// Java's codeSource field is deliberately omitted: Admin never reads it, and 
Go
+// has no equivalent notion of a class file origin.
+type ServiceDefinition struct {
+       // CanonicalName is the service interface name, always taken verbatim 
from
+       // the exported URL's Interface(). See BuildFromURL for why this must 
not be
+       // re-derived by reflection.
+       CanonicalName string `json:"canonicalName"`
+       // Methods holds one entry per canonical method the builder considers 
safe to
+       // expose. This is not the same set as Parameters["methods"]; see 
Parameters.
+       Methods []MethodDefinition `json:"methods"`
+       // Parameters carries the full provider URL parameter map, matching 
Java's
+       // serviceDefinition.setParameters(url.getParameters()).
+       //
+       // Parameters["methods"] is the runtime method set from the exported 
URL and
+       // includes the SwapCaseFirstRune aliases dubbo-go registers for Java
+       // interop. Methods above holds only canonical names. The two 
intentionally
+       // differ in size; consumers must not assume they match.
+       Parameters map[string]string `json:"parameters"`
+       // Types holds one entry for every composite type reachable from a 
method
+       // signature, including struct, slice, array and map shapes. Pointers 
express
+       // nullability and are folded into Java reference/wrapper spellings.
+       Types []TypeDefinition `json:"types"`
+}
+
+// MethodDefinition describes a single RPC method's signature.
+type MethodDefinition struct {
+       // Name is the canonical wire name: the MethodMapper mapping when one 
exists,
+       // otherwise the Go exported method name. The SwapCaseFirstRune alias is
+       // never published here even though it is routable at runtime.
+       Name string `json:"name"`
+       // ParameterTypes holds the type expression of each parameter, in order,
+       // excluding the receiver and a leading context.Context.
+       ParameterTypes []string `json:"parameterTypes"`
+       // Parameters pairs each parameter type with a positional name.
+       //
+       // Java's equivalent field is @Deprecated and carries TypeDefinition 
elements
+       // with no name at all. Go publishes {name, type} instead because 
Admin's
+       // Parameter message has both fields, and Go reflection cannot recover 
source
+       // parameter names — so the names here are always generated 
(arg0..argN).
+       Parameters []ParameterDefinition `json:"parameters"`
+       // ReturnType is the type expression of the non-error return value, or
+       // VoidReturnType for a method that returns only error.
+       ReturnType string `json:"returnType"`

Review Comment:
   `parameters` 字段不兼容 Java 的定义格式
   
   Go 当前输出:
   
   ```json
   "parameters": [
     {"name": "arg0", "type": "java.lang.String"}
   ]
   ```
   
   但 Java 的 `MethodDefinition.parameters` 类型是 `List<TypeDefinition>`,其中没有 
`name` 字段;参数类型应使用 `type/items/properties` 等结构。参考 [[Java 
MethodDefinition](https://github.com/apache/dubbo/blob/3.3/dubbo-common/src/main/java/org/apache/dubbo/metadata/definition/model/MethodDefinition.java)](https://github.com/apache/dubbo/blob/3.3/dubbo-common/src/main/java/org/apache/dubbo/metadata/definition/model/MethodDefinition.java)
 和 
[[TypeDefinition](https://github.com/apache/dubbo/blob/3.3/dubbo-common/src/main/java/org/apache/dubbo/metadata/definition/model/TypeDefinition.java)](https://github.com/apache/dubbo/blob/3.3/dubbo-common/src/main/java/org/apache/dubbo/metadata/definition/model/TypeDefinition.java)。
   
   这会导致严格的 Java/Jackson 消费者反序列化失败,宽松消费者也会得到错误的兼容字段。建议:
   
   - 让 `parameters` 使用 Java 的 `TypeDefinition` 结构;或
   - 如果 Admin 只依赖 `parameterTypes`,直接省略这个 deprecated 字段;
   - 不要复用 Java 字段名承载新的 `{name,type}` schema。
   
   当前 `json_test.go` 反而把这个不兼容格式固定下来了,需要同步调整。



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