diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py index 10026af350..1c33f77b87 100644 --- a/zerver/lib/markdown/__init__.py +++ b/zerver/lib/markdown/__init__.py @@ -65,7 +65,7 @@ from zerver.lib.outgoing_http import OutgoingSession from zerver.lib.subdomains import is_static_or_current_realm_url from zerver.lib.tex import render_tex from zerver.lib.thumbnail import user_uploads_or_external -from zerver.lib.timeout import TimeoutExpiredError, timeout +from zerver.lib.timeout import timeout from zerver.lib.timezone import common_timezones from zerver.lib.types import LinkifierDict from zerver.lib.url_encoding import encode_stream, hash_util_encode @@ -420,68 +420,13 @@ def has_blockquote_ancestor(element_pair: Optional[ElementPair]) -> bool: @cache_with_key(lambda tweet_id: tweet_id, cache_name="database") def fetch_tweet_data(tweet_id: str) -> Optional[Dict[str, Any]]: - if settings.TEST_SUITE: - from . import testing_mocks - - res = testing_mocks.twitter(tweet_id) - else: - creds = { - "consumer_key": settings.TWITTER_CONSUMER_KEY, - "consumer_secret": settings.TWITTER_CONSUMER_SECRET, - "access_token_key": settings.TWITTER_ACCESS_TOKEN_KEY, - "access_token_secret": settings.TWITTER_ACCESS_TOKEN_SECRET, - } - if not all(creds.values()): - return None - - # We lazily import twitter here because its import process is - # surprisingly slow, and doing so has a significant impact on - # the startup performance of `manage.py` commands. - import twitter - - api = twitter.Api(tweet_mode="extended", **creds) - - try: - # Sometimes Twitter hangs on responses. Timing out here - # will cause the Tweet to go through as-is with no inline - # preview, rather than having the message be rejected - # entirely. This timeout needs to be less than our overall - # formatting timeout. - tweet = timeout(3, lambda: api.GetStatus(tweet_id)) - res = tweet.AsDict() - except TimeoutExpiredError: - # We'd like to try again later and not cache the bad result, - # so we need to re-raise the exception (just as though - # we were being rate-limited) - raise - except twitter.TwitterError as e: - t = e.args[0] - if len(t) == 1 and ("code" in t[0]): - # https://developer.twitter.com/en/docs/basics/response-codes - code = t[0]["code"] - if code in [34, 144, 421, 422]: - # All these "correspond with HTTP 404," and mean - # that the message doesn't exist; return None so - # that we will cache the error. - return None - elif code in [63, 179]: - # 63 is that the account is suspended, 179 is that - # it is now locked; cache the None. - return None - elif code in [88, 130, 131]: - # Code 88 means that we were rate-limited, 130 - # means Twitter is having capacity issues, and 131 - # is other 400-equivalent; in these cases, raise - # the error so we don't cache None and will try - # again later. - raise - # It's not clear what to do in cases of other errors, - # but for now it seems reasonable to log at error - # level (so that we get notified), but then cache the - # failure to proceed with our usual work - markdown_logger.exception("Unknown error fetching tweet data", stack_info=True) - return None - return res + # Twitter removed support for the v1 API that this integration + # used. Given that, there's no point wasting time trying to make + # network requests to Twitter. But we leave this function, because + # existing cached renderings for Tweets is useful. We throw an + # exception rather than returning `None` to avoid caching that the + # link doesn't exist. + raise NotImplementedError("Twitter desupported their v1 API") class OpenGraphSession(OutgoingSession): @@ -1049,6 +994,8 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor): img.set("src", media_url) return tweet + except NotImplementedError: + return None except Exception: # We put this in its own try-except because it requires external # connectivity. If Twitter flakes out, we don't want to not-render diff --git a/zerver/lib/markdown/testing_mocks.py b/zerver/lib/markdown/testing_mocks.py deleted file mode 100644 index 448e64fa7a..0000000000 --- a/zerver/lib/markdown/testing_mocks.py +++ /dev/null @@ -1,233 +0,0 @@ -from typing import Any, Dict, Optional - -import orjson - -NORMAL_TWEET = """{ - "created_at": "Sat Sep 10 22:23:38 +0000 2011", - "favorite_count": 1, - "full_text": "@twitter meets @seepicturely at #tcdisrupt cc.@boscomonkey @episod http://t.co/6J2EgYM", - "hashtags": [ - { - "text": "tcdisrupt" - } - ], - "id": 112652479837110270, - "id_str": "112652479837110273", - "in_reply_to_screen_name": "Twitter", - "in_reply_to_user_id": 783214, - "lang": "en", - "retweet_count": 4, - "source": "Instagram", - "urls": [ - { - "expanded_url": "http://instagr.am/p/MuW67/", - "url": "http://t.co/6J2EgYM" - } - ], - "user": { - "created_at": "Mon May 16 20:07:59 +0000 2011", - "description": "Eoin's photography account. See @mceoin for tweets.", - "followers_count": 3, - "id": 299862462, - "lang": "en", - "location": "Twitter", - "name": "Eoin McMillan", - "profile_background_color": "131516", - "profile_background_image_url": "http://abs.twimg.com/images/themes/theme14/bg.gif", - "profile_background_tile": true, - "profile_image_url": "http://pbs.twimg.com/profile_images/1380912173/Screen_shot_2011-06-03_at_7.35.36_PM_normal.png", - "profile_link_color": "009999", - "profile_sidebar_fill_color": "EFEFEF", - "profile_text_color": "333333", - "screen_name": "imeoin", - "statuses_count": 278, - "url": "http://t.co/p9hKpiGMyN" - }, - "user_mentions": [ - { - "id": 783214, - "name": "Twitter", - "screen_name": "Twitter" - }, - { - "id": 14792670, - "name": "Bosco So", - "screen_name": "boscomonkey" - }, - { - "id": 819797, - "name": "Taylor Singletary", - "screen_name": "episod" - } - ] -}""" - -MENTION_IN_LINK_TWEET = """{ - "created_at": "Sat Sep 10 22:23:38 +0000 2011", - "favorite_count": 1, - "full_text": "http://t.co/@foo", - "hashtags": [ - { - "text": "tcdisrupt" - } - ], - "id": 112652479837110270, - "id_str": "112652479837110273", - "in_reply_to_screen_name": "Twitter", - "in_reply_to_user_id": 783214, - "lang": "en", - "retweet_count": 4, - "source": "Instagram", - "urls": [ - { - "expanded_url": "http://foo.com", - "url": "http://t.co/@foo" - } - ], - "user": { - "created_at": "Mon May 16 20:07:59 +0000 2011", - "description": "Eoin's photography account. See @mceoin for tweets.", - "followers_count": 3, - "id": 299862462, - "lang": "en", - "location": "Twitter", - "name": "Eoin McMillan", - "profile_background_color": "131516", - "profile_background_image_url": "http://abs.twimg.com/images/themes/theme14/bg.gif", - "profile_background_tile": true, - "profile_image_url": "http://pbs.twimg.com/profile_images/1380912173/Screen_shot_2011-06-03_at_7.35.36_PM_normal.png", - "profile_link_color": "009999", - "profile_sidebar_fill_color": "EFEFEF", - "profile_text_color": "333333", - "screen_name": "imeoin", - "statuses_count": 278, - "url": "http://t.co/p9hKpiGMyN" - }, - "user_mentions": [ - { - "id": 783214, - "name": "Foo", - "screen_name": "foo" - } - ] -}""" - -MEDIA_TWEET = """{ - "created_at": "Sat Sep 10 22:23:38 +0000 2011", - "favorite_count": 1, - "full_text": "http://t.co/xo7pAhK6n3", - "id": 112652479837110270, - "id_str": "112652479837110273", - "in_reply_to_screen_name": "Twitter", - "in_reply_to_user_id": 783214, - "lang": "en", - "media": [ - { - "display_url": "pic.twitter.com/xo7pAhK6n3", - "expanded_url": "http://twitter.com/NEVNBoston/status/421654515616849920/photo/1", - "id": 421654515495211010, - "media_url": "http://pbs.twimg.com/media/BdoEjD4IEAIq86Z.jpg", - "media_url_https": "https://pbs.twimg.com/media/BdoEjD4IEAIq86Z.jpg", - "sizes": {"large": {"h": 700, "resize": "fit", "w": 1024}, - "medium": {"h": 410, "resize": "fit", "w": 599}, - "small": {"h": 232, "resize": "fit", "w": 340}, - "thumb": {"h": 150, "resize": "crop", "w": 150}}, - "type": "photo", - "url": "http://t.co/xo7pAhK6n3"} - ], - "retweet_count": 4, - "source": "Instagram", - "user": { - "created_at": "Mon May 16 20:07:59 +0000 2011", - "description": "Eoin's photography account. See @mceoin for tweets.", - "followers_count": 3, - "id": 299862462, - "lang": "en", - "location": "Twitter", - "name": "Eoin McMillan", - "profile_background_color": "131516", - "profile_background_image_url": "http://abs.twimg.com/images/themes/theme14/bg.gif", - "profile_background_tile": true, - "profile_image_url": "http://pbs.twimg.com/profile_images/1380912173/Screen_shot_2011-06-03_at_7.35.36_PM_normal.png", - "profile_link_color": "009999", - "profile_sidebar_fill_color": "EFEFEF", - "profile_text_color": "333333", - "screen_name": "imeoin", - "statuses_count": 278, - "url": "http://t.co/p9hKpiGMyN" - }, - "user_mentions": [ - { - "id": 783214, - "name": "Foo", - "screen_name": "foo" - } - ] -}""" - -EMOJI_TWEET = """{ - "created_at": "Sat Sep 10 22:23:38 +0000 2011", - "favorite_count": 1, - "full_text": "Zulip is 💯% open-source!", - "hashtags": [ - { - "text": "tcdisrupt" - } - ], - "id": 112652479837110270, - "id_str": "112652479837110273", - "in_reply_to_screen_name": "Twitter", - "in_reply_to_user_id": 783214, - "lang": "en", - "retweet_count": 4, - "source": "Instagram", - "user": { - "created_at": "Mon May 16 20:07:59 +0000 2011", - "description": "Eoin's photography account. See @mceoin for tweets.", - "followers_count": 3, - "id": 299862462, - "lang": "en", - "location": "Twitter", - "name": "Eoin McMillan", - "profile_background_color": "131516", - "profile_background_image_url": "http://abs.twimg.com/images/themes/theme14/bg.gif", - "profile_background_tile": true, - "profile_image_url": "http://pbs.twimg.com/profile_images/1380912173/Screen_shot_2011-06-03_at_7.35.36_PM_normal.png", - "profile_link_color": "009999", - "profile_sidebar_fill_color": "EFEFEF", - "profile_text_color": "333333", - "screen_name": "imeoin", - "statuses_count": 278, - "url": "http://t.co/p9hKpiGMyN" - }, - "user_mentions": [ - { - "id": 783214, - "name": "Twitter", - "screen_name": "Twitter" - }, - { - "id": 14792670, - "name": "Bosco So", - "screen_name": "boscomonkey" - }, - { - "id": 819797, - "name": "Taylor Singletary", - "screen_name": "episod" - } - ] -}""" - - -def twitter(tweet_id: str) -> Optional[Dict[str, Any]]: - if tweet_id in ["112652479837110273", "287977969287315456", "287977969287315457"]: - return orjson.loads(NORMAL_TWEET) - elif tweet_id == "287977969287315458": - return orjson.loads(MENTION_IN_LINK_TWEET) - elif tweet_id == "287977969287315459": - return orjson.loads(MEDIA_TWEET) - elif tweet_id == "287977969287315460": - return orjson.loads(EMOJI_TWEET) - else: - return None diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index e3884bc537..0a5344de9a 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -950,258 +950,10 @@ class MarkdownTest(ZulipTestCase): "410766290349879296", ) - def test_inline_interesting_links(self) -> None: - def make_link(url: str) -> str: - return f'{url}' - - normal_tweet_html = ( - '@Twitter " - "meets @seepicturely at #tcdisrupt cc." - '@boscomonkey " - '@episod " - 'http://instagr.am/p/MuW67/" - ) - - mention_in_link_tweet_html = """http://foo.com""" - - media_tweet_html = ( - '' - "http://twitter.com/NEVNBoston/status/421654515616849920/photo/1" - ) - - emoji_in_tweet_html = """Zulip is :100:% open-source!""" - - def make_inline_twitter_preview(url: str, tweet_html: str, image_html: str = "") -> str: - ## As of right now, all previews are mocked to be the exact same tweet - return ( - '
' - '
' - f'' - '' - "" - f"

{tweet_html}

" - "- Eoin McMillan (@imeoin)" - f"{image_html}" - "
" - "
" - ) - - msg = "http://www.twitter.com" - converted = markdown_convert_wrapper(msg) - self.assertEqual(converted, "

{}

".format(make_link("http://www.twitter.com"))) - - msg = "http://www.twitter.com/wdaher/" - converted = markdown_convert_wrapper(msg) - self.assertEqual(converted, "

{}

".format(make_link("http://www.twitter.com/wdaher/"))) - - msg = "http://www.twitter.com/wdaher/status/3" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, "

{}

".format(make_link("http://www.twitter.com/wdaher/status/3")) - ) - - # id too long - msg = "http://www.twitter.com/wdaher/status/2879779692873154569" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

".format( - make_link("http://www.twitter.com/wdaher/status/2879779692873154569") - ), - ) - - # id too large (i.e. tweet doesn't exist) - msg = "http://www.twitter.com/wdaher/status/999999999999999999" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

".format( - make_link("http://www.twitter.com/wdaher/status/999999999999999999") - ), - ) - - msg = "http://www.twitter.com/wdaher/status/287977969287315456" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("http://www.twitter.com/wdaher/status/287977969287315456"), - make_inline_twitter_preview( - "http://www.twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - ), - ) - - msg = "https://www.twitter.com/wdaher/status/287977969287315456" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("https://www.twitter.com/wdaher/status/287977969287315456"), - make_inline_twitter_preview( - "https://www.twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - ), - ) - - msg = "http://twitter.com/wdaher/status/287977969287315456" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315456"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - ), - ) - - # Repeated links will only be converted once - msg = ( - "http://twitter.com/wdaher/status/287977969287315456 " - "http://twitter.com/wdaher/status/287977969287315457 " - "http://twitter.com/wdaher/status/287977969287315457 " - "http://twitter.com/wdaher/status/287977969287315457" - ) - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{} {} {} {}

\n{}{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315456"), - make_link("http://twitter.com/wdaher/status/287977969287315457"), - make_link("http://twitter.com/wdaher/status/287977969287315457"), - make_link("http://twitter.com/wdaher/status/287977969287315457"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315457", normal_tweet_html - ), - ), - ) - - # A max of 3 will be converted - msg = ( - "http://twitter.com/wdaher/status/287977969287315456 " - "http://twitter.com/wdaher/status/287977969287315457 " - "https://twitter.com/wdaher/status/287977969287315456 " - "http://twitter.com/wdaher/status/287977969287315460" - ) - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{} {} {} {}

\n{}{}{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315456"), - make_link("http://twitter.com/wdaher/status/287977969287315457"), - make_link("https://twitter.com/wdaher/status/287977969287315456"), - make_link("http://twitter.com/wdaher/status/287977969287315460"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315457", normal_tweet_html - ), - make_inline_twitter_preview( - "https://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - ), - ) - - # Test smart in-place inlining behavior: - msg = ( - "Paragraph 1: http://twitter.com/wdaher/status/287977969287315456\n\n" - "Paragraph 2\n\n" - "Paragraph 3: http://twitter.com/wdaher/status/287977969287315457" - ) - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

Paragraph 1: {}

\n{}

Paragraph 2

\n

Paragraph 3: {}

\n{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315456"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - make_link("http://twitter.com/wdaher/status/287977969287315457"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315457", normal_tweet_html - ), - ), - ) - - # Tweet has a mention in a URL, only the URL is linked - msg = "http://twitter.com/wdaher/status/287977969287315458" - - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315458"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315458", - mention_in_link_tweet_html, - ), - ), - ) - - # Tweet with an image - msg = "http://twitter.com/wdaher/status/287977969287315459" - - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315459"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315459", - media_tweet_html, - ( - '
' - '' - f"""""" - "" - "
" - ), - ), - ), - ) - - msg = "http://twitter.com/wdaher/status/287977969287315460" - converted = markdown_convert_wrapper(msg) - self.assertEqual( - converted, - "

{}

\n{}".format( - make_link("http://twitter.com/wdaher/status/287977969287315460"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315460", emoji_in_tweet_html - ), - ), - ) - - # Test Twitter previews in spoiler tags. - msg = "```spoiler secret tweet\nTweet: http://twitter.com/wdaher/status/287977969287315456\n```" - converted = markdown_convert_wrapper(msg) - - rendered_spoiler = '
\n

secret tweet

\n
' - self.assertEqual( - converted, - rendered_spoiler.format( - make_link("http://twitter.com/wdaher/status/287977969287315456"), - make_inline_twitter_preview( - "http://twitter.com/wdaher/status/287977969287315456", normal_tweet_html - ), - ), - ) - def test_fetch_tweet_data_settings_validation(self) -> None: with self.settings(TEST_SUITE=False, TWITTER_CONSUMER_KEY=None): - self.assertIs(None, fetch_tweet_data("287977969287315459")) + with self.assertRaises(NotImplementedError): + fetch_tweet_data("287977969287315459") def test_content_has_emoji(self) -> None: self.assertFalse(content_has_emoji_syntax("boring"))