Add support for Gorilla generation

[jamietanna]

So far this seems to have worked for one of my services, but would be good to hear if others have success.

I'll also be adding the examples/ version so folks can see how that works too.

Closes #465.

[jamietanna]

Hey @deepmap-marcinr sorry for the nudge, but wondering if there's any chance you'd be able to have a review of this, to see if it fits in with how we'd want to do this? 🙏

[deepmap-marcinr]

Oh wow, you did a lot of work here. I'm happy to merge this shortly. Looking the generated code looks pretty reasonable - it's interesting how middleware is done by making a chain of functions that return handlers.

[jamietanna]

Thanks! I think that's right, but maybe I should double check. This was mostly followed by using Chi as a base 🤔

[jamietanna]

Having a look through Gorilla's middleware handling code it's similar to Chi's middleware handling code, which both appear to be based on how net/http handles it, so happy for this to be merged when you're happy with it 🙌

[micklove]

@jamietanna , nice work, this is great, have been testing for the last few days, works well 👏
Will your gorilla changes be in the next release?

FYI - I used an earlier commit, on the master branch , to test your stuff, works well.
However, today, when I used the latest master (my build pipeline is just pulling the latest master, till the next release), it appears the mux import is no longer being written out)... example below...

[jamietanna]

Thanks Mick, yep I'm currently using it in production (https://deliveroo.engineering/2022/06/27/openapi-design-first.html) pinned to a commit off HEAD.

That's odd, mind sharing the config file you're using to generate with? I pulled HEAD yesterday and it still seemed to generate OK for me 🤔

And yes, they'll be in for the next release ☺

[micklove]

Cheers Jamie, it appears to be something going on in the GA pipeline (latest HEAD working locally for me too), I'll sort it :)
(I'll dump the details here if they are relevant, and not just PEBKAC)

V Nice blog post BTW, will have a look at some of those tools you mentioned too, your hard work is much appreciated

[corani]

Hi @jamietanna, thanks for the great work!

I had hacked together something similar but would like to switch to this implementation. A few differences I noticed that may be worth to discuss:

1. You're generating type aliases (e.g. type Status = string) while I was generating new types (e.g. type Status string).
2. For enum values you're generating constants named directly after the string value. To avoid collisions I was prefixing them by the type.

Example of both:

status:
  type: string
  enum: [ success, failed ]

// Yours:
type Status = string

const (
    Success Status = "success"
    Failed  Status = "failed"
)

// Mine:
type Status string

const (
    StatusSuccess Status = "success"
    StatusFailed  Status = "failed"
)

3. Using new types allows you to define methods on them, so for enum types I was generating a custom UnmarshalJSON method that validated that the provided enum value was actually allowed by the spec (not sure if this is too much "magic").

func (v *Status) UnmarshalJSON(bs []byte) error {
    var s string

    if err := json.Unmarshal(bs, &s); err != nil {
        return err
    }

    switch Status(s) {
    case StatusSuccess, StatusFailed:
        *v = Status(s)

        return nil
    default:
        return fmt.Errorf("invalid enum value %q", s)
    }
}

4. It appears some useless code is being generated (or some code is missing) for optional query parameters:

// ------------- Optional query parameter "name" -------------
if paramValue := r.URL.Query().Get("name"); paramValue != "" {
    // this is empty
}

// BindQueryParameter

5. When setting the Methods for the Gorilla HandleFunc you're using strings (e.g. "GET") while I used the constants in the http package (e.g. http.MethodGet).

[jamietanna]

Hey, so generally this PR echoes how we've done it in the other servers, and these pieces of feedback may be worth adding as separate issues to get general feedback that we can align all servers with?

For 2 - #625 may be worth looking at and raising a PR if possible - it was originally added in #549 to only selectively do it, but makes sense to be all the time.

[corani]

I've created new issues for them:

1. Feature: Generate new types rather than type aliases #656
2. reuse Enum value collision #625
3. Feature: automatic validation of enum values #657
4. Bug: useless code generated for optional query parameters #658
5. Use http.MethodXXX constants instead of string literals #659