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

feat(net/goai): add enhanced response interface #3896

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

UncleChair
Copy link
Contributor

@UncleChair UncleChair commented Oct 28, 2024

Fixes #3889

  • Add enhanced response interface for openapi config for users to add extra responses and examples
  • Add related unit test
    ok github.com/gogf/gf/v2/net/goai coverage: 80.2% of statements

@gqcn
Copy link
Member

gqcn commented Nov 11, 2024

@UncleChair Hello, thanks for your contribution, but I have a little suggestion for this implementing OpenAPI Operation enhancement:

  • It might be better to add an interface like:
type IEnhanceOperation interface{
    EnhanceOperation(operation *goai.Operation, req interface{}, res interface{})
}
  • The previous defined interface ResponseStatusDef might be removed as it is not released officially.

@gqcn gqcn added the discuss We need discuss to make decision. label Nov 11, 2024
@UncleChair
Copy link
Contributor Author

@gqcn Here are some of my opinions

  • It might be better to add an interface like:

I have also considered to use interface solution to modify the operation, but I regard this feature to be a common method for users to add this to all api for processing some common settings, like what common response are doing. So it may be better to create a hook and users would not need to implement this interface everywhere?

  • The previous defined interface ResponseStatusDef might be removed as it is not released officially.

Since the status was added by private functions, if we use a common interface for users to add status, they may have to implement their own functions, or we may have to expose those features to users, which may not that following the original oai designed usage cover and would increase the complexity. But anyway it is still possible, I could also help expose those functions used for status adding if needed.

@gqcn
Copy link
Member

gqcn commented Nov 12, 2024

@UncleChair

I have also considered to use interface solution to modify the operation, but I regard this feature to be a common method for users to add this to all api for processing some common settings, like what common response are doing. So it may be better to create a hook and users would not need to implement this interface everywhere?

Yes, it is reseanable. But the goai.OpenApiV3 from ghttp.Server.GetOpenApi()already exposed its attributes public, which can be customized by users. And also, the hook function is a little bit ugly, we may not add this hook feature for package goai util it is really necessary. But the IEnhanceOperation interface is good, we can add this as soon as possible.

@UncleChair
Copy link
Contributor Author

UncleChair commented Nov 12, 2024

@gqcn

But the IEnhanceOperation interface is good, we can add this as soon as possible.

Got it, then would we remove the ResponseStatusDef? In that case the getResponseFromObject function may need to be exposed to users or they may have difficulty adding additional status since users may have to implement this again. Or just keep it there to offer an easy usage for user?

Btw, where should the IEnhanceOperation be? Seem it is neither a request nor a response, and users could only define these in api. I am not sure how users could have a better understanding with this interface and found a good place to implement it lol

@gqcn
Copy link
Member

gqcn commented Nov 12, 2024

@UncleChair Indeed, it would be quite complicated for users if only providing IEnhanceOperation. How about this:

  1. We only keep the ResponseStatusDef design, which would be defined on response struct, but rename it to:
// ResponseStatusStruct is the structure for certain response status.
type ResponseStatusStruct = any

// IEnhanceResponseStatus is used to enhance the documentation of the response.
// Normal response structure could implement this interface to provide more information.
type IEnhanceResponseStatus interface {
	EnhanceResponseStatus() map[StatusCode]ResponseStatusStruct
}
  1. We do not provide IEnhanceOperation util it is necessary for some scenario.

@UncleChair
Copy link
Contributor Author

@gqcn Changed the interface related name

  1. We do not provide IEnhanceOperation util it is necessary for some scenario.

Actually I am also tring to generate json files for examples during the goai doc generation process. In some cases I may already have an error list for a response with different status code, For example:

package v1

import (
	"cssgen-api/api"

	"github.com/gogf/gf/v2/errors/gcode"
	"github.com/gogf/gf/v2/frame/g"
)

// parameters should use uppercase
type LoginReq struct {
	g.Meta   `path:"/login" tags:"Auth" method:"post" sum:"User Login"`
	Username string `json:"username" v:"required-without:Email" des:"User Name" eg:"admin"`
	Email    string `json:"email" v:"required-without:Username" des:"User Email" eg:"[email protected]"`
	Password string `json:"password" v:"required" des:"Password" eg:"adminPassword"`
}
type LoginRes struct {
	Token  string `json:"token" des:"JWT token"`
	Expire int64  `json:"expire" des:"JWT expire time"`
}

type LoginRes401 struct {
	g.Meta `status:"401" resEg:"resource/openapi/cssgen-api.api.auth.v1.LoginRes/401.json"`
}

func (r LoginRes) EnhanceResponseStatus() map[int]any {
	return map[int]any{
		401: LoginRes401{},
	}
}

var LoginErrRes = map[int][]gcode.Code{
	401: {
		gcode.New(1, "No related users", nil),
		gcode.New(2, "Wrong username or password", nil),
	}
}

func (r LoginRes) GenerateJsonFile() (path string, err error) {
	return api.GenerateJsonFile(r, LoginErrRes)
}

If I want to generate a static file for the error list, I have to use some mechanism to generate the file first and fill the generated file path resource/openapi/cssgen-api.api.auth.v1.LoginRes/401.json to the related response, which is not that automatic. This may be an edge case for example usage and we should desigen the api first, but the generation process may still important since we may need to sync some changes for error list if we modified it, which is also very common during project development, then I am trying to add a hook for operations in this pr.
I would like to provide some solutions for error list and example file synchronization in development, but since the structure of error list may differ from each other and we could not add a common interface for that, the best way seems to let users define what it should be and offer some hooks for users totrigger the process. Do you have any good idea to solve this usage problem?

@gqcn
Copy link
Member

gqcn commented Nov 13, 2024

#3896 (comment)

I see. It is quite complicated for users to handle OpenAPIv3 structure, it is not easy and convenient. We should encapsulate the complicacy.
How about expose the only predicted attributes that would be customized by users to a public structure, and then we add the attributes one by one to the structure if necessary. For example, we change the EnhanceResponseStatus() map[int]any to:

type EnhancedStatusCode = int
type EnhancedStatusType struct{
    Response any
    Example  any
}
func EnhanceResponseStatus() map[EnhancedStatusCode]EnhancedStatusType

Currently, we add Example to the EnhancedStatusType as it is necessary, we can easily extend it in the future if also necessary.

@UncleChair UncleChair changed the title feat(net/goai): add openapi operation override hook feat(net/goai): add enhanced response interface Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss We need discuss to make decision.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net/goai: Operation object info improvement
3 participants