diff --git a/.gitignore b/.gitignore index 5e297107..2b145fc6 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ until_fail.sh # Secrets apps/*/priv/secrets/* !apps/*/priv/secrets/.keep +**/*.secret.exs # Elixir LS .elixir_ls diff --git a/apps/cf/config/dev.exs b/apps/cf/config/dev.exs index 4ddf87ab..0f82cadb 100644 --- a/apps/cf/config/dev.exs +++ b/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" diff --git a/apps/cf/config/test.exs b/apps/cf/config/test.exs index 400276f9..dfd0f82f 100644 --- a/apps/cf/config/test.exs +++ b/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, diff --git a/apps/cf/lib/videos/captions_fetcher.ex b/apps/cf/lib/videos/captions_fetcher.ex index 7ba7be4f..649d294f 100644 --- a/apps/cf/lib/videos/captions_fetcher.ex +++ b/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 diff --git a/apps/cf/lib/videos/captions_fetcher_test.ex b/apps/cf/lib/videos/captions_fetcher_test.ex index 9295049a..178e08ca 100644 --- a/apps/cf/lib/videos/captions_fetcher_test.ex +++ b/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" diff --git a/apps/cf/lib/videos/captions_fetcher_youtube.ex b/apps/cf/lib/videos/captions_fetcher_youtube.ex index 4403924d..5081c689 100644 --- a/apps/cf/lib/videos/captions_fetcher_youtube.ex +++ b/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, diff --git a/apps/cf/lib/videos/metadata_fetcher.ex b/apps/cf/lib/videos/metadata_fetcher.ex index 96dda2e9..9df30716 100644 --- a/apps/cf/lib/videos/metadata_fetcher.ex +++ b/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 diff --git a/apps/cf/lib/videos/metadata_fetcher_opengraph.ex b/apps/cf/lib/videos/metadata_fetcher_opengraph.ex new file mode 100644 index 00000000..61de6ecd --- /dev/null +++ b/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 diff --git a/apps/cf/lib/videos/metadata_fetcher_test.ex b/apps/cf/lib/videos/metadata_fetcher_test.ex new file mode 100644 index 00000000..948767cf --- /dev/null +++ b/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 diff --git a/apps/cf/lib/videos/metadata_fetcher_youtube.ex b/apps/cf/lib/videos/metadata_fetcher_youtube.ex new file mode 100644 index 00000000..0a20ae01 --- /dev/null +++ b/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 diff --git a/apps/cf/lib/videos/videos.ex b/apps/cf/lib/videos/videos.ex index 1facbbf2..d01d087e 100644 --- a/apps/cf/lib/videos/videos.ex +++ b/apps/cf/lib/videos/videos.ex @@ -17,6 +17,7 @@ defmodule CF.Videos do 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} @@ -163,10 +154,9 @@ 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() @@ -174,4 +164,20 @@ defmodule CF.Videos do {: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 diff --git a/apps/cf/priv/secrets/.keep b/apps/cf/priv/secrets/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/apps/cf/test/videos/videos_test.exs b/apps/cf/test/videos/videos_test.exs index 6efde826..e70cefa9 100644 --- a/apps/cf/test/videos/videos_test.exs +++ b/apps/cf/test/videos/videos_test.exs @@ -5,7 +5,7 @@ defmodule CF.VideosTest do alias CF.Videos alias CF.Accounts.UserPermissions.PermissionsError - defp test_url, do: "__TEST__/#{DB.Utils.TokenGenerator.generate(8)}" + defp test_url, do: "https://www.youtube.com/watch?v=#{DB.Utils.TokenGenerator.generate(11)}" describe "Add video" do test "without enough reputation" do @@ -59,7 +59,13 @@ defmodule CF.VideosTest do describe "Fetch captions" do test "fetch captions" do - video = DB.Factory.insert(:video, provider: "__TEST__", language: "en") + video = + DB.Factory.insert( + :video, + youtube_id: DB.Utils.TokenGenerator.generate(11), + language: "en" + ) + {:ok, captions} = Videos.download_captions(video) assert captions.content == "__TEST-CONTENT__" diff --git a/apps/cf_graphql/lib/resolvers/videos.ex b/apps/cf_graphql/lib/resolvers/videos.ex index a3dd658c..e60bd209 100644 --- a/apps/cf_graphql/lib/resolvers/videos.ex +++ b/apps/cf_graphql/lib/resolvers/videos.ex @@ -15,21 +15,21 @@ defmodule CF.GraphQL.Resolvers.Videos do # Queries def get(_root, %{id: id}, _info) do - case get_video_by_id(id) do + case CF.Videos.get_video_by_id(id) do nil -> {:error, "Video #{id} doesn't exist"} video -> {:ok, video} end end def get(_root, %{hash_id: id}, _info) do - case get_video_by_id(id) do + case CF.Videos.get_video_by_id(id) do nil -> {:error, "Video #{id} doesn't exist"} video -> {:ok, video} end end def get(_root, %{url: url}, _info) do - case get_video_by_url(url) do + case CF.Videos.get_video_by_url(url) do nil -> {:error, "Video with url #{url} doesn't exist"} video -> {:ok, video} end @@ -69,20 +69,4 @@ defmodule CF.GraphQL.Resolvers.Videos do |> Repo.all() |> Enum.group_by(& &1.video_id) end - - # ---- Private ---- - - defp get_video_by_url(url) do - case Video.parse_url(url) do - {provider, id} -> - Video - |> Video.with_speakers() - |> Repo.get_by(provider: provider, provider_id: id) - - nil -> - nil - end - end - - defp get_video_by_id(id), do: Repo.get(Video, id) end diff --git a/apps/cf_graphql/lib/schema/types/video.ex b/apps/cf_graphql/lib/schema/types/video.ex index ff7698a9..e104e889 100644 --- a/apps/cf_graphql/lib/schema/types/video.ex +++ b/apps/cf_graphql/lib/schema/types/video.ex @@ -21,9 +21,15 @@ defmodule CF.GraphQL.Schema.Types.Video do @desc "Video URL" field(:url, non_null(:string), do: resolve(&Resolvers.Videos.url/3)) @desc "Video provider (youtube, vimeo...etc)" - field(:provider, non_null(:string)) + field(:provider, non_null(:string), deprecate: "Use `url` or `youtube_id`") do + resolve(fn _, _, _ -> {:ok, "youtube"} end) + end + @desc "Unique ID used to identify video with provider" - field(:provider_id, non_null(:string)) + field(:provider_id, non_null(:string), deprecate: "Use `url` or `youtube_id`") do + resolve(fn v, _, _ -> {:ok, v.youtube_id} end) + end + @desc "Language of the video represented as a two letters locale" field(:language, :string) @desc "Video insert datetime" @@ -41,6 +47,14 @@ defmodule CF.GraphQL.Schema.Types.Video do resolve(&Resolvers.Videos.statements/3) complexity(join_complexity()) end + + # Video providers + + @desc "Youtube ID for this video" + field(:youtube_id, :string) + + @desc "Offset for all statements on this video when watched with YouTube player" + field(:youtube_offset, non_null(:integer)) end @desc "A list a paginated videos" diff --git a/apps/cf_rest_api/lib/controllers/video_controller.ex b/apps/cf_rest_api/lib/controllers/video_controller.ex index 690fb94a..21895037 100644 --- a/apps/cf_rest_api/lib/controllers/video_controller.ex +++ b/apps/cf_rest_api/lib/controllers/video_controller.ex @@ -5,6 +5,7 @@ defmodule CF.RestApi.VideoController do use CF.RestApi, :controller import CF.Videos + alias CF.RestApi.ChangesetView action_fallback(CF.RestApi.FallbackController) @@ -23,16 +24,6 @@ defmodule CF.RestApi.VideoController do def index(conn, _params), do: render(conn, :index, videos: videos_list()) - @doc """ - List all videos but only return indexes. - """ - @deprecated "Extension/Injector is now using GraphQL API for this" - def index_ids(conn, %{"min_id" => min_id}), - do: json(conn, videos_index(min_id)) - - def index_ids(conn, _), - do: json(conn, videos_index()) - @doc """ Create a new video based on `url`. If it already exist, just returns the video. @@ -44,10 +35,15 @@ defmodule CF.RestApi.VideoController do |> Guardian.Plug.current_resource() |> create!(url, params["is_partner"]) |> case do - {:error, message} -> + {:error, error} when is_binary(error) -> + conn + |> put_status(:unprocessable_entity) + |> json(%{error: error}) + + {:error, changeset = %Ecto.Changeset{}} -> conn |> put_status(:unprocessable_entity) - |> json(%{error: %{url: message}}) + |> json(ChangesetView.render("error.json", %{changeset: changeset})) {:ok, video} -> render(conn, "show.json", video: video) diff --git a/apps/cf_rest_api/lib/router.ex b/apps/cf_rest_api/lib/router.ex index dda93eee..cba25be3 100644 --- a/apps/cf_rest_api/lib/router.ex +++ b/apps/cf_rest_api/lib/router.ex @@ -24,7 +24,6 @@ defmodule CF.RestApi.Router do # ---- Public endpoints ---- get("/", ApiInfoController, :get) get("/videos", VideoController, :index) - get("/videos/index", VideoController, :index_ids) get("/speakers/:slug_or_id", SpeakerController, :show) post("/search/video", VideoController, :search) get("/videos/:video_id/statements", StatementController, :get) diff --git a/apps/cf_rest_api/lib/views/video_view.ex b/apps/cf_rest_api/lib/views/video_view.ex index 5d21685f..89bb5c66 100644 --- a/apps/cf_rest_api/lib/views/video_view.ex +++ b/apps/cf_rest_api/lib/views/video_view.ex @@ -14,8 +14,10 @@ defmodule CF.RestApi.VideoView do id: video.id, hash_id: video.hash_id, title: video.title, - provider: video.provider, - provider_id: video.provider_id, + provider: "youtube", + provider_id: video.youtube_id, + youtube_id: video.youtube_id, + youtube_offset: video.youtube_offset, url: DB.Schema.Video.build_url(video), posted_at: video.inserted_at, speakers: render_many(video.speakers, CF.RestApi.SpeakerView, "speaker.json"), diff --git a/apps/cf_rest_api/test/controllers/video_controller_test.exs b/apps/cf_rest_api/test/controllers/video_controller_test.exs index 1198f2b1..ce3a9efd 100644 --- a/apps/cf_rest_api/test/controllers/video_controller_test.exs +++ b/apps/cf_rest_api/test/controllers/video_controller_test.exs @@ -18,7 +18,7 @@ defmodule CF.RestApi.VideoControllerTest do assert Enum.count(videos) == Enum.count(response) assert random_video.title == - Enum.find(response, &(&1["provider_id"] == random_video.provider_id))["title"] + Enum.find(response, &(&1["youtube_id"] == random_video.youtube_id))["title"] end test "filter on language", %{conn: conn} do @@ -67,6 +67,6 @@ defmodule CF.RestApi.VideoControllerTest do |> post("/videos", %{url: "https://google.fr"}) |> json_response(422) - assert response == %{"error" => %{"url" => "Invalid URL"}} + assert response == %{"errors" => %{"url" => ["invalid_url"]}} end end diff --git a/apps/db/lib/db_schema/video.ex b/apps/db/lib/db_schema/video.ex index c01f5d06..00848e01 100644 --- a/apps/db/lib/db_schema/video.ex +++ b/apps/db/lib/db_schema/video.ex @@ -13,12 +13,17 @@ defmodule DB.Schema.Video do field(:title, :string) field(:hash_id, :string) field(:url, :string, virtual: true) - field(:provider, :string, null: false) - field(:provider_id, :string, null: false) field(:language, :string, null: true) field(:unlisted, :boolean, null: false) field(:is_partner, :boolean, null: false) + # DEPRECATED + # field(:provider, :string) + # field(:provider_id, :string) + + field(:youtube_id, :string) + field(:youtube_offset, :integer, null: false, default: 0) + many_to_many(:speakers, Speaker, join_through: VideoSpeaker, on_delete: :delete_all) has_many(:statements, Statement, on_delete: :delete_all) @@ -28,16 +33,11 @@ defmodule DB.Schema.Video do # Define valid providers @providers_regexs %{ - # Map a provider name to its regex, using named_captures to get the id --------------------↘️ - "youtube" => + # Map a provider name to its regex, using named_captures to get the id ---------↘️ + youtube: ~r/(?:youtube\.com\/(?:[^\/]+\/.+\/|(?:v|e(?:mbed)?)\/|.*[?&]v=)|youtu\.be\/)(?[^"&?\/ ]{11})/i } - # Allow for URLs like `https://anything.com/__TEST__/ID` in tests - if Application.get_env(:db, :env) == :test do - @providers_regexs Map.put(@providers_regexs, "__TEST__", ~r/__TEST__\/(?[^"&?\/ ]+)/) - end - # Define queries @doc """ @@ -108,28 +108,23 @@ defmodule DB.Schema.Video do ## Examples iex> import DB.Schema.Video, only: [build_url: 1] - iex> build_url(%{provider: "youtube", provider_id: "dQw4w9WgXcQ"}) + iex> build_url(%{youtube_id: "dQw4w9WgXcQ"}) "https://www.youtube.com/watch?v=dQw4w9WgXcQ" """ - def build_url(%{provider: "youtube", provider_id: id}), + def build_url(%{youtube_id: id}) when not is_nil(id), do: "https://www.youtube.com/watch?v=#{id}" # Add a special case for building test URLs if Application.get_env(:db, :env) == :test do - def build_url(%{provider: "__TEST__", provider_id: id}), + def build_url(%{youtube_id: id}), do: "__TEST__/#{id}" end @doc """ Returns overview image url for the given video """ - def image_url( - _video = %__MODULE__{ - provider: "youtube", - provider_id: youtube_id - } - ) do - "https://img.youtube.com/vi/#{youtube_id}/hqdefault.jpg" + def image_url(_video = %__MODULE__{youtube_id: id}) when not is_nil(id) do + "https://img.youtube.com/vi/#{id}/hqdefault.jpg" end @doc """ @@ -139,11 +134,10 @@ defmodule DB.Schema.Video do struct |> cast(params, [:url, :title, :language]) |> validate_required([:url, :title]) - |> parse_url() - |> validate_required([:provider, :provider_id]) + |> parse_video_url() |> validate_length(:title, min: 5, max: 120) - |> unique_constraint(:videos_provider_provider_id_index) - # Change "en-US" to "en" + |> unique_constraint(:videos_youtube_id_index) + # Change locales like "en-US" to "en" |> update_change(:language, &hd(String.split(&1, "-"))) end @@ -160,29 +154,14 @@ defmodule DB.Schema.Video do end @doc """ - Parse an URL. - If given a changeset, fill the `provider` and `provider_id` fields or add - an error if URL is not valid. - If given a binary, return {provider, id} or nil if invalid. - - ## Examples - - iex> import DB.Schema.Video, only: [parse_url: 1] - iex> parse_url "" - nil - iex> parse_url "https://www.youtube.com/watch?v=dQw4w9WgXcQ" - {"youtube", "dQw4w9WgXcQ"} - iex> parse_url "https://www.youtube.com/watch?v=" - nil + Given a changeset, fill the `{provider}_id` fields or add an error if URL is not valid. """ - def parse_url(changeset = %Ecto.Changeset{}) do + def parse_video_url(changeset = %Ecto.Changeset{}) do case changeset do %Ecto.Changeset{valid?: true, changes: %{url: url}} -> case parse_url(url) do - {provider, id} -> - changeset - |> put_change(:provider, provider) - |> put_change(:provider_id, id) + {:youtube, id} -> + put_change(changeset, :youtube_id, id) _ -> add_error(changeset, :url, "invalid_url") @@ -193,6 +172,20 @@ defmodule DB.Schema.Video do end end + @doc """ + Parse an URL. + Given a binary, return {provider, id} or nil if invalid. + + ## Examples + + iex> import DB.Schema.Video, only: [parse_url: 1] + iex> parse_url "" + nil + iex> parse_url "https://www.youtube.com/watch?v=dQw4w9WgXcQ" + {:youtube, "dQw4w9WgXcQ"} + iex> parse_url "https://www.youtube.com/watch?v=" + nil + """ def parse_url(url) when is_binary(url) do Enum.find_value(@providers_regexs, fn {provider, regex} -> case Regex.named_captures(regex, url) do diff --git a/apps/db/priv/repo/migrations/20180827123706_add_hash_id_to_videos.exs b/apps/db/priv/repo/migrations/20180827123706_add_hash_id_to_videos.exs index 26b95b4b..781f70a7 100644 --- a/apps/db/priv/repo/migrations/20180827123706_add_hash_id_to_videos.exs +++ b/apps/db/priv/repo/migrations/20180827123706_add_hash_id_to_videos.exs @@ -1,28 +1,31 @@ defmodule DB.Repo.Migrations.AddHashIdToVideos do use Ecto.Migration + import Ecto.Query alias DB.Schema.Video def up do alter table(:videos) do # A size of 10 allows us to go up to 100_000_000_000_000 videos - add :hash_id, :string, size: 10 + add(:hash_id, :string, size: 10) end # Create unique index on hash_id - create unique_index(:videos, [:hash_id]) + create(unique_index(:videos, [:hash_id])) # Flush pending migrations to ensure column is created flush() # Update all existing videos with their hashIds - DB.Repo.all(Video) + Video + |> select([v], v.id) + |> DB.Repo.all() |> Enum.map(&Video.changeset_generate_hash_id/1) |> Enum.map(&DB.Repo.update/1) end def down do alter table(:videos) do - remove :hash_id + remove(:hash_id) end end end diff --git a/apps/db/priv/repo/migrations/20181209205427_videos_providers_as_columns.exs b/apps/db/priv/repo/migrations/20181209205427_videos_providers_as_columns.exs new file mode 100644 index 00000000..d620b8dc --- /dev/null +++ b/apps/db/priv/repo/migrations/20181209205427_videos_providers_as_columns.exs @@ -0,0 +1,62 @@ +defmodule DB.Repo.Migrations.VideosProvidersAsColumns do + @moduledoc """ + This migration changes the `Videos` schema to go from a pair + of {provider, provider_id} to a model where we have multiple `{provider}_id` + column. This will allow to store multiple sources for a single video, with + different offsets to ensure they're all in sync. + """ + + use Ecto.Migration + + def up do + # Add new columns + alter table(:videos) do + add(:youtube_id, :string, null: true, length: 11) + add(:youtube_offset, :integer, null: false, default: 0) + end + + flush() + + # Migrate existing videos - we only have YouTube right now + Ecto.Adapters.SQL.query!(DB.Repo, """ + UPDATE videos SET youtube_id = provider_id; + """) + + flush() + + # Create index + create(unique_index(:videos, :youtube_id)) + + # Remove columns + alter table(:videos) do + remove(:provider) + remove(:provider_id) + end + end + + def down do + # Restore old scheme + alter table(:videos) do + add(:provider, :string) + add(:provider_id, :string) + end + + flush() + + # Migrate existing videos + Ecto.Adapters.SQL.query!(DB.Repo, """ + UPDATE videos SET provider_id = youtube_id, provider = 'youtube'; + """) + + flush() + + # Re-create index + create(unique_index(:videos, [:provider, :provider_id])) + + # Remove columns + alter table(:videos) do + remove(:youtube_id) + remove(:youtube_offset) + end + end +end diff --git a/apps/db/test/support/factory.ex b/apps/db/test/support/factory.ex index 0b56875f..0c19a135 100644 --- a/apps/db/test/support/factory.ex +++ b/apps/db/test/support/factory.ex @@ -50,8 +50,7 @@ defmodule DB.Factory do %Video{ url: "https://www.youtube.com/watch?v=#{youtube_id}", title: Faker.Lorem.sentence(3..8), - provider: "youtube", - provider_id: youtube_id, + youtube_id: youtube_id, hash_id: nil, language: Enum.random(["en", "fr", nil]) }