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

Add client-type-prefix output option (#785) #788

Merged
merged 2 commits into from Oct 27, 2022

Conversation

jedrivisser
Copy link
Contributor

Fix for #785

This is backwards compatible and should change nothing on any existing generation.

If the client-type-prefix output option is set, then the Client struct is prefixed to avoid type name collisions.

Example code generated without prefix:

image

Example code generated with prefix:

image

Copy link
Collaborator

@jamietanna jamietanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting solution - not sure if you'd seen #583 and that a solution introduced is to allow overriding just the OpenAPI types themselves, instead of modifying the Client. Do you think that'd be an option here, or do you think it'd be better to also have the option to rename the generated Client, rather than modifying the spec (especially if the spec's not controlled by you)?

@jedrivisser
Copy link
Contributor Author

That does look cool and useful, but yea as you mentioned, this is useful when I am not in control of the spec as is the case for me now :).

I also had a look at how grpc-go's code generation got around it, and they prefix the Client with a service name:
https://github.com/grpc/grpc-go/blob/v1.50.1/cmd/protoc-gen-go-grpc/grpc.go#L170

So I thought a similar approach might work.

@deepmap-marcinr
Copy link
Contributor

I'd structure this a bit differently.

First, rather than hard coding Client in the templates at all, I'd add a function to the template input to get the client name - this allows client name construction to move into code, minimizing template work, which sucks.

Second, rather than making it a prefix, id's add a config option called "ClientInterfaceTypeName", something very descriptive, and if it's not spcified, you use "Client".

This is much more flexible and maintainable.

@jedrivisser
Copy link
Contributor Author

I have extracted the whole ClientTypeName now and given it a default value of Client if it is not set.

I am using the global state variable directly since I am not doing any additional manipulation, though can change it to a method if needed.

If felt like the right direction between the comment:

// globalState stores all global state. Please don't put global state anywhere
// else so that we can easily track it.
var globalState struct {
	options Configuration
	spec    *openapi3.T
}

and the fact the the global options are already added to the templating:

TemplateFunctions["opts"] = func() Configuration { return globalState.options }

But if you prefer I wrap it in a method like the responseTypeSuffix uses:

// genResponseTypeName creates the name of generated response types (given the operationID):
func genResponseTypeName(operationID string) string {
	return fmt.Sprintf("%s%s", UppercaseFirstCharacter(operationID), responseTypeSuffix)
}

Let me know and I will change it

@deepmap-marcinr deepmap-marcinr merged commit 7b01009 into deepmap:master Oct 27, 2022
@jedrivisser jedrivisser deleted the prefixed-client branch October 27, 2022 19:25
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
* Add client-type-prefix output option (deepmap#785)

* Override whole client type instead of prefixing it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants