Similarityoung commented on code in PR #3280: URL: https://github.com/apache/dubbo-go/pull/3280#discussion_r2992383847
########## protocol/triple/openapi/schema_resolver.go: ########## @@ -0,0 +1,270 @@ +/* + * 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 openapi + +import ( + "reflect" + "strings" + "sync" + "time" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/triple/openapi/model" +) + +type SchemaResolver struct { + config *global.OpenAPIConfig + schemaMap *sync.Map + nameToSchema *sync.Map +} + +func NewSchemaResolver(config *global.OpenAPIConfig) *SchemaResolver { + return &SchemaResolver{ + config: config, + schemaMap: &sync.Map{}, + nameToSchema: &sync.Map{}, + } +} + +func (r *SchemaResolver) Resolve(t reflect.Type) *model.Schema { + return r.resolveType(t) +} + +func (r *SchemaResolver) resolveType(t reflect.Type) *model.Schema { + if t == nil { + return model.NewSchema().SetType(model.SchemaTypeObject) + } + + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + + switch t.Kind() { + case reflect.String: + return model.NewSchema().SetType(model.SchemaTypeString) + case reflect.Bool: + return model.NewSchema().SetType(model.SchemaTypeBoolean) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Int64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Int32 { + s.SetFormat("int32") + } + return s + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Uint64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Uint32 { + s.SetFormat("int32") + } + return s + case reflect.Float32, reflect.Float64: + s := model.NewSchema().SetType(model.SchemaTypeNumber) + if t.Kind() == reflect.Float64 { + s.SetFormat("double") + } else { + s.SetFormat("float") + } + return s + case reflect.Array, reflect.Slice: + return model.NewSchema(). + SetType(model.SchemaTypeArray). + SetItems(r.resolveType(t.Elem())) + case reflect.Map: + return model.NewSchema(). + SetType(model.SchemaTypeObject). + SetAdditionalProperties(r.resolveType(t.Elem())) + case reflect.Struct: + return r.resolveStruct(t) + case reflect.Interface: + return model.NewSchema().SetType(model.SchemaTypeObject) + default: + return model.NewSchema().SetType(model.SchemaTypeObject) + } +} + +func (r *SchemaResolver) resolveStruct(t reflect.Type) *model.Schema { + if t.PkgPath() == "time" && t.Name() == "Time" { + return model.NewSchema(). + SetType(model.SchemaTypeString). + SetFormat("date-time") + } + + if cached, ok := r.schemaMap.Load(t); ok { + if s, ok := cached.(*model.Schema); ok { + return &model.Schema{ + Ref: "#/components/schemas/" + s.Title, + } + } + } + + schema := model.NewSchema(). + SetType(model.SchemaTypeObject). + SetTitle(t.Name()). + SetGoType(t.String()) + + r.schemaMap.Store(t, schema) + r.nameToSchema.Store(t.Name(), schema) + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if !r.isExported(field) { + continue + } + + jsonTag := field.Tag.Get("json") + if jsonTag == "-" { + continue + } + + fieldName := r.getFieldName(field, jsonTag) + if fieldName == "" { + continue + } + + fieldSchema := r.resolveType(field.Type) + schema.AddProperty(fieldName, fieldSchema) + } + + example := r.GenerateExample(schema) + if example != nil { + schema.SetExample(example) + } + + return &model.Schema{ + Ref: "#/components/schemas/" + schema.Title, + } +} + +func (r *SchemaResolver) isExported(f reflect.StructField) bool { + return f.PkgPath == "" +} + +func (r *SchemaResolver) getFieldName(f reflect.StructField, jsonTag string) string { + if jsonTag != "" { + name := strings.Split(jsonTag, ",")[0] + if name != "" { + return name + } + } + return f.Name +} + +func (r *SchemaResolver) GetSchemas() map[string]*model.Schema { + schemas := make(map[string]*model.Schema) + r.schemaMap.Range(func(key, value any) bool { + if t, ok := key.(reflect.Type); ok { + if s, ok := value.(*model.Schema); ok { + name := t.Name() + if name == "" { + name = t.String() + } + schemas[name] = s + } + } + return true + }) + return schemas +} + +func (r *SchemaResolver) GetSchemaName(schema *model.Schema) string { + if schema == nil || schema.Ref == "" { + return "" + } + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + return parts[3] + } + return "" +} + +func (r *SchemaResolver) GetSchemaDefinition(schema *model.Schema) *model.Schema { + if schema == nil { + return nil + } + if schema.Ref == "" { + return schema + } + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + schemaName := parts[3] + if s, ok := r.nameToSchema.Load(schemaName); ok { + if resolvedSchema, ok := s.(*model.Schema); ok { + return resolvedSchema + } + } + } + return nil +} + +func (r *SchemaResolver) GenerateExample(schema *model.Schema) any { + if schema == nil { + return nil + } + + if schema.Ref != "" { + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + schemaName := parts[3] + if s, ok := r.nameToSchema.Load(schemaName); ok { + if resolvedSchema, ok := s.(*model.Schema); ok { + return r.GenerateExample(resolvedSchema) + } + } + } + return nil + } Review Comment: ### Add cycle detection when generating examples for recursive schemas GenerateExample() recursively resolves the target schema whenever it sees a $ref, and resolveStruct() calls it immediately while building each struct schema. For self-referential types such as type Node struct { Next *Node }, this never bottoms out, so OpenAPI generation will overflow the stack instead of producing a document. #### Suggested fix: Add a recursion guard to example generation, keyed by schema name or reflect.Type. Mark a schema as “visiting” before descending into it, and if it is encountered again, stop expanding that branch and return a safe placeholder such as nil or {}. Returning an empty object is usually the most UI-friendly option because it avoids crashes without making the generated example disappear entirely. ########## protocol/triple/openapi/schema_resolver.go: ########## @@ -0,0 +1,270 @@ +/* + * 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 openapi + +import ( + "reflect" + "strings" + "sync" + "time" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/triple/openapi/model" +) + +type SchemaResolver struct { + config *global.OpenAPIConfig + schemaMap *sync.Map + nameToSchema *sync.Map +} + +func NewSchemaResolver(config *global.OpenAPIConfig) *SchemaResolver { + return &SchemaResolver{ + config: config, + schemaMap: &sync.Map{}, + nameToSchema: &sync.Map{}, + } +} + +func (r *SchemaResolver) Resolve(t reflect.Type) *model.Schema { + return r.resolveType(t) +} + +func (r *SchemaResolver) resolveType(t reflect.Type) *model.Schema { + if t == nil { + return model.NewSchema().SetType(model.SchemaTypeObject) + } + + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + + switch t.Kind() { + case reflect.String: + return model.NewSchema().SetType(model.SchemaTypeString) + case reflect.Bool: + return model.NewSchema().SetType(model.SchemaTypeBoolean) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Int64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Int32 { + s.SetFormat("int32") + } + return s + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Uint64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Uint32 { + s.SetFormat("int32") + } + return s + case reflect.Float32, reflect.Float64: + s := model.NewSchema().SetType(model.SchemaTypeNumber) + if t.Kind() == reflect.Float64 { + s.SetFormat("double") + } else { + s.SetFormat("float") + } + return s + case reflect.Array, reflect.Slice: + return model.NewSchema(). + SetType(model.SchemaTypeArray). + SetItems(r.resolveType(t.Elem())) + case reflect.Map: + return model.NewSchema(). + SetType(model.SchemaTypeObject). + SetAdditionalProperties(r.resolveType(t.Elem())) + case reflect.Struct: + return r.resolveStruct(t) + case reflect.Interface: + return model.NewSchema().SetType(model.SchemaTypeObject) + default: + return model.NewSchema().SetType(model.SchemaTypeObject) + } +} + +func (r *SchemaResolver) resolveStruct(t reflect.Type) *model.Schema { + if t.PkgPath() == "time" && t.Name() == "Time" { + return model.NewSchema(). + SetType(model.SchemaTypeString). + SetFormat("date-time") + } + + if cached, ok := r.schemaMap.Load(t); ok { + if s, ok := cached.(*model.Schema); ok { + return &model.Schema{ + Ref: "#/components/schemas/" + s.Title, + } + } + } + + schema := model.NewSchema(). + SetType(model.SchemaTypeObject). + SetTitle(t.Name()). + SetGoType(t.String()) + + r.schemaMap.Store(t, schema) + r.nameToSchema.Store(t.Name(), schema) + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if !r.isExported(field) { + continue + } + + jsonTag := field.Tag.Get("json") + if jsonTag == "-" { + continue + } + + fieldName := r.getFieldName(field, jsonTag) + if fieldName == "" { + continue + } + + fieldSchema := r.resolveType(field.Type) + schema.AddProperty(fieldName, fieldSchema) + } + + example := r.GenerateExample(schema) + if example != nil { + schema.SetExample(example) + } + + return &model.Schema{ + Ref: "#/components/schemas/" + schema.Title, + } +} + +func (r *SchemaResolver) isExported(f reflect.StructField) bool { + return f.PkgPath == "" +} + +func (r *SchemaResolver) getFieldName(f reflect.StructField, jsonTag string) string { + if jsonTag != "" { + name := strings.Split(jsonTag, ",")[0] + if name != "" { + return name + } + } + return f.Name +} + +func (r *SchemaResolver) GetSchemas() map[string]*model.Schema { + schemas := make(map[string]*model.Schema) + r.schemaMap.Range(func(key, value any) bool { + if t, ok := key.(reflect.Type); ok { + if s, ok := value.(*model.Schema); ok { + name := t.Name() + if name == "" { + name = t.String() + } + schemas[name] = s + } + } + return true + }) + return schemas +} + +func (r *SchemaResolver) GetSchemaName(schema *model.Schema) string { + if schema == nil || schema.Ref == "" { + return "" + } + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + return parts[3] + } + return "" +} + +func (r *SchemaResolver) GetSchemaDefinition(schema *model.Schema) *model.Schema { + if schema == nil { + return nil + } + if schema.Ref == "" { + return schema + } + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + schemaName := parts[3] + if s, ok := r.nameToSchema.Load(schemaName); ok { + if resolvedSchema, ok := s.(*model.Schema); ok { + return resolvedSchema + } + } + } + return nil +} + +func (r *SchemaResolver) GenerateExample(schema *model.Schema) any { + if schema == nil { + return nil + } + + if schema.Ref != "" { + parts := strings.Split(schema.Ref, "/") + if len(parts) == 4 && parts[0] == "#" && parts[1] == "components" && parts[2] == "schemas" { + schemaName := parts[3] + if s, ok := r.nameToSchema.Load(schemaName); ok { + if resolvedSchema, ok := s.(*model.Schema); ok { + return r.GenerateExample(resolvedSchema) + } + } + } + return nil + } Review Comment: ### Add cycle detection when generating examples for recursive schemas GenerateExample() recursively resolves the target schema whenever it sees a $ref, and resolveStruct() calls it immediately while building each struct schema. For self-referential types such as type Node struct { Next *Node }, this never bottoms out, so OpenAPI generation will overflow the stack instead of producing a document. #### Suggested fix: Add a recursion guard to example generation, keyed by schema name or reflect.Type. Mark a schema as “visiting” before descending into it, and if it is encountered again, stop expanding that branch and return a safe placeholder such as nil or {}. Returning an empty object is usually the most UI-friendly option because it avoids crashes without making the generated example disappear entirely. ########## protocol/triple/openapi/service.go: ########## @@ -0,0 +1,204 @@ +/* + * 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 openapi + +import ( + "sync" +) + +import ( + "github.com/dubbogo/gost/log/logger" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/triple/openapi/model" +) + +type OpenAPIRequest struct { + Group string + Format string +} + +type Service interface { + GetOpenAPI(req *OpenAPIRequest) *model.OpenAPI + GetOpenAPIGroups() []string + Refresh() +} + +type DefaultService struct { + config *global.OpenAPIConfig + defResolver *DefinitionResolver + encoder *Encoder + + mu sync.RWMutex + openAPIs map[string]*model.OpenAPI + services map[string]*common.ServiceInfo + groups map[string]string +} + +func NewDefaultService(cfg *global.OpenAPIConfig) *DefaultService { + if cfg == nil { + cfg = global.DefaultOpenAPIConfig() + } + s := &DefaultService{ + config: cfg, + services: make(map[string]*common.ServiceInfo), + openAPIs: make(map[string]*model.OpenAPI), + groups: make(map[string]string), + } + s.defResolver = NewDefinitionResolver(s.config, nil) + s.encoder = NewEncoder() + return s +} + +func (s *DefaultService) RegisterService(interfaceName string, info *common.ServiceInfo, group string) { + s.mu.Lock() + defer s.mu.Unlock() + s.services[interfaceName] = info + if group != "" { + s.groups[interfaceName] = group + } + s.openAPIs = nil +} Review Comment: ### Avoid using only interfaceName as the OpenAPI registration key RegisterService() stores both services and groups under a single interfaceName key. If the same Triple interface is exported into multiple OpenAPI groups, or appears under multiple Dubbo group/version combinations, the later registration overwrites the earlier one. As a result, /api-docs/... can retain only one variant of that interface, so the newly added grouping support does not actually work in those cases. #### Suggested fix: Promote the registry key to something that is unique at the document level instead of using only interfaceName. A composite key such as {interfaceName, openapiGroup, dubboGroup, version} would avoid these collisions cleanly. If all dimensions are not available here yet, at minimum openapiGroup needs to be part of the key; otherwise multiple documentation groups for the same interface will always overwrite each other. It would also be cleaner to store the group directly in the composite key and remove the parallel groups map so the registry has a single source of truth. ########## protocol/triple/openapi/schema_resolver.go: ########## @@ -0,0 +1,270 @@ +/* + * 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 openapi + +import ( + "reflect" + "strings" + "sync" + "time" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/protocol/triple/openapi/model" +) + +type SchemaResolver struct { + config *global.OpenAPIConfig + schemaMap *sync.Map + nameToSchema *sync.Map +} + +func NewSchemaResolver(config *global.OpenAPIConfig) *SchemaResolver { + return &SchemaResolver{ + config: config, + schemaMap: &sync.Map{}, + nameToSchema: &sync.Map{}, + } +} + +func (r *SchemaResolver) Resolve(t reflect.Type) *model.Schema { + return r.resolveType(t) +} + +func (r *SchemaResolver) resolveType(t reflect.Type) *model.Schema { + if t == nil { + return model.NewSchema().SetType(model.SchemaTypeObject) + } + + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + + switch t.Kind() { + case reflect.String: + return model.NewSchema().SetType(model.SchemaTypeString) + case reflect.Bool: + return model.NewSchema().SetType(model.SchemaTypeBoolean) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Int64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Int32 { + s.SetFormat("int32") + } + return s + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + s := model.NewSchema().SetType(model.SchemaTypeInteger) + if t.Kind() == reflect.Uint64 { + s.SetFormat("int64") + } else if t.Kind() == reflect.Uint32 { + s.SetFormat("int32") + } + return s + case reflect.Float32, reflect.Float64: + s := model.NewSchema().SetType(model.SchemaTypeNumber) + if t.Kind() == reflect.Float64 { + s.SetFormat("double") + } else { + s.SetFormat("float") + } + return s + case reflect.Array, reflect.Slice: + return model.NewSchema(). + SetType(model.SchemaTypeArray). + SetItems(r.resolveType(t.Elem())) + case reflect.Map: + return model.NewSchema(). + SetType(model.SchemaTypeObject). + SetAdditionalProperties(r.resolveType(t.Elem())) + case reflect.Struct: + return r.resolveStruct(t) + case reflect.Interface: + return model.NewSchema().SetType(model.SchemaTypeObject) + default: + return model.NewSchema().SetType(model.SchemaTypeObject) + } +} + +func (r *SchemaResolver) resolveStruct(t reflect.Type) *model.Schema { + if t.PkgPath() == "time" && t.Name() == "Time" { + return model.NewSchema(). + SetType(model.SchemaTypeString). + SetFormat("date-time") + } + + if cached, ok := r.schemaMap.Load(t); ok { + if s, ok := cached.(*model.Schema); ok { + return &model.Schema{ + Ref: "#/components/schemas/" + s.Title, + } + } + } + + schema := model.NewSchema(). + SetType(model.SchemaTypeObject). + SetTitle(t.Name()). + SetGoType(t.String()) Review Comment: ### Disambiguate component schema names with package information resolveStruct() currently uses only t.Name() for both the component name and the nameToSchema key. If two request/response types from different Go packages share the same name, for example foo.Request and bar.Request, they will both be emitted as #/components/schemas/Request, and the later one will overwrite the earlier definition. That leaves at least one operation pointing at the wrong schema in the generated document. #### Suggested fix: Use a globally unique schema key instead of the bare type name. The simplest fix is to include package identity in the internal component key, for example t.PkgPath() + "." + t.Name(), and then normalize it into an OpenAPI-safe component name. If you want to keep short display names, you can still append a package-derived suffix only on collisions, but schema.Ref, nameToSchema, and GetSchemas() all need to use the same naming rule consistently. -- 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]
