deepmap
Strict server return type using matching interface
The new strict server looks amazing, but using an interface{} for the return type is not ideal. While its not the best as it would not provide an easy way for the user get the list of types, could an interface be used as the return type instead? This would allow for only specific types which match the interface to be returned, granting some type checking. (the user could create their own type matching the interface, but that would have to be done very intentionally)
This would require a separate error type in the response, but I think it would be better to be more explicit when wanting to returrn an error here anyway.
type PetStoreImpl struct {}
func (*PetStoreImpl) GetPets(ctx context.Context, request GetPetsRequestObject) interface{} {
var result []Pet
// Implement me
return GetPets200JSONResponse(result)
}
could become
type PetStoreImpl struct {}
func (*PetStoreImpl) GetPets(ctx context.Context, request GetPetsRequestObject) (resp IsGetPetsResponse, err error) {
var result []Pet
// Implement me
return GetPets200JSONResponse(result)
}
///in api
type IsGetPetsResponse interface {
CanGetPetsResponse() bool
}
When designing strict server interface I was looking for type checking the response object, but for now go does not offer a great variant implementation. The solution should offer type check for the compiler and should be descriptive for humans. The common community solutions for this problems are:
- A struct of pointers with only one of them initialized. Pretty descriptive, but cumbersome to use, since you have to wrap your response into another struct. Also hard to keep invariants, since user can initialize multiple pointers, or none at all, which we can only validate in runtime.
- An interface with useless methods. Offers good type checking, but not as descriptive because of the duck typing. Making the method public also hints user to implement it in his own types, which will not work :), so its better to make the method private (I think standard library uses this approach somewhere). What I don't like about this method is the need to separate error into separate return value. I like the idea of controllers returning a single response, error or not. It will make it harder to migrate to another solution once go gets proper union types.
- Type constraints introduced in go 1.18 can specify an interface which can only be implemented by concrete types, but for now they only work in generic code and cannot be used as a regular types. The original generic proposal states that using constraints as ordinary interface types may be possible, but AFAIK no work on that has been done.
So I think the best solution for now is to wait for a better solution. As generics mature I'm sure we will get some solutions that will satisfy all the requirements without resorting to hacks.
Given that type unions have been under discussion for more than half a decade, along with the fact that such large changes move slowly (even when part of the original proposal), the best case for getting unions is probably 2-3 years out minimum. (a year each for specific proposal, implementation, and deployment, if not more.) I think it would be a mistake to not make the library better now because of potential features that, at best, will be added several years from now.
Making the method public also hints user to implement it in his own types, which will not work
Making the interface return a value that could be passed directly to ctx.Blob would allow the user to create custom implementations. This technically bypasses the intention of strict server, but honestly, allowing for this bypass for the odd edge case may in fact prove useful. (and accepting interface{} already implies the user can return anything they want anyway)
///in api
type IsGetPetsResponse interface {
CanGetPetsResponse() (code int, contentType string, b []byte)
}
For the default types, the data would do the type check as already exists to determine which one it is, and if its not any of the generated types, it calls the above function and pipes it directly in.
What I don't like about this method is the need to separate error into separate return value
The return format of (T, error) is one of the most common go paradigms I know. And I honestly don't think it would make it any more difficult to migrate than the current implementation, and even during a migration, the paradigm of (UnionedType, error) would still be valid.
If its a requirement for the single return type there is a solution, tho it is a bet more messy. Regardless, wrapping the error with an error struct that matches the interface would do the trick. But as I said, this is more of a hack than anything else.
type StrictError struct {
Error error
}
func (s StrictError) CanGetPetsResponse() []byte{
return nil
}
// (would have functions for each type.)
I get your point, but there are more problems with this solutions. Now, if a response contains no headers, no variable content type or status it will be aliased to a referenced type. This also works with reusable responses when multiple endpoints reuse the same response object (there is also logic to not alias if multiple status codes use the same response). I don't think we can add methods via type alias (will need to double check that later). If we are doing interfaces with methods for response objects, the better solution would be to use visitor pattern and move response logic from a server-specific code to common code. Something like this:
type responseWriter interface {
setHeader(key, value string)
json(statusCode int, value interface{}) error
}
type methodNameResponseObject interface {
visitMethodNameResponse(w responseWriter) error
}
type MethodName200JSONResponse struct {
Body MethodNameResponseData
Headers MethodName200ResponseHeaders
}
func (response MethodName200JSONResponse) visitMethodNameResponse(w responseWriter) error {
w.setHeader("header1", response.Headers.Header1)
return w.json(200, response.Body)
}
As for breaking the strictness in some corner cases for now it can be achieved with middlewares. You can return any type from you controller, catch it in a middleware, process it manually (middlewares get full access to raw contexts\response writers) and return nil to make strict server do nothing.
The middleware work-around for bypassing strict is a tad messy, but no worse than needing to do so to begin with, so fair enough.
As a note, I was not intending for the generated response structs to actually implement the interfaces. They would just do what they currently doing. Only in the case of custom structs would it actually use the interface.
It sounds like you have done a lot of consideration around this already, and while I would prefer the interfaces, if you don't think they will work, then I will leave it.
As a note, I was not intending for the generated response structs to actually implement the interfaces. They would just do what they currently doing. Only in the case of custom structs would it actually use the interface.
How would you return generated response structs then? If they do not implement the interface? As for using the interfaces - it can work, but there is a lot of caveats that need to be addressed and not a simple replacement for empty interfaces. You can make a PR (maybe a draft one if you are unsure) and I will review it to detect possible problems early. The strict code is not yet released properly, so if we can change the API now, it will be easier, than doing it later, after a full release.
How would you return generated response structs then?
Id have the built in/generated structs do exactly what they do right now. Just that to account for the custom objects, should a user choose to return one, instead of this
response := handler(ctx, request)
switch v := response.(type) {
case TestData200JSONResponse:
return ctx.JSON(200, v)
case error:
return v
case nil:
default:
return fmt.Errorf("Unexpected response type: %T", v)
}
return nil
they would have this.
response := handler(ctx, request)
switch v := response.(type) {
case TestData200JSONResponse:
return ctx.JSON(200, v)
case error:
return v
case nil:
default:
return ctx.Blob(response.CanGetPetsResponse())
}
Tho as you mentioned, it would probably be better to use the visitor pattern. This was just from my messing around. Ill see about throwing together a PR next weekend.
Had a few free hours this weekend, so I made a PR by using visitor pattern. see #701. As it turns out you can add methods via aliases, and all tests passed after quick implementation without any issues. @deefdragon can you review it tell if it fixes your issue?
This has been fixed by #701. Thank you.
Out of curisoity @deepmap-marcinr , when do you decide to tag a new version?