added function in openapi3.operation #370

TheRanomial · 2025-01-22T19:36:56Z

Let me explain the purpose of the parameter registration code I added. It handles automatic OpenAPI3 documentation for strongly-typed request parameters.

Consider this example:
type ListBooksParams struct {
APIKey string header:"API-Key"
PageSize *int query:"page_size"
SessionID string cookie:"my-cookie"
}

The parameter registration code I added will:
*Read the struct tags from ListBooksParams
*Create OpenAPI parameter definitions for each tagged field:

This means that when someone looks at your API documentation, they'll see exactly what parameters are expected, where they should be placed (header/query/cookie), and their types - all generated automatically from your Go code's type definitions.
Without this code, you'd have to manually document these parameters in your OpenAPI spec, which could get out of sync with your actual code.

EwenQuim

Thanks for the PR! 🥳

Please fix formatting and add some tests to check that it works as expected.

documentation/docs/guides/controllers.md

ctx.go

openapi.go

EwenQuim

We still need some tests

documentation/docs/guides/controllers.md

EwenQuim

Better, thank you!

But you've made weird choices for testing. Please test your feature, not an helper function! This does not make sense, is there any reason?

openapi.go

register_parameter_test.go

TheRanomial · 2025-01-23T16:47:38Z

Hii @EwenQuim sorry for the mistakes i did previously. I have made some changes,Can you have a look at them.

EwenQuim

Please, when submitting a PR, re-read yourself. There are a lot of things you could have seen yourself.

When submitting a PR, you can click on the "Files changed" tab and review your changes. There, you'll see that there are a lot of changes that are useless, typos or mistakes. Reviewing yourself before asking for a maintainer to review is something usual in CS, don't hesitate to get used to it :)

I hope you'll understand!

ctx.go

examples/petstore/lib/server_test.go

openapi.go

TheRanomial · 2025-01-24T14:10:31Z

Hii @EwenQuim I am extremely sorry because make lint is not working in in laptop so i am not aware of the linting errors if they are present.That is why i reduced the newlines as they were showing me lint error in the actions.

EwenQuim · 2025-01-24T17:41:29Z

No problem! What is not working when you run make ? Can you send us a screenshot so we can debug it for you (and maybe others)?

EwenQuim

Looks very nice! Just a few comments but we're almost there! Thank you for your great work! 🥳

examples/petstore/lib/testdata/doc/openapi.golden.json

openapi.go

EwenQuim · 2025-01-24T17:46:24Z

openapi.go

-		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists != nil {
-			continue
-		}
-		parameter := openapi3.NewPathParameter(pathParam)
-		parameter.Schema = openapi3.NewStringSchema().NewRef()
-		if strings.HasSuffix(pathParam, "...") {
-			parameter.Description += " (might contain slashes)"
+		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists == nil {
+			if strings.Contains(route.Path, "{"+pathParam+"}") {
+				parameter := openapi3.NewPathParameter(pathParam)
+				parameter.Schema = openapi3.NewStringSchema().NewRef()
+				route.Operation.AddParameter(parameter)
+			}


Why did you change it? Did you spot any bug?

One of my test was failing so i had to modify it.

Why resolved???

Fixed tests to make them test the right behaviour

EwenQuim · 2025-01-29T08:19:51Z

I've removed some useless changes and used OptionXxx instead of redefining the OpenAPI logic (so we have a unified parameter declaration).

Thanks a lot @TheRanomial for setting up the structure of all this, this was very useful! Don't hesitate to look at my commits and the current PR.

@dylanhitt what do you think about it?

dylanhitt

Yes this looks great. Not sure what you want to do about that linting issue though.

examples/petstore/lib/testdata/doc/openapi.golden.json

openapi.go

EwenQuim · 2025-01-26T12:26:53Z

openapi.go

-		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists != nil {
-			continue
-		}
-		parameter := openapi3.NewPathParameter(pathParam)
-		parameter.Schema = openapi3.NewStringSchema().NewRef()
-		if strings.HasSuffix(pathParam, "...") {
-			parameter.Description += " (might contain slashes)"
+		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists == nil {
+			if strings.Contains(route.Path, "{"+pathParam+"}") {
+				parameter := openapi3.NewPathParameter(pathParam)
+				parameter.Schema = openapi3.NewStringSchema().NewRef()
+				route.Operation.AddParameter(parameter)
+			}


EwenQuim · 2025-01-28T22:15:33Z

openapi.go

-		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists != nil {
-			continue
-		}
-		parameter := openapi3.NewPathParameter(pathParam)
-		parameter.Schema = openapi3.NewStringSchema().NewRef()
-		if strings.HasSuffix(pathParam, "...") {
-			parameter.Description += " (might contain slashes)"
+		if exists := route.Operation.Parameters.GetByInAndName("path", pathParam); exists == nil {
+			if strings.Contains(route.Path, "{"+pathParam+"}") {
+				parameter := openapi3.NewPathParameter(pathParam)
+				parameter.Schema = openapi3.NewStringSchema().NewRef()
+				route.Operation.AddParameter(parameter)
+			}


Why resolved???

EwenQuim · 2025-01-29T19:04:48Z

Thanks @TheRanomial for this PR! 🎉

Happy to count you in our contributors

added function in openapi3.operation

2002e1e

EwenQuim requested changes Jan 22, 2025

View reviewed changes

documentation/docs/guides/controllers.md Show resolved Hide resolved

ctx.go Outdated Show resolved Hide resolved

Made changes

ce56e65

dylanhitt requested changes Jan 23, 2025

View reviewed changes

openapi.go Outdated Show resolved Hide resolved

openapi.go Outdated Show resolved Hide resolved

EwenQuim requested changes Jan 23, 2025

View reviewed changes

documentation/docs/guides/controllers.md Outdated Show resolved Hide resolved

documentation/docs/guides/controllers.md Outdated Show resolved Hide resolved

applied changes and added test

3f4d45a

EwenQuim requested changes Jan 23, 2025

View reviewed changes

openapi.go Outdated Show resolved Hide resolved

openapi.go Outdated Show resolved Hide resolved

register_parameter_test.go Outdated Show resolved Hide resolved

TheRanomial added 4 commits January 23, 2025 22:18

register_test and function changes

fc8a2d2

solved some errors

62c749d

fixed linting errors

48d79f6

lint errors

3b7390d

EwenQuim requested changes Jan 24, 2025

View reviewed changes

TheRanomial added 3 commits January 24, 2025 15:03

rectified the mistakes

f2d9f54

rectified linting errors

f4fa049

rectified linting errors-2

3f636ac

EwenQuim requested changes Jan 24, 2025

View reviewed changes

fixed linting issues

86adb82

TheRanomial requested review from EwenQuim and dylanhitt January 25, 2025 21:31

TheRanomial and others added 6 commits January 28, 2025 18:15

Merge branch 'main' into strongly_typed_params

d7fd44f

Merge branch 'main' into strongly_typed_params

e16096d

Fixed OpenAPI generation for path params

5f824f3

Fixed tests to make them test the right behaviour

Used OptionXxx instead of creating the parameter manually

43036de

Remove unused RegisterParameters function

68d0e82

Added warning for future syntax for strongly typed params

0962405

dylanhitt approved these changes Jan 29, 2025

View reviewed changes

EwenQuim added 3 commits January 29, 2025 19:44

docs: Use quotes for the incoming syntax warning

6c10c2e

Merge branch 'main' into pr/TheRanomial/370

c2c66de

Moved Route struct to route.go

d542e38

EwenQuim approved these changes Jan 29, 2025

View reviewed changes

EwenQuim merged commit 59cbb1a into go-fuego:main Jan 29, 2025
4 of 5 checks passed

EwenQuim mentioned this pull request Jan 29, 2025

Strongly typed params - declaration #357

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added function in openapi3.operation #370

added function in openapi3.operation #370

TheRanomial commented Jan 22, 2025 •

edited

Loading

EwenQuim left a comment •

edited

Loading

EwenQuim left a comment

EwenQuim left a comment

TheRanomial commented Jan 23, 2025

EwenQuim left a comment

TheRanomial commented Jan 24, 2025

EwenQuim commented Jan 24, 2025

EwenQuim left a comment

EwenQuim Jan 24, 2025

TheRanomial Jan 24, 2025

EwenQuim Jan 26, 2025

EwenQuim Jan 28, 2025

EwenQuim commented Jan 29, 2025

dylanhitt left a comment

EwenQuim Jan 26, 2025

EwenQuim Jan 28, 2025

EwenQuim commented Jan 29, 2025

added function in openapi3.operation #370

added function in openapi3.operation #370

Conversation

TheRanomial commented Jan 22, 2025 • edited Loading

EwenQuim left a comment • edited Loading

Choose a reason for hiding this comment

EwenQuim left a comment

Choose a reason for hiding this comment

EwenQuim left a comment

Choose a reason for hiding this comment

TheRanomial commented Jan 23, 2025

EwenQuim left a comment

Choose a reason for hiding this comment

TheRanomial commented Jan 24, 2025

EwenQuim commented Jan 24, 2025

EwenQuim left a comment

Choose a reason for hiding this comment

EwenQuim Jan 24, 2025

Choose a reason for hiding this comment

TheRanomial Jan 24, 2025

Choose a reason for hiding this comment

EwenQuim Jan 26, 2025

Choose a reason for hiding this comment

EwenQuim Jan 28, 2025

Choose a reason for hiding this comment

EwenQuim commented Jan 29, 2025

dylanhitt left a comment

Choose a reason for hiding this comment

EwenQuim Jan 26, 2025

Choose a reason for hiding this comment

EwenQuim Jan 28, 2025

Choose a reason for hiding this comment

EwenQuim commented Jan 29, 2025

TheRanomial commented Jan 22, 2025 •

edited

Loading

EwenQuim left a comment •

edited

Loading