Alanxtl commented on code in PR #3711: URL: https://github.com/apache/dubbo-go/pull/3711#discussion_r4024717376
########## metadata/definition/types.go: ########## @@ -0,0 +1,561 @@ +/* + * 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 + +import ( + "errors" + "fmt" + "reflect" + "sort" + "strings" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/internal/genericfield" +) + +// maxTypeDepth bounds structural nesting. The visited set already terminates +// reference cycles, so this only catches pathologically deep composite types +// (a [][][]...[]T built by code generation, say) before they exhaust the stack. +const maxTypeDepth = 64 + +// unsupportedError marks a type or method the builder refuses to publish. +// +// Refusing is deliberate: a schema that silently degrades an unrepresentable +// type produces requests the provider cannot realize, and the failure surfaces +// at call time on the Admin side rather than at publish time here. Java has no +// equivalent problem — erasure yields an incomplete TypeDefinition, but Java has +// no chan or func to describe in the first place. +type unsupportedError struct { + subject string + reason string +} + +func (e *unsupportedError) Error() string { + return fmt.Sprintf("%s is not supported: %s", e.subject, e.reason) +} + +func unsupported(subject, reason string) error { + return &unsupportedError{subject: subject, reason: reason} +} + +// IsUnsupported reports whether err marks a type or method the builder +// deliberately declined to publish, as opposed to an internal failure. +func IsUnsupported(err error) bool { + var target *unsupportedError + return errors.As(err, &target) +} + +// blockedNamedTypes are named types the Generalizer or hessian2 codec treats +// specially, but for which the MCP proposal has not yet defined a JSON wire +// representation. Expanding them as plain structs would publish their internal +// layout (time.Time's wall/ext/loc, big.Int's neg/abs) as if it were the +// contract, which is worse than not publishing the method at all. +// +// Named scalars are deliberately absent: time.Duration is an int64 that +// round-trips as a number like any other, so the scalar branch below handles it +// correctly. Only types whose structure would be misrepresented belong here. +var blockedNamedTypes = map[string]string{ + "time.Time": "no JSON wire representation is defined for time.Time yet", + "math/big.Int": "no JSON wire representation is defined for math/big types yet", + "math/big.Float": "no JSON wire representation is defined for math/big types yet", + "math/big.Rat": "no JSON wire representation is defined for math/big types yet", + "encoding/json.RawMessage": "raw JSON has no declarable structure", +} + +// Java type names used by the published contract. +// +// The definition speaks Java's type vocabulary rather than Go's. That is not a +// concession to any particular consumer: it is the vocabulary dubbo-go's own +// generic runtime already uses. filter/generic matches the caller-supplied +// $invoke types against protocol/dubbo/hessian2.GetJavaName output when it +// decides whether to unwrap a packed variadic tail, so a Go-spelled contract +// would describe names that dubbo-go itself does not recognize. +// +// Struct names are the exception and stay Go-derived unless the type declares a +// Java class name; see resolveStruct. +const ( + javaBoolean = "boolean" + javaByte = "byte" + javaShort = "short" + javaInt = "int" + javaLong = "long" + javaFloat = "float" + javaDouble = "double" + javaString = "java.lang.String" + javaMap = "java.util.Map" +) + +// javaWrappers maps each primitive spelling to its boxed counterpart. +// +// Go's T versus *T is exactly Java's primitive versus wrapper distinction, and +// consumers read nullability off that: a primitive rejects null, a reference +// type accepts it. Expressing it through the type name means the contract needs +// no separate nullability flag, which Provider metadata has never carried. +var javaWrappers = map[string]string{ + javaBoolean: "java.lang.Boolean", + javaByte: "java.lang.Byte", + javaShort: "java.lang.Short", + javaInt: "java.lang.Integer", + javaLong: "java.lang.Long", + javaFloat: "java.lang.Float", + javaDouble: "java.lang.Double", +} + +// javaScalarName returns the Java spelling of a Go scalar. +// +// Unsigned kinds widen to the next signed type that holds their whole range. +// The Java schema is then wider than the Go value on both sides; generic +// realization validates the target Go bit width so negative or oversized +// inputs fail instead of wrapping. +// +// uint and uint64 have no such landing spot: their range runs past Java's long. +// They are refused rather than published under an invented name, which is what +// the existing hessian helper does with its non-Java "unsigned long". +func javaScalarName(t reflect.Type, nullable bool) (string, error) { + var primitive string + switch t.Kind() { + case reflect.Bool: + primitive = javaBoolean + case reflect.Int8: + primitive = javaByte + case reflect.Uint8: + // short, not byte. Java's byte is signed, so a consumer decoding + // against it accepts only -128..127 and rejects a Go uint8 of 128..255 + // before the call ever leaves. Widening costs nothing and keeps the + // whole range callable. + // + // This also decides []byte, since []byte and []uint8 are one type in Go. + // Spelling it byte[] would match what hessian2 writes on the wire, but + // that alignment buys nothing when half the element values cannot be + // expressed in the schema either way. + primitive = javaShort + case reflect.Int16: + primitive = javaShort + case reflect.Uint16, reflect.Int32: + primitive = javaInt + case reflect.Uint32, reflect.Int, reflect.Int64: + primitive = javaLong + case reflect.Uint, reflect.Uint64: + return "", unsupported(t.String(), + "unsigned 64-bit integers exceed the range of every Java integer type") + case reflect.Float32: + primitive = javaFloat + case reflect.Float64: + primitive = javaDouble + case reflect.String: + // Already a reference type; there is no unboxed spelling to choose. + return javaString, nil + default: + return "", unsupported(t.String(), "not a scalar kind") + } + + if nullable { + return javaWrappers[primitive], nil + } + return primitive, nil +} + +// typeCollector resolves reflect.Types into type expressions and accumulates a +// TypeDefinition for every composite type it walks through. +type typeCollector struct { + defs map[string]*TypeDefinition +} + +func newTypeCollector() *typeCollector { + return &typeCollector{defs: make(map[string]*TypeDefinition)} +} + +// resolve returns the type expression for t, recording definitions for t and +// everything reachable from it. +func (c *typeCollector) resolve(t reflect.Type) (string, error) { + return c.resolveAt(t, 0) +} + +func (c *typeCollector) resolveAt(t reflect.Type, depth int) (string, error) { + if t == nil { + return "", unsupported("<nil type>", "type is nil") + } + if depth > maxTypeDepth { + return "", unsupported(t.String(), fmt.Sprintf("type nesting exceeds %d levels", maxTypeDepth)) + } + + // Pointers carry nullability, not structure. Java expresses the same + // distinction with primitive versus wrapper class rather than in the type + // expression, so strip the indirection here and let the scalar branch pick + // the right spelling. For struct, list and map the wrapper form is already + // nullable, so the flag changes nothing. + nullable := false + for t.Kind() == reflect.Pointer { + t = t.Elem() + nullable = true + depth++ + if depth > maxTypeDepth { + return "", unsupported(t.String(), + fmt.Sprintf("type nesting exceeds %d levels", maxTypeDepth)) + } + } + + if name := namedTypeKey(t); name != "" { + if reason, blocked := blockedNamedTypes[name]; blocked { + return "", unsupported(name, reason) + } + } + + switch t.Kind() { + case reflect.Bool, reflect.String, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64: + // Scalars are published under their Java spelling even when the Go type + // is named. A `type UserID int64` travels the generic wire as a bare + // 64-bit integer, so publishing "UserID" would name something the caller + // can neither construct nor recognize. + return javaScalarName(t, nullable) + + case reflect.Slice, reflect.Array: + // Array syntax, not List<T>. Java generics cannot hold a primitive, so + // List<byte> would not be a type anyone could write; the array form is + // valid for every element kind. It is also what + // protocol/dubbo/hessian2.GetJavaName produces, which keeps the + // published contract and the runtime's own spelling in agreement — and + // it is what makes a Go []byte land on Java's byte[], the form hessian2 + // already puts on the wire. + // + // A Go array's length has no Java counterpart, so [N]T and []T are + // spelled the same. + // Go []byte is Hessian binary and Java byte[] regardless of uint8's + // scalar spelling. Scalar uint8 widens to short so 0..255 is directly + // representable; inside a byte container the wire identity wins, and the + // generic realizer preserves Java's signed byte bits on the way back. + if t.Elem().Kind() == reflect.Uint8 { + return c.recordContainer(javaByte) + } + return c.resolveContainer(t.Elem(), depth) + + case reflect.Map: + if t.Key().Kind() != reflect.String { + return "", unsupported(t.String(), + "only string-keyed maps can be expressed as JSON objects") + } + // The key is fixed at String rather than resolved: a named string key + // still serializes as a JSON object key, and admitting map[UserID]T + // would imply a key schema JSON cannot carry. + return c.resolveMap(t.Elem(), depth) + + case reflect.Struct: + return c.resolveStruct(t, depth) + + case reflect.Interface: + // Including the empty interface would publish a field whose schema is + // "anything", which Admin cannot turn into a request form and the + // provider cannot realize into a concrete Go value. + return "", unsupported(t.String(), "interface types have no declarable structure") + + case reflect.Chan, reflect.Func, reflect.UnsafePointer: + return "", unsupported(t.String(), "type cannot cross an RPC boundary") + + case reflect.Complex64, reflect.Complex128: + return "", unsupported(t.String(), "complex numbers have no cross-language representation") + + case reflect.Uintptr: + return "", unsupported(t.String(), "uintptr is a process-local address") + + default: + return "", unsupported(t.String(), "unhandled reflect kind "+t.Kind().String()) + } +} + +// resolveContainer resolves a slice or array element and records a list entry +// whose single item is the element type. +// +// Container entries exist because Admin resolves a parameter by looking up its +// exact ParameterTypes string in types[], then walks items/properties from +// there. Publishing only the element T while the parameter reads List<T> leaves +// Admin with no path to T's fields. +// +// Exactly one item is what marks this as a collection rather than a map: Java's +// MapTypeBuilder appends one entry per actual type argument, so a Map yields two +// and consumers tell the two apart by arity, not by name. +func (c *typeCollector) resolveContainer( + elem reflect.Type, + depth int, +) (string, error) { + elemExpr, err := c.resolveAt(elem, depth+1) + if err != nil { + return "", err + } + return c.recordContainer(elemExpr) +} + +func (c *typeCollector) recordContainer(elemExpr string) (string, error) { + expr := elemExpr + "[]" + if _, seen := c.defs[expr]; !seen { + c.put(expr, &TypeDefinition{Type: expr, Items: []string{elemExpr}}) + } + return expr, nil +} + +// resolveMap records a map entry carrying both type arguments. +// +// Two items, key first. A single item would be read as a collection of that +// item, silently turning an object schema into an array one. +// +// The value is boxed: it sits inside a generic argument list, where Java admits +// only reference types. Map<java.lang.String,long> is not a type. +func (c *typeCollector) resolveMap(value reflect.Type, depth int) (string, error) { + valueExpr, err := c.resolveAt(value, depth+1) + if err != nil { + return "", err + } + valueExpr = boxed(valueExpr) + expr := javaMap + "<" + javaString + "," + valueExpr + ">" + + if _, seen := c.defs[expr]; !seen { + c.put(expr, &TypeDefinition{Type: expr, Items: []string{javaString, valueExpr}}) + } + return expr, nil +} + +// boxed returns the wrapper spelling of a primitive, or expr unchanged when it +// is already a reference type. +func boxed(expr string) string { + if wrapper, primitive := javaWrappers[expr]; primitive { + return wrapper + } + return expr +} + +func (c *typeCollector) resolveStruct(t reflect.Type, depth int) (string, error) { + expr := namedTypeKey(t) + if expr == "" { + // An anonymous struct literal in a signature has no name for Admin to + // key on, and no stable identity across builds. + return "", unsupported(t.String(), "anonymous struct types cannot be named in a contract") + } + + // Reserve the key before descending so a self-referential type terminates. + // The placeholder is replaced below once fields resolve; on failure it is + // removed again so a later, successful path can retry the same type. + if _, seen := c.defs[expr]; seen { + return expr, nil + } + c.put(expr, &TypeDefinition{Type: expr}) + + properties, err := c.structProperties(t, expr, depth) + if err != nil { + c.remove(expr) + return "", err + } + c.defs[expr].Properties = properties + return expr, nil +} + +func (c *typeCollector) structProperties(t reflect.Type, expr string, depth int) (map[string]string, error) { + properties := make(map[string]string) + // matchSets records, per wire name, the field that claimed it. mapstructure + // matches case-insensitively, so collisions are detected on the folded name. + matchSets := make(map[string]string) + flattening := map[reflect.Type]bool{t: true} + if err := c.collectStructProperties(t, expr, "", depth, properties, matchSets, flattening); err != nil { + return nil, err + } + return properties, nil +} + +func (c *typeCollector) collectStructProperties( + t reflect.Type, + expr string, + path string, + depth int, + properties map[string]string, + matchSets map[string]string, + flattening map[reflect.Type]bool, +) error { + for i := range t.NumField() { + field := t.Field(i) + if field.PkgPath != "" { + // Unexported. MapGeneralizer skips these too (CanInterface is + // false), so they are genuinely absent from the wire format. + continue + } + + tag := genericfield.ParseMTag(field) + if tag.Ignore { + continue + } + if len(tag.UnknownOptions) > 0 { + return unsupported(field.Name, + fmt.Sprintf("m tag option %q has no declarable schema", tag.UnknownOptions[0])) + } + + owner := path + field.Name + if tag.Squash { + fieldType := field.Type + fieldDepth := depth + 1 + if fieldDepth > maxTypeDepth { + return unsupported(field.Type.String(), + fmt.Sprintf("type nesting exceeds %d levels", maxTypeDepth)) + } Review Comment: The builder flattens pointer-to-struct fields tagged m:squash, but MapGeneralizer.Generalize returns nil for a nil pointer and then fails the squash type assertion; MapGeneralizer.Realize also rejects the nil pointer as unsupported for squash. An optional embedded struct is therefore advertised in the service definition but generic invocation fails when the field is omitted. Either reject pointer squash here or make both conversion paths accept nil consistently, and add a nil-pointer round-trip test. -- 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]
