Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate Schemas using comment markers #424

Open
austince opened this issue Sep 23, 2021 · 5 comments
Open

Generate Schemas using comment markers #424

austince opened this issue Sep 23, 2021 · 5 comments

Comments

@austince
Copy link

austince commented Sep 23, 2021

Has there been a discussion in openapi3gen to use comment markers instead of/ in addition to the struct tags for schema generation? An example of this approach is seen in kubebuilder (https://kubebuilder.io/reference/markers.html) and in goswagger.

Of course, this would be a significant change to just using reflection and struct tags, since we'd need to parse the source files directly, but is one way to generate more expressive schemas.

Does anyone have other options for adding more information, like descriptions, validations, etc. to generated schemas without using struct tags?

@fenollp
Copy link
Collaborator

fenollp commented Oct 2, 2021

There hasn't been discussion on this.

WRT other ways of adding info: in what way are you limited here? Maybe explain your problem boundaries.

@felixvd
Copy link

felixvd commented Dec 13, 2021

In my case, the structTags already contain the actual tags, so those would have to be removed manually if the description was taken from the tags. The fields are also already documented like this:

type Geometry struct {
	ID string `json:"id,omitempty" yaml:"id,omitempty" msgpack:"id,omitempty" datastruct:"id"`

	Name *string `json:"name,omitempty" yaml:"name,omitempty" msgpack:"name,omitempty"`

	// Transform of the geometry in link space
	Transform *linalg.Transform `json:"transform,omitempty" yaml:"transform,omitempty,flow" msgpack:"transform,omitempty"`

	// Type of geometry. Can be one of: box, sphere, cylinder, trimesh
	Type *string `json:"type,omitempty" yaml:"type,omitempty" msgpack:"type,omitempty"`
        ....

The comments above each field and type are called Doc when parsed in the AST, so this usage appears to be idiomatic. It would make sense if this package parsed them for the description field as well.

@fenollp
Copy link
Collaborator

fenollp commented Dec 13, 2021

I suggest going the way of SchemaCustomizer: a new openapi3gen option that takes in the ast.Node value of the current field being generated and from there one should be able to set description fields or to add schema validation information (value types, ranges, patterns, ...)
PRs welcomed ;)

@felixvd
Copy link

felixvd commented Dec 14, 2021

I tested this yesterday, which retrieves the comments from the AST (inefficiently, but who cares?):

const loadMode = packages.NeedName |
	packages.NeedFiles |
	packages.NeedCompiledGoFiles |
	packages.NeedImports |
	packages.NeedDeps |
	packages.NeedTypes |
	packages.NeedSyntax |
	packages.NeedTypesInfo


func GetGenDeclFromReflectType(t reflect.Type) (*ast.GenDecl, error) {
	loadConfig := new(packages.Config)
	loadConfig.Mode = loadMode
	loadConfig.Fset = token.NewFileSet()
	
	pkgs, err := packages.Load(loadConfig, t.PkgPath())
	if err != nil {
		panic(err)
	}
	
	for _, pkg := range pkgs {
		for _, syn := range pkg.Syntax {  // syn is of type *ast.File
			for _, dec := range syn.Decls {
				if gen, ok := dec.(*ast.GenDecl); ok && gen.Tok == token.TYPE {
					for _, spec := range gen.Specs {
						if ts, ok := spec.(*ast.TypeSpec); ok {
							if ts.Name.Name == t.Name() {
								return gen, nil
							}
						}
					}
				}
			}
		}
	}
	return nil, errors.New("Type not found??")
}

func GetCommentForTypeFromAST(gen *ast.GenDecl, t reflect.Type) (string, error) {
	for _, spec := range gen.Specs {
		if ts, ok := spec.(*ast.TypeSpec); ok {
			if ts.Name.Name == t.Name() && ts.Doc != nil {
				for _, docline := range ts.Doc.List {
					fmt.Println(docline.Text[3:])
					// TODO: Format with newlines and return as output
				}	
				return "FOUND (as struct doc)", nil
			}
		}
	}
	
	// If no doc exists for the struct, it might be declared as "type MyType struct {...}", so it is on the genDecl level
	if len(gen.Specs) == 1 {
		if gen.Doc != nil {
			if len(gen.Doc.List) > 0 {
				for _, docline := range gen.Doc.List {
					fmt.Println(docline.Text[3:])
					// TODO: Format with newlines and return as output
				}
				return "FOUND (as genDecl doc)", nil
			}
		}
	}

	return "", nil
}

func GetCommentForFieldFromAST(gen *ast.GenDecl, fieldName string) (string, error) {
	for _, spec := range gen.Specs {
		if ts, ok := spec.(*ast.TypeSpec); ok {			
			st, ok := ts.Type.(*ast.StructType)
			if !ok {
				continue
			}
			for i := 0; i < len(st.Fields.List); i++ {
				field := st.Fields.List[i]
				if field.Names[0].Name == fieldName {
					if field.Doc != nil {
						for j := 0; j < len(field.Doc.List); j++ {
							fmt.Println(field.Doc.List[j].Text[3:])
							// TODO: Format with newlines and return as output
						} 
						return "FOUND", nil
					} else {
						return "", nil
					}
				}
			}
		}
	}
	return "", errors.New("Type not found??")
}

// my type (doc line 1/2)
// my type (doc line 2/2)
type Mytype struct {
	// my comment line 1 of 2
	// my comment line 2 of 2
	Myfield string

	// my 2nd comment line 1/1
	Myfield2 int

	// ending comment
}

func main() {
	testobj := Mytype{"test", 5}
	gen, err := GetGenDeclFromReflectType(reflect.TypeOf(testobj))
	if err != nil {
		panic(err)
	}
	
	ret, err := GetCommentForTypeFromAST(gen, reflect.TypeOf(testobj))
	fmt.Println("===1", ret)
	ret, err = GetCommentForFieldFromAST(gen, "Myfield")
	fmt.Println("===2", ret)
	return
}

Returns:

my type (doc line 1/2)
my type (doc line 2/2)
===1 FOUND (as genDecl doc)
my comment line 1 of 2
my comment line 2 of 2
===2 FOUND

@felixvd
Copy link

felixvd commented Dec 24, 2021

I made a prototype here, attempting to use only the SchemaCustomizer. I wanted to avoid changing too much inside this package, so this seemed to be the leanest approach. The interesting functions that handle the AST are here.

The first problem is that the customizer only receives the field, but not the (parent) type that owns the comments. I added that to the customizer args.

Another problem is that only the jsonname (taken from the tag) is known, but the field's actual name is easier to compare. I passed that as well.

The performance is atrocious, because the AST is built and searched from scratch every time the customizer is called. A ~3000 line schema takes over 30 minutes to generate with this prototype, and openapi3.Content about 2 minutes. The generator will need to store and pass the AST node to the/a customizer function to achieve acceptable performance.

I think adding ast.Node to TypeInfo (when available) and passing the TypeInfo to the SchemaCustomizer is the way to proceed. But parsing the AST will affect performance negatively. Let me know if you have thoughts on this or a better idea @fenollp . Making the AST parsing conditional could be an option.

PS: I know there's a bug with the fieldName for array fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants