We really need documentation on how middleware is processed, as I now found out. The last middleware is the one which is called first, so:

x-go-middlewares: [auth,something-that-needs-auth]

will run something-that-needs-auth first. Ordering is intact, however, you would really think it is the other way around.

I needed something to be able to set the go types to custom made types. x-go-type allows for this but currently only for types which are defined in the same scope as the generated code. As this is highly impractical for me, I introduced x-go-type-external.

I did not rewrite the x-go-type as a) I really didn't want to destroy that part 😄 b) I think it makes sense to have those two options seperated as one is generally for scope level and one gives you all the options you need, but needs more options filled out.

How does it work?

You can use it the same way as x-go-type and it overwrites the given standard type:

x-go-type-external:
    type: CustomString
    import: github.com/discord-gophers/goapi-gen/examples/petstore-expanded/types
    alias: customTypes

This gives you the option to reference the custom type, here CustomString, and specify the import path. The alias option is available so we can make sure not to make go angry because we import time and github.com/tiehm/time for example but assign it an alias. alias is currently mandatory but could be made optional as you might not need it for every external type.

The resultung code uses the alias and type to generate the typing, for example:

type NewPet struct {
	// Name of the pet
        //         vvv alias       vvv type          
	Name customTypes.CustomString `json:"name"`

	// Type of the pet
	Tag *string `json:"tag,omitempty"`
}

The working example is also attached in the petstore-extended example.

Please forgive me if this is not the cleanest code or if I made spelling mistakes in the PR... it is 5:30 am but I really wanted to try this out.

I would love some feedback on it and of course suggestions on how to make this better, as I am sure my approach is not the best possible.

Right now you have to manually decode every request body on every request handler which uses request bodies. It's almost always pretty much the same stuff like

var newPet NewPet
if err := json.NewDecoder(r.Body).Decode(&newPet); err != nil {
	sendPetstoreError(w, http.StatusBadRequest, "Invalid format for NewPet")
	return
}

This could be circumvented by introducing a feature to automatically generate/decode the bodies in the generated request handler before it is passed to the handler which is written by the developer.

As an example from the petstore example: We switch from

// ServerInterface represents all server handlers.
type ServerInterface interface {
	// ...
	// Creates a new pet
	// (POST /pets)
	AddPet(w http.ResponseWriter, r *http.Request)
	// ...
}

to

// ServerInterface represents all server handlers.
type ServerInterface interface {
	// ...
	// Creates a new pet
	// (POST /pets)
	AddPet(w http.ResponseWriter, r *http.Request, body NewPet)
	// ...
}

and directly pass the body the handler which then can implement it and directly use it:

func (p *PetStore) AddPet(w http.ResponseWriter, r *http.Request, body NewPet) {
	fmt.Sprintf("name is %s", body.Name)
}

instead of

func (p *PetStore) AddPet(w http.ResponseWriter, r *http.Request) {
	var body NewPet
	if err := json.NewDecoder(r.Body).Decode(&newPet); err != nil {
		sendPetstoreError(w, http.StatusBadRequest, "Invalid format for NewPet")
		return
	}
	fmt.Sprintf("name is %s", body.Name)
}

This could also implement required checks on body fields and error if those fail automatically without checking it everytime you decode the request body.

The issue behind this is obviously that you now have less control over your error handling for this specific case. A solution could be a global optional function which can be changed to spit out dynamic errors if needed. An example could be:

// maybe a bit too long of a function name :)
func dynamicDecodeErrorsOnRequiredFieldFails(route, field string, err error) string {
    return fmt.Sprintf("invalid body: field %s has issue %s", field, err.Error())
}

or anything like that. This way custom errors could be kept while letting the codegen take care of decoding the body. Another option could be to not pass the body structure itself to the handler but a struct which has the body in it (if it was successfully decoded) and the error if there is any. This way you have full control over everything but this of course adds a bit more code and changes types.

As title. This will require a secret called GITHUB_TOKEN, however.

Not sure who wants to generate one or if we want a machine user for discord-gophers,

Context

For methods such as DELETE, PATCH, PUT and POST, we are often met with the need of just returning a response code.

Having a helper method landing after #80 (returning a *Response) directly means that we are obliged to either return something like

(we will go with openapi as the generated package name)

var r openapi.Response
r.Status(http.StatusNoContent)
return &r

Or to write the status code directly, similarly to this code @diamondburned refactored.

w.WriteHeader(http.StatusNoContent)
return nil

Proposal

Generating a function to help with this

func ResponseCode(code int) *Response {
	return &Response{
		statusCode: code,
	}
}

Which would turn this situation into

return openapi.ResponseCode(http.StatusNoContent)

I think this would make every single return meaningful, such that developers could identify code paths that actually respond to the request simpler, rather than having them search for status code being set, or not, earlier.

• return correctly required param
• delete dead code
• replace usage of deprecated function
• update go:generate command of moq to use latest, since it seems to be causing issues.
• update required param error tests

POSSIBLE BREAKING: multiple word call to title helper function (no change in generation, and it generates valid code, but to be aware if needs be).

Unmarshalling a string to an enum currently unmarshals an empty string because the value is never set and persisted internally.

This MR fixes two issues:

• UnmarshalJSON never updated the internal value after unmarshaling.
• Methods must have a pointer receiver for updates to take effect.

Currently, enums are converted into const values of a particular type, such as:

type WebhookEvent string

const (
	WebhookEventShippingUpdated WebhookEvent = "ShippingUpdated"
)

This is suboptimal since any parameter that accepts a WebhookEvent would still accept any arbitrary string.

A good workaround would be to use struct-based enums, allowing for a greater measure of type safety, generating code similar to:

type WebhookEvent struct {
	value string
}

func (e WebhookEvent) String() string {
	return e.value
}

var (
	Unknown   = WebhookEvent{""}
	ShippingUpdated = WebhookEvent{"ShippingUpdated"}
)

Why

Having a few thousand lines of generated code in a single file isn't always readable. Having the ability to break down those files for easier navigation is something I was using before, but had to run 2 commands.

With the revamp of the cli, I am hoping that this can be natively supported.

Example

This following snippet should generate 2 files, a types one, with all the structs and possibly Binder methods, and a server one, with the routing, binding and interfaces.

Example command

goapi-gen --server=server.gen.go --types=types.gen.go

Edge case

By default, the server relies on the models, so in case the typed flag is empty, the types should be generated too, in the server.

The current repository has a lot of redundant directories and has no Go code in the project root. This can most likely be updated to be more easy to work with, as well as easier to go install.

In a first step to making this hard-fork simpler to maintain and better to use for members of discord gophers, dropping Echo support due to the lack of need and additional complexity is a must.

Tasks:

• [ ] Edit CLI
• [ ] Edit README
• [ ] Remove Templates
• [ ] Remove Middleware code

This PR cleans up the ugly middleware map in favor of a clean, generated struct.

The goal is to profit from the excellent autocompletion provided by IDEs to work faster.

As an added bonus, if you had previously used a middleware and forgot it in the spec, the code wont compile, as opposed to silently running, ensuring not ever forgetting a middleware both in the code and in the spec (If you've never typed it, I can't do anything for you)

This PR adds a Reponser interface, which enables us to construct any form of custom type and turn them into responses accepted by goapi-gen.

The goal of this proposal is to shorten the error handling code, and clean up ugly multi-line errors made when constructing a non-standard/specified response body.

The example provided with the petstore may not be the best because of it's short nature, but we can easily save dozens of lines of code, and standardize better with such an API.

Right now, returning concrete types is nice for well defined return types, such as 200s, but once you want to return simple errors, it becomes a pain.

For example, let's say I want to return an InternalServerError with an error code and clarification.

With the status quo, this looks like:

	return CreateThingJSONDefaultResponse(
		DefaultResponse{Code: "spiky-sun-turtle", Message: http.StatusText(http.StatusInternalServerError)},
	).Status(500)

This gets really verbose really fast. What we could have instead is

type InternalServerError struct {
    Code    string `json:"code"`
    Message string `json:"message"`
}

func (e InternalServerError) Respond() *Response {
    return Response{Code: 500, Body: e, ContentType: "application/json"}
}

and use it like so:

return InternalServerError{Code: "goop-turtle", Message: http.StatusText(http.StatusInternalServerError)}

Any type implementing

type Responder interface {
     Respond() *Response
}

Could be returned, thus easily implementing custom errors in a much less verbose way.

This would also allow us to have possible hooks in responses, to avoid having more telemetry/logging directly in the handler, since those types could only wrap parts intended for the users, while we could abstract the telemetry to our own functions, thus making error handling go from 5/7 lines per error to 1 or 2.

goapi-gen currently errors out with an "in": "formData" parameter:

        "consumes": [
          "multipart/form-data"
        ],
        "parameters": [
          {
            "in": "formData",
            "name": "content",
            "type": "file",
            "description": "The content of the file.",
            "required": true
          }
        ],

yields the error

error: could not generate code: error creating operation definitions: error describing global parameters for POST//assets: error generating type for param (content): parameter 'content' has no schema or content

Problem

goapi-gen currently does not autogenerate anything for security. Instead, the user is supposed to use a long-winded way of adding a middleware which also doesn't support dependency injection via context.Contexts.

The current method also requires the user to manually verify authorization information by reading the *http.Request instance within the authentication callback.

Below is an example of such code for BearerAuth:

func (h handler) authenticate(ctx context.Context, in *openapi3filter.AuthenticationInput) error {
	switch in.SecuritySchemeName {
	case "BearerAuth":
		auth := in.RequestValidationInput.Request.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			return fmt.Errorf("invalid BearerAuth header: missing prefix")
		}

		auth = strings.TrimPrefix(auth, "Bearer ")

		s, err := h.server.AuthorizerServer().Authorize(ctx, auth)
		if err != nil {
			return err
		}

		// Checking is done, but s cannot be passed down!
		return nil
	default:
		return fmt.Errorf("unsupported auth scheme %q", in.SecuritySchemeName)
	}
}

This code is then used like so:

	validator := middleware.OapiRequestValidatorWithOptions(nil, &middleware.Options{
		Options: openapi3filter.Options{
			AuthenticationFunc: handler.authenticate,
		},
	})

This abstraction by itself isn't any easier than just using a regular middleware, except for the fact that it's still not possible to inject any value into later handlers. For comparison, such a middleware would look something like this:

func (h handler) mustAuthorize(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		token := r.Header.Get("Authorization")
		if !strings.HasPrefix(token, "Bearer ") {
			h.writeError(w, 401, fmt.Errorf("invalid BearerAuth header: missing prefix"))
			return
		}

		token = strings.TrimPrefix(token, "Bearer ")

		s, err := h.server.AuthorizerServer().Authorize(r.Context(), token)
		if err != nil {
			h.writeError(w, 401, err)
			return
		}

		ctx := context.WithValue(r.Context(), sessionKey, s)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

Clearly, goapi-gen isn't generating much at all when it comes to security.

Proposal

A possible way to improve this situation would be to make goapi-gen generate middlewares that inject the related security information for the user to check from a middleware.

For example, given the following JSON (with all routes and schemas omitted):

{
  "security": [
    {
      "BearerAuth": [],
    }
  ],  
  "components": {
    "securitySchemes": {
      "BearerAuth": {
        "type": "http",
        "scheme": "bearer"
      }
    }
  }
}

goapi-gen can generate code that handles checking for a valid security scheme and validating the Authorization: Bearer header. It can then generate the above callbacks, which can be tucked inside the autogenerated ServerOptions.

For the above example, that would look like this:

// UnauthorizedError exists so the user can
type UnauthorizedError struct {
	error
	SecuritySchemer SecuritySchemer
}

type ctxKey uint8

const (
	_ ctxKey = iota
	securityCtxKey
)

// SecuritySchemer describes any of the security methods.
type SecuritySchemer interface {
	SecurityScheme() *openapi3.SecurityScheme
	SecuritySchemeName() string
}

// SecuritySchemeFromContext returns a SecuritySchemer for the current endpoint,
// if any. The user should type-switch on the returned result.
func SecuritySchemeFromContext(ctx context.Context) SecuritySchemer {
	ss, _ := ctx.Value(securityCtxKey).(SecuritySchemer)
	return ss
}

// BearerAuthScheme is a SecuritySchemer type for BearerAuth.
type BearerAuthScheme struct {
	// Bearer contains the token from the Authorization header.
	Bearer string
}

// I'm not sure what goes in here.
func (s *BearerAuthScheme) SecurityScheme() *openapi3.SecurityScheme { return nil }

// SecuritySchemeName implements SecuritySchemer.
func (s *BearerAuthScheme) SecuritySchemeName() string { "BearerAuth" }

func handleBearerAuth(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		scheme := &BearerAuthScheme{}

		auth := r.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			err := fmt.Errorf("invalid format for Authorization header: missing prefix")
			siw.ErrorHandlerFunc(w, r, &UnauthorizedError{err, scheme})
			return
		}

		auth = strings.TrimPrefix(auth, "Bearer")

		ctx = context.WithValue(ctx, securityCtxKey, scheme)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

type ServerOptions struct {
	BaseURL            string
	BaseRouter         chi.Router
	Middlewares        map[string]func(http.Handler) http.Handler
	SecurityMiddleware func(next http.Handler) http.Handler
	ErrorHandlerFunc   func(w http.ResponseWriter, r *http.Request, err error)
}

// Handler creates http.Handler with routing matching OpenAPI spec.
func Handler(si ServerInterface, opts ...ServerOption) http.Handler {
	options := &ServerOptions{
		BaseURL:     "/",
		BaseRouter:  chi.NewRouter(),
		Middlewares: make(map[string]func(http.Handler) http.Handler),
		ErrorHandlerFunc: func(w http.ResponseWriter, r *http.Request, err error) {
			http.Error(w, err.Error(), http.StatusBadRequest)
		},
	}

	for _, f := range opts {
		f(options)
	}

	r := options.BaseRouter
	wrapper := ServerInterfaceWrapper{
		Handler:          si,
		Middlewares:      options.Middlewares,
		ErrorHandlerFunc: options.ErrorHandlerFunc,
	}

	r.Route(options.BaseURL, func(r chi.Router) {
		r.Post("/login", wrapper.Login)
		r.Post("/register", wrapper.Register)

		r.Group(func(r chi.Router) {
			r.Use(handleBearerAuth)
			r.Use(options.SecurityMiddleware)

			r.Get("/posts", wrapper.NextPosts)
			r.Get("/posts/liked", wrapper.GetLikedPosts)
			r.Delete("/posts/{id}", wrapper.DeletePosts)
			r.Get("/users/self", wrapper.GetSelf)
			r.Get("/users/{id}", wrapper.GetUser)
		})
	})

	return r
}

The user can then inject the middleware like so:

openapi.WithSecurityMiddleware(func(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		ctx := r.Context()

		switch ss := openapi.SecuritySchemeFromContext(ctx).(type) {
		case *openapi.BearerAuthScheme:
			s, err := h.server.AuthorizerServer().Authorize(ctx, ss.Bearer)
			if err != nil {
				// TODO: consider a better and more expressive way.
				h.writeError(w, 401, err)
				return
			}

			ctx = context.WithValue(ctx, sessionKey, s)
		}

		next.ServeHTTP(w, r.WithContext(ctx))
	})
})

A few important notes:

• There should be a better way of writing errors from middlewares than the user calling their own writeError callback.
• r.Group is required here because the user's SecurityMiddleware must be called after our security middleware handlers (handleBearerAuth in this case) are called. Because of this, the user cannot arbitrarily add a middleware.
• Not filling out SecurityMiddleware is a runtime error, not a compile-time error.
• The user must type-assert the error in their ErrorHandlerFunc for the server to possibly respond with a correct status code.
  • This can possibly be improved by making handleBearerAuth not call ErrorHandlerFunc and instead write its own code, like so:

func handleBearerAuth(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		scheme := &BearerAuthScheme{}

		auth := r.Header.Get("Authorization")
		if !strings.HasPrefix(auth, "Bearer ") {
			http.Error(w, "invalid format for Authorization header: missing prefix", 401)
			return
		}

		auth = strings.TrimPrefix(auth, "Bearer")

		ctx = context.WithValue(ctx, securityCtxKey, scheme)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

A very important note regarding the above snippet: there is no way for the user to change how the error is written if they feed the backend an invalid header. That is, unfortunately, on par with what OapiRequestValidatorWithOptions is currently doing, which is a terrible idea!

func OapiRequestValidatorWithOptions(swagger *openapi3.T, options *Options) func(next http.Handler) http.Handler {
	router, err := gorillamux.NewRouter(swagger)
	if err != nil {
		panic(err)
	}

	return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			// validate request
			if statusCode, err := validateRequest(r, router, options); err != nil {
				http.Error(w, err.Error(), statusCode) // !!!
				return
			}

			// serve
			next.ServeHTTP(w, r)
		})
	}

}

Alternatives

I've considered alternatives to injecting into context.Context, including having a callback that the user implements specifically for authenticating. Something like this:

type SecurityHandlers struct {
	BearerAuth func(next http.Handler) http.Handler
	BearerAuth func(ctx context.Context, token string) (context.Context, error)
}

This API, while slightly more declarative (in that it doesn't require pulling an instance from the blackbox that is context.Context), it has other annoying flaws: the first BearerAuth does no parsing at all, while the second BearerAuth has a generally very weird method signature.