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

Doesn't working with import alias for struct fields #483

Closed
dsxack opened this issue Jul 29, 2019 · 23 comments
Closed

Doesn't working with import alias for struct fields #483

dsxack opened this issue Jul 29, 2019 · 23 comments

Comments

@dsxack
Copy link
Contributor

dsxack commented Jul 29, 2019

Describe the bug
When we add import alias for custom type of struct field with this custom type that is not working.

To Reproduce
Put the struct with custom type from other package and add alias for this custom type:

package build

import (
       	domainapp "domain/application"
)

type ListResponse struct {
	BuildInfos []domainapp.BuildInfo `json:"data"`
	Err        error                 `json:"-"`
}

And some handler with comment

// @Success 200 {object} build.ListResponse

Run yaml:

swag init -g ./main.go --parseDependency --parseVendor

Result:

build.ListResponse:
    properties:
      data:
        type: string
    type: object

Expected behavior
Expected result yaml:

build.ListResponse:
    properties:
      data:
        items:
          $ref: '#/definitions/application.BuildInfo'
        type: array
    type: object

Your swag version
swag version v1.6.2

Your go version
go version go1.12.5 linux/amd64

@dsxack dsxack changed the title Doesnt working alias for struct fields Doesn't working alias for struct fields Jul 29, 2019
@dsxack dsxack changed the title Doesn't working alias for struct fields Doesn't working with import alias for struct fields Jul 29, 2019
@dsxack
Copy link
Contributor Author

dsxack commented Jul 29, 2019

I can make PR if this issue will be accepted.

@ubogdan
Copy link
Contributor

ubogdan commented Jul 30, 2019

I will be happy to see this issue finally fixed. (it was "solved" few time in the past #242, #294, #183)

@alexanderbez
Copy link

Any updates on this? I'm having an issue where I have two structs defined in a package A and those two structs both have internal types of different packages with the same name, hence I need to use an alias. But aliases don't get captured by swag :-/

@tac0turtle
Copy link

tac0turtle commented Dec 2, 2019

@dsxack do you still have an interest in committing a PR to fix this, this is currently blocking us as well

@alexanderbez
Copy link

I would be happy to look into opening a PR in fixing this. Is there any reference or guide into getting started here? What should I be looking at to update?

Was #298 closed solely due to lack of activity and unit testing?

@dsxack
Copy link
Contributor Author

dsxack commented Dec 2, 2019

@marbar3778 yes, thanks for reminding me, I'm going to make PR for it tomorrow.

dsxack pushed a commit to dsxack/swag that referenced this issue Dec 3, 2019
dsxack pushed a commit to dsxack/swag that referenced this issue Dec 3, 2019
@dsxack
Copy link
Contributor Author

dsxack commented Dec 3, 2019

@marbar3778, @alexanderbez, fix is merged into master. Could you check it in your project?

@alexanderbez
Copy link

Yes, we will report back!

@tac0turtle
Copy link

Tested it out and ran into a similar issue as before. The aliasing works, but it seems to ignore the aliased name of the type. If two types share the same name, in our case types.Params is aliased, then in the swagger file it shows as the aliased name but incorrect type definition.

@dsxack
Copy link
Contributor Author

dsxack commented Dec 4, 2019

Could you provide example of code and expected result and actual result in json or yaml?

@tac0turtle
Copy link

https://github.com/cosmos/cosmos-sdk/blob/787c902fb6d3a259012b0d78b1f2b11aeb14f1da/x/mint/client/rest/swagger.go#L12 & https://github.com/cosmos/cosmos-sdk/blob/787c902fb6d3a259012b0d78b1f2b11aeb14f1da/x/slashing/client/rest/swagger.go#L25

share the same type name, so I aliased it. In the swagger file, the aliased name appears but the type that the aliased type is supposed to have is not the actual type.

as you can see here: https://github.com/cosmos/cosmos-sdk/blob/787c902fb6d3a259012b0d78b1f2b11aeb14f1da/client/rest/docs/docs.go#L4062 & https://github.com/cosmos/cosmos-sdk/blob/787c902fb6d3a259012b0d78b1f2b11aeb14f1da/client/rest/docs/docs.go#L4106 here the types are different they contain different names, but the underlying type is the same, which is incorrect

@alexanderbez
Copy link

alexanderbez commented Dec 6, 2019

What @marbar3778 stated is correct. The aliased names are now being used (thanks!), but the underlying concrete type -- the value that alias refers to, is not being used which is the entire motivation of using an alias.

In general, I am curious how other projects (currently) handle this? It's not uncommon at all for two or more distinct packages to have the same type name (e.g. we use Params in each package). Surely, they must be rendered correctly somehow? Or is the current solution to just ensure they have different names? If so, I hesitate to do this because something like foo.FooParams and bar.BarParams is not idiomatic Go.

@dsxack
Copy link
Contributor Author

dsxack commented Dec 9, 2019

@alexanderbez, @marbar3778, thank you for describing your issue.

I propose a solution:

Add the // @definitionAlias <alias name> annotation that defines the name of the object definition.

Example:

Package github.com/cosmos/cosmos-sdk/x/mint/internal/types:

package "types"

// @definitionAlias mint.types.Params <==== the alias of definition
type Params struct {
     // ... other things
}

Package github.com/cosmos/cosmos-sdk/x/slashing/internal/types:

package "types"

// @definitionAlias slashing.types.Params
type Params struct {
     // ... other things
}

We will turn it into:

mint.types.Params:
    type: object
    properties:
         ...

slashing.types.Params:
    type: object
    properties:
         ...

That will be ok? @ubogdan, could you approve this proposition and I start work on it?

@ubogdan
Copy link
Contributor

ubogdan commented Dec 9, 2019

@dsxack the main idea sounds great.

I'm wondering how going to add // @definitionAlias slashing.types.Params into an external package (like the ones you are going to load with the help of go.mod).

If the definition goes into code that will be used to generate swag documentation, how are u going to match "slashing.types.Params" with "github.com/cosmos/cosmos-sdk/x/slashing/internal/types"?

@dsxack
Copy link
Contributor Author

dsxack commented Dec 10, 2019

@ubogdan, thank you for your useful comment.

To handle an external package, we could build a definition name with a full package import path for all definitions. But it isn't backward compatible and too verbose.

Tonight I'm going to see how it implemented in swagger generator libraries in other languages.

@srini-apty
Copy link

Any update on this?

@dsxack
Copy link
Contributor Author

dsxack commented Dec 19, 2019

I tried to find which swagger spec should be generated when we have go types with the same names in different packages.

I looked to specs some of the big go projects which I worked with: Kubernetes and Ory Hydra.

Authors of these specs put unique names of definitions with package prefix.

I think we should use @definitionName comment above the struct instead of @definitionAlias comment. If we want
to import an external package, it should be worrying itself about the unique name of definition by puts @definitionName with package prefix. Or we can provide the optional command line flag to generate definitions names with package prefix.

@definitionName comment is will be like // swagger:model Client comment from go-swagger

@ubogdan, could you respond to it?

@ubogdan
Copy link
Contributor

ubogdan commented Dec 21, 2019

@dsxack we can't change the default behavior of swag.

The definition is going to be used in 2 places
In params / requests

// @Param id path model.Account true "Account ID"

and in response

// @Success 200 {object} model.Account

Any of this should not be changed and they need to be relative to the package imports. We only need to change the name of the $ref.

Let's pretend model.Account is located in github.com/swagg/sample/pkg/model. How do you think it should be the definition name ?
From my perspective, this pattern should be unique enough.

          "$ref": "#/definitions/jackfan.us.kg.swagg.sample.pkg.model.Account"

Anyway, I'm afraid it won't work if the project is outside GOPATH when not using go modules.

We can use definitionAlias github.com/swagg/sample/pkg/model.Account Account to produce a shorter ref like

          "$ref": "#/definitions/Account"

@sdghchj
Copy link
Member

sdghchj commented Mar 23, 2020

Fixed, try latest version, please.

@sdghchj sdghchj closed this as completed Mar 23, 2020
@Blquinn
Copy link

Blquinn commented Aug 27, 2020

@sdghchj Still not working in v1.6.7.

Can someone explain why we would have to change the default behavior of swag to accommodate this?

Say I have:

import bar "github.com/foo/foo"

// @Success 200 {object} bar.Baz
func getFoo() {}

I imagine the logic to be, "if unable to resolve the import "bar" via the real package name, try finding it by its import alias".

Is that how it is supposed to work? Can someone explain why this doesn't work.

@sdghchj
Copy link
Member

sdghchj commented Aug 27, 2020

@sdghchj Still not working in v1.6.7.

Can someone explain why we would have to change the default behavior of swag to accommodate this?

Say I have:

import bar "github.com/foo/foo"

// @Success 200 {object} bar.Baz
func getFoo() {}

I imagine the logic to be, "if unable to resolve the import "bar" via the real package name, try finding it by its import alias".

Is that how it is supposed to work? Can someone explain why this doesn't work.

Package alias is not perfectly supported by v1.6.7, that is, only support in struct members, not in API comments.
Master branch has fixed it. Have a try.

@Blquinn
Copy link

Blquinn commented Sep 2, 2020

@sdghchj I see, thanks for the reply. Is there an issue for aliases in api comments?

@sdghchj
Copy link
Member

sdghchj commented Sep 4, 2020

@Blquinn #736

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

No branches or pull requests

7 participants