Repository: Neoteroi/BlackSheep
Pull request #320 (2023-03-12 17:40:22)

session.set doesn't set modified flag
not sure why when everything else that modifies the session does. probably missing something.

RobertoPrevato: Hi @scottrutherford 
Thank you for your contribution. 🙇🏼‍♂️ It's definitely a mistake from my side. I didn´t noticed because I don´t use sessions myself in my web apps, since many years. I notice now that `__del__` is also not working properly.

Would you be so kind to add also the following to your changes?

```diff
diff --git a/blacksheep/sessions/__init__.py b/blacksheep/sessions/__init__.py
index 52eacdb..e69aa1a 100644
--- a/blacksheep/sessions/__init__.py
+++ b/blacksheep/sessions/__init__.py
@@ -47,6 +47,7 @@ class Session:
         self._values[name] = value

     def __delitem__(self, name: str) -> None:
+        self._modified = True
         del self._values[name]

     def __contains__(self, name: str) -> bool:
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
index b811ae2..12884c4 100644
--- a/tests/test_sessions.py
+++ b/tests/test_sessions.py
@@ -138,6 +138,26 @@ def test_session_modified():
     assert session.modified is True


+def test_session_modified_set():
+    session = Session()
+
+    assert session.modified is False
+
+    session.set("foo", "lorem ipsum")
+
+    assert session.modified is True
+
+
+def test_session_modified_del():
+    session = Session({"foo": "lorem ipsum"})
+
+    assert session.modified is False
+
+    del session["foo"]
+
+    assert session.modified is True
+
+
 def test_session_key_error():
     session = Session()
```
scottrutherford: Here you go, enjoying Blacksheep, thank you!
codecov-commenter: ## [Codecov](https://codecov.io/gh/Neoteroi/BlackSheep/pull/320?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) Report
Patch coverage: **`100.00`**% and no project coverage change.
> Comparison is base [(`4200d1c`)](https://codecov.io/gh/Neoteroi/BlackSheep/commit/4200d1cc13790ae431a8a03df7ae0ba08e5b4dee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) 97.93% compared to head [(`49556bc`)](https://codecov.io/gh/Neoteroi/BlackSheep/pull/320?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) 97.93%.

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi)

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #320   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files          61       61           
  Lines        5801     5803    +2     
=======================================
+ Hits         5681     5683    +2     
  Misses        120      120           
```


| [Impacted Files](https://codecov.io/gh/Neoteroi/BlackSheep/pull/320?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi) | Coverage Δ | |
|---|---|---|
| [blacksheep/sessions/\_\_init\_\_.py](https://codecov.io/gh/Neoteroi/BlackSheep/pull/320?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi#diff-YmxhY2tzaGVlcC9zZXNzaW9ucy9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/Neoteroi/BlackSheep/pull/320?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Neoteroi).

RobertoPrevato: Thank You for your kind words. I'm working hard these days to improve several things for version 2 (alpha on PyPi https://pypi.org/project/blacksheep/2.0a3/).

Patch:
diff --git a/blacksheep/sessions/__init__.py b/blacksheep/sessions/__init__.py
index 52eacdb..39cfc84 100644
--- a/blacksheep/sessions/__init__.py
+++ b/blacksheep/sessions/__init__.py
@@ -33,6 +33,7 @@ class Session:
         return self._values.get(name, default)
 
     def set(self, name: str, value: Any) -> None:
+        self._modified = True
         self._values[name] = value
 
     def update(self, values: Mapping[str, Any]) -> None:
@@ -47,6 +48,7 @@ class Session:
         self._values[name] = value
 
     def __delitem__(self, name: str) -> None:
+        self._modified = True
         del self._values[name]
 
     def __contains__(self, name: str) -> bool:


Test patch:
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
index b811ae2..12884c4 100644
--- a/tests/test_sessions.py
+++ b/tests/test_sessions.py
@@ -138,6 +138,26 @@ def test_session_modified():
     assert session.modified is True
 
 
+def test_session_modified_set():
+    session = Session()
+
+    assert session.modified is False
+
+    session.set("foo", "lorem ipsum")
+
+    assert session.modified is True
+
+
+def test_session_modified_del():
+    session = Session({"foo": "lorem ipsum"})
+
+    assert session.modified is False
+
+    del session["foo"]
+
+    assert session.modified is True
+
+
 def test_session_key_error():
     session = Session()