Frontends: refactor with %Frontend{} struct

.gitignore

@@ -10,6 +10,7 @@
 /test/fixtures/test_tmp.txt
 /test/fixtures/image_tmp.jpg
 /test/tmp/
+/test/frontend_static_test/
 /doc
 /instance
 /priv/ssh_keys
@@ -56,4 +57,4 @@ pleroma.iml
 
 # Editor temp files
 /*~
-/*#
+/*#

lib/mix/tasks/pleroma/frontend.ex

@@ -7,6 +7,8 @@ defmodule Mix.Tasks.Pleroma.Frontend do
 
   import Mix.Pleroma
 
+  alias Pleroma.Frontend
+
   @shortdoc "Manages bundled Pleroma frontends"
 
   @moduledoc File.read!("docs/administration/CLI_tasks/frontend.md")
@@ -16,7 +18,7 @@ def run(["install", "none" | _args]) do
     "none"
   end
 
-  def run(["install", frontend | args]) do
+  def run(["install", name | args]) do
     start_pleroma()
 
     {options, [], []} =
@@ -24,13 +26,19 @@ def run(["install", frontend | args]) do
         args,
         strict: [
           ref: :string,
-          static_dir: :string,
           build_url: :string,
           build_dir: :string,
           file: :string
         ]
       )
 
-    Pleroma.Frontend.install(frontend, options)
+    options
+    |> Keyword.put(:name, name)
+    |> opts_to_frontend()
+    |> Frontend.install()
+  end
+
+  defp opts_to_frontend(opts) do
+    struct(Frontend, opts)
   end
 end

lib/pleroma/frontend.ex

@@ -4,39 +4,40 @@
 
 defmodule Pleroma.Frontend do
   alias Pleroma.Config
+  alias Pleroma.Frontend
 
   require Logger
 
-  def install(name, opts \\ []) do
-    frontend_info = %{
-      "ref" => opts[:ref],
-      "build_url" => opts[:build_url],
-      "build_dir" => opts[:build_dir]
-    }
+  @unknown_name "unknown"
 
-    frontend_info =
-      [:frontends, :available, name]
-      |> Config.get(%{})
-      |> Map.merge(frontend_info, fn _key, config, cmd ->
-        # This only overrides things that are actually set
-        cmd || config
-      end)
+  defstruct [:name, :ref, :git, :build_url, :build_dir, :file, :"custom-http-headers"]
 
-    ref = frontend_info["ref"]
+  def install(%Frontend{} = frontend) do
+    frontend
+    |> maybe_put_name()
+    |> hydrate()
+    |> validate!()
+    |> do_install()
+  end
 
-    unless ref do
-      raise "No ref given or configured"
-    end
+  defp maybe_put_name(%{name: nil} = fe), do: Map.put(fe, :name, @unknown_name)
+  defp maybe_put_name(fe), do: fe
 
+  # Merges a named frontend with the provided one
+  defp hydrate(%Frontend{name: name} = frontend) do
+    get_named_frontend(name)
+    |> merge(frontend)
+  end
+
+  defp do_install(%Frontend{ref: ref, name: name} = frontend) do
     dest = Path.join([dir(), name, ref])
 
     label = "#{name} (#{ref})"
     tmp_dir = Path.join(dir(), "tmp")
 
-    with {_, :ok} <-
-           {:download_or_unzip, download_or_unzip(frontend_info, tmp_dir, opts[:file])},
+    with {_, :ok} <- {:download_or_unzip, download_or_unzip(frontend, tmp_dir)},
          Logger.info("Installing #{label} to #{dest}"),
-         :ok <- install_frontend(frontend_info, tmp_dir, dest) do
+         :ok <- install_frontend(frontend, tmp_dir, dest) do
       File.rm_rf!(tmp_dir)
       Logger.info("Frontend #{label} installed to #{dest}")
     else
@@ -50,21 +51,17 @@ def install(name, opts \\ []) do
     end
   end
 
-  def dir(opts \\ []) do
-    if is_nil(opts[:static_dir]) do
-      Pleroma.Config.get!([:instance, :static_dir])
-    else
-      opts[:static_dir]
-    end
+  def dir do
+    Config.get!([:instance, :static_dir])
     |> Path.join("frontends")
   end
 
-  defp download_or_unzip(frontend_info, temp_dir, nil),
-    do: download_build(frontend_info, temp_dir)
+  defp download_or_unzip(%Frontend{file: nil} = frontend, dest),
+    do: download_build(frontend, dest)
 
-  defp download_or_unzip(_frontend_info, temp_dir, file) do
+  defp download_or_unzip(%Frontend{file: file}, dest) do
     with {:ok, zip} <- File.read(Path.expand(file)) do
-      unzip(zip, temp_dir)
+      unzip(zip, dest)
     end
   end
 
@@ -87,9 +84,13 @@ def unzip(zip, dest) do
     end
   end
 
-  defp download_build(frontend_info, dest) do
-    Logger.info("Downloading pre-built bundle for #{frontend_info["name"]}")
-    url = String.replace(frontend_info["build_url"], "${ref}", frontend_info["ref"])
+  def parse_build_url(%Frontend{ref: ref, build_url: build_url}) do
+    String.replace(build_url, "${ref}", ref)
+  end
+
+  defp download_build(%Frontend{name: name} = frontend, dest) do
+    Logger.info("Downloading pre-built bundle for #{name}")
+    url = parse_build_url(frontend)
 
     with {:ok, %{status: 200, body: zip_body}} <-
            Pleroma.HTTP.get(url, [], pool: :media, recv_timeout: 120_000) do
@@ -100,11 +101,46 @@ defp download_build(frontend_info, dest) do
     end
   end
 
-  defp install_frontend(frontend_info, source, dest) do
-    from = frontend_info["build_dir"] || "dist"
+  defp install_frontend(%Frontend{} = frontend, source, dest) do
+    from = frontend.build_dir || "dist"
     File.rm_rf!(dest)
     File.mkdir_p!(dest)
     File.cp_r!(Path.join([source, from]), dest)
     :ok
   end
+
+  # Converts a named frontend into a %Frontend{} struct
+  def get_named_frontend(name) do
+    [:frontends, :available, name]
+    |> Config.get(%{})
+    |> from_map()
+  end
+
+  def merge(%Frontend{} = fe1, %Frontend{} = fe2) do
+    Map.merge(fe1, fe2, fn _key, v1, v2 ->
+      # This only overrides things that are actually set
+      v1 || v2
+    end)
+  end
+
+  def validate!(%Frontend{ref: ref} = fe) when is_binary(ref), do: fe
+  def validate!(_), do: raise("No ref given or configured")
+
+  def from_map(frontend) when is_map(frontend) do
+    struct(Frontend, atomize_keys(frontend))
+  end
+
+  def to_map(%Frontend{} = frontend) do
+    frontend
+    |> Map.from_struct()
+    |> stringify_keys()
+  end
+
+  defp atomize_keys(map) do
+    Map.new(map, fn {k, v} -> {String.to_existing_atom(k), v} end)
+  end
+
+  defp stringify_keys(map) do
+    Map.new(map, fn {k, v} -> {to_string(k), v} end)
+  end
 end

lib/pleroma/web/admin_api/controllers/frontend_controller.ex

@@ -6,6 +6,7 @@ defmodule Pleroma.Web.AdminAPI.FrontendController do
   use Pleroma.Web, :controller
 
   alias Pleroma.Config
+  alias Pleroma.Frontend
   alias Pleroma.Web.Plugs.OAuthScopesPlug
 
   plug(Pleroma.Web.ApiSpec.CastAndValidate)
@@ -29,12 +30,18 @@ def index(conn, _params) do
   end
 
   def install(%{body_params: params} = conn, _params) do
-    with :ok <- Pleroma.Frontend.install(params.name, Map.delete(params, :name)) do
+    frontend = params_to_frontend(params)
+
+    with :ok <- Frontend.install(frontend) do
       index(conn, %{})
     end
   end
 
   defp installed do
-    File.ls!(Pleroma.Frontend.dir())
+    File.ls!(Frontend.dir())
+  end
+
+  defp params_to_frontend(params) when is_map(params) do
+    struct(Frontend, params)
   end
 end

test/pleroma/frontend_test.exs

@@ -18,31 +18,32 @@ defmodule Pleroma.FrontendTest do
   end
 
   test "it downloads and unzips a known frontend" do
-    clear_config([:frontends, :available], %{
-      "pleroma" => %{
-        "ref" => "fantasy",
-        "name" => "pleroma",
-        "build_url" => "http://gensokyo.2hu/builds/${ref}"
-      }
-    })
+    frontend = %Frontend{
+      ref: "fantasy",
+      name: "pleroma",
+      build_url: "http://gensokyo.2hu/builds/${ref}"
+    }
+
+    clear_config([:frontends, :available], %{"pleroma" => Frontend.to_map(frontend)})
 
     Tesla.Mock.mock(fn %{url: "http://gensokyo.2hu/builds/fantasy"} ->
       %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/frontend_dist.zip")}
     end)
 
-    Frontend.install("pleroma")
+    Frontend.install(frontend)
 
     assert File.exists?(Path.join([@dir, "frontends", "pleroma", "fantasy", "test.txt"]))
   end
 
   test "it also works given a file" do
-    clear_config([:frontends, :available], %{
-      "pleroma" => %{
-        "ref" => "fantasy",
-        "name" => "pleroma",
-        "build_dir" => ""
-      }
-    })
+    frontend = %Frontend{
+      ref: "fantasy",
+      name: "pleroma",
+      build_dir: "",
+      file: "test/fixtures/tesla_mock/frontend.zip"
+    }
+
+    clear_config([:frontends, :available], %{"pleroma" => Frontend.to_map(frontend)})
 
     folder = Path.join([@dir, "frontends", "pleroma", "fantasy"])
     previously_existing = Path.join([folder, "temp"])
@@ -50,23 +51,86 @@ test "it also works given a file" do
     File.write!(previously_existing, "yey")
     assert File.exists?(previously_existing)
 
-    Frontend.install("pleroma", file: "test/fixtures/tesla_mock/frontend.zip")
+    Frontend.install(frontend)
 
     assert File.exists?(Path.join([folder, "test.txt"]))
     refute File.exists?(previously_existing)
   end
 
   test "it downloads and unzips unknown frontends" do
+    frontend = %Frontend{
+      ref: "baka",
+      build_url: "http://gensokyo.2hu/madeup.zip",
+      build_dir: ""
+    }
+
     Tesla.Mock.mock(fn %{url: "http://gensokyo.2hu/madeup.zip"} ->
       %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/frontend.zip")}
     end)
 
-    Frontend.install("unknown",
-      ref: "baka",
-      build_url: "http://gensokyo.2hu/madeup.zip",
-      build_dir: ""
-    )
+    Frontend.install(frontend)
 
     assert File.exists?(Path.join([@dir, "frontends", "unknown", "baka", "test.txt"]))
   end
+
+  test "merge/2 only overrides nil values" do
+    fe1 = %Frontend{name: "pleroma"}
+    fe2 = %Frontend{name: "soapbox", ref: "fantasy"}
+    expected = %Frontend{name: "pleroma", ref: "fantasy"}
+    assert Frontend.merge(fe1, fe2) == expected
+  end
+
+  test "validate!/1 raises if :ref isn't set" do
+    fe = %Frontend{name: "pleroma"}
+    assert_raise(RuntimeError, fn -> Frontend.validate!(fe) end)
+  end
+
+  test "validate!/1 returns the frontend" do
+    fe = %Frontend{name: "pleroma", ref: "fantasy"}
+    assert Frontend.validate!(fe) == fe
+  end
+
+  test "from_map/1 parses a map into a %Frontend{} struct" do
+    map = %{"name" => "pleroma", "ref" => "fantasy"}
+    expected = %Frontend{name: "pleroma", ref: "fantasy"}
+    assert Frontend.from_map(map) == expected
+  end
+
+  test "to_map/1 returns the frontend as a map with string keys" do
+    frontend = %Frontend{name: "pleroma", ref: "fantasy"}
+
+    expected = %{
+      "name" => "pleroma",
+      "ref" => "fantasy",
+      "build_dir" => nil,
+      "build_url" => nil,
+      "custom-http-headers" => nil,
+      "file" => nil,
+      "git" => nil
+    }
+
+    assert Frontend.to_map(frontend) == expected
+  end
+
+  test "parse_build_url/1 replaces ${ref}" do
+    frontend = %Frontend{
+      name: "pleroma",
+      ref: "fantasy",
+      build_url: "http://gensokyo.2hu/builds/${ref}"
+    }
+
+    expected = "http://gensokyo.2hu/builds/fantasy"
+    assert Frontend.parse_build_url(frontend) == expected
+  end
+
+  test "dir/0 returns the frontend dir" do
+    assert Frontend.dir() == "test/frontend_static_test/frontends"
+  end
+
+  test "get_named_frontend/1 returns a frontend from the config" do
+    frontend = %Frontend{name: "pleroma", ref: "fantasy"}
+    clear_config([:frontends, :available], %{"pleroma" => Frontend.to_map(frontend)})
+
+    assert Frontend.get_named_frontend("pleroma") == frontend
+  end
 end