Repository: encode/starlette

Issue:
mypy errors when accessing Request.state
From https://www.starlette.io/requests/:
> Other state
>
> If you want to store additional information on the request you can do so using request.state.
>
> For example:
>
> request.state.time_started = time.time()

This causes mypy errors since any accessed/set attributes aren't present in the `State` class. As far as I can tell, there isn't a good way to tell mypy to expect certain attributes to potentially be set.

I could imagine solutions that somehow make `Request.state` generic, and those would probably be the most type-safe, but that seems like it would take a lot of work for little gain, and could make the API a little uglier.

As a workaround, I propose adding a `def __getattr__` in an `if typing.TYPE_CHECKING:` block within the class definition. This will silence the mypy errors without changing anything about how the class behaves during runtime.

One counter-argument to this approach would be that this would prevent state attribute accesses from being called out by mypy even if you want the warnings. But then you'd have to add `# type: ignore` to disable it, and would lose any ability to check the continued validity going forward. Given the suggested usage pattern, I just don't see much value to keeping these errors (though I could be convinced otherwise).

Alternatively, the state could be replaced by a `Dict[str, Any]` (or something that behaves more like one anyway), or given one as an attribute.

Discussion:

Patch:
diff --git a/starlette/middleware/base.py b/starlette/middleware/base.py
index ea5afb2..67d35cc 100644
--- a/starlette/middleware/base.py
+++ b/starlette/middleware/base.py
@@ -29,7 +29,7 @@ class BaseHTTPMiddleware:
         loop = asyncio.get_event_loop()
         queue = asyncio.Queue()  # type: asyncio.Queue
 
-        scope = dict(request)
+        scope = request.scope
         receive = request.receive
         send = queue.put
 
diff --git a/starlette/requests.py b/starlette/requests.py
index 44433a0..be05f29 100644
--- a/starlette/requests.py
+++ b/starlette/requests.py
@@ -18,8 +18,22 @@ class ClientDisconnect(Exception):
     pass
 
 
-class State:
-    pass
+class State(object):
+    def __init__(self, state: typing.Dict = {}):
+        super(State, self).__setattr__("_state", state)
+
+    def __setattr__(self, key: typing.Any, value: typing.Any) -> None:
+        self._state[key] = value
+
+    def __getattr__(self, key: typing.Any) -> typing.Any:
+        try:
+            return self._state[key]
+        except KeyError:
+            message = "'{}' object has no attribute '{}'"
+            raise AttributeError(message.format(self.__class__.__name__, key))
+
+    def __delattr__(self, key: typing.Any) -> None:
+        del self._state[key]
 
 
 class HTTPConnection(Mapping):
@@ -30,42 +44,42 @@ class HTTPConnection(Mapping):
 
     def __init__(self, scope: Scope, receive: Receive = None) -> None:
         assert scope["type"] in ("http", "websocket")
-        self._scope = scope
+        self.scope = scope
 
     def __getitem__(self, key: str) -> str:
-        return self._scope[key]
+        return self.scope[key]
 
     def __iter__(self) -> typing.Iterator[str]:
-        return iter(self._scope)
+        return iter(self.scope)
 
     def __len__(self) -> int:
-        return len(self._scope)
+        return len(self.scope)
 
     @property
     def app(self) -> typing.Any:
-        return self._scope["app"]
+        return self.scope["app"]
 
     @property
     def url(self) -> URL:
         if not hasattr(self, "_url"):
-            self._url = URL(scope=self._scope)
+            self._url = URL(scope=self.scope)
         return self._url
 
     @property
     def headers(self) -> Headers:
         if not hasattr(self, "_headers"):
-            self._headers = Headers(scope=self._scope)
+            self._headers = Headers(scope=self.scope)
         return self._headers
 
     @property
     def query_params(self) -> QueryParams:
         if not hasattr(self, "_query_params"):
-            self._query_params = QueryParams(self._scope["query_string"])
+            self._query_params = QueryParams(self.scope["query_string"])
         return self._query_params
 
     @property
     def path_params(self) -> dict:
-        return self._scope.get("path_params", {})
+        return self.scope.get("path_params", {})
 
     @property
     def cookies(self) -> typing.Dict[str, str]:
@@ -82,38 +96,41 @@ class HTTPConnection(Mapping):
 
     @property
     def client(self) -> Address:
-        host, port = self._scope.get("client") or (None, None)
+        host, port = self.scope.get("client") or (None, None)
         return Address(host=host, port=port)
 
     @property
     def session(self) -> dict:
         assert (
-            "session" in self._scope
+            "session" in self.scope
         ), "SessionMiddleware must be installed to access request.session"
-        return self._scope["session"]
+        return self.scope["session"]
 
     @property
     def auth(self) -> typing.Any:
         assert (
-            "auth" in self._scope
+            "auth" in self.scope
         ), "AuthenticationMiddleware must be installed to access request.auth"
-        return self._scope["auth"]
+        return self.scope["auth"]
 
     @property
     def user(self) -> typing.Any:
         assert (
-            "user" in self._scope
+            "user" in self.scope
         ), "AuthenticationMiddleware must be installed to access request.user"
-        return self._scope["user"]
+        return self.scope["user"]
 
     @property
     def state(self) -> State:
-        if "state" not in self._scope:
-            self._scope["state"] = State()
-        return self._scope["state"]
+        if not hasattr(self, "_state"):
+            # Ensure 'state' has an empty dict if it's not already populated.
+            self.scope.setdefault("state", {})
+            # Create a state instance with a reference to the dict in which it should store info
+            self._state = State(self.scope["state"])
+        return self._state
 
     def url_for(self, name: str, **path_params: typing.Any) -> str:
-        router = self._scope["router"]
+        router = self.scope["router"]
         url_path = router.url_path_for(name, **path_params)
         return url_path.make_absolute_url(base_url=self.url)
 
@@ -132,7 +149,7 @@ class Request(HTTPConnection):
 
     @property
     def method(self) -> str:
-        return self._scope["method"]
+        return self.scope["method"]
 
     @property
     def receive(self) -> Receive:


Test patch:
diff --git a/tests/middleware/test_base.py b/tests/middleware/test_base.py
index 04fa7d6..09a1d0b 100644
--- a/tests/middleware/test_base.py
+++ b/tests/middleware/test_base.py
@@ -84,3 +84,46 @@ def test_middleware_decorator():
     response = client.get("/homepage")
     assert response.text == "Homepage"
     assert response.headers["Custom"] == "Example"
+
+
+def test_state_data_across_multiple_middlewares():
+    expected_value1 = "foo"
+    expected_value2 = "bar"
+
+    class aMiddleware(BaseHTTPMiddleware):
+        async def dispatch(self, request, call_next):
+            request.state.foo = expected_value1
+            print("a", dict(request))
+            response = await call_next(request)
+            return response
+
+    class bMiddleware(BaseHTTPMiddleware):
+        async def dispatch(self, request, call_next):
+            request.state.bar = expected_value2
+            print("b", dict(request))
+            response = await call_next(request)
+            response.headers["X-State-Foo"] = request.state.foo
+            return response
+
+    class cMiddleware(BaseHTTPMiddleware):
+        async def dispatch(self, request, call_next):
+            print("c", dict(request))
+            response = await call_next(request)
+            print("c", dict(request))
+            response.headers["X-State-Bar"] = request.state.bar
+            return response
+
+    app = Starlette()
+    app.add_middleware(aMiddleware)
+    app.add_middleware(bMiddleware)
+    app.add_middleware(cMiddleware)
+
+    @app.route("/")
+    def homepage(request):
+        return PlainTextResponse("OK")
+
+    client = TestClient(app)
+    response = client.get("/")
+    assert response.text == "OK"
+    assert response.headers["X-State-Foo"] == expected_value1
+    assert response.headers["X-State-Bar"] == expected_value2
diff --git a/tests/test_requests.py b/tests/test_requests.py
index 270181e..03c74c4 100644
--- a/tests/test_requests.py
+++ b/tests/test_requests.py
@@ -2,7 +2,7 @@ import asyncio
 
 import pytest
 
-from starlette.requests import ClientDisconnect, Request
+from starlette.requests import ClientDisconnect, Request, State
 from starlette.responses import JSONResponse, Response
 from starlette.testclient import TestClient
 
@@ -240,11 +240,25 @@ def test_request_is_disconnected():
     assert disconnected_after_response
 
 
+def test_request_state_object():
+    scope = {"state": {"old": "foo"}}
+
+    s = State(scope["state"])
+
+    s.new = "value"
+    assert s.new == "value"
+
+    del s.new
+
+    with pytest.raises(AttributeError):
+        s.new
+
+
 def test_request_state():
     async def app(scope, receive, send):
         request = Request(scope, receive)
         request.state.example = 123
-        response = JSONResponse({"state.example": request["state"].example})
+        response = JSONResponse({"state.example": request.state.example})
         await response(scope, receive, send)
 
     client = TestClient(app)
diff --git a/tests/test_responses.py b/tests/test_responses.py
index c7ca0ac..f90bde4 100644
--- a/tests/test_responses.py
+++ b/tests/test_responses.py
@@ -8,11 +8,11 @@ from starlette.background import BackgroundTask
 from starlette.requests import Request
 from starlette.responses import (
     FileResponse,
+    JSONResponse,
     RedirectResponse,
     Response,
     StreamingResponse,
     UJSONResponse,
-    JSONResponse,
 )
 from starlette.testclient import TestClient