Merge pull request #581 from girardinsamuel/fix/session-with-middlewares

Fix setting session when middleware redirects instead of continuing request flow

src/masonite/middleware/route/SessionMiddleware.py

@@ -17,7 +17,6 @@ def before(self, request, response):
         return request
 
     def after(self, request, _):
-        Session.save()
         return request
 
     def with_input(self):

src/masonite/sessions/Session.py

@@ -92,7 +92,8 @@ def set(self, key: str, value: Any) -> None:
         except json.decoder.JSONDecodeError:
             pass
 
-        return self.added.update({key: value})
+        self.added.update({key: value})
+        self.save()
 
     def increment(self, key: str, count: int = 1) -> None:
         """Increment session key with given count."""
@@ -118,6 +119,7 @@ def get(self, key: str) -> Any:
                 pass
             self.flashed.pop(key)
             self.deleted_flashed.append(key)
+            self.save()
             return value
 
         value = self.get_data().get(key)
@@ -137,12 +139,14 @@ def pull(self, key: str) -> Any:
     def flush(self) -> None:
         """Delete all keys from session."""
         self.deleted += list(self.get_data().keys())
+        self.save()
 
     def delete(self, key: str) -> "None|Any":
         """Delete the given key from session."""
         self.deleted.append(key)
         if key in self.flashed:
             self.flashed.pop(key)
+        self.save()
 
     def flash(self, key: str, value: Any) -> None:
         """Save temporary value into session."""
@@ -155,6 +159,7 @@ def flash(self, key: str, value: Any) -> None:
             pass
 
         self.flashed.update({key: value})
+        self.save()
 
     def all(self) -> dict:
         """Get all session data."""

tests/features/session/test_cookie_session.py

@@ -55,7 +55,7 @@ def test_can_save_session(self):
         session = self.application.make("session")
         session.start()
         session.set("key1", "test1")
-        session.save()
+
         self.assertEqual(response.cookie("s_key1"), "test1")
 
     def test_can_delete_session(self):
@@ -70,7 +70,6 @@ def test_can_delete_session(self):
         session.delete("key")
         self.assertEqual(session.get("key"), None)
 
-        session.save()
         self.assertEqual(response.cookie("s_key"), None)
         self.assertTrue("s_key" in response.cookie_jar.deleted_cookies)
 
@@ -86,8 +85,6 @@ def test_can_pull_session(self):
         key = session.pull("key")
         self.assertEqual(key, "test")
         self.assertEqual(session.get("key"), None)
-
-        session.save()
         self.assertEqual(response.cookie("s_key"), None)
         self.assertTrue("s_key" in response.cookie_jar.deleted_cookies)
 
@@ -102,7 +99,6 @@ def test_can_flush_session(self):
 
         session.flush()
         self.assertEqual(session.get("key"), None)
-        session.save()
         self.assertEqual(response.cookie("s_key"), None)
         self.assertTrue("s_key" in response.cookie_jar.deleted_cookies)
 
@@ -114,6 +110,24 @@ def test_can_flash(self):
         session.flash("key", "test")
         self.assertEqual(session.get("key"), "test")
         self.assertEqual(session.get("key"), None)
-
-        session.save()
         self.assertEqual(response.cookie("f_key"), None)
+
+    def test_flash_two_keys_does_not_duplicate_data(self):
+        request = self.make_request()
+        response = self.make_response()
+        session = self.application.make("session")
+        session.start()
+        session.flash("key", "test")
+        session.flash("key2", "test2")
+
+        self.assertTrue(session.has("key"))
+        self.assertTrue(session.has("key2"))
+        self.assertTrue(response.cookie_jar.exists("f_key"))
+        self.assertTrue(response.cookie_jar.exists("f_key2"))
+
+        self.assertEqual(session.get("key"), "test")
+
+        self.assertFalse(session.has("key"))
+        self.assertTrue(session.has("key2"))
+        self.assertFalse(response.cookie_jar.exists("f_key"))
+        self.assertTrue(response.cookie_jar.exists("f_key2"))

tests/features/session/test_requests.py

@@ -0,0 +1,62 @@
+from tests import TestCase
+from src.masonite.middleware import Middleware
+from src.masonite.routes import Route
+
+
+class TestUpdateMiddleware(Middleware):
+    def before(self, request, response):
+        request.session.set("key", "value")
+        request.session.flash("flash_key", "flash_value")
+        return request
+
+    def after(self, request, response):
+        return request
+
+
+class TestUpdateAndRedirectsMiddleware(Middleware):
+    def before(self, request, response):
+        request.session.set("key", "value")
+        return (
+            response.redirect("/home")
+            .with_success("Success message")
+            .with_errors("Error message")
+        )
+
+    def after(self, request, response):
+        return request
+
+
+class TestUpdatingSessionFromMiddlewares(TestCase):
+    def setUp(self):
+        super().setUp()
+        self.application.make("middleware").add(
+            {
+                "test": TestUpdateMiddleware,
+                "test_redirect": TestUpdateAndRedirectsMiddleware,
+            }
+        )
+        self.setRoutes(
+            Route.get("/", "WelcomeController@test").middleware("test"),
+            Route.get("/redirect", "WelcomeController@test").middleware(
+                "test_redirect"
+            ),
+            Route.get("/home", "WelcomeController@test"),
+        )
+
+    def test_updating_session_in_middleware_before(self):
+        self.get("/").assertOk().assertSessionHas("key", "value").assertSessionHas(
+            "flash_key", "flash_value"
+        )
+
+    def test_updating_session_and_redirects_in_middleware_before(self):
+        response = (
+            self.get("/redirect")
+            .assertRedirect("/home")
+            .assertSessionHas("key", "value")
+            .response
+        )
+        # here when loading /home redirected route cookies needs to be set by the previous
+        # response.
+        assert response.cookie_jar.exists("f_success")
+        assert response.cookie_jar.exists("f_errors")
+        assert response.cookie_jar.exists("s_key")