labstack / echo

High performance, minimalist Go web framework
https://echo.labstack.com
MIT License
29.52k stars 2.21k forks source link

Support for custom response types #2157

Closed sazzer closed 1 year ago

sazzer commented 2 years ago

Issue Description

Right now, Echo makes it easy to write handlers that return JSON - that is, application/json - documents. However, it doesn't make it as easy as it could to write handlers that return specific JSON formats - for example, application/vnd.siren+json or application/hal+json.

Right now I have to do the following:

    c.Response().Header().Add("content-type", "application/vnd.siren+json")
    return c.JSON(http.StatusOK, siren.New(payload))

in every handler, which means I have to duplicate the content-type every time.

I'm unsure if it fits in with the design principles of Echo, but it would be relatively easy to check if the response value implements certain interfaces and if so use them to augment the response. That would then allow the clients to return rich objects and Echo automatically does the right thing with the response.

For example, if there was:

package siren

func (d Document) WithContentType() string {
    return "application/vnd.siren+json"
}

Then any time a siren.Document is passed to c.JSON then it could use this content-type instead.

This could be used for content-type, but also for other headers if appropriate.

Cheers

sazzer commented 2 years ago

An alternative solution for this could be something like Context.RespondWith(r ResponseMessage) where ResponseMessage is simply:

type ResponseMessage interface {
    SendResponse(c echo.Context)
}

This way, client-types can implement this interface and understand how to render themselves. That would then allow for handlers to just do c.ResponseWith(siren.New(payload)) and the siren.Document knows how to render itself, with correct content-type and anything else of interest.

aldas commented 2 years ago

If all your responses are same (non-default content-type) then you could use custom echo.JSONSerializer and change the content-type there. Search for https://github.com/labstack/echo/search?q=JSONSerializer (probably/should work)

This works as https://github.com/labstack/echo/blob/feaa6ede6a4bbe4a0f25e8fed022868134ccbc6b/context.go#L482-L486

aldas commented 2 years ago

and there is no shame to have you own application specific utility functions.

sazzer commented 2 years ago

Ooh - That's interesting.

I've already done very similar with a custom HTTPErrorHandler to be able to return RFC-7807 application/problem+json responses, so this would seem to be basically the same solution :)

Cheers

sazzer commented 2 years ago

It's not perfect, because Context.JSON sets the content type before calling JSONSerializer.Serialize. That means that if for any reason the handler wants to set a different content-type and then return a SIREN document then it gets messy.

But that is unlikely, and this does work :)

A couple of possible improvements though:

Maybe this could be done the same as for errors, with an HTTPResponseHandler function that has the above as the default implementation. Though I realise that's likely to be quite complicated with the current support for multiple different content-types as methods that are all slightly different to each other.

Alternatively, as you say, just have custom functionality works. That's what I've done before as it happens, and it does work. It's just a slight shame to need to do that when the underlying framework could handle it really quite neatly. And I suspect with the growing interest in various hypermedia and linked data formats (SIREN, HAL, RDF, etc) that this could be useful for a lot of people.

Cheers

aldas commented 2 years ago

If all of your handlers respond with siren you could do

type MyJSONSerializer struct {
    echo.DefaultJSONSerializer
}

func (d MyJSONSerializer) Serialize(c echo.Context, i interface{}, indent string) error {
    c.Response().Header().Set(echo.HeaderContentType, "application/vnd.siren+json")
    return d.DefaultJSONSerializer.Serialize(c, i, indent)
}

func main() {
    e := echo.New()
    e.JSONSerializer = MyJSONSerializer{}

    e.Use(middleware.Logger())
    e.Use(middleware.Recover())

    e.GET("", func(c echo.Context) error {
        return c.JSON(http.StatusOK, map[string]string{"msg": "ok"})
    })

    if err := e.Start(":8080"); err != http.ErrServerClosed {
        log.Fatal(err)
    }
}
x@x:~/code$ curl -v http://localhost:8080
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.74.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/vnd.siren+json
< Date: Thu, 14 Apr 2022 07:30:50 GMT
< Content-Length: 13
< 
{"msg":"ok"}
* Connection #0 to host localhost left intact

As Echo uses interface for echo.Context we can not just add new methods. And there is a point where we need to consider if every (uncommon ones especially) 4liner needs to exist in library. This does not means that this could not exist - but special content-types would affect almost every other "response" methods also. Ala c.String, c.XML etc

That utility function would be:

func JSONSiren(c echo.Context, payload interface{}) {
    c.Response().Header().Set(echo.HeaderContentType, "application/vnd.siren+json")
    json.NewEncoder(c.Response()).Encode(payload)
}
sazzer commented 2 years ago

I always forget the way inheritance works like that :)

And I get that you can't just add methods to Context for everything. That way madness lies, and all that :)

But my thinking had been that adding a single method like RespondWith(ResponseMessage) would then mean that anything able to implement the ResponseMessage interface can effectively send themselves. Obviously removing methods from Context is hard, but something like that could pave the way not only to removing many of the methods but also to allowing custom content types to be supported with no extra effort.

For example, instead of doing c.JSON(http.StatusOK, payload) vs c.XML(http.StatusOK, payload), you would just do c.RespondWith(http.StatusOK, echo.JSONResponse(payload)) or c.RespondWith(http.StatusOK, echo.XMLResponse(payload)). Naively - and I do not know either Echo or Go that well as yet, so there may be good reasons to not do this! - that seems both cleaner and more extensible.

But for now I've got something that works well enough for me, so I'm happy :)

Cheers

aldas commented 2 years ago

I think I understand the reasoning here - for this usecase ResponseMessage and c.RespondWith(http.StatusOK, echo.JSONResponse(payload)) seem eloquent solution but 99% of cases this is overkill and unnecessary. Moreover - you still need to implement ResponseMessage interface and therefore in comparison to creating custom helper/utility function you are still not saving development time or lines of code.

Compare:

func JSONSiren(c echo.Context, code int, payload interface{}) error {
    c.Response().Header().Set(echo.HeaderContentType, "application/vnd.siren+json")
    c.Response().Status = code
    return c.JSON(code, payload)
}

e.GET("/test", func(c echo.Context) error {
    resp := map[string]string{"msg": "OK"}
    return JSONSiren(c, http.StatusOK, resp)
})

or even


func AsJSONSiren(c echo.Context) echo.Context {
    c.Response().Header().Set(echo.HeaderContentType, "application/vnd.siren+json")
    return c
}

e.GET("/test", func(c echo.Context) error {
    resp := map[string]string{"msg": "OK"}
    return AsJSONSiren(c).JSON(http.StatusOK, resp)
})

to

type JSONSiren interface {}
// somewhere here is method for JSONSiren to write response

e.GET("/test", func(c echo.Context) error {
    resp := map[string]string{"msg": "OK"}
    return c.RespondWith(http.StatusOK, JSONResponse(resp)) // way more to type in each handler 
})

I think creating functions for small thing like that is more in Go spirit than implementing interfaces just to respond something to the client and does not make Echo API surface more complex.

lammel commented 1 year ago

I guess we can close this, as no further discussion came up. Keep your utility functions coming :)