.gitignore

@@ -29,5 +29,3 @@ frenzy-*.tar
 # secrets files as long as you replace their contents by environment
 # variables.
 /config/*.secret.exs
-
-.elixir_ls/

lib/frenzy/feed.ex

@@ -35,7 +35,6 @@ defmodule Frenzy.Feed do
     field :site_url, :string
     field :title, :string
     field :favicon, :string
-    field :favicon_url, :string
 
     belongs_to :group, Frenzy.Group
     belongs_to :pipeline, Frenzy.Pipeline
@@ -53,7 +52,6 @@ defmodule Frenzy.Feed do
           site_url: String.t() | nil,
           title: String.t() | nil,
           favicon: String.t() | nil,
-          favicon_url: String.t() | nil,
           group: Frenzy.Group.t() | Ecto.Association.NotLoaded.t(),
           pipeline: Frenzy.Pipeline.t() | nil | Ecto.Association.NotLoaded.t(),
           items: [Frenzy.Item.t()] | Ecto.Association.NotLoaded.t(),
@@ -70,8 +68,7 @@ defmodule Frenzy.Feed do
       :site_url,
       :last_updated,
       :pipeline_id,
-      :favicon,
+      :favicon
-      :favicon_url
     ])
     |> validate_required([:feed_url])
   end

lib/frenzy/task/fetch_favicon.ex

@@ -21,40 +21,44 @@ defmodule Frenzy.Task.FetchFavicon do
 
     Logger.debug("Fetching favicon for #{site_url}")
 
-    favicon_url = fetch_favicon_url_from_webpage(site_url) || URI.merge(site_url, "/favicon.ico")
+    case fetch_favicon_from_webpage(site_url) do
-
+      {:ok, favicon_data} ->
-    with %Feed{favicon_url: old_url} when old_url != favicon_url <- feed,
+        changeset = Feed.changeset(feed, %{favicon: favicon_data})
-         {:ok, favicon_data} <- fetch_favicon_data(favicon_url) do
-      IO.inspect(favicon_url)
-      changeset = Feed.changeset(feed, %{favicon: favicon_data, favicon_url: favicon_url})
         {:ok, _feed} = Repo.update(changeset)
-    else
-      _ ->
-        :ok
-    end
-  end
-
-  @spec fetch_favicon_url_from_webpage(url :: String.t()) :: String.t()
-
-  defp fetch_favicon_url_from_webpage(url) when is_binary(url) do
-    case HTTP.get(url) do
-      {:ok, %HTTPoison.Response{body: body, status_code: code}} when code in 200..299 ->
-        extract_favicon_url(url, body)
-
-      {:ok, %HTTPoison.Response{status_code: code}} ->
-        Logger.debug("Unhandled HTTP code #{code} for '#{url}'")
-        nil
 
       {:error, reason} ->
-        Logger.debug("Error fetching webpage for favicon: #{inspect(reason)}")
+        Logger.info("Couldn't fetch favicon for #{site_url}: #{reason}")
-        nil
+
+        favicon_uri =
+          %{URI.parse(site_url) | path: "/favicon.ico", query: nil, fragment: nil}
+          |> URI.to_string()
+
+        Logger.info("Trying default path: #{favicon_uri}")
+
+        case fetch_favicon_data(favicon_uri, site_url) do
+          {:ok, favicon_data} ->
+            changeset = Feed.changeset(feed, %{favicon: favicon_data})
+            {:ok, _feed} = Repo.update(changeset)
+
+          {:error, reason} ->
+            Logger.info("Couldn't fetch default /favicon.ico for #{site_url}: #{reason}")
+        end
     end
   end
 
-  defp fetch_favicon_url_from_webpage(_), do: {:error, "URL must be a string"}
+  defp fetch_favicon_from_webpage(url) when is_binary(url) do
+    case HTTP.get(url) do
+      {:ok, %HTTPoison.Response{body: body}} ->
+        extract_favicon(url, body)
 
-  @spec extract_favicon_url(page_url :: String.t(), body :: term()) :: String.t()
+      {:error, _reason} = err ->
-  defp extract_favicon_url(page_url, body) do
+        err
+    end
+  end
+
+  defp fetch_favicon_from_webpage(_), do: {:error, "URL must be a string"}
+
+  defp extract_favicon(page_url, body) do
     html_tree = Floki.parse(body)
 
     case Floki.find(html_tree, "link[rel=icon]") do
@@ -85,38 +89,34 @@ defmodule Frenzy.Task.FetchFavicon do
           nil ->
             {:error, "No link[rel=icon] with type of image/png"}
 
+          # todo: try requesting /favicon.ico
+
           link ->
             link
             |> Floki.attribute("href")
             |> List.first()
-            |> case do
+            |> fetch_favicon_data(page_url)
-              href when is_binary(href) ->
-                URI.merge(page_url, href) |> to_string()
-
-              _ ->
-                nil
-            end
         end
     end
   end
 
-  @spec fetch_favicon_data(favicon_url :: String.t()) :: {:ok, String.t()} | :error
+  defp fetch_favicon_data(favicon_url, site_url) when is_binary(favicon_url) do
-  defp fetch_favicon_data(favicon_url) do
+    # handle relative URIs, set default scheme if not provided
-    Logger.debug("Fetching favicon from: '#{favicon_url}'")
+    absolute_url =
+      favicon_url
+      |> URI.parse()
+      |> HTTP.resolve_uri(URI.parse(site_url))
 
-    case HTTP.get(favicon_url) do
+    case HTTP.get(absolute_url) do
-      {:ok, %HTTPoison.Response{body: body, status_code: code}} when code in 200..299 ->
+      {:ok, %HTTPoison.Response{body: body}} ->
         {:ok, "data:image/png;base64,#{Base.encode64(body)}"}
 
-      {:ok, %HTTPoison.Response{status_code: code}} ->
+      {:error, _reason} = err ->
-        Logger.debug("Unhandled HTTP code #{code} for '#{favicon_url}'")
+        err
-        :error
+    end
+  end
 
-      {:error, reason} ->
+  defp fetch_favicon_data(_, _), do: {:error, "No or invalid href for link"}
-        Logger.debug("Error fetching favicon: #{inspect(reason)}")
-        :error
-    end
-  end
 
   # from https://github.com/elixir-plug/plug/blob/v1.8.3/lib/plug/request_id.ex#L60
   defp generate_task_id() do

lib/frenzy/update_feeds.ex

@@ -87,9 +87,6 @@ defmodule Frenzy.UpdateFeeds do
         case FeedParser.parse(body, content_type) do
           {:ok, rss} ->
             update_feed_from_rss(feed, rss)
-
-          {:error, reason} ->
-            Logger.error("Unable to parse feed at '#{feed.feed_url}': #{inspect(reason)}")
         end
 
       {:ok, %HTTPoison.Response{status_code: 404}} ->

priv/repo/migrations/20200529233101_feeds_add_favicon_url.exs

@@ -1,9 +0,0 @@
-defmodule Frenzy.Repo.Migrations.FeedsAddFaviconUrl do
-  use Ecto.Migration
-
-  def change do
-    alter table(:feeds) do
-      add :favicon_url, :string, default: nil
-    end
-  end
-end