Remove Client-Side Parameter Validation (#3024)

telegram/_bot.py

@@ -96,7 +96,7 @@ from telegram._utils.files import is_local_file, parse_file_input
 from telegram._utils.types import DVInput, FileInput, JSONDict, ODVInput, ReplyMarkup
 from telegram._webhookinfo import WebhookInfo
 from telegram.constants import InlineQueryLimit
-from telegram.error import InvalidToken, TelegramError
+from telegram.error import InvalidToken
 from telegram.request import BaseRequest, RequestData
 from telegram.request._httpxrequest import HTTPXRequest
 from telegram.request._requestparameter import RequestParameter
@@ -2092,6 +2092,8 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.Message`: On success, if edited message is not an inline message, the
             edited message is returned, otherwise :obj:`True` is returned.
         """
+        # The location parameter is a convenience functionality added by us, so enforcing the
+        # mutual exclusivity here is nothing that Telegram would handle anyway
         if not (all([latitude, longitude]) or location):
             raise ValueError(
                 "Either location or latitude and longitude must be passed as argument."
@@ -2284,11 +2286,20 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
+        # The venue parameter is a convenience functionality added by us, so enforcing the
+        # mutual exclusivity here is nothing that Telegram would handle anyway
         if not (venue or all([latitude, longitude, address, title])):
             raise ValueError(
                 "Either venue or latitude, longitude, address and title must be "
                 "passed as arguments."
             )
+        if not (
+            bool(venue) ^ any([latitude, longitude, address, title])
+        ):  # pylint: disable=superfluous-parens
+            raise ValueError(
+                "Either venue or latitude, longitude, address and title must be "
+                "passed as arguments. Not both."
+            )
 
         if isinstance(venue, Venue):
             latitude = venue.location.latitude
@@ -2405,10 +2416,19 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
+        # The contact parameter is a convenience functionality added by us, so enforcing the
+        # mutual exclusivity here is nothing that Telegram would handle anyway
         if (not contact) and (not all([phone_number, first_name])):
             raise ValueError(
                 "Either contact or phone_number and first_name must be passed as arguments."
             )
+        if not (
+            bool(contact) ^ any([phone_number, first_name])
+        ):  # pylint: disable=superfluous-parens
+            raise ValueError(
+                "Either contact or phone_number and first_name must be passed as arguments. "
+                "Not both."
+            )
 
         if isinstance(contact, Contact):
             phone_number = contact.phone_number
@@ -3407,12 +3427,6 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        if inline_message_id is None and (chat_id is None or message_id is None):
-            raise ValueError(
-                "edit_message_caption: Both chat_id and message_id are required when "
-                "inline_message_id is not specified"
-            )
-
         data: JSONDict = {"parse_mode": parse_mode}
 
         if caption:
@@ -3492,12 +3506,6 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
         Raises:
             :class:`telegram.error.TelegramError`
         """
-        if inline_message_id is None and (chat_id is None or message_id is None):
-            raise ValueError(
-                "edit_message_media: Both chat_id and message_id are required when "
-                "inline_message_id is not specified"
-            )
-
         data: JSONDict = {"media": media}
 
         if chat_id:
@@ -3568,12 +3576,6 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        if inline_message_id is None and (chat_id is None or message_id is None):
-            raise ValueError(
-                "edit_message_reply_markup: Both chat_id and message_id are required when "
-                "inline_message_id is not specified"
-            )
-
         data: JSONDict = {}
 
         if chat_id:
@@ -4668,27 +4670,9 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        ok = bool(ok)
-
-        if ok and (shipping_options is None or error_message is not None):
-            raise TelegramError(
-                "answerShippingQuery: If ok is True, shipping_options "
-                "should not be empty and there should not be error_message"
-            )
-
-        if not ok and (shipping_options is not None or error_message is None):
-            raise TelegramError(
-                "answerShippingQuery: If ok is False, error_message "
-                "should not be empty and there should not be shipping_options"
-            )
-
         data: JSONDict = {"shipping_query_id": shipping_query_id, "ok": ok}
 
-        if ok:
+        if shipping_options is not None:
-            if not shipping_options:
-                # not using an assert statement directly here since they are removed in
-                # the optimized bytecode
-                raise AssertionError
             data["shipping_options"] = [option.to_dict() for option in shipping_options]
         if error_message is not None:
             data["error_message"] = error_message
@@ -4759,15 +4743,6 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        ok = bool(ok)
-
-        if not (ok ^ (error_message is not None)):  # pylint: disable=superfluous-parens
-            raise TelegramError(
-                "answerPreCheckoutQuery: If ok is True, there should "
-                "not be error_message; if ok is False, error_message "
-                "should not be empty"
-            )
-
         data: JSONDict = {"pre_checkout_query_id": pre_checkout_query_id, "ok": ok}
 
         if error_message is not None:
@@ -5275,14 +5250,7 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        if creates_join_request and member_limit:
+        data: JSONDict = {"chat_id": chat_id}
-            raise ValueError(
-                "If `creates_join_request` is `True`, `member_limit` can't be specified."
-            )
-
-        data: JSONDict = {
-            "chat_id": chat_id,
-        }
 
         if expire_date is not None:
             data["expire_date"] = expire_date
@@ -5380,11 +5348,6 @@ class Bot(TelegramObject, AbstractAsyncContextManager):
             :class:`telegram.error.TelegramError`
 
         """
-        if creates_join_request and member_limit:
-            raise ValueError(
-                "If `creates_join_request` is `True`, `member_limit` can't be specified."
-            )
-
         link = invite_link.invite_link if isinstance(invite_link, ChatInviteLink) else invite_link
         data: JSONDict = {"chat_id": chat_id, "invite_link": link}
 

tests/test_bot.py

@@ -64,7 +64,7 @@ from telegram import (
 from telegram._utils.datetime import from_timestamp, to_timestamp
 from telegram._utils.defaultvalue import DefaultValue
 from telegram.constants import ChatAction, InlineQueryLimit, MenuButtonType, ParseMode
-from telegram.error import BadRequest, InvalidToken, NetworkError, TelegramError
+from telegram.error import BadRequest, InvalidToken, NetworkError
 from telegram.ext import ExtBot, InvalidCallbackData
 from telegram.helpers import escape_markdown
 from telegram.request import BaseRequest, HTTPXRequest, RequestData
@@ -1527,10 +1527,6 @@ class TestBot:
 
         assert message.caption == "new caption"
 
-    async def test_edit_message_caption_without_required(self, bot):
-        with pytest.raises(ValueError, match="Both chat_id and message_id are required when"):
-            await bot.edit_message_caption(caption="new_caption")
-
     @pytest.mark.skip(reason="need reference to an inline message")
     async def test_edit_message_caption_inline(self):
         pass
@@ -1544,11 +1540,6 @@ class TestBot:
 
         assert message is not True
 
-    async def test_edit_message_reply_markup_without_required(self, bot):
-        new_markup = InlineKeyboardMarkup([[InlineKeyboardButton(text="test", callback_data="1")]])
-        with pytest.raises(ValueError, match="Both chat_id and message_id are required when"):
-            await bot.edit_message_reply_markup(reply_markup=new_markup)
-
     @pytest.mark.skip(reason="need reference to an inline message")
     async def test_edit_reply_markup_inline(self):
         pass
@@ -1871,24 +1862,6 @@ class TestBot:
         monkeypatch.setattr(bot.request, "post", make_assertion)
         assert await bot.answer_shipping_query(1, False, error_message="Not enough fish")
 
-    async def test_answer_shipping_query_errors(self, monkeypatch, bot):
-        shipping_options = ShippingOption(1, "option1", [LabeledPrice("price", 100)])
-
-        with pytest.raises(TelegramError, match="should not be empty and there should not be"):
-            await bot.answer_shipping_query(1, True, error_message="Not enough fish")
-
-        with pytest.raises(TelegramError, match="should not be empty and there should not be"):
-            await bot.answer_shipping_query(1, False)
-
-        with pytest.raises(TelegramError, match="should not be empty and there should not be"):
-            await bot.answer_shipping_query(1, False, shipping_options=shipping_options)
-
-        with pytest.raises(TelegramError, match="should not be empty and there should not be"):
-            await bot.answer_shipping_query(1, True)
-
-        with pytest.raises(AssertionError):
-            await bot.answer_shipping_query(1, True, shipping_options=[])
-
     # TODO: Needs improvement. Need incoming pre checkout queries to test
     async def test_answer_pre_checkout_query_ok(self, monkeypatch, bot):
         # For now just test that our internals pass the correct data
@@ -1910,13 +1883,6 @@ class TestBot:
         monkeypatch.setattr(bot.request, "post", make_assertion)
         assert await bot.answer_pre_checkout_query(1, False, error_message="Not enough fish")
 
-    async def test_answer_pre_checkout_query_errors(self, monkeypatch, bot):
-        with pytest.raises(TelegramError, match="should not be"):
-            await bot.answer_pre_checkout_query(1, True, error_message="Not enough fish")
-
-        with pytest.raises(TelegramError, match="should not be empty"):
-            await bot.answer_pre_checkout_query(1, False)
-
     @flaky(3, 1)
     async def test_restrict_chat_member(self, bot, channel_id, chat_permissions):
         # TODO: Add bot to supergroup so this can be tested properly
@@ -2007,16 +1973,6 @@ class TestBot:
         assert isinstance(invite_link, str)
         assert invite_link != ""
 
-    async def test_create_edit_invite_link_mutually_exclusive_arguments(self, bot, channel_id):
-        data = {"chat_id": channel_id, "member_limit": 17, "creates_join_request": True}
-
-        with pytest.raises(ValueError, match="`member_limit` can't be specified"):
-            await bot.create_chat_invite_link(**data)
-
-        data.update({"invite_link": "https://invite.link"})
-        with pytest.raises(ValueError, match="`member_limit` can't be specified"):
-            await bot.edit_chat_invite_link(**data)
-
     @flaky(3, 1)
     async def test_edit_revoke_chat_invite_link_passing_link_objects(self, bot, channel_id):
         invite_link = await bot.create_chat_invite_link(chat_id=channel_id)

tests/test_contact.py

@@ -127,6 +127,15 @@ class TestContact:
         with pytest.raises(ValueError, match="Either contact or phone_number and first_name"):
             await bot.send_contact(chat_id=chat_id)
 
+    async def test_send_mutually_exclusive(self, bot, chat_id, contact):
+        with pytest.raises(ValueError, match="Not both"):
+            await bot.send_contact(
+                chat_id=chat_id,
+                contact=contact,
+                phone_number=contact.phone_number,
+                first_name=contact.first_name,
+            )
+
     def test_to_dict(self, contact):
         contact_dict = contact.to_dict()
 

tests/test_venue.py

@@ -135,6 +135,17 @@ class TestVenue:
         with pytest.raises(ValueError, match="Either venue or latitude, longitude, address and"):
             await bot.send_venue(chat_id=chat_id)
 
+    async def test_send_venue_mutually_exclusive(self, bot, chat_id, venue):
+        with pytest.raises(ValueError, match="Not both"):
+            await bot.send_venue(
+                chat_id=chat_id,
+                latitude=1,
+                longitude=1,
+                address="address",
+                title="title",
+                venue=venue,
+            )
+
     def test_to_dict(self, venue):
         venue_dict = venue.to_dict()