Merge pull request #63 from CaptainFact/refactor/video-provider-id

Videos can now handle multiple providers

.gitignore

@@ -17,6 +17,7 @@ until_fail.sh
 # Secrets
 apps/*/priv/secrets/*
 !apps/*/priv/secrets/.keep
+**/*.secret.exs
 
 # Elixir LS
 .elixir_ls

apps/cf/config/dev.exs

@@ -31,3 +31,6 @@ config :rollbax,
 
 # Mails
 config :cf, CF.Mailer, adapter: Bamboo.LocalAdapter
+
+# Import local secrets if any - use wildcard to ignore errors
+import_config "*dev.secret.exs"

apps/cf/config/test.exs

@@ -23,9 +23,9 @@ config :cf, CF.Mailer, adapter: Bamboo.TestAdapter
 # Reduce the number of round for encryption during tests
 config :bcrypt_elixir, :log_rounds, 4
 
-# Captions mock for testing
-config :cf,
-  captions_fetcher: CF.Videos.CaptionsFetcherTest
+# Behaviours mock for testing
+config :cf, captions_fetcher: CF.Videos.CaptionsFetcherTest
+config :cf, use_test_video_metadata_fetcher: true
 
 # Configure Rollbar (errors reporting)
 config :rollbax,

apps/cf/lib/actions/action_creator.ex

@@ -100,7 +100,7 @@ defmodule CF.Actions.ActionCreator do
       user_id,
       :video,
       :update,
-      video_id: video.video_id,
+      video_id: video.id,
       changes: changes
     )
   end

apps/cf/lib/videos/captions_fetcher.ex

@@ -3,6 +3,5 @@ defmodule CF.Videos.CaptionsFetcher do
   Fetch captions for videos.
   """
 
-  @callback fetch(String.t(), String.t()) ::
-              {:ok, DB.Schema.VideoCaption.t()} | {:error, binary()}
+  @callback fetch(DB.Schema.Video.t()) :: {:ok, DB.Schema.VideoCaption.t()} | {:error, binary()}
 end

apps/cf/lib/videos/captions_fetcher_test.ex

@@ -6,7 +6,7 @@ defmodule CF.Videos.CaptionsFetcherTest do
   @behaviour CF.Videos.CaptionsFetcher
 
   @impl true
-  def fetch(_provider_id, _locale) do
+  def fetch(_video) do
     captions = %DB.Schema.VideoCaption{
       content: "__TEST-CONTENT__",
       format: "xml"

apps/cf/lib/videos/captions_fetcher_youtube.ex

@@ -6,7 +6,7 @@ defmodule CF.Videos.CaptionsFetcherYoutube do
   @behaviour CF.Videos.CaptionsFetcher
 
   @impl true
-  def fetch(youtube_id, locale) do
+  def fetch(%{youtube_id: youtube_id, locale: locale}) do
     with {:ok, content} <- fetch_captions_content(youtube_id, locale) do
       captions = %DB.Schema.VideoCaption{
         content: content,

apps/cf/lib/videos/metadata_fetcher.ex

@@ -1,76 +1,16 @@
 defmodule CF.Videos.MetadataFetcher do
   @moduledoc """
-  Methods to fetch metadata (title, language) from videos
+  Fetch metadata for video.
   """
 
-  require Logger
-
-  alias Kaur.Result
-  alias GoogleApi.YouTube.V3.Connection, as: YouTubeConnection
-  alias GoogleApi.YouTube.V3.Api.Videos, as: YouTubeVideos
-  alias GoogleApi.YouTube.V3.Model.Video, as: YouTubeVideo
-  alias GoogleApi.YouTube.V3.Model.VideoListResponse, as: YouTubeVideoList
-
-  alias DB.Schema.Video
+  @type video_metadata :: %{
+          title: String.t(),
+          language: String.t(),
+          url: String.t()
+        }
 
   @doc """
-  Fetch metadata from video. Returns an object containing  :title, :url and :language
-
-  Usage:
-  iex> fetch_video_metadata("https://www.youtube.com/watch?v=OhWRT3PhMJs")
-  iex> fetch_video_metadata({"youtube", "OhWRT3PhMJs"})
+  Takes an URL, fetch the metadata and return them
   """
-  def fetch_video_metadata(nil),
-    do: {:error, "Invalid URL"}
-
-  if Application.get_env(:db, :env) == :test do
-    def fetch_video_metadata(url = "__TEST__/" <> _id) do
-      {:ok, %{title: "__TEST-TITLE__", url: url}}
-    end
-  end
-
-  def fetch_video_metadata(url) when is_binary(url),
-    do: fetch_video_metadata(Video.parse_url(url))
-
-  def fetch_video_metadata({"youtube", provider_id}) do
-    case Application.get_env(:cf, :youtube_api_key) do
-      nil ->
-        Logger.warn("No YouTube API key provided. Falling back to HTML fetcher")
-        fetch_video_metadata_html("youtube", provider_id)
-
-      api_key ->
-        fetch_video_metadata_api("youtube", provider_id, api_key)
-    end
-  end
-
-  defp fetch_video_metadata_api("youtube", provider_id, api_key) do
-    YouTubeConnection.new()
-    |> YouTubeVideos.youtube_videos_list("snippet", id: provider_id, key: api_key)
-    |> Result.map_error(fn e -> "YouTube API Error: #{inspect(e)}" end)
-    |> Result.keep_if(&(!Enum.empty?(&1.items)), "Video doesn't exist")
-    |> Result.map(fn %YouTubeVideoList{items: [video = %YouTubeVideo{} | _]} ->
-      %{
-        title: video.snippet.title,
-        language: video.snippet.defaultLanguage || video.snippet.defaultAudioLanguage,
-        url: Video.build_url(%{provider: "youtube", provider_id: provider_id})
-      }
-    end)
-  end
-
-  defp fetch_video_metadata_html(provider, id) do
-    url = Video.build_url(%{provider: provider, provider_id: id})
-
-    case HTTPoison.get(url) do
-      {:ok, %HTTPoison.Response{body: body}} ->
-        meta = Floki.attribute(body, "meta[property='og:title']", "content")
-
-        case meta do
-          [] -> {:error, "Page does not contains an OpenGraph title attribute"}
-          [title] -> {:ok, %{title: HtmlEntities.decode(title), url: url}}
-        end
-
-      {_, _} ->
-        {:error, "Remote URL didn't respond correctly"}
-    end
-  end
+  @callback fetch_video_metadata(String.t()) :: {:ok, video_metadata} | {:error, binary()}
 end

apps/cf/lib/videos/metadata_fetcher_opengraph.ex

@@ -0,0 +1,25 @@
+defmodule CF.Videos.MetadataFetcher.Opengraph do
+  @moduledoc """
+  Methods to fetch metadata (title, language) from videos
+  """
+
+  @behaviour CF.Videos.MetadataFetcher
+
+  @doc """
+  Fetch metadata from video using OpenGraph tags.
+  """
+  def fetch_video_metadata(url) do
+    case HTTPoison.get(url) do
+      {:ok, %HTTPoison.Response{body: body}} ->
+        meta = Floki.attribute(body, "meta[property='og:title']", "content")
+
+        case meta do
+          [] -> {:error, "Page does not contains an OpenGraph title attribute"}
+          [title] -> {:ok, %{title: HtmlEntities.decode(title), url: url}}
+        end
+
+      {_, _} ->
+        {:error, "Remote URL didn't respond correctly"}
+    end
+  end
+end

apps/cf/lib/videos/metadata_fetcher_test.ex

@@ -0,0 +1,18 @@
+defmodule CF.Videos.MetadataFetcher.Test do
+  @moduledoc """
+  Methods to fetch metadata (title, language) from videos
+  """
+
+  @behaviour CF.Videos.MetadataFetcher
+
+  @doc """
+  Fetch metadata from video using OpenGraph tags.
+  """
+  def fetch_video_metadata(url) do
+    {:ok,
+     %{
+       title: "__TEST-TITLE__",
+       url: url
+     }}
+  end
+end

apps/cf/lib/videos/metadata_fetcher_youtube.ex

@@ -0,0 +1,51 @@
+defmodule CF.Videos.MetadataFetcher.Youtube do
+  @moduledoc """
+  Methods to fetch metadata (title, language) from videos
+  """
+
+  @behaviour CF.Videos.MetadataFetcher
+
+  require Logger
+
+  alias Kaur.Result
+  alias GoogleApi.YouTube.V3.Connection, as: YouTubeConnection
+  alias GoogleApi.YouTube.V3.Api.Videos, as: YouTubeVideos
+  alias GoogleApi.YouTube.V3.Model.Video, as: YouTubeVideo
+  alias GoogleApi.YouTube.V3.Model.VideoListResponse, as: YouTubeVideoList
+
+  alias DB.Schema.Video
+  alias CF.Videos.MetadataFetcher
+
+  @doc """
+  Fetch metadata from video. Returns an object containing  :title, :url and :language
+  """
+  def fetch_video_metadata(nil),
+    do: {:error, "Invalid URL"}
+
+  def fetch_video_metadata(url) when is_binary(url) do
+    {:youtube, youtube_id} = Video.parse_url(url)
+
+    case Application.get_env(:cf, :youtube_api_key) do
+      nil ->
+        Logger.warn("No YouTube API key provided. Falling back to HTML fetcher")
+        MetadataFetcher.Opengraph.fetch_video_metadata(url)
+
+      api_key ->
+        do_fetch_video_metadata(youtube_id, api_key)
+    end
+  end
+
+  defp do_fetch_video_metadata(youtube_id, api_key) do
+    YouTubeConnection.new()
+    |> YouTubeVideos.youtube_videos_list("snippet", id: youtube_id, key: api_key)
+    |> Result.map_error(fn e -> "YouTube API Error: #{inspect(e)}" end)
+    |> Result.keep_if(&(!Enum.empty?(&1.items)), "remote_video_404")
+    |> Result.map(fn %YouTubeVideoList{items: [video = %YouTubeVideo{} | _]} ->
+      %{
+        title: video.snippet.title,
+        language: video.snippet.defaultLanguage || video.snippet.defaultAudioLanguage,
+        url: Video.build_url(%{youtube_id: youtube_id})
+      }
+    end)
+  end
+end

apps/cf/lib/videos/videos.ex

@@ -6,17 +6,18 @@ defmodule CF.Videos do
   import Ecto.Query, warn: false
   import CF.Videos.MetadataFetcher
   import CF.Videos.CaptionsFetcher
+  import CF.Actions.ActionCreator, only: [action_update: 2]
 
   alias Ecto.Multi
   alias DB.Repo
   alias DB.Schema.Video
-  alias DB.Schema.Statement
   alias DB.Schema.Speaker
   alias DB.Schema.VideoSpeaker
   alias DB.Schema.VideoCaption
 
   alias CF.Actions.ActionCreator
   alias CF.Accounts.UserPermissions
+  alias CF.Videos.MetadataFetcher
 
   @captions_fetcher Application.get_env(:cf, :captions_fetcher)
 
@@ -34,28 +35,17 @@ defmodule CF.Videos do
   def videos_list(filters, false),
     do: Repo.all(Video.query_list(Video, filters))
 
-  @doc """
-  Index videos, returning only their id, provider_id and provider.
-  Accepted filters are the same than for `videos_list/1`
-  """
-  def videos_index(from_id \\ 0) do
-    Video
-    |> select([v], %{id: v.id, provider: v.provider, provider_id: v.provider_id})
-    |> where([v], v.id > ^from_id)
-    |> Repo.all()
-  end
-
   @doc """
   Return the corresponding video if it has already been added, `nil` otherwise
   """
   def get_video_by_url(url) do
     case Video.parse_url(url) do
-      {provider, id} ->
+      {:youtube, id} ->
         Video
         |> Video.with_speakers()
-        |> Repo.get_by(provider: provider, provider_id: id)
+        |> Repo.get_by(youtube_id: id)
 
-      nil ->
+      _ ->
         nil
     end
   end
@@ -74,7 +64,8 @@ defmodule CF.Videos do
   def create!(user, video_url, is_partner \\ nil) do
     UserPermissions.check!(user, :add, :video)
 
-    with {:ok, metadata} <- fetch_video_metadata(video_url) do
+    with metadata_fetcher when not is_nil(metadata_fetcher) <- get_metadata_fetcher(video_url),
+         {:ok, metadata} <- metadata_fetcher.(video_url) do
       # Videos posted by publishers are recorded as partner unless explicitely
       # specified otherwise (false)
       base_video = %Video{is_partner: user.is_publisher && is_partner != false}
@@ -106,33 +97,20 @@ defmodule CF.Videos do
   Returns {:ok, statements} if success, {:error, reason} otherwise.
   Returned statements contains only an id and a key
   """
-  def shift_statements(user, video_id, offset) when is_integer(offset) do
+  def shift_statements(user, video_id, offsets) do
     UserPermissions.check!(user, :update, :video)
-    statements_query = where(Statement, [s], s.video_id == ^video_id)
+    video = Repo.get!(Video, video_id)
+    changeset = Video.changeset_shift_offsets(video, offsets)
 
     Multi.new()
-    |> Multi.update_all(
-      :statements_update,
-      statements_query,
-      [inc: [time: offset]],
-      returning: [:id, :time]
-    )
-    |> Multi.insert(
-      :action,
-      ActionCreator.action(
-        user.id,
-        :video,
-        :update,
-        video_id: video_id,
-        changes: %{"statements_time" => offset}
-      )
-    )
+    |> Multi.update(:video, changeset)
+    |> Multi.insert(:action_update, action_update(user.id, changeset))
     |> Repo.transaction()
     |> case do
-      {:ok, %{statements_update: {_, statements}}} ->
-        {:ok, Enum.map(statements, &%{id: &1.id, time: &1.time})}
+      {:ok, %{video: video}} ->
+        {:ok, video}
 
-      {:error, _, reason, _} ->
+      {:error, _operation, reason, _changes} ->
         {:error, reason}
     end
   end
@@ -163,15 +141,30 @@ defmodule CF.Videos do
 
   Usage:
   iex> download_captions(video)
-  iex> download_captions(video)
   """
   def download_captions(video = %Video{}) do
-    with {:ok, captions} <- @captions_fetcher.fetch(video.provider_id, video.language) do
+    with {:ok, captions} <- @captions_fetcher.fetch(video) do
       captions
       |> VideoCaption.changeset(%{video_id: video.id})
       |> Repo.insert()
 
       {:ok, captions}
     end
   end
+
+  defp get_metadata_fetcher(video_url) do
+    cond do
+      Application.get_env(:cf, :use_test_video_metadata_fetcher) ->
+        &MetadataFetcher.Test.fetch_video_metadata/1
+
+      # We only support YouTube for now
+      # TODO Use a Regex here
+      video_url ->
+        &MetadataFetcher.Youtube.fetch_video_metadata/1
+
+      # Use a default fetcher that retrieves info from OpenGraph tags
+      true ->
+        &MetadataFetcher.Opengraph.fetch_video_metadata/1
+    end
+  end
 end