Send favicons in Fever API

Fixes a bug where items without titles could not be marked as read

config/config.exs

@@ -20,7 +20,7 @@ config :frenzy, FrenzyWeb.Endpoint,
 # Configures Elixir's Logger
 config :logger, :console,
   format: "$time $metadata[$level] $message\n",
-  metadata: [:request_id, :item_task_id]
+  metadata: [:request_id, :item_task_id, :favicon_task_id]
 
 # Use Jason for JSON parsing in Phoenix
 config :phoenix, :json_library, Jason

lib/frenzy/feed.ex

@@ -34,6 +34,7 @@ defmodule Frenzy.Feed do
     field :last_updated, :utc_datetime
     field :site_url, :string
     field :title, :string
+    field :favicon, :string
 
     belongs_to :group, Frenzy.Group
     belongs_to :pipeline, Frenzy.Pipeline
@@ -50,6 +51,7 @@ defmodule Frenzy.Feed do
           last_updated: DateTime.t() | nil,
           site_url: String.t() | nil,
           title: String.t() | nil,
+          favicon: 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(),
@@ -65,7 +67,8 @@ defmodule Frenzy.Feed do
       :feed_url,
       :site_url,
       :last_updated,
-      :pipeline_id
+      :pipeline_id,
+      :favicon
     ])
     |> validate_required([:feed_url])
   end

lib/frenzy/http.ex

@@ -0,0 +1,34 @@
+defmodule Frenzy.HTTP do
+  require Logger
+  @redirect_codes [301, 302]
+
+  # @spec get(url :: String.t()) :: {:ok, HTTPoison.Response.t()} | {:error, String.()}
+  def get(url) do
+    case HTTPoison.get(url) do
+      {:ok, %HTTPoison.Response{status_code: 200} = response} ->
+        {:ok, response}
+
+      {:ok, %HTTPoison.Response{status_code: status_code, headers: headers}}
+      when status_code in @redirect_codes ->
+        headers
+        |> Enum.find(fn {name, _value} -> name == "Location" end)
+        |> case do
+          {"Location", new_url} ->
+            Logger.debug("Got 301 redirect from #{url} to #{new_url}")
+            get(new_url)
+
+          _ ->
+            {:error, "Missing Location header for redirect"}
+        end
+
+      {:ok, %HTTPoison.Response{status_code: 403}} ->
+        {:error, "403 Forbidden"}
+
+      {:ok, %HTTPoison.Response{status_code: 404}} ->
+        {:error, "404 Not Found"}
+
+      {:error, %HTTPoison.Error{reason: reason}} ->
+        {:error, reason}
+    end
+  end
+end

lib/frenzy/item.ex

@@ -59,7 +59,7 @@ defmodule Frenzy.Item do
           url: String.t(),
           read: boolean(),
           read_date: DateTime.t(),
-          title: String.t(),
+          title: String.t() | nil,
           tombstone: boolean(),
           feed: Frenzy.Feed.t() | Ecto.Association.NotLoaded.t(),
           inserted_at: NaiveDateTime.t(),
@@ -70,6 +70,6 @@ defmodule Frenzy.Item do
   def changeset(item, attrs) do
     item
     |> cast(attrs, [:guid, :title, :url, :creator, :date, :content, :read, :read_date, :tombstone])
-    |> validate_required([:guid, :title, :url, :date, :content, :feed])
+    |> validate_required([:guid, :url, :date, :content, :feed])
   end
 end

lib/frenzy/pipeline/scrape_stage.ex

@@ -1,5 +1,6 @@
 defmodule Frenzy.Pipeline.ScrapeStage do
   require Logger
+  alias Frenzy.HTTP
   alias Frenzy.Pipeline.Stage
   @behaviour Stage
 
@@ -65,13 +66,13 @@ defmodule Frenzy.Pipeline.ScrapeStage do
     Logger.debug("Getting article from #{url}")
 
     url
-    |> HTTPoison.get()
+    |> HTTP.get()
     |> case do
       {:ok, response} ->
         handle_response(url, response, opts)
 
-      {:error, %HTTPoison.Error{reason: reason}} ->
-        {:error, "HTTPoison error: #{reason}"}
+      {:error, reason} ->
+        {:error, "Couldn't scrape article: #{reason}"}
     end
   end
 
@@ -79,7 +80,7 @@ defmodule Frenzy.Pipeline.ScrapeStage do
 
   @spec handle_response(String.t(), HTTPoison.Response.t(), String.t()) ::
           {:ok, String.t()} | {:error, String.t()}
-  defp handle_response(url, %HTTPoison.Response{status_code: 200, body: body}, opts) do
+  defp handle_response(url, %HTTPoison.Response{body: body}, opts) do
     case opts["extractor"] do
       "builtin" ->
         {:ok, Readability.article(body)}
@@ -111,36 +112,7 @@ defmodule Frenzy.Pipeline.ScrapeStage do
     end
   end
 
-  defp handle_response(_url, %HTTPoison.Response{status_code: 404}, _extractor) do
-    {:error, "404 not found"}
-  end
-
-  defp handle_response(
-         url,
-         %HTTPoison.Response{status_code: status_code, headers: headers},
-         extractor
-       )
-       when status_code in [301, 302] do
-    headers
-    |> Enum.find(fn {name, _value} -> name == "Location" end)
-    |> case do
-      {"Location", new_url} ->
-        Logger.debug("Got 301 redirect from #{url} to #{new_url}")
-        get_article_content(new_url, extractor)
-
-      _ ->
-        {:error, "Missing Location header for redirect"}
-    end
-  end
-
-  defp handle_response(_url, %HTTPoison.Response{status_code: 403}, _extractor) do
-    {:error, "403 Forbidden"}
-  end
-
-  defp handle_response(_url, %HTTPoison.Response{} = response, _extractor) do
-    {:error, "No handler for response #{inspect(response)}"}
-  end
-
+  #
   # Generates a helper function for the article with the given URI that takes an HTML element and,
   # if it's an <img> element whose src attribute does not have a hostname, adds the hostname and
   # scheme to the element.
@@ -177,8 +149,8 @@ defmodule Frenzy.Pipeline.ScrapeStage do
 
   # convert images to data URIs so that they're stored by clients as part of the body
   defp image_to_data_uri(src, true) do
-    case HTTPoison.get(src) do
-      {:ok, %HTTPoison.Response{status_code: 200, body: body, headers: headers}} ->
+    case HTTP.get(src) do
+      {:ok, %HTTPoison.Response{body: body, headers: headers}} ->
         {"Content-Type", content_type} =
           Enum.find(headers, fn {header, _value} -> header == "Content-Type" end)
 

lib/frenzy/task/create_item.ex

@@ -1,4 +1,4 @@
-defmodule Frenzy.CreateItemTask do
+defmodule Frenzy.Task.CreateItem do
   require Logger
   use Task
   alias Frenzy.Repo

lib/frenzy/task/fetch_favicon.ex

@@ -0,0 +1,112 @@
+defmodule Frenzy.Task.FetchFavicon do
+  require Logger
+  use Task
+  alias Frenzy.{HTTP, Repo, Feed}
+
+  def start_link(feed) do
+    Task.start_link(__MODULE__, :run, [feed])
+  end
+
+  def run(feed) do
+    Logger.metadata(favicon_task_id: generate_task_id())
+
+    case fetch_favicon_from_webpage(feed.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 favicon for #{feed.site_url}: #{reason}")
+
+        favicon_uri =
+          %{URI.parse(feed.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) 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 #{feed.site_url}: #{reason}")
+        end
+    end
+  end
+
+  defp fetch_favicon_from_webpage(url) do
+    case HTTP.get(url) do
+      {:ok, %HTTPoison.Response{body: body}} ->
+        extract_favicon(body)
+
+      {:error, _reason} = err ->
+        err
+    end
+  end
+
+  defp extract_favicon(body) do
+    html_tree = Floki.parse(body)
+
+    case Floki.find(html_tree, "link[rel=icon]") do
+      [] ->
+        {:error, "No element matching link[rel=icon]"}
+
+      links ->
+        links
+        |> Enum.find(fn link ->
+          link
+          |> Floki.attribute("type")
+          |> Enum.map(&String.downcase/1)
+          |> Enum.any?(&(&1 == "image/png"))
+          |> case do
+            false ->
+              link
+              |> Floki.attribute("href")
+              # bad hack for missing type attr
+              |> Enum.any?(&String.contains?(&1, ".png"))
+
+            true ->
+              true
+          end
+
+          # todo: support more image types
+        end)
+        |> case do
+          nil ->
+            {:error, "No link[rel=icon] with type of image/png"}
+
+          # todo: try requesting /favicon.ico
+
+          link ->
+            link
+            |> Floki.attribute("href")
+            |> List.first()
+            |> fetch_favicon_data()
+        end
+    end
+  end
+
+  defp fetch_favicon_data(nil), do: {:error, "No href for link"}
+
+  defp fetch_favicon_data(url) do
+    case HTTP.get(url) do
+      {:ok, %HTTPoison.Response{body: body}} ->
+        {:ok, "data:image/png;base64,#{Base.encode64(body)}"}
+
+      {:error, _reason} = err ->
+        err
+    end
+  end
+
+  # from https://github.com/elixir-plug/plug/blob/v1.8.3/lib/plug/request_id.ex#L60
+  defp generate_task_id() do
+    binary = <<
+      System.system_time(:nanosecond)::64,
+      :erlang.phash2({node(), self()}, 16_777_216)::24,
+      :erlang.unique_integer()::32
+    >>
+
+    Base.url_encode64(binary)
+  end
+end

lib/frenzy/update_feeds.ex

@@ -1,6 +1,7 @@
 defmodule Frenzy.UpdateFeeds do
   use GenServer
-  alias Frenzy.{Repo, Feed, Item, CreateItemTask}
+  alias Frenzy.{HTTP, Repo, Feed, Item}
+  alias Frenzy.Task.{CreateItem, FetchFavicon}
   import Ecto.Query
   require Logger
 
@@ -49,9 +50,11 @@ defmodule Frenzy.UpdateFeeds do
   defp prune_old_items() do
     {count, _} =
       from(i in Item,
-        where: i.read and not i.tombstone,
-        # where: i.read_date <= from_now(-1, "week"),
-        where: i.read_date <= from_now(-1, "minute"),
+        where: not i.tombstone,
+        # todo: these time intervals should be configurable by the admin
+        where:
+          (i.read and i.read_date <= from_now(-1, "week")) or
+            (not i.read and i.inserted_at <= from_now(-2, "week")),
         update: [
           set: [tombstone: true, content: nil, creator: nil, date: nil, url: nil, title: nil]
         ]
@@ -119,16 +122,18 @@ defmodule Frenzy.UpdateFeeds do
         last_updated: (rss.last_updated || DateTime.utc_now()) |> Timex.Timezone.convert(:utc)
       })
 
-    Repo.update(changeset)
+    {:ok, feed} = Repo.update(changeset)
+
+    if is_nil(feed.favicon) do
+      FetchFavicon.run(feed)
+    end
 
     feed = Repo.preload(feed, [:items])
 
     Enum.each(rss.items, fn entry ->
       # todo: use Repo.exists for this
       if !Enum.any?(feed.items, fn item -> item.guid == entry.guid end) do
-        CreateItemTask.start_link(feed, entry)
-        # Task.start_link(__MODULE__, :create_item, [feed, entry])
-        # spawn(__MODULE__, :create_item, [feed, entry])
+        CreateItem.start_link(feed, entry)
       end
     end)
   end

lib/frenzy_web/controllers/fever_controller.ex

@@ -50,7 +50,7 @@ defmodule FrenzyWeb.FeverController do
   end
 
   defp fever_response(user, params) do
-    %{api_version: 2, auth: 1}
+    %{api_version: 3, auth: 1}
     |> mark(user, params)
     |> unread_recently_read(user, params)
     |> feeds(user, params)
@@ -158,9 +158,26 @@ defmodule FrenzyWeb.FeverController do
     end
   end
 
-  defp favicons(res, _user, %{"favicons" => _}) do
+  defp favicons(res, user, %{"favicons" => _}) do
+    favicons =
+      user.groups
+      |> Enum.flat_map(& &1.feeds)
+      |> Enum.map(fn feed ->
+        case feed.favicon do
+          nil ->
+            nil
+
+          favicon ->
+            %{
+              id: feed.id,
+              data: favicon |> String.trim_leading("data:")
+            }
+        end
+      end)
+      |> Enum.reject(&is_nil/1)
+
     res
-    |> Map.put(:favicons, [])
+    |> Map.put(:favicons, favicons)
   end
 
   defp favicons(res, _, _), do: res
@@ -211,7 +228,7 @@ defmodule FrenzyWeb.FeverController do
             {id, _} = id |> String.trim() |> Integer.parse()
             item = Repo.get(Item, id)
 
-            if item.feed_id in feed_ids do
+            if not is_nil(item) and item.feed_id in feed_ids do
               item
             else
               nil
@@ -221,24 +238,22 @@ defmodule FrenzyWeb.FeverController do
 
         Map.has_key?(params, "since_id") ->
           since = Repo.get(Item, params["since_id"])
-          {since, _} = Integer.parse(since)
 
           Repo.all(
             from i in Item,
               where: i.feed_id in ^feed_ids,
-              where: i.inserted_at > ^since,
+              where: i.inserted_at > ^since.inserted_at,
               order_by: [asc: :id],
               limit: 50
           )
 
         Map.has_key?(params, "max_id") ->
           max = Repo.get(Item, params["max_id"])
-          {max, _} = Integer.parse(max)
 
           Repo.all(
             from i in Item,
               where: i.feed_id in ^feed_ids,
-              where: i.inserted_at < ^max,
+              where: i.inserted_at < ^max.inserted_at,
               order_by: [desc: :id],
               limit: 50
           )

lib/frenzy_web/controllers/item_controller.ex

@@ -44,7 +44,7 @@ defmodule FrenzyWeb.ItemController do
         read_date: Timex.now()
       })
 
-    Repo.update(changeset)
+    {:ok, item} = Repo.update(changeset)
     redirect(conn, to: Routes.item_path(Endpoint, :show, item.id))
   end
 

lib/frenzy_web/router.ex

@@ -62,6 +62,7 @@ defmodule FrenzyWeb.Router do
   scope "/", FrenzyWeb do
     pipe_through :api
 
+    post "/", FeverController, :post
     get "/fever", FeverController, :get
     get "/api/fever.php", FeverController, :get
     post "/fever", FeverController, :post

lib/frenzy_web/templates/account/show.html.eex

@@ -1,24 +1,35 @@
 <h1>Account Settings</h1>
-<h2><pre><%= @user.username %></pre></h2>
+<h2>Username: <code><%= @user.username %></code></h2>
 
-<a href="<%= Routes.account_path(@conn, :change_password) %>" class="btn btn-secondary">Change Password</a>
 
-<a href="<%= Routes.account_path(@conn, :change_fever_password) %>" class="btn btn-secondary">Change Fever Password</a>
-
-<section class="mt-4">
-  <h2>Import/Export Data</h2>
+<section class="card mt-4">
+  <h4 class="card-header">Import/Export Data</h4>
+  <ul class="list-group list-group-flush">
+    <li class="list-group-item">
       <%= form_for @conn, Routes.account_path(@conn, :import), [method: :post, multipart: true], fn f -> %>
-    <%= file_input f, :file %>
+        <%= file_input f, :file, required: true %>
         <%= submit "Import OPML", class: "btn btn-primary" %>
       <% end %>
+    </li>
+    <li class="list-group-item">
       <%= form_for @conn, Routes.account_path(@conn, :export), [method: :post, class: "mt-2"], fn f -> %>
         <%= submit "Export OPML", class: "btn btn-primary" %>
       <% end %>
+    </li>
+  </ul>
 </section>
 
-<section class="mt-4">
-  <h2>Approved Clients</h2>
+<section class="card mt-4">
+  <h4 class="card-header">Security</h4>
 
+  <ul class="list-group list-group-flush">
+    <li class="list-group-item">
+<a href="<%= Routes.account_path(@conn, :change_password) %>" class="btn btn-secondary">Change Password</a>
+
+<a href="<%= Routes.account_path(@conn, :change_fever_password) %>" class="btn btn-secondary">Change Fever Password</a>
+    </li>
+    <li class="list-group-item">
+    <h5 class="card-title">Approved Clients</h5>
    <table class="table table-striped">
       <thead>
         <tr>
@@ -45,5 +56,7 @@
           </tr>
         <% end %>
       </tbody>
-  </table>
+    </table> </li>
+  </ul>
+
 </section>

lib/frenzy_web/templates/feed/show.html.eex

@@ -1,4 +1,9 @@
-<h1><%= @feed.title %></h1>
+<h1 class="feed-title">
+  <%= if @feed.favicon do %>
+    <img src="<%= @feed.favicon %>" alt="<%= @feed.title %> favicon" class="favicon">
+  <% end %>
+  <%= @feed.title %>
+</h1>
 
 <%= form_tag Routes.feed_path(@conn, :refresh, @feed.id), method: :post, class: "d-inline" do %>
   <%= submit "Refresh Feed", class: "btn btn-primary" %>

lib/frenzy_web/templates/group/new.html.eex

@@ -1,3 +1,5 @@
+<h1>New Group</h1>
+
 <%= form_for @changeset, Routes.group_path(@conn, :create), fn form -> %>
   <div class="form-group row">
     <label for="title" class="col-sm-2 col-form-label">Title</label>

lib/frenzy_web/templates/group/show.html.eex

@@ -29,7 +29,12 @@
           <a href="<%= Routes.feed_path(@conn, :show, feed.id) %>"><%= feed.feed_url %></a>
         </td>
         <td>
-          <a href="<%= feed.site_url %>"><%= feed.title %></a>
+          <a href="<%= feed.site_url %>" class="feed-title">
+            <%= if feed.favicon do %>
+              <img src="<%= feed.favicon %>" alt="<%= feed.title %> favicon" class="favicon">
+            <% end %>
+            <%= feed.title %>
+          </a>
         </td>
       </tr>
     <% end %>

lib/frenzy_web/templates/item/show.html.eex

@@ -2,15 +2,19 @@
   <a href="<%= @item.url %>" target="_blank" rel="noopener noreferrer"><%= @item.title || "(Untitled)" %></a>    
 </h1>
 
-<%= if @item.read do %>
+<div class="item-controls">
+  <a href="<%= Routes.feed_path(@conn, :show, @item.feed_id) %>" class="mr-2"><%= @item.feed.title %></a>
+
+  <%= if @item.read do %>
     <%= form_tag Routes.item_path(@conn, :unread, @item.id), method: :post do %>
-    <%= submit "Mark as Unread", class: "btn btn-secondary" %>
+      <%= submit "Mark as Unread", class: "btn btn-sm btn-secondary" %>
     <% end %>
-<% else %>
+  <% else %>
     <%= form_tag Routes.item_path(@conn, :read, @item.id), method: :post do %>
-    <%= submit "Mark as Read", class: "btn btn-secondary" %>
+      <%= submit "Mark as Read", class: "btn btn-sm btn-secondary" %>
     <% end %>
-<% end %>
+  <% end %>
+</div>
 
 <article class="mt-4">
   <%= raw(@item.content) %>

lib/frenzy_web/templates/layout/app.html.eex

@@ -58,8 +58,13 @@
                       </summary>
                       <ul class="nav flex-column">
                         <%= for feed <- group.feeds do %>
-                          <li class="nav-item">
-                            <a href="<%= Routes.feed_path(@conn, :show, feed.id) %>" class="nav-link"><%= feed.title %></a>
+                          <li class="nav-item feed-nav-item">
+                            <a href="<%= Routes.feed_path(@conn, :show, feed.id) %>" class="nav-link">
+                              <%= if feed.favicon do %>
+                                <img src="<%= feed.favicon %>" alt="<%= feed.title %> favicon" class="favicon">
+                              <% end %>
+                              <%= feed.title %>
+                            </a>
                           </li>
                         <% end %>
                       </ul>

priv/repo/migrations/20191110165504_feeds_add_favicon.exs

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

priv/static/css/app.css

@@ -9,6 +9,10 @@ blockquote {
 	border-left: 4px solid lightgray;
 }
 
+body {
+	overflow-y: scroll;
+}
+
 label.sidebar-toggle:hover {
 	cursor: pointer;
 }
@@ -115,3 +119,27 @@ label.sidebar-toggle > .oi {
 	text-decoration: none;
 }
 
+.sidebar summary a {
+	display: inline-block;
+	width: 90%;
+}
+
+.item-controls a {
+	vertical-align: middle;
+}
+
+.item-controls form {
+	display: inline-block;
+}
+
+
+.favicon {
+	height: 1em;
+	vertical-align: baseline;
+}
+
+.feed-nav-item .favicon,
+.feed-nav-item .nav-link {
+	display: inline-block;
+}
+