labstack / echo

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

Proposal: Application state #2670

Closed behnambm closed 1 month ago

behnambm commented 3 months ago

Application state

This proposal seeks to introduce an Application State Management feature in Echo. This feature will enable the ability to access a store application wide. Mmiddlewares and request handlers will be able to access this store because they both have access to Echo struct. The state's data will not change from request to request.

Similar functionality is available in other frameworks, such as FastAPI.

API

Set(key string, val any) Get(key string) (any, bool)

We need to discuss about some cases that needs to be considered in the API design. For example, should we override the already existing data? Or return an error?

Sample

package main

import (
    "fmt"
    "github.com/labstack/echo/v4"
    "net/http"
)

func main() {
    e := echo.New()

    testClient := http.Client{}
    testClient.Timeout = 42
    e.State.Set("myClient", &testClient)

    e.GET("/", hello)

    e.Logger.Fatal(e.Start(":1323"))
}

func hello(c echo.Context) error {
    client, ok := c.Echo().State.Get("myClient")
    httpClient, ok := client.(*http.Client)
    if ok {
        fmt.Println("Timeout :", httpClient.Timeout)
    }

    return c.String(http.StatusOK, "Hello, World!")
}
behnambm commented 3 months ago

@aldas Would you please take a look at this?

aldas commented 3 months ago

@behnambm this seems a lot like dependency injection containers. DI is hard to test/mock. Consider this approach

package main

import (
    "fmt"
    "github.com/labstack/echo/v4"
    "net/http"
)

func main() {
    e := echo.New()

    g := e.Group("/users")
    RegisterUserRoutes(g)  // users.RegisterUserRoutes(g) <-- if it would be moved to separate package

    e.Logger.Fatal(e.Start(":1323"))
}

///// in bigger application you would divide code by domains. so this is example for user related domain
// package users

type userCtrl struct {
    // here you could add things that you want to access from handlers
    httpClient *http.Client 
}

func RegisterUserRoutes(g *echo.Group) {
    userCtrl := userCtrl{httpClient: &http.Client{Timeout: 42}}

    g.GET("/", userCtrl.index)
}

func (u *userCtrl) index(c echo.Context) error {
    fmt.Println("Timeout :", u.httpClient.Timeout)

    return c.String(http.StatusOK, "Hello, World!")
}

with this approach it is easy to test these handler methods and code is cleaner etc.

aldas commented 3 months ago

Also please see this comment https://github.com/labstack/echo/issues/2075#issuecomment-1016819041

aldas commented 3 months ago

a little bit off-topic but https://grafana.com/blog/2024/02/09/how-i-write-http-services-in-go-after-13-years/ from Mat has excellent ideas for designing around testability

behnambm commented 3 months ago

@aldas Thanks for the response and also for sharing the link to the article by Mat Ryer.

I appreciate the alternative approach you’ve suggested, especially in terms of keeping the code modular and easy to test. However, I believe the proposed application state management feature provides value in specific scenarios where a centralized, application-wide state is necessary or advantageous.

Here are a few points to consider:

behnambm commented 1 month ago

@aldas It's been a while since this issue has been open, and I would appreciate it if you could take another look.

I recommend checking out the Actix web framework, written in Rust, which has similar state features as proposed in this issue. You can find more details in this link.

aldas commented 1 month ago

I am not in favor of add this.

As adding handler to structs that have dependencies as members/fields is better solution. It is cleaner and does not involve "containers for dependency injection" like things. I am well aware that in dynamically typed languages that concept is quite popular. struct fields is more go-like approach (look how standard library does things). I do not think bringing these things to strongly typed language like Go, promotes concepts that Go is known for - simplicity, readability. Dependency containers are somewhat on par with Java Annotations that is another "dark hidden magic" which is not easy to reason just by looking at code.

  1. compare the complexity here with these 2 approaches:
    client, ok := c.Echo().State.Get("myClient")
    httpClient, ok := client.(*http.Client)
    if ok {
    fmt.Println("Timeout :", httpClient.Timeout)
    }

    vs

    func (u *userCtrl) index(c echo.Context) error {
    fmt.Println("Timeout :", u.httpClient.Timeout)

this is far superior approach if you have more than 1 dependency that you need to access as you do not need to cast+check anything as struct is strongly typed.

  1. you can already today have pretty much same functionality if you would create middleware that injects these things info context and in that case your handle part would simplify to.
client, ok := c.Echo().State.Get("myClient")
httpClient, ok := client.(*http.Client)

you would do

httpClient, ok :=c.Get("myClient").(*http.Client)
  1. there is no reason what you could not add function to the same thing and have that State container as package level object.
    client, ok := state.Get("myClient")

state being here name of the package. Moreover - all these casts are redundant if that State would have all these global objects as public fields.

There is not need to make this Echo field/feature.

behnambm commented 1 month ago

If it doesn’t make sense to add this feature to Echo, I’ll go ahead and close the issue. @aldas, thank you for considering this feature proposal.