Feat: Add Fiber server support by Jleagle · Pull Request #813 · oapi-codegen/oapi-codegen

Jleagle · 2022-10-24T12:00:20Z

Fixes #301

Massive thank you to @gnirb for already doing some of the PR.

gnirb · 2023-02-08T08:46:25Z

Hello @Jleagle, how is the PR going? Can I be of any assistance?

Jleagle · 2023-02-09T20:38:08Z

@gnirb
Hi, i made a start and merged in most of your work. Last time i tested it, it didn't actually generate anything 🤷. Not had much time to find out why recently. Any help would be appreciated.

gnirb · 2023-02-16T13:37:30Z

@gnirb Hi, i made a start and merged in most of your work. Last time i tested it, it didn't actually generate anything 🤷. Not had much time to find out why recently. Any help would be appreciated.

Anything outstanding now before removing "draft"-status for the PR?

Jleagle · 2023-02-20T14:22:07Z

@deepmap-marcinr Hi, any chance one of your team could take a look at this please? Thanks!

mfiumara · 2023-02-26T23:15:38Z

Nice work, is it already usable from your branch? Or still requires testing?

Jleagle · 2023-02-27T11:04:38Z

@mfiumara
I have not started using it in production yet but so far everything works, lots of the code comes from @gnirb's branch who uses it internally too. More testing can only be a good thing though.

leonklingele · 2023-01-13T02:28:45Z

 // Code generated by github.com/deepmap/oapi-codegen version (devel) DO NOT EDIT.
 package headdigitofhttpheader

-import (


Why was this removed?

Don't know, but I'll revert it

leonklingele · 2023-01-13T02:29:11Z

 //
 // Code generated by github.com/deepmap/oapi-codegen version (devel) DO NOT EDIT.
 package head_digit_of_operation_id
-


Why was this removed?

leonklingele · 2023-01-13T02:40:16Z

 	github.com/gin-gonic/gin v1.8.1
 	github.com/go-chi/chi/v5 v5.0.8
+	github.com/gofiber/adaptor/v2 v2.1.30
+	github.com/gofiber/fiber/v2 v2.40.1


This should be updated.

leonklingele · 2023-01-13T02:51:34Z

@@ -0,0 +1,129 @@
+//go:generate go run github.com/deepmap/oapi-codegen/cmd/oapi-codegen --config=server.cfg.yaml ../strict-schema.yaml


Why isn't this file a 1:1 copy of internal/test/strict-server/echo/server.go?

Not sure, changing..

leonklingele · 2023-03-04T13:42:53Z

+	// Parameter object where we will unmarshal all parameters from the context
+	var params FindPetsParams
+
+	query, err := url.ParseQuery(string(c.Request().URI().QueryString()))


This could use fiber's func (c *Ctx) QueryParser(out interface{}) error func https://docs.gofiber.io/api/ctx/#queryparser

After some investigation it seems like this requires all the generated request struct query fields to be tagged with query: <name>.

Gonna leave it as a future improvement, since it looks like it will require changes to the type generation potentially affecting the other server implementations too.

leonklingele · 2023-03-04T13:43:07Z

+// FindPets operation middleware
+func (siw *ServerInterfaceWrapper) FindPets(c *fiber.Ctx) error {
+
+	var err error


This doesn't do anything.

leonklingele · 2023-03-04T13:50:03Z

Good job! 🥳 Added some comments.

deepmap-marcinr · 2023-03-06T17:17:31Z

This looks good overall, with some cosmetics issues.

Can you rearrange your imports into 3 sections - stdlib, third party imports, than oapi-codegen imports? It's what other files do.

This change is isolated to Fiber mainly, but it gives us yet another router to support. I really need to think about making this modular somehow, so that we don't need to change the core oapi-codegen to provide router specific code, but this isn't anything to do with your commit. Please respond to the good questions by @leonklingele and I'm happy to merge once everything is resolved.

Jleagle · 2023-04-10T20:31:34Z

This looks good overall, with some cosmetics issues.

Can you rearrange your imports into 3 sections - stdlib, third party imports, than oapi-codegen imports? It's what other files do.

This change is isolated to Fiber mainly, but it gives us yet another router to support. I really need to think about making this modular somehow, so that we don't need to change the core oapi-codegen to provide router specific code, but this isn't anything to do with your commit. Please respond to the good questions by @leonklingele and I'm happy to merge once everything is resolved.

Imports updated.

@gnirb Would you be able to help answer leonklingele's questions?

leonklingele · 2023-04-10T21:44:35Z

Sorry! Meant to say t.Helper()

gnirb · 2023-04-11T08:07:51Z

@gnirb Would you be able to help answer leonklingele's questions?

Sure, I'll take a look!

gnirb · 2023-04-11T09:30:41Z

Opened a PR to address the comments https://github.com/Jleagle/oapi-codegen/pull/29, please take a look @Jleagle

gnirb · 2023-04-11T11:07:12Z

Alright @leonklingele comments addressed, please take another look if this feels good to go

Jleagle · 2023-04-27T13:19:37Z

This looks good overall, with some cosmetics issues.

Can you rearrange your imports into 3 sections - stdlib, third party imports, than oapi-codegen imports? It's what other files do.

This change is isolated to Fiber mainly, but it gives us yet another router to support. I really need to think about making this modular somehow, so that we don't need to change the core oapi-codegen to provide router specific code, but this isn't anything to do with your commit. Please respond to the good questions by @leonklingele and I'm happy to merge once everything is resolved.

@deepmap-marcinr. I think everything has been resolved 👍

leonklingele

Last-minute review incoming 🫨

Almost perfect, just some small modifications which also make it a bit faster by removing unnecessary type conversions. Thanks for your work here, @Jleagle!

leonklingele · 2023-05-13T18:28:23Z

@Jleagle please also do one final rebase to master :)

Co-authored-by: leonklingele <git@leonklingele.de>

leonklingele

lgtm! @deepmap-marcinr what do you say? :)

deepmap-marcinr · 2023-06-02T00:15:42Z

lgtm! @deepmap-marcinr what do you say? :)

This is a big change and another router to maintain, but it's nicely isolated. Could you rebase and fix the conflicts, and then I'll merge it, and release it in 1.13.0, which I intend to release as soon as this commit is ready.

deepmap-marcinr · 2023-06-02T14:23:13Z

I pulled your change to test locally, and some things are missing.

run go mod tidy
regenerate all your fiber boilerplate
Tests fail:

# github.com/deepmap/oapi-codegen/internal/test/strict-server/fiber
internal/test/strict-server/fiber/server.gen.go:810:10: request.PType undefined (type ReservedGoKeywordParametersRequestObject has no field or method PType)

- Update modules - Add missing test handler - Fix variable naming in strict server wrappers

deepmap-marcinr · 2023-06-02T16:38:37Z

@Jleagle - please see the PR I sent you which fixes these problems.
https://github.com/Jleagle/oapi-codegen/pull/31

After you merge that, pls make lint pass.

Updates to 813

* Revert generated * Add to cmd * Add to codegen * Fiber * Fix imports * Templates * Split types and code * Rename aliases * Strict * tests * Validate middleware * Templates * Update petstore * Generate * Generate * Readme * Fix tests * Fix build * tweaks to get it running * fixed linter errors, even though other servers seem to just ignore these * Fix option yaml * Fix tests * Mod tidy * Remove extra space * Check errors * Update pkg/fiber-middleware/oapi_validate_test.go Co-authored-by: leonklingele <git@leonklingele.de> * Update pkg/fiber-middleware/oapi_validate_test.go Co-authored-by: leonklingele <git@leonklingele.de> * Update pkg/fiber-middleware/oapi_validate_test.go Co-authored-by: leonklingele <git@leonklingele.de> * Update pkg/fiber-middleware/oapi_validate_test.go Co-authored-by: leonklingele <git@leonklingele.de> * Update pkg/fiber-middleware/oapi_validate_test.go Co-authored-by: leonklingele <git@leonklingele.de> * Separate oapi-codegen imports * Use t.Helper() * fixing comments * Sort imports * Use WriteString * Fix imports * Mods * Generate * Add missing config * Use new adaptor * Use new adaptor * Fix compilation and test issues - Update modules - Add missing test handler - Fix variable naming in strict server wrappers --------- Co-authored-by: Carl Bring <111354161+gnirb@users.noreply.github.com> Co-authored-by: leonklingele <git@leonklingele.de> Co-authored-by: marcinromaszewicz <marcinromaszewicz@deepmap.ai>

Jleagle marked this pull request as ready for review February 16, 2023 21:10

leonklingele reviewed Mar 4, 2023

View reviewed changes

Comment thread pkg/fiber-middleware/oapi_validate_test.go

Comment thread pkg/fiber-middleware/oapi_validate_test.go Outdated

leonklingele reviewed Mar 4, 2023

View reviewed changes

leonklingele reviewed May 13, 2023

View reviewed changes

Comment thread pkg/fiber-middleware/oapi_validate.go Outdated

leonklingele mentioned this pull request May 13, 2023

Fiber support #1022

Closed

Jleagle added 8 commits May 14, 2023 21:08

Revert generated

aac7e58

Add to cmd

a2d3029

Add to codegen

6804a3b

Fiber

e98165b

Fix imports

4b511df

Templates

9723041

Split types and code

95bac95

Rename aliases

cd77c32

Jleagle and others added 15 commits May 14, 2023 21:23

Update pkg/fiber-middleware/oapi_validate_test.go

9950d5f

Co-authored-by: leonklingele <git@leonklingele.de>

Update pkg/fiber-middleware/oapi_validate_test.go

72f97a0

Co-authored-by: leonklingele <git@leonklingele.de>

Update pkg/fiber-middleware/oapi_validate_test.go

f7374b7

Co-authored-by: leonklingele <git@leonklingele.de>

Update pkg/fiber-middleware/oapi_validate_test.go

532ee1d

Co-authored-by: leonklingele <git@leonklingele.de>

Separate oapi-codegen imports

75cf747

Use t.Helper()

84953e7

fixing comments

fab7a48

Sort imports

4030fff

Use WriteString

25b5227

Fix imports

6198c7f

Mods

5d342cb

Generate

dfafdc4

Add missing config

22089bf

Use new adaptor

c58882a

Use new adaptor

c6e3a43

leonklingele approved these changes Jun 1, 2023

View reviewed changes

Jleagle and others added 2 commits June 2, 2023 08:40

Merge remote-tracking branch 'upstream/master'

6f1ff00

Merge remote-tracking branch 'origin/master' into fiber

91094f6

marcinromaszewicz added 2 commits June 2, 2023 09:01

Merge remote-tracking branch 'origin/master' into fiber

88e1309

Fix compilation and test issues

7fc79c6

- Update modules - Add missing test handler - Fix variable naming in strict server wrappers

deepmap-marcinr reviewed Jun 2, 2023

View reviewed changes

Comment thread pkg/codegen/templates/strict/strict-fiber.tmpl Outdated

Comment thread internal/test/strict-server/fiber/server.go

Jleagle added 2 commits June 2, 2023 18:08

Merge pull request #31 from deepmap/updates-to-813

b890a90

Updates to 813

Merge branch 'deepmap:master' into master

4f5751f

deepmap-marcinr merged commit 7aa85bb into oapi-codegen:master Jun 2, 2023

		@@ -0,0 +1,129 @@
		//go:generate go run github.com/deepmap/oapi-codegen/cmd/oapi-codegen --config=server.cfg.yaml ../strict-schema.yaml

Uh oh!

Conversation

Jleagle commented Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnirb commented Feb 8, 2023

Uh oh!

Jleagle commented Feb 9, 2023

Uh oh!

gnirb commented Feb 16, 2023

Uh oh!

Jleagle commented Feb 20, 2023

Uh oh!

mfiumara commented Feb 26, 2023

Uh oh!

Jleagle commented Feb 27, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnirb Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonklingele commented Mar 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deepmap-marcinr commented Mar 6, 2023

Uh oh!

Jleagle commented Apr 10, 2023

Uh oh!

leonklingele commented Apr 10, 2023 via email

Uh oh!

gnirb commented Apr 11, 2023

Uh oh!

gnirb commented Apr 11, 2023

Uh oh!

gnirb commented Apr 11, 2023

Uh oh!

Jleagle commented Apr 27, 2023

Uh oh!

leonklingele left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leonklingele commented May 13, 2023

Uh oh!

Uh oh!

leonklingele left a comment

Choose a reason for hiding this comment

Jleagle commented Oct 24, 2022 •

edited

Loading

gnirb Apr 11, 2023 •

edited

Loading