From dce4a3c98e75b867e4661a7907ecc785e3422057 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 29 May 2023 09:19:45 -0700 Subject: [PATCH] markdown: Remove most of Twitter integration. Twitter removed their v1 API. We take care to keep the existing cached results around for now, and to not poison that cache, since we might be able replace this with something that can still use the existing cache. --- zerver/lib/markdown/__init__.py | 73 ++------ zerver/lib/markdown/testing_mocks.py | 233 ------------------------- zerver/tests/test_markdown.py | 252 +-------------------------- 3 files changed, 12 insertions(+), 546 deletions(-) delete mode 100644 zerver/lib/markdown/testing_mocks.py 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 ( - '
' - '" - "
" - ) - - 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"))