From 0538c3b0180c9b610b5aa61ea5e3bcb88d51f6f2 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 10 Jan 2022 14:36:08 +0530
Subject: [PATCH 01/10] Add drop_user/chat_data

---
 telegram/ext/_dispatcher.py | 76 +++++++++++++++++++++++++++++++++++--
 tests/test_dispatcher.py    | 48 +++++++++++++++++++++++
 2 files changed, 121 insertions(+), 3 deletions(-)

diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index 248f4fdafe5..e44221df0ac 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -78,11 +78,11 @@ def callback(update, context):
     Note:
         Has no effect, if the handler or error handler is run asynchronously.
 
-    Attributes:
-        state (:obj:`object`): Optional. The next state of the conversation.
-
     Args:
         state (:obj:`object`, optional): The next state of the conversation.
+
+    Attributes:
+        state (:obj:`object`): Optional. The next state of the conversation.
     """
 
     __slots__ = ('state',)
@@ -631,6 +631,76 @@ def remove_handler(self, handler: Handler, group: int = DEFAULT_GROUP) -> None:
             if not self.handlers[group]:
                 del self.handlers[group]
 
+    def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -> None:
+        """Used for deleting a key from the persistence.
+
+        Args:
+            chat_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
+                will be deleted even if it is not empty. Mutually exclusive with
+                :paramref:`all_empty_entries`.
+            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all empty entries from
+                :attr:`chat_data` will be deleted. Mutually exclusive with :paramref:`chat_id`.
+
+        Raises:
+            :exc:`ValueError`: When both :paramref:`chat_id` and :paramref:`all_empty_entries` are
+                provided, or none of them are passed.
+        """
+
+        if chat_id and all_empty_entries:
+            raise ValueError("You must pass either `chat_id` or `all_empty_entries` not both.")
+
+        if chat_id is None and not all_empty_entries:
+            raise ValueError("You must pass either `chat_id` or `all_empty_entries`.")
+
+        if chat_id:
+            if chat_id not in self.chat_data:
+                raise ValueError("The specified `chat_id` is not present in `chat_data`")
+            del self.chat_data[chat_id]
+
+        elif all_empty_entries:
+            chat_ids = list(self.chat_data.keys())
+
+            for _id in chat_ids:
+                if not self.chat_data[_id]:
+                    del self.chat_data[_id]
+
+        self.update_persistence()
+
+    def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -> None:
+        """Used for deleting a key from the persistence.
+
+        Args:
+            user_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
+                will be deleted even if it is not empty. Mutually exclusive with
+                :paramref:`all_empty_entries`.
+            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all empty entries from
+                :attr:`user_data` will be deleted. Mutually exclusive with :paramref:`user_id`.
+
+        Raises:
+            :exc:`ValueError`: When both :paramref:`user_id` and :paramref:`all_empty_entries` are
+                provided, or none of them are passed.
+        """
+
+        if user_id and all_empty_entries:
+            raise ValueError("You must pass either `user_id` or `all_empty_entries` not both.")
+
+        if user_id is None and not all_empty_entries:
+            raise ValueError("You must pass either `user_id` or `all_empty_entries`.")
+
+        if user_id:
+            if user_id not in self.user_data:
+                raise ValueError("The specified `user_id` is not present in `user_data`")
+            del self.user_data[user_id]
+
+        elif all_empty_entries:
+            user_ids = list(self.user_data.keys())
+
+            for u_id in user_ids:
+                if not self.user_data[u_id]:
+                    del self.user_data[u_id]
+
+        self.update_persistence()
+
     def update_persistence(self, update: object = None) -> None:
         """Update :attr:`user_data`, :attr:`chat_data` and :attr:`bot_data` in :attr:`persistence`.
 
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index 339a5c603a7..d7ff7dd5f06 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -879,6 +879,54 @@ def callback(update, context):
         assert not dp.persistence.test_flag_user_data
         assert dp.persistence.test_flag_chat_data
 
+    @pytest.mark.parametrize(
+        "remove_all,c_id,expected",
+        [
+            (True, 123, None),
+            (None, None, None),
+            (True, None, {321: {'not_empty': 'no'}, 222: "remove_me"}),
+            (False, 321, {123: [], 222: "remove_me"}),
+            (False, 111, None),
+        ],
+    )
+    def test_drop_chat_data(self, dp, remove_all, c_id, expected):
+        dp.chat_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+
+        if (remove_all and c_id) or (not remove_all and not c_id):
+            with pytest.raises(ValueError, match="You must pass either"):
+                dp.drop_chat_data(c_id, all_empty_entries=remove_all)
+        else:
+            if c_id is not None and c_id not in dp.chat_data:
+                with pytest.raises(ValueError, match="The specified"):
+                    dp.drop_chat_data(c_id)
+            else:
+                dp.drop_chat_data(c_id, all_empty_entries=remove_all)
+                assert dp.chat_data == expected
+
+    @pytest.mark.parametrize(
+        "remove_all,u_id,expected",
+        [
+            (True, 123, None),
+            (None, None, None),
+            (True, None, {321: {'not_empty': 'no'}, 222: "remove_me"}),
+            (False, 321, {123: [], 222: "remove_me"}),
+            (False, 111, None),
+        ],
+    )
+    def test_drop_user_data(self, dp, remove_all, u_id, expected):
+        dp.user_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+
+        if (remove_all and u_id) or (not remove_all and not u_id):
+            with pytest.raises(ValueError, match="You must pass either"):
+                dp.drop_user_data(u_id, all_empty_entries=remove_all)
+        else:
+            if u_id is not None and u_id not in dp.user_data:
+                with pytest.raises(ValueError, match="The specified"):
+                    dp.drop_user_data(u_id)
+            else:
+                dp.drop_user_data(u_id, all_empty_entries=remove_all)
+                assert dp.user_data == expected
+
     def test_update_persistence_once_per_update(self, monkeypatch, dp):
         def update_persistence(*args, **kwargs):
             self.count += 1

From 4832f0adb829805c451112a42d1f324317e1c492 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 10 Jan 2022 17:21:16 +0530
Subject: [PATCH 02/10] Make dp.user/chat_data read-only

---
 telegram/ext/_dispatcher.py | 83 ++++++++++++++++++++++++-------------
 tests/conftest.py           |  4 +-
 tests/test_dispatcher.py    | 13 +++++-
 3 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index e44221df0ac..cf30215dc42 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -37,7 +37,9 @@
     TypeVar,
     TYPE_CHECKING,
     Tuple,
+    Mapping,
 )
+from types import MappingProxyType
 from uuid import uuid4
 
 from telegram import Update
@@ -111,8 +113,6 @@ class Dispatcher(Generic[BT, CCT, UD, CD, BD, JQ, PT]):
             instance to pass onto handler callbacks.
         workers (:obj:`int`, optional): Number of maximum concurrent worker threads for the
             ``@run_async`` decorator and :meth:`run_async`.
-        user_data (:obj:`defaultdict`): A dictionary handlers can use to store data for the user.
-        chat_data (:obj:`defaultdict`): A dictionary handlers can use to store data for the chat.
         bot_data (:obj:`dict`): A dictionary handlers can use to store data for the bot.
         persistence (:class:`telegram.ext.BasePersistence`): Optional. The persistence class to
             store data that should be persistent over restarts.
@@ -144,8 +144,8 @@ class Dispatcher(Generic[BT, CCT, UD, CD, BD, JQ, PT]):
         'persistence',
         'update_queue',
         'job_queue',
-        'user_data',
-        'chat_data',
+        '_user_data',
+        '_chat_data',
         'bot_data',
         '_update_persistence_lock',
         'handlers',
@@ -198,8 +198,8 @@ def __init__(
                 stacklevel=stack_level,
             )
 
-        self.user_data: DefaultDict[int, UD] = defaultdict(self.context_types.user_data)
-        self.chat_data: DefaultDict[int, CD] = defaultdict(self.context_types.chat_data)
+        self._user_data: DefaultDict[int, UD] = defaultdict(self.context_types.user_data)
+        self._chat_data: DefaultDict[int, CD] = defaultdict(self.context_types.chat_data)
         self.bot_data = self.context_types.bot_data()
         self.persistence: Optional[BasePersistence] = None
         self._update_persistence_lock = Lock()
@@ -213,12 +213,12 @@ def __init__(
             self.persistence.set_bot(self.bot)
 
             if self.persistence.store_data.user_data:
-                self.user_data = self.persistence.get_user_data()
-                if not isinstance(self.user_data, defaultdict):
+                self._user_data = self.persistence.get_user_data()
+                if not isinstance(self._user_data, defaultdict):
                     raise ValueError("user_data must be of type defaultdict")
             if self.persistence.store_data.chat_data:
-                self.chat_data = self.persistence.get_chat_data()
-                if not isinstance(self.chat_data, defaultdict):
+                self._chat_data = self.persistence.get_chat_data()
+                if not isinstance(self._chat_data, defaultdict):
                     raise ValueError("chat_data must be of type defaultdict")
             if self.persistence.store_data.bot_data:
                 self.bot_data = self.persistence.get_bot_data()
@@ -282,6 +282,27 @@ def _set_singleton(cls, val: Optional['Dispatcher']) -> None:
         cls.logger.debug('Setting singleton dispatcher as %s', val)
         cls.__singleton = weakref.ref(val) if val else None
 
+    @property
+    def chat_data(self) -> Mapping[int, CD]:
+        """
+        :obj:`types.MappingProxyType`: A dictionary handlers can use to store data for the chat.
+
+        .. versionchanged:: 14.0
+           :attr:`chat_data` is now read-only
+
+        """
+        return MappingProxyType(self._chat_data)
+
+    @property
+    def user_data(self) -> Mapping[int, UD]:
+        """:obj:`types.MappingProxyType`: A dictionary handlers can use to store data for the user.
+
+        .. versionchanged:: 14.0
+           :attr:`user_data` is now read-only
+
+        """
+        return MappingProxyType(self._user_data)
+
     @classmethod
     def get_instance(cls) -> 'Dispatcher':
         """Get the singleton instance of this class.
@@ -632,7 +653,9 @@ def remove_handler(self, handler: Handler, group: int = DEFAULT_GROUP) -> None:
                 del self.handlers[group]
 
     def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -> None:
-        """Used for deleting a key from the persistence.
+        """Used for deleting a key from the :attr:`chat_data`.
+
+        .. versionadded:: 14.0
 
         Args:
             chat_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
@@ -653,24 +676,26 @@ def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -
             raise ValueError("You must pass either `chat_id` or `all_empty_entries`.")
 
         if chat_id:
-            if chat_id not in self.chat_data:
+            if chat_id not in self._chat_data:
                 raise ValueError("The specified `chat_id` is not present in `chat_data`")
-            del self.chat_data[chat_id]
+            del self._chat_data[chat_id]
 
         elif all_empty_entries:
-            chat_ids = list(self.chat_data.keys())
+            chat_ids = list(self._chat_data.keys())
 
             for _id in chat_ids:
-                if not self.chat_data[_id]:
-                    del self.chat_data[_id]
+                if not self._chat_data[_id]:
+                    del self._chat_data[_id]
 
         self.update_persistence()
 
     def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -> None:
-        """Used for deleting a key from the persistence.
+        """Used for deleting a key from the :attr:`user_data`.
+
+        .. versionadded:: 14.0
 
         Args:
-            user_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
+            user_id (:obj:`int`, optional): The user id to delete from the persistence. The entry
                 will be deleted even if it is not empty. Mutually exclusive with
                 :paramref:`all_empty_entries`.
             all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all empty entries from
@@ -688,16 +713,16 @@ def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -
             raise ValueError("You must pass either `user_id` or `all_empty_entries`.")
 
         if user_id:
-            if user_id not in self.user_data:
+            if user_id not in self._user_data:
                 raise ValueError("The specified `user_id` is not present in `user_data`")
-            del self.user_data[user_id]
+            del self._user_data[user_id]
 
         elif all_empty_entries:
-            user_ids = list(self.user_data.keys())
+            user_ids = list(self._user_data.keys())
 
             for u_id in user_ids:
-                if not self.user_data[u_id]:
-                    del self.user_data[u_id]
+                if not self._user_data[u_id]:
+                    del self._user_data[u_id]
 
         self.update_persistence()
 
@@ -713,10 +738,10 @@ def update_persistence(self, update: object = None) -> None:
 
     def __update_persistence(self, update: object = None) -> None:
         if self.persistence:
-            # We use list() here in order to decouple chat_ids from self.chat_data, as dict view
+            # We use list() here in order to decouple chat_ids from self._chat_data, as dict view
             # objects will change, when the dict does and we want to loop over chat_ids
-            chat_ids = list(self.chat_data.keys())
-            user_ids = list(self.user_data.keys())
+            chat_ids = list(self._chat_data.keys())
+            user_ids = list(self._user_data.keys())
 
             if isinstance(update, Update):
                 if update.effective_chat:
@@ -745,13 +770,13 @@ def __update_persistence(self, update: object = None) -> None:
             if self.persistence.store_data.chat_data:
                 for chat_id in chat_ids:
                     try:
-                        self.persistence.update_chat_data(chat_id, self.chat_data[chat_id])
+                        self.persistence.update_chat_data(chat_id, self._chat_data[chat_id])
                     except Exception as exc:
                         self.dispatch_error(update, exc)
             if self.persistence.store_data.user_data:
                 for user_id in user_ids:
                     try:
-                        self.persistence.update_user_data(user_id, self.user_data[user_id])
+                        self.persistence.update_user_data(user_id, self._user_data[user_id])
                     except Exception as exc:
                         self.dispatch_error(update, exc)
 
@@ -828,7 +853,7 @@ def dispatch_error(
 
         Returns:
             :obj:`bool`: :obj:`True` if one of the error handlers raised
-                :class:`telegram.ext.DispatcherHandlerStop`. :obj:`False`, otherwise.
+            :class:`telegram.ext.DispatcherHandlerStop`. :obj:`False`, otherwise.
         """
         async_args = None if not promise else promise.args
         async_kwargs = None if not promise else promise.kwargs
diff --git a/tests/conftest.py b/tests/conftest.py
index 1583118e690..44e890a4000 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -194,8 +194,8 @@ def dp(_dp):
     # Reset the dispatcher first
     while not _dp.update_queue.empty():
         _dp.update_queue.get(False)
-    _dp.chat_data = defaultdict(dict)
-    _dp.user_data = defaultdict(dict)
+    _dp._chat_data = defaultdict(dict)
+    _dp._user_data = defaultdict(dict)
     _dp.bot_data = {}
     _dp.persistence = None
     _dp.handlers = {}
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index d7ff7dd5f06..faec7bace4f 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -125,6 +125,15 @@ def test_manual_init_warning(self, recwarn):
         )
         assert recwarn[0].filename == __file__, "stacklevel is incorrect!"
 
+    @pytest.mark.parametrize("data", ["chat_data", "user_data"])
+    def test_chat_user_data_read_only(self, dp, data):
+        read_only_data = getattr(dp, data)
+        writable_data = getattr(dp, f"_{data}")
+        writable_data[123] = 321
+        assert read_only_data == writable_data
+        with pytest.raises(TypeError):
+            read_only_data[111] = 123
+
     @pytest.mark.parametrize(
         'builder',
         (DispatcherBuilder(), UpdaterBuilder()),
@@ -890,7 +899,7 @@ def callback(update, context):
         ],
     )
     def test_drop_chat_data(self, dp, remove_all, c_id, expected):
-        dp.chat_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+        dp._chat_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
 
         if (remove_all and c_id) or (not remove_all and not c_id):
             with pytest.raises(ValueError, match="You must pass either"):
@@ -914,7 +923,7 @@ def test_drop_chat_data(self, dp, remove_all, c_id, expected):
         ],
     )
     def test_drop_user_data(self, dp, remove_all, u_id, expected):
-        dp.user_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+        dp._user_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
 
         if (remove_all and u_id) or (not remove_all and not u_id):
             with pytest.raises(ValueError, match="You must pass either"):

From f8295bec151681d292b16c336f9b0a809fa38ba9 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 10 Jan 2022 17:29:52 +0530
Subject: [PATCH 03/10] deepsource

---
 telegram/ext/_dispatcher.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index cf30215dc42..a193958c0fd 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -668,7 +668,6 @@ def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -
             :exc:`ValueError`: When both :paramref:`chat_id` and :paramref:`all_empty_entries` are
                 provided, or none of them are passed.
         """
-
         if chat_id and all_empty_entries:
             raise ValueError("You must pass either `chat_id` or `all_empty_entries` not both.")
 
@@ -705,7 +704,6 @@ def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -
             :exc:`ValueError`: When both :paramref:`user_id` and :paramref:`all_empty_entries` are
                 provided, or none of them are passed.
         """
-
         if user_id and all_empty_entries:
             raise ValueError("You must pass either `user_id` or `all_empty_entries` not both.")
 

From 56a47bde37d4af903c1ab85f28e1b6ce73c38ae0 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Sun, 16 Jan 2022 04:55:31 +0530
Subject: [PATCH 04/10] Address review

---
 telegram/ext/_dispatcher.py | 106 ++++++++++++++++++++----------------
 tests/conftest.py           |   5 +-
 tests/test_dispatcher.py    |  22 ++++++--
 3 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index a193958c0fd..49e6fdd51f8 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -113,6 +113,24 @@ class Dispatcher(Generic[BT, CCT, UD, CD, BD, JQ, PT]):
             instance to pass onto handler callbacks.
         workers (:obj:`int`, optional): Number of maximum concurrent worker threads for the
             ``@run_async`` decorator and :meth:`run_async`.
+        chat_data (:obj:`types.MappingProxyType`): A dictionary handlers can use to store data for
+            the chat.
+
+            .. versionchanged:: 14.0
+                :attr:`chat_data` is now read-only
+
+            .. tip::
+               Manually modifying :attr:`chat_data` is almost never needed and unadvisable.
+
+        user_data (:obj:`types.MappingProxyType`): A dictionary handlers can use to store data for
+            the user.
+
+            .. versionchanged:: 14.0
+               :attr:`user_data` is now read-only
+
+            .. tip::
+               Manually modifying :attr:`user_data` is almost never needed and unadvisable.
+
         bot_data (:obj:`dict`): A dictionary handlers can use to store data for the bot.
         persistence (:class:`telegram.ext.BasePersistence`): Optional. The persistence class to
             store data that should be persistent over restarts.
@@ -145,7 +163,9 @@ class Dispatcher(Generic[BT, CCT, UD, CD, BD, JQ, PT]):
         'update_queue',
         'job_queue',
         '_user_data',
+        'user_data',
         '_chat_data',
+        'chat_data',
         'bot_data',
         '_update_persistence_lock',
         'handlers',
@@ -198,10 +218,17 @@ def __init__(
                 stacklevel=stack_level,
             )
 
-        self._user_data: DefaultDict[int, UD] = defaultdict(self.context_types.user_data)
-        self._chat_data: DefaultDict[int, CD] = defaultdict(self.context_types.chat_data)
+        self._user_data: DefaultDict[int, UD]
+        self._chat_data: DefaultDict[int, CD]
+        # Read only mapping-
+        self.user_data: Mapping[int, UD]
+        self.chat_data: Mapping[int, CD]
+
+        self._set_user_data(defaultdict(self.context_types.user_data))
+        self._set_chat_data(defaultdict(self.context_types.chat_data))
         self.bot_data = self.context_types.bot_data()
-        self.persistence: Optional[BasePersistence] = None
+
+        self.persistence: Optional[BasePersistence]
         self._update_persistence_lock = Lock()
         if persistence:
             if not isinstance(persistence, BasePersistence):
@@ -213,11 +240,11 @@ def __init__(
             self.persistence.set_bot(self.bot)
 
             if self.persistence.store_data.user_data:
-                self._user_data = self.persistence.get_user_data()
+                self._set_user_data(self.persistence.get_user_data())
                 if not isinstance(self._user_data, defaultdict):
                     raise ValueError("user_data must be of type defaultdict")
             if self.persistence.store_data.chat_data:
-                self._chat_data = self.persistence.get_chat_data()
+                self._set_chat_data(self.persistence.get_chat_data())
                 if not isinstance(self._chat_data, defaultdict):
                     raise ValueError("chat_data must be of type defaultdict")
             if self.persistence.store_data.bot_data:
@@ -282,27 +309,6 @@ def _set_singleton(cls, val: Optional['Dispatcher']) -> None:
         cls.logger.debug('Setting singleton dispatcher as %s', val)
         cls.__singleton = weakref.ref(val) if val else None
 
-    @property
-    def chat_data(self) -> Mapping[int, CD]:
-        """
-        :obj:`types.MappingProxyType`: A dictionary handlers can use to store data for the chat.
-
-        .. versionchanged:: 14.0
-           :attr:`chat_data` is now read-only
-
-        """
-        return MappingProxyType(self._chat_data)
-
-    @property
-    def user_data(self) -> Mapping[int, UD]:
-        """:obj:`types.MappingProxyType`: A dictionary handlers can use to store data for the user.
-
-        .. versionchanged:: 14.0
-           :attr:`user_data` is now read-only
-
-        """
-        return MappingProxyType(self._user_data)
-
     @classmethod
     def get_instance(cls) -> 'Dispatcher':
         """Get the singleton instance of this class.
@@ -318,6 +324,18 @@ def get_instance(cls) -> 'Dispatcher':
             return cls.__singleton()  # type: ignore[return-value] # pylint: disable=not-callable
         raise RuntimeError(f'{cls.__name__} not initialized or multiple instances exist')
 
+    def _set_chat_data(self, data: DefaultDict[int, CD]) -> None:
+        """Used for assigning a new value to the underlying chat_data dictionary. Also updates
+        the read-only mapping."""
+        self._chat_data = data
+        self.chat_data = MappingProxyType(self._chat_data)
+
+    def _set_user_data(self, data: DefaultDict[int, UD]) -> None:
+        """Used for assigning a new value to the underlying user_data dictionary. Also updates
+        the read-only mapping."""
+        self._user_data = data
+        self.user_data = MappingProxyType(self._user_data)
+
     def _pooled(self) -> None:
         thr_name = current_thread().name
         while 1:
@@ -660,30 +678,26 @@ def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -
         Args:
             chat_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
                 will be deleted even if it is not empty. Mutually exclusive with
-                :paramref:`all_empty_entries`.
-            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all empty entries from
-                :attr:`chat_data` will be deleted. Mutually exclusive with :paramref:`chat_id`.
+                ``all_empty_entries``.
+            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all entries which evaluates
+                to :obj:`False` in a boolean context in :attr:`chat_data` will be deleted.
+                Mutually exclusive with ``chat_id``.
 
         Raises:
-            :exc:`ValueError`: When both :paramref:`chat_id` and :paramref:`all_empty_entries` are
+            :exc:`ValueError`: When both ``chat_id`` and ``all_empty_entries`` are
                 provided, or none of them are passed.
         """
-        if chat_id and all_empty_entries:
-            raise ValueError("You must pass either `chat_id` or `all_empty_entries` not both.")
-
-        if chat_id is None and not all_empty_entries:
+        if not bool(chat_id) ^ bool(all_empty_entries):
             raise ValueError("You must pass either `chat_id` or `all_empty_entries`.")
 
         if chat_id:
-            if chat_id not in self._chat_data:
-                raise ValueError("The specified `chat_id` is not present in `chat_data`")
             del self._chat_data[chat_id]
 
         elif all_empty_entries:
             chat_ids = list(self._chat_data.keys())
 
             for _id in chat_ids:
-                if not self._chat_data[_id]:
+                if not bool(self._chat_data[_id]):
                     del self._chat_data[_id]
 
         self.update_persistence()
@@ -696,30 +710,26 @@ def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -
         Args:
             user_id (:obj:`int`, optional): The user id to delete from the persistence. The entry
                 will be deleted even if it is not empty. Mutually exclusive with
-                :paramref:`all_empty_entries`.
-            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all empty entries from
-                :attr:`user_data` will be deleted. Mutually exclusive with :paramref:`user_id`.
+                ``all_empty_entries``.
+            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all entries which evaluates
+                to :obj:`False` in a boolean context in :attr:`user_data` will be deleted.
+                Mutually exclusive with ``user_id``.
 
         Raises:
-            :exc:`ValueError`: When both :paramref:`user_id` and :paramref:`all_empty_entries` are
+            :exc:`ValueError`: When both ``user_id`` and ``all_empty_entries`` are
                 provided, or none of them are passed.
         """
-        if user_id and all_empty_entries:
-            raise ValueError("You must pass either `user_id` or `all_empty_entries` not both.")
-
-        if user_id is None and not all_empty_entries:
+        if not bool(user_id) ^ bool(all_empty_entries):
             raise ValueError("You must pass either `user_id` or `all_empty_entries`.")
 
         if user_id:
-            if user_id not in self._user_data:
-                raise ValueError("The specified `user_id` is not present in `user_data`")
             del self._user_data[user_id]
 
         elif all_empty_entries:
             user_ids = list(self._user_data.keys())
 
             for u_id in user_ids:
-                if not self._user_data[u_id]:
+                if not bool(self._user_data[u_id]):
                     del self._user_data[u_id]
 
         self.update_persistence()
diff --git a/tests/conftest.py b/tests/conftest.py
index 44e890a4000..b8391342df5 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -194,10 +194,9 @@ def dp(_dp):
     # Reset the dispatcher first
     while not _dp.update_queue.empty():
         _dp.update_queue.get(False)
-    _dp._chat_data = defaultdict(dict)
-    _dp._user_data = defaultdict(dict)
+    _dp._set_chat_data(defaultdict(dict))
+    _dp._set_user_data(defaultdict(dict))
     _dp.bot_data = {}
-    _dp.persistence = None
     _dp.handlers = {}
     _dp.error_handlers = {}
     _dp.exception_event = Event()
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index faec7bace4f..1613eda7797 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -897,16 +897,23 @@ def callback(update, context):
             (False, 321, {123: [], 222: "remove_me"}),
             (False, 111, None),
         ],
+        ids=[
+            "Passing all parameters (error)",
+            "Passing no parameters (error)",
+            "test all_empty removal",
+            "test chat_id removal",
+            "test no key in data (error)",
+        ],
     )
     def test_drop_chat_data(self, dp, remove_all, c_id, expected):
-        dp._chat_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+        dp._chat_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
 
         if (remove_all and c_id) or (not remove_all and not c_id):
             with pytest.raises(ValueError, match="You must pass either"):
                 dp.drop_chat_data(c_id, all_empty_entries=remove_all)
         else:
             if c_id is not None and c_id not in dp.chat_data:
-                with pytest.raises(ValueError, match="The specified"):
+                with pytest.raises(KeyError):
                     dp.drop_chat_data(c_id)
             else:
                 dp.drop_chat_data(c_id, all_empty_entries=remove_all)
@@ -921,16 +928,23 @@ def test_drop_chat_data(self, dp, remove_all, c_id, expected):
             (False, 321, {123: [], 222: "remove_me"}),
             (False, 111, None),
         ],
+        ids=[
+            "Passing all parameters (error)",
+            "Passing no parameters (error)",
+            "test all_empty removal",
+            "test chat_id removal",
+            "test no key in data (error)",
+        ],
     )
     def test_drop_user_data(self, dp, remove_all, u_id, expected):
-        dp._user_data = {123: [], 321: {'not_empty': 'no'}, 222: "remove_me"}
+        dp._user_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
 
         if (remove_all and u_id) or (not remove_all and not u_id):
             with pytest.raises(ValueError, match="You must pass either"):
                 dp.drop_user_data(u_id, all_empty_entries=remove_all)
         else:
             if u_id is not None and u_id not in dp.user_data:
-                with pytest.raises(ValueError, match="The specified"):
+                with pytest.raises(KeyError):
                     dp.drop_user_data(u_id)
             else:
                 dp.drop_user_data(u_id, all_empty_entries=remove_all)

From ae44b7c630b1b2a955e8c48e13c1d0b1c6bba8c9 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Sun, 16 Jan 2022 22:09:02 +0530
Subject: [PATCH 05/10] Review 2

---
 telegram/ext/_dispatcher.py | 68 +++++++++----------------------------
 tests/test_dispatcher.py    | 68 ++++++++++---------------------------
 2 files changed, 34 insertions(+), 102 deletions(-)

diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index 49e6fdd51f8..ee46d3039b7 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -326,13 +326,15 @@ def get_instance(cls) -> 'Dispatcher':
 
     def _set_chat_data(self, data: DefaultDict[int, CD]) -> None:
         """Used for assigning a new value to the underlying chat_data dictionary. Also updates
-        the read-only mapping."""
+        the read-only mapping.
+        """
         self._chat_data = data
         self.chat_data = MappingProxyType(self._chat_data)
 
     def _set_user_data(self, data: DefaultDict[int, UD]) -> None:
         """Used for assigning a new value to the underlying user_data dictionary. Also updates
-        the read-only mapping."""
+        the read-only mapping.
+        """
         self._user_data = data
         self.user_data = MappingProxyType(self._user_data)
 
@@ -670,67 +672,29 @@ def remove_handler(self, handler: Handler, group: int = DEFAULT_GROUP) -> None:
             if not self.handlers[group]:
                 del self.handlers[group]
 
-    def drop_chat_data(self, chat_id: int = None, all_empty_entries: bool = False) -> None:
+    def drop_chat_data(self, chat_id: int) -> None:
         """Used for deleting a key from the :attr:`chat_data`.
 
         .. versionadded:: 14.0
 
         Args:
-            chat_id (:obj:`int`, optional): The chat id to delete from the persistence. The entry
-                will be deleted even if it is not empty. Mutually exclusive with
-                ``all_empty_entries``.
-            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all entries which evaluates
-                to :obj:`False` in a boolean context in :attr:`chat_data` will be deleted.
-                Mutually exclusive with ``chat_id``.
-
-        Raises:
-            :exc:`ValueError`: When both ``chat_id`` and ``all_empty_entries`` are
-                provided, or none of them are passed.
+            chat_id (:obj:`int`): The chat id to delete from the persistence. The entry
+                will be deleted even if it is not empty.
         """
-        if not bool(chat_id) ^ bool(all_empty_entries):
-            raise ValueError("You must pass either `chat_id` or `all_empty_entries`.")
-
-        if chat_id:
-            del self._chat_data[chat_id]
-
-        elif all_empty_entries:
-            chat_ids = list(self._chat_data.keys())
-
-            for _id in chat_ids:
-                if not bool(self._chat_data[_id]):
-                    del self._chat_data[_id]
+        del self._chat_data[chat_id]
 
         self.update_persistence()
 
-    def drop_user_data(self, user_id: int = None, all_empty_entries: bool = False) -> None:
+    def drop_user_data(self, user_id: int) -> None:
         """Used for deleting a key from the :attr:`user_data`.
 
         .. versionadded:: 14.0
 
         Args:
-            user_id (:obj:`int`, optional): The user id to delete from the persistence. The entry
-                will be deleted even if it is not empty. Mutually exclusive with
-                ``all_empty_entries``.
-            all_empty_entries (:obj:`bool`, optional): If :obj:`True`, all entries which evaluates
-                to :obj:`False` in a boolean context in :attr:`user_data` will be deleted.
-                Mutually exclusive with ``user_id``.
-
-        Raises:
-            :exc:`ValueError`: When both ``user_id`` and ``all_empty_entries`` are
-                provided, or none of them are passed.
+            user_id (:obj:`int`): The user id to delete from the persistence. The entry
+                will be deleted even if it is not empty.
         """
-        if not bool(user_id) ^ bool(all_empty_entries):
-            raise ValueError("You must pass either `user_id` or `all_empty_entries`.")
-
-        if user_id:
-            del self._user_data[user_id]
-
-        elif all_empty_entries:
-            user_ids = list(self._user_data.keys())
-
-            for u_id in user_ids:
-                if not bool(self._user_data[u_id]):
-                    del self._user_data[u_id]
+        del self._user_data[user_id]
 
         self.update_persistence()
 
@@ -748,8 +712,8 @@ def __update_persistence(self, update: object = None) -> None:
         if self.persistence:
             # We use list() here in order to decouple chat_ids from self._chat_data, as dict view
             # objects will change, when the dict does and we want to loop over chat_ids
-            chat_ids = list(self._chat_data.keys())
-            user_ids = list(self._user_data.keys())
+            chat_ids = list(self.chat_data.keys())
+            user_ids = list(self.user_data.keys())
 
             if isinstance(update, Update):
                 if update.effective_chat:
@@ -778,13 +742,13 @@ def __update_persistence(self, update: object = None) -> None:
             if self.persistence.store_data.chat_data:
                 for chat_id in chat_ids:
                     try:
-                        self.persistence.update_chat_data(chat_id, self._chat_data[chat_id])
+                        self.persistence.update_chat_data(chat_id, self.chat_data[chat_id])
                     except Exception as exc:
                         self.dispatch_error(update, exc)
             if self.persistence.store_data.user_data:
                 for user_id in user_ids:
                     try:
-                        self.persistence.update_user_data(user_id, self._user_data[user_id])
+                        self.persistence.update_user_data(user_id, self.user_data[user_id])
                     except Exception as exc:
                         self.dispatch_error(update, exc)
 
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index 1613eda7797..80d3032e5c3 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -889,66 +889,34 @@ def callback(update, context):
         assert dp.persistence.test_flag_chat_data
 
     @pytest.mark.parametrize(
-        "remove_all,c_id,expected",
-        [
-            (True, 123, None),
-            (None, None, None),
-            (True, None, {321: {'not_empty': 'no'}, 222: "remove_me"}),
-            (False, 321, {123: [], 222: "remove_me"}),
-            (False, 111, None),
-        ],
-        ids=[
-            "Passing all parameters (error)",
-            "Passing no parameters (error)",
-            "test all_empty removal",
-            "test chat_id removal",
-            "test no key in data (error)",
-        ],
+        "c_id,expected",
+        [(321, {123: [], 222: "remove_me"}), (111, None)],
+        ids=["test chat_id removal", "test no key in data (error)"],
     )
-    def test_drop_chat_data(self, dp, remove_all, c_id, expected):
+    def test_drop_chat_data(self, dp, c_id, expected):
         dp._chat_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
 
-        if (remove_all and c_id) or (not remove_all and not c_id):
-            with pytest.raises(ValueError, match="You must pass either"):
-                dp.drop_chat_data(c_id, all_empty_entries=remove_all)
+        if c_id is not None and c_id not in dp.chat_data:
+            with pytest.raises(KeyError):
+                dp.drop_chat_data(c_id)
         else:
-            if c_id is not None and c_id not in dp.chat_data:
-                with pytest.raises(KeyError):
-                    dp.drop_chat_data(c_id)
-            else:
-                dp.drop_chat_data(c_id, all_empty_entries=remove_all)
-                assert dp.chat_data == expected
+            dp.drop_chat_data(c_id)
+            assert dp.chat_data == expected
 
     @pytest.mark.parametrize(
-        "remove_all,u_id,expected",
-        [
-            (True, 123, None),
-            (None, None, None),
-            (True, None, {321: {'not_empty': 'no'}, 222: "remove_me"}),
-            (False, 321, {123: [], 222: "remove_me"}),
-            (False, 111, None),
-        ],
-        ids=[
-            "Passing all parameters (error)",
-            "Passing no parameters (error)",
-            "test all_empty removal",
-            "test chat_id removal",
-            "test no key in data (error)",
-        ],
+        "u_id,expected",
+        [(321, {123: [], 222: "remove_me"}), (111, None)],
+        ids=["test user_id removal", "test no key in data (error)"],
     )
-    def test_drop_user_data(self, dp, remove_all, u_id, expected):
+    def test_drop_user_data(self, dp, u_id, expected):
         dp._user_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
 
-        if (remove_all and u_id) or (not remove_all and not u_id):
-            with pytest.raises(ValueError, match="You must pass either"):
-                dp.drop_user_data(u_id, all_empty_entries=remove_all)
+        if u_id is not None and u_id not in dp.user_data:
+            with pytest.raises(KeyError):
+                dp.drop_user_data(u_id)
         else:
-            if u_id is not None and u_id not in dp.user_data:
-                with pytest.raises(KeyError):
-                    dp.drop_user_data(u_id)
-            else:
-                dp.drop_user_data(u_id, all_empty_entries=remove_all)
-                assert dp.user_data == expected
+            dp.drop_user_data(u_id)
+            assert dp.user_data == expected
 
     def test_update_persistence_once_per_update(self, monkeypatch, dp):
         def update_persistence(*args, **kwargs):

From 766d049856e3957e0525ef81f231ec49c07d35e3 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 17 Jan 2022 05:43:39 +0530
Subject: [PATCH 06/10] Add Persistence.drop_chat/user_data

Also fix a broken persistence test
---
 telegram/ext/_basepersistence.py   |  50 ++++++++----
 telegram/ext/_dictpersistence.py   |  24 ++++++
 telegram/ext/_dispatcher.py        |   6 +-
 telegram/ext/_picklepersistence.py |  36 +++++++++
 tests/test_dispatcher.py           |  18 +++++
 tests/test_persistence.py          | 126 ++++++++++++++++++-----------
 6 files changed, 193 insertions(+), 67 deletions(-)

diff --git a/telegram/ext/_basepersistence.py b/telegram/ext/_basepersistence.py
index a3b97e67c75..7caafbe8b32 100644
--- a/telegram/ext/_basepersistence.py
+++ b/telegram/ext/_basepersistence.py
@@ -520,6 +520,40 @@ def update_bot_data(self, data: BD) -> None:
                 The :attr:`telegram.ext.Dispatcher.bot_data`.
         """
 
+    @abstractmethod
+    def update_callback_data(self, data: CDCData) -> None:
+        """Will be called by the :class:`telegram.ext.Dispatcher` after a handler has
+        handled an update.
+
+        .. versionadded:: 13.6
+
+        .. versionchanged:: 14.0
+           Changed this method into an ``@abstractmethod``.
+
+        Args:
+            data (Optional[Tuple[List[Tuple[:obj:`str`, :obj:`float`, \
+                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]):
+                The relevant data to restore :class:`telegram.ext.CallbackDataCache`.
+        """
+
+    @abstractmethod
+    def drop_chat_data(self, chat_id: int) -> None:
+        """Will be called by the :class:`telegram.ext.Dispatcher`, when using
+        :meth:`~telegram.ext.Dispatcher.drop_chat_data`.
+
+        Args:
+            chat_id (:obj:`int`): The chat id to delete from the persistence.
+        """
+
+    @abstractmethod
+    def drop_user_data(self, user_id: int) -> None:
+        """Will be called by the :class:`telegram.ext.Dispatcher`, when using
+        :meth:`~telegram.ext.Dispatcher.drop_user_data`.
+
+        Args:
+            user_id (:obj:`int`): The user id to delete from the persistence.
+        """
+
     @abstractmethod
     def refresh_user_data(self, user_id: int, user_data: UD) -> None:
         """Will be called by the :class:`telegram.ext.Dispatcher` before passing the
@@ -570,22 +604,6 @@ def refresh_bot_data(self, bot_data: BD) -> None:
                 The ``bot_data``.
         """
 
-    @abstractmethod
-    def update_callback_data(self, data: CDCData) -> None:
-        """Will be called by the :class:`telegram.ext.Dispatcher` after a handler has
-        handled an update.
-
-        .. versionadded:: 13.6
-
-        .. versionchanged:: 14.0
-           Changed this method into an ``@abstractmethod``.
-
-        Args:
-            data (Optional[Tuple[List[Tuple[:obj:`str`, :obj:`float`, \
-                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]):
-                The relevant data to restore :class:`telegram.ext.CallbackDataCache`.
-        """
-
     @abstractmethod
     def flush(self) -> None:
         """Will be called by :class:`telegram.ext.Updater` upon receiving a stop signal. Gives the
diff --git a/telegram/ext/_dictpersistence.py b/telegram/ext/_dictpersistence.py
index 65be8ae35e5..0ecd454e83a 100644
--- a/telegram/ext/_dictpersistence.py
+++ b/telegram/ext/_dictpersistence.py
@@ -361,6 +361,30 @@ def update_callback_data(self, data: CDCData) -> None:
         self._callback_data = (data[0], data[1].copy())
         self._callback_data_json = None
 
+    def drop_chat_data(self, chat_id: int) -> None:
+        """Will delete the specified key from the :attr:`chat_data`.
+
+        Args:
+            chat_id (:obj:`int`): The chat id to delete from the persistence.
+        """
+        if self._chat_data is None:
+            self._chat_data = defaultdict(dict)
+        else:
+            del self._chat_data[chat_id]
+            self._chat_data_json = None
+
+    def drop_user_data(self, user_id: int) -> None:
+        """Will delete the specified key from the :attr:`user_data`.
+
+        Args:
+            user_id (:obj:`int`): The user id to delete from the persistence.
+        """
+        if self._user_data is None:
+            self._user_data = defaultdict(dict)
+        else:
+            del self._user_data[user_id]
+            self._user_data_json = None
+
     def refresh_user_data(self, user_id: int, user_data: Dict) -> None:
         """Does nothing.
 
diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index ee46d3039b7..2d5ec433c7f 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -683,7 +683,8 @@ def drop_chat_data(self, chat_id: int) -> None:
         """
         del self._chat_data[chat_id]
 
-        self.update_persistence()
+        if self.persistence:
+            self.persistence.drop_chat_data(chat_id)
 
     def drop_user_data(self, user_id: int) -> None:
         """Used for deleting a key from the :attr:`user_data`.
@@ -696,7 +697,8 @@ def drop_user_data(self, user_id: int) -> None:
         """
         del self._user_data[user_id]
 
-        self.update_persistence()
+        if self.persistence:
+            self.persistence.drop_user_data(user_id)
 
     def update_persistence(self, update: object = None) -> None:
         """Update :attr:`user_data`, :attr:`chat_data` and :attr:`bot_data` in :attr:`persistence`.
diff --git a/telegram/ext/_picklepersistence.py b/telegram/ext/_picklepersistence.py
index df549fd2972..f8410818a4c 100644
--- a/telegram/ext/_picklepersistence.py
+++ b/telegram/ext/_picklepersistence.py
@@ -392,6 +392,42 @@ def update_callback_data(self, data: CDCData) -> None:
             else:
                 self._dump_singlefile()
 
+    def drop_chat_data(self, chat_id: int) -> None:
+        """Will delete the specified key from the :attr:`chat_data` and depending on
+        :attr:`on_flush` save the pickle file.
+
+        Args:
+            chat_id (:obj:`int`): The chat id to delete from the persistence.
+        """
+        if self.chat_data is None:
+            self.chat_data = defaultdict(self.context_types.chat_data)
+        else:
+            del self.chat_data[chat_id]
+
+        if not self.on_flush:
+            if not self.single_file:
+                self._dump_file(Path(f"{self.filepath}_chat_data"), self.chat_data)
+            else:
+                self._dump_singlefile()
+
+    def drop_user_data(self, user_id: int) -> None:
+        """Will delete the specified key from the :attr:`user_data` and depending on
+        :attr:`on_flush` save the pickle file.
+
+        Args:
+            user_id (:obj:`int`): The user id to delete from the persistence.
+        """
+        if self.user_data is None:
+            self.user_data = defaultdict(self.context_types.user_data)
+        else:
+            del self.user_data[user_id]
+
+        if not self.on_flush:
+            if not self.single_file:
+                self._dump_file(Path(f"{self.filepath}_user_data"), self.user_data)
+            else:
+                self._dump_singlefile()
+
     def refresh_user_data(self, user_id: int, user_data: UD) -> None:
         """Does nothing.
 
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index 80d3032e5c3..a79b512346d 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -630,6 +630,12 @@ def get_bot_data(self):
             def update_bot_data(self, data):
                 raise Exception
 
+            def drop_chat_data(self, chat_id):
+                pass
+
+            def drop_user_data(self, user_id):
+                pass
+
             def get_chat_data(self):
                 return defaultdict(dict)
 
@@ -756,6 +762,12 @@ def update_chat_data(self, chat_id, data):
             def update_user_data(self, user_id, data):
                 self.update(data)
 
+            def drop_user_data(self, user_id):
+                pass
+
+            def drop_chat_data(self, chat_id):
+                pass
+
             def get_chat_data(self):
                 pass
 
@@ -834,6 +846,12 @@ def update_user_data(self, user_id, data):
             def update_conversation(self, name, key, new_state):
                 pass
 
+            def drop_chat_data(self, chat_id):
+                pass
+
+            def drop_user_data(self, user_id):
+                pass
+
             def get_conversations(self, name):
                 pass
 
diff --git a/tests/test_persistence.py b/tests/test_persistence.py
index 941a29f5dc2..a37d772b671 100644
--- a/tests/test_persistence.py
+++ b/tests/test_persistence.py
@@ -50,14 +50,14 @@
     JobQueue,
     ContextTypes,
 )
+from telegram.ext._callbackdatacache import _KeyboardData
 
 
 @pytest.fixture(autouse=True)
-def change_directory(tmp_path):
+def change_directory(tmp_path: Path):
     orig_dir = Path.cwd()
-    # Switch to a temporary directory so we don't have to worry about cleaning up files
-    # (str() for py<3.6)
-    os.chdir(str(tmp_path))
+    # Switch to a temporary directory, so we don't have to worry about cleaning up files
+    os.chdir(tmp_path)
     yield
     # Go back to original directory
     os.chdir(orig_dir)
@@ -99,6 +99,12 @@ def update_user_data(self, user_id, data):
     def get_callback_data(self):
         raise NotImplementedError
 
+    def drop_user_data(self, user_id):
+        raise NotImplementedError
+
+    def drop_chat_data(self, chat_id):
+        raise NotImplementedError
+
     def refresh_user_data(self, user_id, user_data):
         raise NotImplementedError
 
@@ -159,6 +165,12 @@ def update_user_data(self, user_id, data):
         def update_callback_data(self, data):
             self.callback_data = data
 
+        def drop_user_data(self, user_id):
+            pass
+
+        def drop_chat_data(self, chat_id):
+            pass
+
         def update_conversation(self, name, key, new_state):
             raise NotImplementedError
 
@@ -257,7 +269,7 @@ def test_abstract_methods(self, base_persistence):
         with pytest.raises(
             TypeError,
             match=(
-                'flush, get_bot_data, get_callback_data, '
+                'drop_chat_data, drop_user_data, flush, get_bot_data, get_callback_data, '
                 'get_chat_data, get_conversations, '
                 'get_user_data, refresh_bot_data, refresh_chat_data, '
                 'refresh_user_data, update_bot_data, update_callback_data, '
@@ -339,7 +351,7 @@ def get_callback_data():
         u.dispatcher.chat_data[442233]['test5'] = 'test6'
         assert u.dispatcher.chat_data[442233]['test5'] == 'test6'
 
-    @pytest.mark.parametrize('run_async', [True, False], ids=['synchronous', 'run_async'])
+    @pytest.mark.parametrize('run_async', [True, False], ids=['run_async', 'synchronous'])
     def test_dispatcher_integration_handlers(
         self,
         dp,
@@ -368,8 +380,6 @@ def get_callback_data():
         base_persistence.get_chat_data = get_chat_data
         base_persistence.get_bot_data = get_bot_data
         base_persistence.get_callback_data = get_callback_data
-        # base_persistence.update_chat_data = lambda x: x
-        # base_persistence.update_user_data = lambda x: x
         base_persistence.refresh_bot_data = lambda x: x
         base_persistence.refresh_chat_data = lambda x, y: x
         base_persistence.refresh_user_data = lambda x, y: x
@@ -383,7 +393,7 @@ def callback_known_user(update, context):
                 pytest.fail('bot_data corrupt')
 
         def callback_known_chat(update, context):
-            if not context.chat_data['test3'] == 'test4':
+            if not context.chat_data[3] == 'test4':
                 pytest.fail('chat_data corrupt')
             if not context.bot_data == bot_data:
                 pytest.fail('bot_data corrupt')
@@ -398,20 +408,17 @@ def callback_unknown_user_or_chat(update, context):
             context.user_data[1] = 'test7'
             context.chat_data[2] = 'test8'
             context.bot_data['test0'] = 'test0'
-            context.bot.callback_data_cache.put('test0')
+            # Let's now delete user1 and chat1
+            context.dispatcher.drop_chat_data(-67890)
+            context.dispatcher.drop_user_data(12345)
+            # Test setting new keyboard callback data-
+            context.bot.callback_data_cache._keyboard_data['id'] = _KeyboardData(
+                'id', button_data={'button3': 'test3'}
+            )
 
-        known_user = MessageHandler(
-            filters.User(user_id=12345),
-            callback_known_user,
-        )
-        known_chat = MessageHandler(
-            filters.Chat(chat_id=-67890),
-            callback_known_chat,
-        )
-        unknown = MessageHandler(
-            filters.ALL,
-            callback_unknown_user_or_chat,
-        )
+        known_user = MessageHandler(filters.User(user_id=12345), callback_known_user)  # user1
+        known_chat = MessageHandler(filters.Chat(chat_id=-67890), callback_known_chat)  # chat1
+        unknown = MessageHandler(filters.ALL, callback_unknown_user_or_chat)  # user2 and chat2
         dp.add_handler(known_user)
         dp.add_handler(known_chat)
         dp.add_handler(unknown)
@@ -420,51 +427,64 @@ def callback_unknown_user_or_chat(update, context):
         chat1 = Chat(id=-67890, type='group')
         chat2 = Chat(id=-987654, type='group')
         m = Message(1, None, chat2, from_user=user1)
-        u = Update(0, m)
-        with caplog.at_level(logging.ERROR):
-            dp.process_update(u)
-        rec = caplog.records[-1]
-        assert rec.getMessage() == 'No error handlers are registered, logging exception.'
-        assert rec.levelname == 'ERROR'
-        rec = caplog.records[-2]
-        assert rec.getMessage() == 'No error handlers are registered, logging exception.'
-        assert rec.levelname == 'ERROR'
-        rec = caplog.records[-3]
-        assert rec.getMessage() == 'No error handlers are registered, logging exception.'
-        assert rec.levelname == 'ERROR'
+        u_known_user = Update(0, m)
+        dp.process_update(u_known_user)
+        # 4 errors which arise since update_*_data are raising NotImplementedError here.
+        assert len(caplog.records) == 4
         m.from_user = user2
         m.chat = chat1
-        u = Update(1, m)
-        dp.process_update(u)
+        u_known_chat = Update(1, m)
+        dp.process_update(u_known_chat)
         m.chat = chat2
-        u = Update(2, m)
+        u_unknown_user_or_chat = Update(2, m)
 
         def save_bot_data(data):
             if 'test0' not in data:
                 pytest.fail()
 
-        def save_chat_data(data):
-            if -987654 not in data:
+        def save_chat_data(_id, data):
+            if 2 not in data:  # data should be: {2: 'test8'}
                 pytest.fail()
 
-        def save_user_data(data):
-            if 54321 not in data:
+        def save_user_data(_id, data):
+            if 1 not in data:  # data should be: {1: 'test7'}
                 pytest.fail()
 
         def save_callback_data(data):
-            if not assert_data_in_cache(dp.bot.callback_data, 'test0'):
+            if not assert_data_in_cache(dp.bot.callback_data_cache, 'test3'):
                 pytest.fail()
 
+        # Functions to check deletion-
+        def delete_user_data(user_id):
+            if 12345 != user_id:
+                pytest.fail("The id being deleted is not of user1's")
+            user_data.pop(user_id, None)
+
+        def delete_chat_data(chat_id):
+            if -67890 != chat_id:
+                pytest.fail("The chat id being deleted is not of chat1's")
+            chat_data.pop(chat_id, None)
+
         base_persistence.update_chat_data = save_chat_data
         base_persistence.update_user_data = save_user_data
         base_persistence.update_bot_data = save_bot_data
         base_persistence.update_callback_data = save_callback_data
-        dp.process_update(u)
+        base_persistence.drop_chat_data = delete_chat_data
+        base_persistence.drop_user_data = delete_user_data
+        dp.process_update(u_unknown_user_or_chat)
 
+        # Test callback_unknown_user_or_chat worked correctly-
         assert dp.user_data[54321][1] == 'test7'
         assert dp.chat_data[-987654][2] == 'test8'
         assert dp.bot_data['test0'] == 'test0'
-        assert assert_data_in_cache(dp.bot.callback_data_cache, 'test0')
+        assert assert_data_in_cache(dp.bot.callback_data_cache, 'test3')
+        assert 12345 not in dp.user_data  # Tests if dp.drop_user_data worked or not
+        assert -67890 not in dp.chat_data
+        assert len(caplog.records) == 8  # Errors double since new update is processed.
+        for r in caplog.records:
+            assert issubclass(r.exc_info[0], NotImplementedError)
+            assert r.getMessage() == 'No error handlers are registered, logging exception.'
+            assert r.levelname == 'ERROR'
 
     @pytest.mark.parametrize(
         'store_user_data', [True, False], ids=['store_user_data-True', 'store_user_data-False']
@@ -475,7 +495,7 @@ def save_callback_data(data):
     @pytest.mark.parametrize(
         'store_bot_data', [True, False], ids=['store_bot_data-True', 'store_bot_data-False']
     )
-    @pytest.mark.parametrize('run_async', [True, False], ids=['synchronous', 'run_async'])
+    @pytest.mark.parametrize('run_async', [True, False], ids=['run_async', 'synchronous'])
     def test_persistence_dispatcher_integration_refresh_data(
         self,
         dp,
@@ -1042,15 +1062,11 @@ def test_pickle_behaviour_with_slots(self, pickle_persistence):
         assert retrieved == bot_data
 
     def test_no_files_present_multi_file(self, pickle_persistence):
-        assert pickle_persistence.get_user_data() == defaultdict(dict)
         assert pickle_persistence.get_user_data() == defaultdict(dict)
         assert pickle_persistence.get_chat_data() == defaultdict(dict)
-        assert pickle_persistence.get_chat_data() == defaultdict(dict)
-        assert pickle_persistence.get_bot_data() == {}
         assert pickle_persistence.get_bot_data() == {}
         assert pickle_persistence.get_callback_data() is None
         assert pickle_persistence.get_conversations('noname') == {}
-        assert pickle_persistence.get_conversations('noname') == {}
 
     def test_no_files_present_single_file(self, pickle_persistence):
         pickle_persistence.single_file = True
@@ -1342,6 +1358,8 @@ def test_updating_multi_file(self, pickle_persistence, good_pickle_files):
         with Path('pickletest_user_data').open('rb') as f:
             user_data_test = defaultdict(dict, pickle.load(f))
         assert user_data_test == user_data
+        pickle_persistence.drop_user_data(67890)
+        assert 67890 not in pickle_persistence.get_user_data()
 
         chat_data = pickle_persistence.get_chat_data()
         chat_data[-12345]['test3']['test4'] = 'test6'
@@ -1354,6 +1372,8 @@ def test_updating_multi_file(self, pickle_persistence, good_pickle_files):
         with Path('pickletest_chat_data').open('rb') as f:
             chat_data_test = defaultdict(dict, pickle.load(f))
         assert chat_data_test == chat_data
+        pickle_persistence.drop_chat_data(-67890)
+        assert -67890 not in pickle_persistence.get_chat_data()
 
         bot_data = pickle_persistence.get_bot_data()
         bot_data['test3']['test4'] = 'test6'
@@ -1408,6 +1428,8 @@ def test_updating_single_file(self, pickle_persistence, good_pickle_files):
         with Path('pickletest').open('rb') as f:
             user_data_test = defaultdict(dict, pickle.load(f)['user_data'])
         assert user_data_test == user_data
+        pickle_persistence.drop_user_data(67890)
+        assert 67890 not in pickle_persistence.get_user_data()
 
         chat_data = pickle_persistence.get_chat_data()
         chat_data[-12345]['test3']['test4'] = 'test6'
@@ -1420,6 +1442,8 @@ def test_updating_single_file(self, pickle_persistence, good_pickle_files):
         with Path('pickletest').open('rb') as f:
             chat_data_test = defaultdict(dict, pickle.load(f)['chat_data'])
         assert chat_data_test == chat_data
+        pickle_persistence.drop_chat_data(-67890)
+        assert -67890 not in pickle_persistence.get_chat_data()
 
         bot_data = pickle_persistence.get_bot_data()
         bot_data['test3']['test4'] = 'test6'
@@ -2132,6 +2156,8 @@ def test_updating(
         dict_persistence.update_user_data(12345, user_data[12345])
         assert dict_persistence.user_data == user_data
         assert dict_persistence.user_data_json == json.dumps(user_data)
+        dict_persistence.drop_user_data(67890)
+        assert 67890 not in dict_persistence.user_data
 
         chat_data = dict_persistence.get_chat_data()
         chat_data[-12345]['test3']['test4'] = 'test6'
@@ -2144,6 +2170,8 @@ def test_updating(
         dict_persistence.update_chat_data(-12345, chat_data[-12345])
         assert dict_persistence.chat_data == chat_data
         assert dict_persistence.chat_data_json == json.dumps(chat_data)
+        dict_persistence.drop_chat_data(-67890)
+        assert -67890 not in dict_persistence.chat_data
 
         bot_data = dict_persistence.get_bot_data()
         bot_data['test3']['test4'] = 'test6'

From cb786f61102ac6ff05da38248087cf680b1a9fc0 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 17 Jan 2022 05:49:41 +0530
Subject: [PATCH 07/10] versionadded tags

---
 telegram/ext/_basepersistence.py   | 4 ++++
 telegram/ext/_dictpersistence.py   | 4 ++++
 telegram/ext/_picklepersistence.py | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/telegram/ext/_basepersistence.py b/telegram/ext/_basepersistence.py
index 7caafbe8b32..f8e666ff226 100644
--- a/telegram/ext/_basepersistence.py
+++ b/telegram/ext/_basepersistence.py
@@ -541,6 +541,8 @@ def drop_chat_data(self, chat_id: int) -> None:
         """Will be called by the :class:`telegram.ext.Dispatcher`, when using
         :meth:`~telegram.ext.Dispatcher.drop_chat_data`.
 
+        .. versionadded:: 14.0
+
         Args:
             chat_id (:obj:`int`): The chat id to delete from the persistence.
         """
@@ -550,6 +552,8 @@ def drop_user_data(self, user_id: int) -> None:
         """Will be called by the :class:`telegram.ext.Dispatcher`, when using
         :meth:`~telegram.ext.Dispatcher.drop_user_data`.
 
+        .. versionadded:: 14.0
+
         Args:
             user_id (:obj:`int`): The user id to delete from the persistence.
         """
diff --git a/telegram/ext/_dictpersistence.py b/telegram/ext/_dictpersistence.py
index 0ecd454e83a..d4ea00a42d4 100644
--- a/telegram/ext/_dictpersistence.py
+++ b/telegram/ext/_dictpersistence.py
@@ -364,6 +364,8 @@ def update_callback_data(self, data: CDCData) -> None:
     def drop_chat_data(self, chat_id: int) -> None:
         """Will delete the specified key from the :attr:`chat_data`.
 
+        .. versionadded:: 14.0
+
         Args:
             chat_id (:obj:`int`): The chat id to delete from the persistence.
         """
@@ -376,6 +378,8 @@ def drop_chat_data(self, chat_id: int) -> None:
     def drop_user_data(self, user_id: int) -> None:
         """Will delete the specified key from the :attr:`user_data`.
 
+        .. versionadded:: 14.0
+
         Args:
             user_id (:obj:`int`): The user id to delete from the persistence.
         """
diff --git a/telegram/ext/_picklepersistence.py b/telegram/ext/_picklepersistence.py
index f8410818a4c..3cff78528bc 100644
--- a/telegram/ext/_picklepersistence.py
+++ b/telegram/ext/_picklepersistence.py
@@ -396,6 +396,8 @@ def drop_chat_data(self, chat_id: int) -> None:
         """Will delete the specified key from the :attr:`chat_data` and depending on
         :attr:`on_flush` save the pickle file.
 
+        .. versionadded:: 14.0
+
         Args:
             chat_id (:obj:`int`): The chat id to delete from the persistence.
         """
@@ -414,6 +416,8 @@ def drop_user_data(self, user_id: int) -> None:
         """Will delete the specified key from the :attr:`user_data` and depending on
         :attr:`on_flush` save the pickle file.
 
+        .. versionadded:: 14.0
+
         Args:
             user_id (:obj:`int`): The user id to delete from the persistence.
         """

From 687b559b9bed30b79276c3fa520506332f6ee4d4 Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Mon, 17 Jan 2022 16:42:35 +0530
Subject: [PATCH 08/10] More test coverage

---
 telegram/ext/_basepersistence.py |  2 +-
 tests/test_persistence.py        | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/telegram/ext/_basepersistence.py b/telegram/ext/_basepersistence.py
index f8e666ff226..3c1f2e797e4 100644
--- a/telegram/ext/_basepersistence.py
+++ b/telegram/ext/_basepersistence.py
@@ -532,7 +532,7 @@ def update_callback_data(self, data: CDCData) -> None:
 
         Args:
             data (Optional[Tuple[List[Tuple[:obj:`str`, :obj:`float`, \
-                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]):
+                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]]):
                 The relevant data to restore :class:`telegram.ext.CallbackDataCache`.
         """
 
diff --git a/tests/test_persistence.py b/tests/test_persistence.py
index a37d772b671..096d7c4b814 100644
--- a/tests/test_persistence.py
+++ b/tests/test_persistence.py
@@ -1929,6 +1929,12 @@ def test_with_context_types(self, ud, cd, bd, singlefile):
         assert isinstance(persistence.get_bot_data(), bd)
         assert persistence.get_bot_data() == 0
 
+        persistence.user_data = None
+        persistence.chat_data = None
+        persistence.drop_user_data(123)
+        persistence.drop_chat_data(123)
+        assert isinstance(persistence.get_user_data(), defaultdict)
+        assert isinstance(persistence.get_chat_data(), defaultdict)
         persistence.user_data = None
         persistence.chat_data = None
         persistence.update_user_data(1, ud(1))
@@ -2158,6 +2164,9 @@ def test_updating(
         assert dict_persistence.user_data_json == json.dumps(user_data)
         dict_persistence.drop_user_data(67890)
         assert 67890 not in dict_persistence.user_data
+        dict_persistence._user_data = None
+        dict_persistence.drop_user_data(123)
+        assert isinstance(dict_persistence.get_user_data(), defaultdict)
 
         chat_data = dict_persistence.get_chat_data()
         chat_data[-12345]['test3']['test4'] = 'test6'
@@ -2172,6 +2181,9 @@ def test_updating(
         assert dict_persistence.chat_data_json == json.dumps(chat_data)
         dict_persistence.drop_chat_data(-67890)
         assert -67890 not in dict_persistence.chat_data
+        dict_persistence._chat_data = None
+        dict_persistence.drop_chat_data(123)
+        assert isinstance(dict_persistence.get_chat_data(), defaultdict)
 
         bot_data = dict_persistence.get_bot_data()
         bot_data['test3']['test4'] = 'test6'

From 05bc8b038d34409e04a2f1e0ee181e8a49c36e3a Mon Sep 17 00:00:00 2001
From: Harshil <37377066+harshil21@users.noreply.github.com>
Date: Wed, 19 Jan 2022 04:28:38 +0530
Subject: [PATCH 09/10] Review 3

Update a ValueError message
---
 telegram/ext/_basepersistence.py   |  2 +-
 telegram/ext/_dictpersistence.py   | 14 ++++----
 telegram/ext/_dispatcher.py        | 38 +++++---------------
 telegram/ext/_picklepersistence.py | 10 +++---
 tests/conftest.py                  |  7 ++--
 tests/test_dispatcher.py           | 30 ++++++----------
 tests/test_persistence.py          | 56 +++++++++++++-----------------
 7 files changed, 60 insertions(+), 97 deletions(-)

diff --git a/telegram/ext/_basepersistence.py b/telegram/ext/_basepersistence.py
index 3c1f2e797e4..81a16a66371 100644
--- a/telegram/ext/_basepersistence.py
+++ b/telegram/ext/_basepersistence.py
@@ -457,7 +457,7 @@ def get_callback_data(self) -> Optional[CDCData]:
 
         Returns:
             Optional[Tuple[List[Tuple[:obj:`str`, :obj:`float`, \
-                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]:
+                Dict[:obj:`str`, :obj:`Any`]]], Dict[:obj:`str`, :obj:`str`]]]:
                 The restored meta data or :obj:`None`, if no data was stored.
         """
 
diff --git a/telegram/ext/_dictpersistence.py b/telegram/ext/_dictpersistence.py
index d4ea00a42d4..df3f9aa71f5 100644
--- a/telegram/ext/_dictpersistence.py
+++ b/telegram/ext/_dictpersistence.py
@@ -370,10 +370,9 @@ def drop_chat_data(self, chat_id: int) -> None:
             chat_id (:obj:`int`): The chat id to delete from the persistence.
         """
         if self._chat_data is None:
-            self._chat_data = defaultdict(dict)
-        else:
-            del self._chat_data[chat_id]
-            self._chat_data_json = None
+            return
+        self._chat_data.pop(chat_id, None)
+        self._chat_data_json = None
 
     def drop_user_data(self, user_id: int) -> None:
         """Will delete the specified key from the :attr:`user_data`.
@@ -384,10 +383,9 @@ def drop_user_data(self, user_id: int) -> None:
             user_id (:obj:`int`): The user id to delete from the persistence.
         """
         if self._user_data is None:
-            self._user_data = defaultdict(dict)
-        else:
-            del self._user_data[user_id]
-            self._user_data_json = None
+            return
+        self._user_data.pop(user_id, None)
+        self._user_data_json = None
 
     def refresh_user_data(self, user_id: int, user_data: Dict) -> None:
         """Does nothing.
diff --git a/telegram/ext/_dispatcher.py b/telegram/ext/_dispatcher.py
index 2d5ec433c7f..3bba0df15e0 100644
--- a/telegram/ext/_dispatcher.py
+++ b/telegram/ext/_dispatcher.py
@@ -218,14 +218,12 @@ def __init__(
                 stacklevel=stack_level,
             )
 
-        self._user_data: DefaultDict[int, UD]
-        self._chat_data: DefaultDict[int, CD]
+        self._user_data: DefaultDict[int, UD] = defaultdict(self.context_types.user_data)
+        self._chat_data: DefaultDict[int, CD] = defaultdict(self.context_types.chat_data)
         # Read only mapping-
-        self.user_data: Mapping[int, UD]
-        self.chat_data: Mapping[int, CD]
+        self.user_data: Mapping[int, UD] = MappingProxyType(self._user_data)
+        self.chat_data: Mapping[int, CD] = MappingProxyType(self._chat_data)
 
-        self._set_user_data(defaultdict(self.context_types.user_data))
-        self._set_chat_data(defaultdict(self.context_types.chat_data))
         self.bot_data = self.context_types.bot_data()
 
         self.persistence: Optional[BasePersistence]
@@ -240,13 +238,9 @@ def __init__(
             self.persistence.set_bot(self.bot)
 
             if self.persistence.store_data.user_data:
-                self._set_user_data(self.persistence.get_user_data())
-                if not isinstance(self._user_data, defaultdict):
-                    raise ValueError("user_data must be of type defaultdict")
+                self._user_data.update(self.persistence.get_user_data())
             if self.persistence.store_data.chat_data:
-                self._set_chat_data(self.persistence.get_chat_data())
-                if not isinstance(self._chat_data, defaultdict):
-                    raise ValueError("chat_data must be of type defaultdict")
+                self._chat_data.update(self.persistence.get_chat_data())
             if self.persistence.store_data.bot_data:
                 self.bot_data = self.persistence.get_bot_data()
                 if not isinstance(self.bot_data, self.context_types.bot_data):
@@ -257,7 +251,7 @@ def __init__(
                 persistent_data = self.persistence.get_callback_data()
                 if persistent_data is not None:
                     if not isinstance(persistent_data, tuple) and len(persistent_data) != 2:
-                        raise ValueError('callback_data must be a 2-tuple')
+                        raise ValueError('callback_data must be a tuple of length 2')
                     # Mypy doesn't know that persistence.set_bot (see above) already checks that
                     # self.bot is an instance of ExtBot if callback_data should be stored ...
                     self.bot.callback_data_cache = CallbackDataCache(  # type: ignore[attr-defined]
@@ -324,20 +318,6 @@ def get_instance(cls) -> 'Dispatcher':
             return cls.__singleton()  # type: ignore[return-value] # pylint: disable=not-callable
         raise RuntimeError(f'{cls.__name__} not initialized or multiple instances exist')
 
-    def _set_chat_data(self, data: DefaultDict[int, CD]) -> None:
-        """Used for assigning a new value to the underlying chat_data dictionary. Also updates
-        the read-only mapping.
-        """
-        self._chat_data = data
-        self.chat_data = MappingProxyType(self._chat_data)
-
-    def _set_user_data(self, data: DefaultDict[int, UD]) -> None:
-        """Used for assigning a new value to the underlying user_data dictionary. Also updates
-        the read-only mapping.
-        """
-        self._user_data = data
-        self.user_data = MappingProxyType(self._user_data)
-
     def _pooled(self) -> None:
         thr_name = current_thread().name
         while 1:
@@ -681,7 +661,7 @@ def drop_chat_data(self, chat_id: int) -> None:
             chat_id (:obj:`int`): The chat id to delete from the persistence. The entry
                 will be deleted even if it is not empty.
         """
-        del self._chat_data[chat_id]
+        self._chat_data.pop(chat_id, None)  # type: ignore[arg-type]
 
         if self.persistence:
             self.persistence.drop_chat_data(chat_id)
@@ -695,7 +675,7 @@ def drop_user_data(self, user_id: int) -> None:
             user_id (:obj:`int`): The user id to delete from the persistence. The entry
                 will be deleted even if it is not empty.
         """
-        del self._user_data[user_id]
+        self._user_data.pop(user_id, None)  # type: ignore[arg-type]
 
         if self.persistence:
             self.persistence.drop_user_data(user_id)
diff --git a/telegram/ext/_picklepersistence.py b/telegram/ext/_picklepersistence.py
index 3cff78528bc..3fcbb995424 100644
--- a/telegram/ext/_picklepersistence.py
+++ b/telegram/ext/_picklepersistence.py
@@ -402,9 +402,8 @@ def drop_chat_data(self, chat_id: int) -> None:
             chat_id (:obj:`int`): The chat id to delete from the persistence.
         """
         if self.chat_data is None:
-            self.chat_data = defaultdict(self.context_types.chat_data)
-        else:
-            del self.chat_data[chat_id]
+            return
+        self.chat_data.pop(chat_id, None)  # type: ignore[arg-type]
 
         if not self.on_flush:
             if not self.single_file:
@@ -422,9 +421,8 @@ def drop_user_data(self, user_id: int) -> None:
             user_id (:obj:`int`): The user id to delete from the persistence.
         """
         if self.user_data is None:
-            self.user_data = defaultdict(self.context_types.user_data)
-        else:
-            del self.user_data[user_id]
+            return
+        self.user_data.pop(user_id, None)  # type: ignore[arg-type]
 
         if not self.on_flush:
             if not self.single_file:
diff --git a/tests/conftest.py b/tests/conftest.py
index b8391342df5..bd7b5276164 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -28,6 +28,7 @@
 from threading import Thread, Event
 from time import sleep
 from typing import Callable, List, Iterable, Any
+from types import MappingProxyType
 
 import pytest
 import pytz
@@ -194,8 +195,10 @@ def dp(_dp):
     # Reset the dispatcher first
     while not _dp.update_queue.empty():
         _dp.update_queue.get(False)
-    _dp._set_chat_data(defaultdict(dict))
-    _dp._set_user_data(defaultdict(dict))
+    _dp._chat_data = defaultdict(dict)
+    _dp._user_data = defaultdict(dict)
+    _dp.chat_data = MappingProxyType(_dp._chat_data)  # Rebuild the mapping so it updates
+    _dp.user_data = MappingProxyType(_dp._user_data)
     _dp.bot_data = {}
     _dp.handlers = {}
     _dp.error_handlers = {}
diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py
index a79b512346d..6a077049a22 100644
--- a/tests/test_dispatcher.py
+++ b/tests/test_dispatcher.py
@@ -908,33 +908,23 @@ def callback(update, context):
 
     @pytest.mark.parametrize(
         "c_id,expected",
-        [(321, {123: [], 222: "remove_me"}), (111, None)],
-        ids=["test chat_id removal", "test no key in data (error)"],
+        [(321, {222: "remove_me"}), (111, {321: {'not_empty': 'no'}, 222: "remove_me"})],
+        ids=["test chat_id removal", "test no key in data (no error)"],
     )
     def test_drop_chat_data(self, dp, c_id, expected):
-        dp._chat_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
-
-        if c_id is not None and c_id not in dp.chat_data:
-            with pytest.raises(KeyError):
-                dp.drop_chat_data(c_id)
-        else:
-            dp.drop_chat_data(c_id)
-            assert dp.chat_data == expected
+        dp._chat_data.update({321: {'not_empty': 'no'}, 222: "remove_me"})
+        dp.drop_chat_data(c_id)
+        assert dp.chat_data == expected
 
     @pytest.mark.parametrize(
         "u_id,expected",
-        [(321, {123: [], 222: "remove_me"}), (111, None)],
-        ids=["test user_id removal", "test no key in data (error)"],
+        [(321, {222: "remove_me"}), (111, {321: {'not_empty': 'no'}, 222: "remove_me"})],
+        ids=["test user_id removal", "test no key in data (no error)"],
     )
     def test_drop_user_data(self, dp, u_id, expected):
-        dp._user_data.update({123: [], 321: {'not_empty': 'no'}, 222: "remove_me"})
-
-        if u_id is not None and u_id not in dp.user_data:
-            with pytest.raises(KeyError):
-                dp.drop_user_data(u_id)
-        else:
-            dp.drop_user_data(u_id)
-            assert dp.user_data == expected
+        dp._user_data.update({321: {'not_empty': 'no'}, 222: "remove_me"})
+        dp.drop_user_data(u_id)
+        assert dp.user_data == expected
 
     def test_update_persistence_once_per_update(self, monkeypatch, dp):
         def update_persistence(*args, **kwargs):
diff --git a/tests/test_persistence.py b/tests/test_persistence.py
index 096d7c4b814..66ffbe7ff82 100644
--- a/tests/test_persistence.py
+++ b/tests/test_persistence.py
@@ -296,52 +296,40 @@ def test_conversationhandler_addition(self, dp, base_persistence):
     def test_dispatcher_integration_init(
         self, bot, base_persistence, chat_data, user_data, bot_data, callback_data
     ):
-        def get_user_data():
+        # Bad data testing-
+        def bad_get_bot_data():
             return "test"
 
-        def get_chat_data():
-            return "test"
-
-        def get_bot_data():
+        def bad_get_callback_data():
             return "test"
 
-        def get_callback_data():
-            return "test"
-
-        base_persistence.get_user_data = get_user_data
-        base_persistence.get_chat_data = get_chat_data
-        base_persistence.get_bot_data = get_bot_data
-        base_persistence.get_callback_data = get_callback_data
+        # Good data testing-
+        def good_get_user_data():
+            return user_data
 
-        with pytest.raises(ValueError, match="user_data must be of type defaultdict"):
-            UpdaterBuilder().bot(bot).persistence(base_persistence).build()
+        def good_get_chat_data():
+            return chat_data
 
-        def get_user_data():
-            return user_data
+        def good_get_bot_data():
+            return bot_data
 
-        base_persistence.get_user_data = get_user_data
-        with pytest.raises(ValueError, match="chat_data must be of type defaultdict"):
-            UpdaterBuilder().bot(bot).persistence(base_persistence).build()
+        def good_get_callback_data():
+            return callback_data
 
-        def get_chat_data():
-            return chat_data
+        base_persistence.get_user_data = good_get_user_data  # No errors to be tested so
+        base_persistence.get_chat_data = good_get_chat_data
+        base_persistence.get_bot_data = bad_get_bot_data
+        base_persistence.get_callback_data = bad_get_callback_data
 
-        base_persistence.get_chat_data = get_chat_data
         with pytest.raises(ValueError, match="bot_data must be of type dict"):
             UpdaterBuilder().bot(bot).persistence(base_persistence).build()
 
-        def get_bot_data():
-            return bot_data
-
-        base_persistence.get_bot_data = get_bot_data
-        with pytest.raises(ValueError, match="callback_data must be a 2-tuple"):
+        base_persistence.get_bot_data = good_get_bot_data
+        with pytest.raises(ValueError, match="callback_data must be a tuple of length 2"):
             UpdaterBuilder().bot(bot).persistence(base_persistence).build()
 
-        def get_callback_data():
-            return callback_data
-
         base_persistence.bot = None
-        base_persistence.get_callback_data = get_callback_data
+        base_persistence.get_callback_data = good_get_callback_data
         u = UpdaterBuilder().bot(bot).persistence(base_persistence).build()
         assert u.dispatcher.bot is base_persistence.bot
         assert u.dispatcher.bot_data == bot_data
@@ -1511,6 +1499,9 @@ def test_save_on_flush_multi_files(self, pickle_persistence, good_pickle_files):
         pickle_persistence.update_user_data(54321, user_data[54321])
         assert pickle_persistence.user_data == user_data
 
+        pickle_persistence.drop_user_data(0)
+        assert pickle_persistence.user_data == user_data
+
         with Path('pickletest_user_data').open('rb') as f:
             user_data_test = defaultdict(dict, pickle.load(f))
         assert not user_data_test == user_data
@@ -1522,6 +1513,9 @@ def test_save_on_flush_multi_files(self, pickle_persistence, good_pickle_files):
         pickle_persistence.update_chat_data(54321, chat_data[54321])
         assert pickle_persistence.chat_data == chat_data
 
+        pickle_persistence.drop_chat_data(0)
+        assert pickle_persistence.user_data == user_data
+
         with Path('pickletest_chat_data').open('rb') as f:
             chat_data_test = defaultdict(dict, pickle.load(f))
         assert not chat_data_test == chat_data

From 2bbd2b4779fdefb8fb6434014dd2d9c023fbc91d Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Wed, 19 Jan 2022 12:33:38 +0100
Subject: [PATCH 10/10] Fix animation tests

---
 tests/test_animation.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/test_animation.py b/tests/test_animation.py
index 3f7c6c85149..b07394eea0a 100644
--- a/tests/test_animation.py
+++ b/tests/test_animation.py
@@ -76,9 +76,8 @@ def test_creation(self, animation):
         assert animation.file_unique_id != ''
 
     def test_expected_values(self, animation):
-        assert animation.file_size == self.file_size
         assert animation.mime_type == self.mime_type
-        assert animation.file_name == self.file_name
+        assert animation.file_name.startswith('game.gif') == self.file_name.startswith('game.gif')
         assert isinstance(animation.thumb, PhotoSize)
 
     @flaky(3, 1)
@@ -122,7 +121,6 @@ def make_assertion(url, data, **kwargs):
     def test_get_and_download(self, bot, animation):
         new_file = bot.get_file(animation.file_id)
 
-        assert new_file.file_size == self.file_size
         assert new_file.file_id == animation.file_id
         assert new_file.file_path.startswith('https://')