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

@@ -48,52 +48,54 @@ type Site struct {
 	ImageSite bool   `json:"imageSite"`
 }
 
-const YoutubeOembedLink = "https://www.youtube.com/oembed?format=json&url=%s"
+const (
-const TwitterOembedLink = "https://publish.twitter.com/oembed?url=%s"
+	YoutubeOembedLink = "https://www.youtube.com/oembed?format=json&url=%s"
-const GiphyOembedLink = "https://giphy.com/services/oembed?url=%s"
+	TwitterOembedLink = "https://publish.twitter.com/oembed?url=%s"
+	GiphyOembedLink   = "https://giphy.com/services/oembed?url=%s"
+)
 
-var httpClient = http.Client{
+var (
-	Timeout: 30 * time.Second,
+	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) {
+func GetOembed(name, endpoint, url string, data interface{}) error {
-	oembedLink := fmt.Sprintf(YoutubeOembedLink, url)
+	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)
+	return json.Unmarshal(jsonBytes, &data)
-	if err != nil {
-		return data, fmt.Errorf("can't unmarshall json %w", err)
-	}
-
-	return data, nil
 }
 
 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
-	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
 }
 
 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 <br>
 	s = strings.ReplaceAll(s, "https://", "\nhttps://") // Displays links in next line
 	s = html.UnescapeString(s)                          // Parses html special characters like &#225;
@@ -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())
 	}
 }

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")
+	ts := []struct {
-	require.NoError(t, err)
+		URL        string
-	require.Equal(t, statusTownhall.Site, previewData.Site)
+		ShouldFail bool
-	require.Equal(t, statusTownhall.Title, previewData.Title)
+	}{
-	require.Equal(t, statusTownhall.ThumbnailURL, previewData.ThumbnailURL)
+		{"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")
+	for _, u := range ts {
-	require.NoError(t, err)
+		previewData, err := GetLinkPreviewData(u.URL)
-	require.Equal(t, statusTownhall.Site, previewData.Site)
+		if u.ShouldFail {
-	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)
+			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")
+	ts := []struct {
-	require.NoError(t, err)
+		URL        string
-	require.Equal(t, statusProfile.Site, previewData3.Site)
+		Expected   LinkPreviewData
-	require.Equal(t, statusProfile.Title, previewData3.Title)
+		ShouldFail bool
-	require.Equal(t, statusProfile.ThumbnailURL, "")
+	}{
-
+		{"https://twitter.com/ethstatus/status/1419674733885407236", statusTweet1, false},
-	_, err = GetLinkPreviewData("https://www.test.com/unknown")
+		{"https://twitter.com/ethstatus/status/1420035091997278214", statusTweet2, false},
-	require.Error(t, err)
+		{"https://twitter.com/ethstatus", statusProfile, false},
-
+		{"https://www.test.com/unknown", LinkPreviewData{}, true},
+	}
+
+	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)
+	}
 }