openapi3gen: add `CreateComponentSchemas` option to export object schemas to components #914

EnriqueL8 · 2024-02-22T12:20:39Z

PR to start the discussion on #909

Signed-off-by: Enrique Lacal <[email protected]>

sskserk · 2024-02-22T13:54:00Z

@EnriqueL8 let me try to test it and thus help you.

sskserk · 2024-02-22T13:54:12Z

EnriqueL8 · 2024-02-22T14:31:17Z

There is a problem in some cases where it actually makes it worse - I think introducing an annotation to ignore structs to be schemas would be helpful as well. Maybe using the struct tag?

So in the case where an api returns an object such as:

{
   size: 50
   objects: [
       <array-with-objects>
   ]
}

The top level should be a schema on the path itself but the type for the object should be in #/components/schema

fenollp · 2024-02-22T15:59:03Z

Hi! Please run ./docs.sh and push the diff so CI can continue.

EnriqueL8 · 2024-02-22T16:22:16Z

Thanks - just making a few more change and will run it

Signed-off-by: Enrique Lacal <[email protected]>

sskserk · 2024-02-22T20:46:07Z

@EnriqueL8,

I'm trying to compile my project having replaced original kin-openapi with your branch.

Observe the problem:

../../../go/pkg/mod/github.com/davidebianchi/[email protected]/main.go:115:19: cannot use openapi3.Paths{} (value of type openapi3.Paths) as *openapi3.Paths value in assignment
../../../go/pkg/mod/github.com/davidebianchi/[email protected]/operation.go:31:22: invalid argument: cannot make openapi3.Responses; type must be slice, map, or channel
../../../go/pkg/mod/github.com/davidebianchi/[email protected]/route.go:101:29: invalid argument: cannot make openapi3.Responses; type must be slice, map, or channel

I guess it because the [email protected] refers to the incompatible version of kin-openapi

Exactly the same problem as mentioned here https://github.com/davidebianchi/gswagger/actions/runs/7914093080/job/21603115911

sskserk · 2024-02-22T21:45:47Z

@EnriqueL8 there is one problem. If there are different types in different packages have the same name then the "NewSchemaRefForValue" produces one component, the same ref.

Example:

package root:

 // Imported package with conflicting type name
import "github.com/getkin/kin-openapi/openapi3gen/subpkg"

type AnotherStruct struct {
  Field1 string `json:"field1"`
  Field2 string `json:"field2"`
  Field3 string `json:"field3"`
}
type RecursiveType struct {
  Field1        string        `json:"field1"`
  Field2        string        `json:"field2"`
  Field3        string        `json:"field3"`
  AnotherStruct AnotherStruct `json:"children,omitempty"`
  
  // Use imported type
  AnotherStruct2         subpkg.AnotherStruct    `json:"children,omitempty"`
}

package subpkg:

type AnotherStruct struct {
  Field4 string `json:"field1"`
}

EnriqueL8 · 2024-02-22T21:58:17Z

Aah I see - is that something that we should document as a limitation as it seems quite improbable? The same thing would occur if that type is cyclic

sskserk · 2024-02-22T22:06:15Z

@EnriqueL8, I think it makes to supply an optional type name generator and use it at openapi3gen.go:315 ... diving into the code

sskserk · 2024-02-22T22:41:22Z

@EnriqueL8 please take a look at your forked repository. I have created a MR into your developed branch. I propose a new option making it possible to customize the generated type name

EnriqueL8 · 2024-02-23T10:04:44Z

thanks @sskserk I'll take a look

type name builder

EnriqueL8 · 2024-02-23T10:06:41Z

Added your commit

sskserk · 2024-02-23T10:12:44Z

@EnriqueL8 let me create a couple of additional tests. I'll bring an additional MR into the current branch.

EnriqueL8 · 2024-02-23T10:22:08Z

@sskserk awesome - I'm also testing this

Signed-off-by: Enrique Lacal <[email protected]>

.github/docs/openapi3gen_pack.txt

EnriqueL8 · 2024-02-23T15:04:08Z

I'm wondering if we should have a customiser for ignoring components instead of only top level? will leave to the reviewer to decide @fenollp fyi

EnriqueL8 · 2024-02-23T17:27:26Z

@sskserk I've found another issue here when you have a generic field, ignoring the top level is not enough if it's nested so you want to be able to not add generics to component schemas as you will get one per type use which is awful

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 · 2024-02-23T17:59:27Z

Pushed up a commit for handling generics - I'm sure there is a better way so any thoughts appreciated

Signed-off-by: Enrique Lacal <[email protected]>

sskserk · 2024-02-28T16:36:09Z

@EnriqueL8 & @fenollp , let me just share the news. A related project has been fixed as well davidebianchi/gswagger#148

EnriqueL8 · 2024-02-28T17:50:22Z

This is ready for review

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 · 2024-02-29T14:39:37Z

@fenollp I'm seeing a lot of issues with found unresolved ref , it seem that the package so far made an assumption that all refs are resolved? So no reference should exist in the final document?

It might be that I am using validate wrong and should be loading the document generated into a loader and the validate it? Okay that works if I do:

SwaggerGen.Generate a document
Load that into a loader (this will resolve the references)
Validate that document I loaded

Maybe there is a flag on the validate to allow references? or is that not a valid OpenAPI3 document?

EnriqueL8 · 2024-03-04T18:01:04Z

I've noticed another issue with map[string]interface{} as we are using the type object to export the schema. Ideally we shouldn't be exporting the top level maps to schemas.

@fenollp if you get a chance would appreciate a first review as well 😃

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 · 2024-03-04T18:11:45Z

Alright pushed up fix for maps, only perform the export for structs

sskserk · 2024-03-11T15:56:02Z

@fenollp do you think this MR is ready to be merged?

EnriqueL8 · 2024-03-18T15:35:28Z

@fenollp any bandwidth to take a look at this? Note would appreciate your input on #914 (comment)

fenollp

Mostly nitpicks, LGTM!

openapi3gen/subpkg/sub_type.go

openapi3gen/openapi3gen_test.go

openapi3gen/openapi3gen.go

fenollp · 2024-03-22T19:11:27Z

openapi3gen/openapi3gen.go

+
+	// Best way I could find to check that
+	// this current type is a generic
+	isGeneric, err := regexp.Match(`^.*\[.*\]$`, []byte(t.Name()))


How about just checking that the last rune is ]?

would be an array in that case?

ah yes, then runes[-1] == ']' && runes[-2] != '['

openapi3gen/openapi3gen.go

openapi3gen/openapi3gen_test.go

EnriqueL8 · 2024-03-25T13:30:18Z

@fenollp thanks for the review! Will push changes up soon

address comment

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 · 2024-03-28T11:51:33Z

@fenollp Ready for re-review 😃

fenollp

Looks like it's all down to that regex thing and we're good :)

fenollp · 2024-03-29T18:44:43Z

openapi3gen/openapi3gen.go

+
+	// Best way I could find to check that
+	// this current type is a generic
+	isGeneric, err := regexp.Match(`^.*\[.*\]$`, []byte(t.Name()))


ah yes, then runes[-1] == ']' && runes[-2] != '['

fenollp · 2024-04-06T09:32:03Z

Hey there! Please rebase & I'll make this move along.

fenollp

Rebase please

EnriqueL8 · 2024-04-09T11:32:50Z

@fenollp Thanks, sorry was away and not available to rebase! It's a shame that the commit was squashed and not awarded to the contributors of the PR

fenollp · 2024-04-09T11:50:43Z

Hi @EnriqueL8, you're right. I messed up while rebasing your PR and the shared attribution went to someone that's not even taken part in that part of the lib. I should have checked, sorry.

I do not mean to steal anyone's work and would very much prefer to give attribution where it's deserved (e.g. here). So I suggest this: send a PR that both reverts then applies your commit (add some changes if you want but that's not needed). At least this way attribution is assured. If you have other ideas, I'm open :)

EnriqueL8 · 2024-04-09T12:05:37Z

Sounds good - I'll do that!

EnriqueL8 · 2024-04-09T15:29:31Z

@fenollp I think this solves it #937

Add ability to export object schemas to components

27f199f

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 mentioned this pull request Feb 22, 2024

#/components/schemas across routes #909

Closed

fenollp changed the title ~~Add ability to export object schemas to components~~ openapi3gen: add CreateComponentSchemas option to export object schemas to components Feb 22, 2024

EnriqueL8 added 3 commits February 22, 2024 16:22

feat: Add ability to ignore exporting top level schema

89d9be2

Signed-off-by: Enrique Lacal <[email protected]>

Update docs

b5c51e1

Signed-off-by: Enrique Lacal <[email protected]>

fix test format

63a1f28

Signed-off-by: Enrique Lacal <[email protected]>

type name builder

2cc5392

Merge pull request #2 from sskserk/type_name_builder

37ccde6

type name builder

EnriqueL8 added 2 commits February 23, 2024 12:06

Add docs

341c02f

Signed-off-by: Enrique Lacal <[email protected]>

fix tests and sub type

dbc7a0c

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 marked this pull request as ready for review February 23, 2024 14:54

EnriqueL8 commented Feb 23, 2024

View reviewed changes

.github/docs/openapi3gen_pack.txt Outdated Show resolved Hide resolved

Add support to ignore exporting generics

0c6163e

Signed-off-by: Enrique Lacal <[email protected]>

fix check for object

eb85980

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 added 2 commits February 29, 2024 13:49

Rename fields to export

e0111a1

Signed-off-by: Enrique Lacal <[email protected]>

fix docs

a357586

Signed-off-by: Enrique Lacal <[email protected]>

EnriqueL8 mentioned this pull request Feb 29, 2024

feat: export input and output schemas to components in OpenAPI document hyperledger/firefly-common#131

Draft

fix: exporting schemas that are maps

0883c33

Signed-off-by: Enrique Lacal <[email protected]>

fenollp requested changes Mar 22, 2024

View reviewed changes

sskserk and others added 5 commits March 25, 2024 22:09

address comment

7a9b932

Merge pull request #6 from sskserk/type_name_builder

98749de

address comment

Review comments and fixes

cfcfe42

Signed-off-by: Enrique Lacal <[email protected]>

Exclude subpkg docs

3f39f82

Signed-off-by: Enrique Lacal <[email protected]>

Fix linting

83b175e

Signed-off-by: Enrique Lacal <[email protected]>

fenollp requested changes Mar 29, 2024

View reviewed changes

fenollp requested changes Apr 6, 2024

View reviewed changes

fenollp mentioned this pull request Apr 6, 2024

openapi3gen: add CreateComponentSchemas option to export object schemas to components #935

Merged

fenollp linked an issue Apr 6, 2024 that may be closed by this pull request

#/components/schemas across routes #909

Closed

fenollp closed this in #935 Apr 6, 2024

openapi3gen: add CreateComponentSchemas option to export object schemas to components #914

openapi3gen: add CreateComponentSchemas option to export object schemas to components #914

Conversation

EnriqueL8 commented Feb 22, 2024 • edited Loading

sskserk commented Feb 22, 2024

sskserk commented Feb 22, 2024

EnriqueL8 commented Feb 22, 2024 • edited Loading

fenollp commented Feb 22, 2024

EnriqueL8 commented Feb 22, 2024

sskserk commented Feb 22, 2024 • edited Loading

sskserk commented Feb 22, 2024 • edited Loading

EnriqueL8 commented Feb 22, 2024

sskserk commented Feb 22, 2024

sskserk commented Feb 22, 2024 • edited Loading

EnriqueL8 commented Feb 23, 2024

EnriqueL8 commented Feb 23, 2024

sskserk commented Feb 23, 2024

EnriqueL8 commented Feb 23, 2024

EnriqueL8 commented Feb 23, 2024

EnriqueL8 commented Feb 23, 2024 • edited Loading

EnriqueL8 commented Feb 23, 2024

sskserk commented Feb 28, 2024

EnriqueL8 commented Feb 28, 2024

EnriqueL8 commented Feb 29, 2024 • edited Loading

EnriqueL8 commented Mar 4, 2024

EnriqueL8 commented Mar 4, 2024

sskserk commented Mar 11, 2024

EnriqueL8 commented Mar 18, 2024

fenollp left a comment

Choose a reason for hiding this comment

fenollp Mar 22, 2024

Choose a reason for hiding this comment

EnriqueL8 Mar 25, 2024

Choose a reason for hiding this comment

fenollp Mar 29, 2024

Choose a reason for hiding this comment

EnriqueL8 commented Mar 25, 2024

EnriqueL8 commented Mar 28, 2024

fenollp left a comment

Choose a reason for hiding this comment

fenollp Mar 29, 2024

Choose a reason for hiding this comment

fenollp commented Apr 6, 2024

fenollp left a comment

Choose a reason for hiding this comment

EnriqueL8 commented Apr 9, 2024

fenollp commented Apr 9, 2024

EnriqueL8 commented Apr 9, 2024

EnriqueL8 commented Apr 9, 2024

openapi3gen: add `CreateComponentSchemas` option to export object schemas to components #914

openapi3gen: add `CreateComponentSchemas` option to export object schemas to components #914

EnriqueL8 commented Feb 22, 2024 •

edited

Loading

EnriqueL8 commented Feb 22, 2024 •

edited

Loading

sskserk commented Feb 22, 2024 •

edited

Loading

sskserk commented Feb 22, 2024 •

edited

Loading

sskserk commented Feb 22, 2024 •

edited

Loading

EnriqueL8 commented Feb 23, 2024 •

edited

Loading

EnriqueL8 commented Feb 29, 2024 •

edited

Loading