From 505ef579eb2d0300010163a872ddb8736a162d71 Mon Sep 17 00:00:00 2001 From: Samuel Hawksby-Robinson Date: Tue, 16 Aug 2022 11:44:46 +0100 Subject: [PATCH] Refactor to urls to try to remove http client use in tests shelfing it at the moment, if it becomes a problem in the future then may be we can address it then. For the moment I've just left the refactoring I already did. --- protocol/urls/urls.go | 115 ++++++++++++------------------------- protocol/urls/urls_test.go | 76 ++++++++++++------------ 2 files changed, 78 insertions(+), 113 deletions(-) diff --git a/protocol/urls/urls.go b/protocol/urls/urls.go index c39561244..6555842ac 100644 --- a/protocol/urls/urls.go +++ b/protocol/urls/urls.go @@ -48,52 +48,54 @@ type Site struct { ImageSite bool `json:"imageSite"` } -const YoutubeOembedLink = "https://www.youtube.com/oembed?format=json&url=%s" -const TwitterOembedLink = "https://publish.twitter.com/oembed?url=%s" -const GiphyOembedLink = "https://giphy.com/services/oembed?url=%s" +const ( + YoutubeOembedLink = "https://www.youtube.com/oembed?format=json&url=%s" + TwitterOembedLink = "https://publish.twitter.com/oembed?url=%s" + GiphyOembedLink = "https://giphy.com/services/oembed?url=%s" +) -var httpClient = http.Client{ - Timeout: 30 * time.Second, -} +var ( + httpClient = http.Client{Timeout: 30 * time.Second} +) func LinkPreviewWhitelist() []Site { return []Site{ - Site{ + { Title: "Status", Address: "our.status.im", ImageSite: false, }, - Site{ + { Title: "YouTube", Address: "youtube.com", ImageSite: false, }, - Site{ + { Title: "YouTube shortener", Address: "youtu.be", ImageSite: false, }, - Site{ + { Title: "Twitter", Address: "twitter.com", ImageSite: false, }, - Site{ + { Title: "GIPHY GIFs shortener", Address: "gph.is", ImageSite: true, }, - Site{ + { Title: "GIPHY GIFs", Address: "giphy.com", ImageSite: true, }, - Site{ + { Title: "GIPHY GIFs subdomain", Address: "media.giphy.com", ImageSite: true, }, - Site{ + { Title: "GitHub", Address: "github.com", ImageSite: false, @@ -110,7 +112,7 @@ func LinkPreviewWhitelist() []Site { }, // Medium unfurling is failing - https://github.com/status-im/status-go/issues/2192 // - // Site{ + // { // Title: "Medium", // Address: "medium.com", // ImageSite: false, @@ -118,7 +120,7 @@ func LinkPreviewWhitelist() []Site { } } -func GetURLContent(url string) (data []byte, err error) { +func getURLContent(url string) (data []byte, err error) { response, err := httpClient.Get(url) if err != nil { return data, fmt.Errorf("can't get content from link %s", url) @@ -127,64 +129,44 @@ func GetURLContent(url string) (data []byte, err error) { return ioutil.ReadAll(response.Body) } -func GetYoutubeOembed(url string) (data YoutubeOembedData, err error) { - oembedLink := fmt.Sprintf(YoutubeOembedLink, url) +func GetOembed(name, endpoint, url string, data interface{}) error { + oembedLink := fmt.Sprintf(endpoint, url) - jsonBytes, err := GetURLContent(oembedLink) + jsonBytes, err := getURLContent(oembedLink) if err != nil { - return data, fmt.Errorf("can't get bytes from youtube oembed response on %s link", oembedLink) + return fmt.Errorf("can't get bytes from %s oembed response on %s link", name, oembedLink) } - err = json.Unmarshal(jsonBytes, &data) - if err != nil { - return data, fmt.Errorf("can't unmarshall json %w", err) - } - - return data, nil + return json.Unmarshal(jsonBytes, &data) } func GetYoutubePreviewData(link string) (previewData LinkPreviewData, err error) { - oembedData, err := GetYoutubeOembed(link) + oembedData := new(YoutubeOembedData) + err = GetOembed("Youtube", YoutubeOembedLink, link, &oembedData) if err != nil { - return previewData, err + return } previewData.Title = oembedData.Title previewData.Site = oembedData.ProviderName previewData.ThumbnailURL = oembedData.ThumbnailURL - - return previewData, nil -} - -func GetTwitterOembed(url string) (data TwitterOembedData, err error) { - oembedLink := fmt.Sprintf(TwitterOembedLink, url) - jsonBytes, err := GetURLContent(oembedLink) - if err != nil { - return data, fmt.Errorf("can't get bytes from twitter oembed response on %s link", oembedLink) - } - - err = json.Unmarshal(jsonBytes, &data) - if err != nil { - return data, fmt.Errorf("can't unmarshall json %w", err) - } - - return data, nil + return } func GetTwitterPreviewData(link string) (previewData LinkPreviewData, err error) { - oembedData, err := GetTwitterOembed(link) + oembedData := new(TwitterOembedData) + err = GetOembed("Twitter", TwitterOembedLink, link, oembedData) if err != nil { return previewData, err } - previewData.Title = GetReadableTextFromTweetHTML(oembedData.HTML) + previewData.Title = getReadableTextFromTweetHTML(oembedData.HTML) previewData.Site = oembedData.ProviderName return previewData, nil } -func GetReadableTextFromTweetHTML(s string) string { - +func getReadableTextFromTweetHTML(s string) string { s = strings.ReplaceAll(s, "\u003Cbr\u003E", "\n") // Adds line break for all
s = strings.ReplaceAll(s, "https://", "\nhttps://") // Displays links in next line s = html.UnescapeString(s) // Parses html special characters like á @@ -197,9 +179,7 @@ func GetReadableTextFromTweetHTML(s string) string { } func GetGenericLinkPreviewData(link string) (previewData LinkPreviewData, err error) { - // nolint: gosec res, err := httpClient.Get(link) - if err != nil { return previewData, fmt.Errorf("can't get content from link %s", link) } @@ -213,34 +193,18 @@ func GetGenericLinkPreviewData(link string) (previewData LinkPreviewData, err er } func GetGenericImageLinkPreviewData(title string, link string) (previewData LinkPreviewData, err error) { - url, _ := url.Parse(link) + u, _ := url.Parse(link) previewData.Title = title - previewData.Site = strings.ToLower(url.Hostname()) + previewData.Site = strings.ToLower(u.Hostname()) previewData.ThumbnailURL = link previewData.Height = 0 previewData.Width = 0 return previewData, nil } -func GetGiphyOembed(url string) (data GiphyOembedData, err error) { - oembedLink := fmt.Sprintf(GiphyOembedLink, url) - - jsonBytes, err := GetURLContent(oembedLink) - - if err != nil { - return data, fmt.Errorf("can't get bytes from Giphy oembed response at %s", oembedLink) - } - - err = json.Unmarshal(jsonBytes, &data) - if err != nil { - return data, fmt.Errorf("can't unmarshall json %w", err) - } - - return data, nil -} - func GetGiphyPreviewData(link string) (previewData LinkPreviewData, err error) { - oembedData, err := GetGiphyOembed(link) + oembedData := new(GiphyOembedData) + err = GetOembed("Giphy", GiphyOembedLink, link, oembedData) if err != nil { return previewData, err } @@ -257,9 +221,7 @@ func GetGiphyPreviewData(link string) (previewData LinkPreviewData, err error) { // Giphy has a shortener service called gph.is, the oembed service doesn't work with shortened urls, // so we need to fetch the long url first func GetGiphyLongURL(shortURL string) (longURL string, err error) { - // nolint: gosec res, err := http.Get(shortURL) - if err != nil { return longURL, fmt.Errorf("can't get bytes from Giphy's short url at %s", shortURL) } @@ -275,7 +237,6 @@ func GetGiphyLongURL(shortURL string) (longURL string, err error) { func GetGiphyShortURLPreviewData(shortURL string) (data LinkPreviewData, err error) { longURL, err := GetGiphyLongURL(shortURL) - if err != nil { return data, err } @@ -284,12 +245,12 @@ func GetGiphyShortURLPreviewData(shortURL string) (data LinkPreviewData, err err } func GetLinkPreviewData(link string) (previewData LinkPreviewData, err error) { - url, err := url.Parse(link) + u, err := url.Parse(link) if err != nil { return previewData, fmt.Errorf("cant't parse link %s", link) } - hostname := strings.ToLower(url.Hostname()) + hostname := strings.ToLower(u.Hostname()) switch hostname { case "youtube.com", "youtu.be", "www.youtube.com": @@ -305,6 +266,6 @@ func GetLinkPreviewData(link string) (previewData LinkPreviewData, err error) { case "media.tenor.com", "tenor.com": return GetGenericImageLinkPreviewData("Tenor", link) default: - return previewData, fmt.Errorf("link %s isn't whitelisted. Hostname - %s", link, url.Hostname()) + return previewData, fmt.Errorf("link %s isn't whitelisted. Hostname - %s", link, u.Hostname()) } } diff --git a/protocol/urls/urls_test.go b/protocol/urls/urls_test.go index 3dcbe711a..297b6edaa 100644 --- a/protocol/urls/urls_test.go +++ b/protocol/urls/urls_test.go @@ -8,28 +8,33 @@ import ( ) func TestGetLinkPreviewData(t *testing.T) { - statusTownhall := LinkPreviewData{ Site: "YouTube", Title: "Status Town Hall #67 - 12 October 2020", ThumbnailURL: "https://i.ytimg.com/vi/mzOyYtfXkb0/hqdefault.jpg", } - previewData, err := GetLinkPreviewData("https://www.youtube.com/watch?v=mzOyYtfXkb0") - require.NoError(t, err) - require.Equal(t, statusTownhall.Site, previewData.Site) - require.Equal(t, statusTownhall.Title, previewData.Title) - require.Equal(t, statusTownhall.ThumbnailURL, previewData.ThumbnailURL) + ts := []struct { + URL string + ShouldFail bool + }{ + {"https://www.youtube.com/watch?v=mzOyYtfXkb0", false}, + {"https://youtu.be/mzOyYtfXkb0", false}, + {"https://www.test.com/unknown", true}, + } - previewData, err = GetLinkPreviewData("https://youtu.be/mzOyYtfXkb0") - require.NoError(t, err) - require.Equal(t, statusTownhall.Site, previewData.Site) - require.Equal(t, statusTownhall.Title, previewData.Title) - require.Equal(t, statusTownhall.ThumbnailURL, previewData.ThumbnailURL) - - _, err = GetLinkPreviewData("https://www.test.com/unknown") - require.Error(t, err) + for _, u := range ts { + previewData, err := GetLinkPreviewData(u.URL) + if u.ShouldFail { + require.Error(t, err) + continue + } + require.NoError(t, err) + require.Equal(t, statusTownhall.Site, previewData.Site) + require.Equal(t, statusTownhall.Title, previewData.Title) + require.Equal(t, statusTownhall.ThumbnailURL, previewData.ThumbnailURL) + } } // split at "." and ignore the first item @@ -130,43 +135,42 @@ func TestStatusLinkPreviewData(t *testing.T) { // } func TestTwitterLinkPreviewData(t *testing.T) { - statusTweet1 := LinkPreviewData{ Site: "Twitter", Title: "Crypto isn't going anywhere.— Status (@ethstatus) July 26, 2021", } - - previewData1, err := GetLinkPreviewData("https://twitter.com/ethstatus/status/1419674733885407236") - require.NoError(t, err) - require.Equal(t, statusTweet1.Site, previewData1.Site) - require.Equal(t, statusTweet1.Title, previewData1.Title) - require.Equal(t, statusTweet1.ThumbnailURL, "") - statusTweet2 := LinkPreviewData{ Site: "Twitter", Title: "🎉 Status v1.15 is a go! 🎉\n\n📌 Pin important messages in chats and groups" + "\n✏️ Edit messages after sending\n🔬 Scan QR codes with the browser\n⚡️ FASTER app navigation!" + "\nhttps://t.co/qKrhDArVKb— Status (@ethstatus) July 27, 2021", } - - previewData2, err := GetLinkPreviewData("https://twitter.com/ethstatus/status/1420035091997278214") - require.NoError(t, err) - require.Equal(t, statusTweet2.Site, previewData2.Site) - require.Equal(t, statusTweet2.Title, previewData2.Title) - require.Equal(t, statusTweet2.ThumbnailURL, "") - statusProfile := LinkPreviewData{ Site: "Twitter", Title: "Tweets by ethstatus", } - previewData3, err := GetLinkPreviewData("https://twitter.com/ethstatus") - require.NoError(t, err) - require.Equal(t, statusProfile.Site, previewData3.Site) - require.Equal(t, statusProfile.Title, previewData3.Title) - require.Equal(t, statusProfile.ThumbnailURL, "") + ts := []struct { + URL string + Expected LinkPreviewData + ShouldFail bool + }{ + {"https://twitter.com/ethstatus/status/1419674733885407236", statusTweet1, false}, + {"https://twitter.com/ethstatus/status/1420035091997278214", statusTweet2, false}, + {"https://twitter.com/ethstatus", statusProfile, false}, + {"https://www.test.com/unknown", LinkPreviewData{}, true}, + } - _, err = GetLinkPreviewData("https://www.test.com/unknown") - require.Error(t, err) + for _, u := range ts { + previewData, err := GetLinkPreviewData(u.URL) + if u.ShouldFail { + require.Error(t, err) + continue + } + require.NoError(t, err) + require.Equal(t, u.Expected.Site, previewData.Site) + require.Equal(t, u.Expected.Title, previewData.Title) + require.Equal(t, u.Expected.ThumbnailURL, previewData.ThumbnailURL) + } }