From 60bcd88b681c76b0345ed0b5a2a2f85f89301c83 Mon Sep 17 00:00:00 2001 From: Senpos Date: Sun, 3 May 2020 17:26:08 +0300 Subject: [PATCH 1/6] Fix IDFilter behavior when single str id passed --- aiogram/dispatcher/filters/builtin.py | 34 +++++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/aiogram/dispatcher/filters/builtin.py b/aiogram/dispatcher/filters/builtin.py index 0a81998a..2334b343 100644 --- a/aiogram/dispatcher/filters/builtin.py +++ b/aiogram/dispatcher/filters/builtin.py @@ -12,6 +12,19 @@ from aiogram.dispatcher.filters.filters import BoundFilter, Filter from aiogram.types import CallbackQuery, Message, InlineQuery, Poll, ChatType +IDFilterArgumentType = typing.Union[typing.Iterable[typing.Union[int, str]], str, int] + + +def extract_filter_ids(id_filter_argument: IDFilterArgumentType) -> typing.List[int]: + # since "str" is also an "Iterable", we have to check for it first + if isinstance(id_filter_argument, str): + return [int(id_filter_argument)] + if isinstance(id_filter_argument, Iterable): + return [int(item) for (item) in id_filter_argument] + # the last possible type is a single "int" + return [id_filter_argument] + + class Command(Filter): """ You can handle commands by using this filter. @@ -543,12 +556,11 @@ class ExceptionsFilter(BoundFilter): except: return False - class IDFilter(Filter): def __init__(self, - user_id: Optional[Union[Iterable[Union[int, str]], str, int]] = None, - chat_id: Optional[Union[Iterable[Union[int, str]], str, int]] = None, + user_id: Optional[IDFilterArgumentType] = None, + chat_id: Optional[IDFilterArgumentType] = None, ): """ :param user_id: @@ -557,18 +569,14 @@ class IDFilter(Filter): if user_id is None and chat_id is None: raise ValueError("Both user_id and chat_id can't be None") - self.user_id = None - self.chat_id = None + self.user_id: Optional[IDFilterArgumentType] = None + self.chat_id: Optional[IDFilterArgumentType] = None + if user_id: - if isinstance(user_id, Iterable): - self.user_id = list(map(int, user_id)) - else: - self.user_id = [int(user_id), ] + self.user_id = extract_filter_ids(user_id) + if chat_id: - if isinstance(chat_id, Iterable): - self.chat_id = list(map(int, chat_id)) - else: - self.chat_id = [int(chat_id), ] + self.chat_id = extract_filter_ids(chat_id) @classmethod def validate(cls, full_config: typing.Dict[str, typing.Any]) -> typing.Optional[typing.Dict[str, typing.Any]]: From ac7758aeb03bf186a701fde0dd2bce4254165d82 Mon Sep 17 00:00:00 2001 From: Senpos Date: Sun, 3 May 2020 18:00:35 +0300 Subject: [PATCH 2/6] Make self.user_id and self.chat_id Set[int] in IDFilter --- aiogram/dispatcher/filters/builtin.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/aiogram/dispatcher/filters/builtin.py b/aiogram/dispatcher/filters/builtin.py index 2334b343..2c7bcaea 100644 --- a/aiogram/dispatcher/filters/builtin.py +++ b/aiogram/dispatcher/filters/builtin.py @@ -15,14 +15,14 @@ from aiogram.types import CallbackQuery, Message, InlineQuery, Poll, ChatType IDFilterArgumentType = typing.Union[typing.Iterable[typing.Union[int, str]], str, int] -def extract_filter_ids(id_filter_argument: IDFilterArgumentType) -> typing.List[int]: +def extract_filter_ids(id_filter_argument: IDFilterArgumentType) -> typing.Set[int]: # since "str" is also an "Iterable", we have to check for it first if isinstance(id_filter_argument, str): - return [int(id_filter_argument)] + return {int(id_filter_argument),} if isinstance(id_filter_argument, Iterable): - return [int(item) for (item) in id_filter_argument] + return {int(item) for (item) in id_filter_argument} # the last possible type is a single "int" - return [id_filter_argument] + return {id_filter_argument,} class Command(Filter): @@ -569,8 +569,8 @@ class IDFilter(Filter): if user_id is None and chat_id is None: raise ValueError("Both user_id and chat_id can't be None") - self.user_id: Optional[IDFilterArgumentType] = None - self.chat_id: Optional[IDFilterArgumentType] = None + self.user_id: Optional[typing.Set[int]] = None + self.chat_id: Optional[typing.Set[int]] = None if user_id: self.user_id = extract_filter_ids(user_id) From eb48319f3a39bfdab395b186df4304d658d11e7e Mon Sep 17 00:00:00 2001 From: Senpos Date: Sun, 17 May 2020 16:22:33 +0300 Subject: [PATCH 3/6] Change name for chat id type and helper to extract it --- aiogram/dispatcher/filters/builtin.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/aiogram/dispatcher/filters/builtin.py b/aiogram/dispatcher/filters/builtin.py index 2c7bcaea..9a773e2d 100644 --- a/aiogram/dispatcher/filters/builtin.py +++ b/aiogram/dispatcher/filters/builtin.py @@ -12,10 +12,10 @@ from aiogram.dispatcher.filters.filters import BoundFilter, Filter from aiogram.types import CallbackQuery, Message, InlineQuery, Poll, ChatType -IDFilterArgumentType = typing.Union[typing.Iterable[typing.Union[int, str]], str, int] +ChatIDArgumentType = typing.Union[typing.Iterable[typing.Union[int, str]], str, int] -def extract_filter_ids(id_filter_argument: IDFilterArgumentType) -> typing.Set[int]: +def extract_chat_ids(id_filter_argument: ChatIDArgumentType) -> typing.Set[int]: # since "str" is also an "Iterable", we have to check for it first if isinstance(id_filter_argument, str): return {int(id_filter_argument),} @@ -559,8 +559,8 @@ class ExceptionsFilter(BoundFilter): class IDFilter(Filter): def __init__(self, - user_id: Optional[IDFilterArgumentType] = None, - chat_id: Optional[IDFilterArgumentType] = None, + user_id: Optional[ChatIDArgumentType] = None, + chat_id: Optional[ChatIDArgumentType] = None, ): """ :param user_id: @@ -573,10 +573,10 @@ class IDFilter(Filter): self.chat_id: Optional[typing.Set[int]] = None if user_id: - self.user_id = extract_filter_ids(user_id) + self.user_id = extract_chat_ids(user_id) if chat_id: - self.chat_id = extract_filter_ids(chat_id) + self.chat_id = extract_chat_ids(chat_id) @classmethod def validate(cls, full_config: typing.Dict[str, typing.Any]) -> typing.Optional[typing.Dict[str, typing.Any]]: From 6d53463880e6b1b8acf02f534b3eb926ac1d478f Mon Sep 17 00:00:00 2001 From: Senpos Date: Sun, 17 May 2020 16:27:50 +0300 Subject: [PATCH 4/6] Refactor AdminFilter to use extract_chat_ids helper as in IDFilter --- aiogram/dispatcher/filters/builtin.py | 28 +++++++++++++-------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/aiogram/dispatcher/filters/builtin.py b/aiogram/dispatcher/filters/builtin.py index 9a773e2d..a01bac7b 100644 --- a/aiogram/dispatcher/filters/builtin.py +++ b/aiogram/dispatcher/filters/builtin.py @@ -15,14 +15,14 @@ from aiogram.types import CallbackQuery, Message, InlineQuery, Poll, ChatType ChatIDArgumentType = typing.Union[typing.Iterable[typing.Union[int, str]], str, int] -def extract_chat_ids(id_filter_argument: ChatIDArgumentType) -> typing.Set[int]: +def extract_chat_ids(chat_id: ChatIDArgumentType) -> typing.Set[int]: # since "str" is also an "Iterable", we have to check for it first - if isinstance(id_filter_argument, str): - return {int(id_filter_argument),} - if isinstance(id_filter_argument, Iterable): - return {int(item) for (item) in id_filter_argument} + if isinstance(chat_id, str): + return {int(chat_id), } + if isinstance(chat_id, Iterable): + return {int(item) for (item) in chat_id} # the last possible type is a single "int" - return {id_filter_argument,} + return {chat_id, } class Command(Filter): @@ -622,22 +622,20 @@ class AdminFilter(Filter): is_chat_admin is required for InlineQuery. """ - def __init__(self, is_chat_admin: Optional[Union[Iterable[Union[int, str]], str, int, bool]] = None): + def __init__(self, is_chat_admin: Optional[Union[ChatIDArgumentType, bool]] = None): self._check_current = False self._chat_ids = None if is_chat_admin is False: raise ValueError("is_chat_admin cannot be False") - if is_chat_admin: - if isinstance(is_chat_admin, bool): - self._check_current = is_chat_admin - if isinstance(is_chat_admin, Iterable): - self._chat_ids = list(is_chat_admin) - else: - self._chat_ids = [is_chat_admin] - else: + if not is_chat_admin: self._check_current = True + return + + if isinstance(is_chat_admin, bool): + self._check_current = is_chat_admin + self._chat_ids = extract_chat_ids(is_chat_admin) @classmethod def validate(cls, full_config: typing.Dict[str, typing.Any]) -> typing.Optional[typing.Dict[str, typing.Any]]: From 6b1c7d3b36a09990b3cb01175b42b1b388ecf015 Mon Sep 17 00:00:00 2001 From: Senpos Date: Sun, 17 May 2020 16:56:11 +0300 Subject: [PATCH 5/6] Add tests for extract_chat_ids --- .../test_filters/test_builtin.py | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/tests/test_dispatcher/test_filters/test_builtin.py b/tests/test_dispatcher/test_filters/test_builtin.py index 86344cec..a26fc139 100644 --- a/tests/test_dispatcher/test_filters/test_builtin.py +++ b/tests/test_dispatcher/test_filters/test_builtin.py @@ -1,6 +1,12 @@ +from typing import Set + import pytest -from aiogram.dispatcher.filters.builtin import Text +from aiogram.dispatcher.filters.builtin import ( + Text, + extract_chat_ids, + ChatIDArgumentType, +) class TestText: @@ -16,3 +22,50 @@ class TestText: config = {param: value} res = Text.validate(config) assert res == {key: value} + + +@pytest.mark.parametrize( + ('chat_id', 'expected'), + ( + pytest.param('-64856280', {-64856280,}, id='single negative int as string'), + pytest.param('64856280', {64856280,}, id='single positive int as string'), + pytest.param(-64856280, {-64856280,}, id='single negative int'), + pytest.param(64856280, {64856280,}, id='single positive negative int'), + pytest.param( + ['-64856280'], {-64856280,}, id='list of single negative int as string' + ), + pytest.param([-64856280], {-64856280,}, id='list of single negative int'), + pytest.param( + ['-64856280', '-64856280'], + {-64856280,}, + id='list of two duplicated negative ints as strings', + ), + pytest.param( + ['-64856280', -64856280], + {-64856280,}, + id='list of one negative int as string and one negative int', + ), + pytest.param( + [-64856280, -64856280], + {-64856280,}, + id='list of two duplicated negative ints', + ), + pytest.param( + iter(['-64856280']), + {-64856280,}, + id='iterator from a list of single negative int as string', + ), + pytest.param( + [10000000, 20000000, 30000000], + {10000000, 20000000, 30000000}, + id='list of several positive ints', + ), + pytest.param( + [10000000, '20000000', -30000000], + {10000000, 20000000, -30000000}, + id='list of positive int, positive int as string, negative int', + ), + ), +) +def test_extract_chat_ids(chat_id: ChatIDArgumentType, expected: Set[int]): + assert extract_chat_ids(chat_id) == expected From 5bbb36372afa93ed3f2acdd4980e0e0f8396179f Mon Sep 17 00:00:00 2001 From: evgfilim1 Date: Sun, 17 May 2020 19:34:24 +0500 Subject: [PATCH 6/6] Small whitespace fix --- aiogram/dispatcher/filters/builtin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiogram/dispatcher/filters/builtin.py b/aiogram/dispatcher/filters/builtin.py index a01bac7b..5fe01dde 100644 --- a/aiogram/dispatcher/filters/builtin.py +++ b/aiogram/dispatcher/filters/builtin.py @@ -556,8 +556,8 @@ class ExceptionsFilter(BoundFilter): except: return False -class IDFilter(Filter): +class IDFilter(Filter): def __init__(self, user_id: Optional[ChatIDArgumentType] = None, chat_id: Optional[ChatIDArgumentType] = None,