litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.66k stars 384 forks source link

Documentation: Using custom state class does not update state? #1111

Closed arossi07 closed 1 year ago

arossi07 commented 1 year ago

Hi, using the example from the docs on how to use custom state does not increment the value:

from starlite import Starlite, State, get

class MyState(State):
    count: int = 0

    def increment(self) -> None:
        self.count += 1

@get("/")
def handler(state: MyState) -> dict:
    state.increment()
    return state.dict()

app = Starlite(route_handlers=[handler])

What am I missing?

provinzkraut commented 1 year ago
from starlite import Starlite, State, get, TestClient

class MyState(State):
    count: int = 0

    def increment(self) -> None:
        self.count += 1

@get("/")
def handler(state: MyState) -> dict:
    state.increment()
    return state.dict()

app = Starlite(route_handlers=[handler])

with TestClient(app=app) as client:
    print(client.get("/").json())

For me, this prints {'count': 1}. What behaviour are you getting?

arossi07 commented 1 year ago

I get also {'count': 1}, but I would expect it to increment by one on repeating calls and not stay constant.

provinzkraut commented 1 year ago

Okay so, first of all: The example is wrong and your assumption is correct. That's what should happen, but it can't, since this isn't how Sate works.

Subclassing State is just for typing purposes, initializing the attributes doesn't actually set the initial state. What it does do however is preventing the AttributeError MyState.increment() should raise, since count is not inside the internal dictionary of State that stores the actual values:

from starlite import Starlite, State, get, TestClient

class MyState(State):
    count: int

    def increment(self) -> None:
        self.count += 1

@get("/")
def handler(state: MyState) -> dict:
    state.increment()
    return state.dict()

app = Starlite(route_handlers=[handler])

with TestClient(app=app) as client:
    print(client.get("/").json())  # {'status_code': 500, 'detail': 'AttributeError()'}

This will return an error. To get the desired behaviour, we need to change the code to:

from starlite import Starlite, State, TestClient, get

class MyState(State):
    count: int

    def increment(self) -> None:
        if not hasattr(self, "count"):
            self.count = 0

        self.count += 1

@get("/")
def handler(state: MyState) -> dict:
    state.increment()
    return state.dict()

app = Starlite(route_handlers=[handler])

with TestClient(app=app) as client:
    client.get("/")
    print(client.get("/").json())  # {'count': 2}

or implement it the way it's intended, using initial_state:

from starlite import Starlite, State, get, TestClient

class MyState(State):
    count: int

    def increment(self) -> None:
        self.count += 1

@get("/")
def handler(state: MyState) -> dict:
    state.increment()
    return state.dict()

app = Starlite(route_handlers=[handler], initial_state={"count": 0})

with TestClient(app=app) as client:
    client.get("/")
    print(client.get("/").json())  # {'count': 2}

Conclusion

  1. The example is wrong an needs to be fixed
  2. Subclassing state can be a bit misleading, as setting default values doesn't work as expected

Open questions

Should we maybe support setting default values in such a way? We could maybe combine this with the generic state we added to v2.0, so you can actually just pass a class and it will be used for typing and default values. @Goldziher, this is your brainchild, wdyt?

arossi07 commented 1 year ago

Thanks for your help! It works now. I think it would be nice to be able to set your custom State in the class.