Convert to ports (#5)

* Remove erlexec dependency

* feat: async verify worker has started

* test: move infinite to script

* fix: timeout response

* fix: unix compile [Closes #169398412]

* feat: allow database patterns as worker param

This allows us to expand paths in an application, which is not possible from the configuration file.

* OTP 22 compatible with Elixir 1.7-1.10

* Install make on CI server

libmagic-dev contains magic.h

Fix this error:

  ** (Mix) Could not compile with "make" (exit status: 2).

* Fix make install

* Fix plurality

* Fix credo warning

* Allow multiple error messages

OS X and Linux return different errors messages.

* Allow named GenServer processes

* Types

* Disable test broken in ci

Works locally, not in CI.

test/gen_magic_test.exs:50
     ** (EXIT from #PID<0.1672.0>) shutdown

3 files changed, 90 insertions, 78 deletions

diff --git a/lib/gen_magic.ex b/lib/gen_magic.ex
index 93b3545..c60e357 100644
--- a/lib/gen_magic.ex
+++ b/lib/gen_magic.ex
@@ -3,25 +3,17 @@ defmodule GenMagic do
   Top-level namespace for GenMagic, the libMagic client for Elixir.
   """
 
+  alias GenMagic.ApprenticeServer
+
   @doc """
   Top-level convenience function which creates an ad-hoc process. Usually
   this will be wrapped in a pool established by the author of the application
   that uses the library.
   """
   def perform(path) do
-    {:ok, pid} = __MODULE__.ApprenticeServer.start_link()
-    result = GenServer.call(pid, {:perform, path})
+    {:ok, pid} = ApprenticeServer.start_link([])
+    result = ApprenticeServer.file(pid, path)
     :ok = GenServer.stop(pid)
     result
   end
-
-  def perform_infinite(path) do
-    {:ok, pid} = __MODULE__.ApprenticeServer.start_link()
-    perform_infinite(path, pid)
-  end
-
-  defp perform_infinite(path, pid, count \\ 0) do
-    IO.inspect([count, GenServer.call(pid, {:perform, path})])
-    perform_infinite(path, pid, count + 1)
-  end
 end
diff --git a/lib/gen_magic/apprentice_server.ex b/lib/gen_magic/apprentice_server.ex
index 815d14f..004ba12 100644
--- a/lib/gen_magic/apprentice_server.ex
+++ b/lib/gen_magic/apprentice_server.ex
@@ -1,87 +1,96 @@
 defmodule GenMagic.ApprenticeServer do
   @moduledoc """
   Provides access to the underlying libMagic client which performs file introspection.
+
+  This server needs to be supervised, as if it receives any unexpected error, it will terminate.
   """
 
   alias GenMagic.Configuration
   use GenServer
 
-  def start_link(args \\ []) do
-    GenServer.start_link(__MODULE__, args)
+  @type result() :: [mime_type: String.t(), encoding: String.t(), content: String.t()]
+
+  @worker_timeout Configuration.get_worker_timeout()
+
+  def start_link([]) do
+    database_patterns = Configuration.get_database_patterns()
+    GenServer.start_link(__MODULE__, database_patterns: database_patterns)
   end
 
-  defmodule State do
-    defstruct pid: nil, ospid: nil, started: false, count: 0
+  def start_link(name: name) do
+    database_patterns = Configuration.get_database_patterns()
+    GenServer.start_link(__MODULE__, [database_patterns: database_patterns], name: name)
   end
 
-  def init(_) do
-    {:ok, %State{}}
+  def start_link([database_patterns: _] = args) do
+    GenServer.start_link(__MODULE__, args)
   end
 
-  def handle_call(message, from, %{started: false} = state) do
-    case start(state) do
-      {:ok, state} -> handle_call(message, from, state)
-      {:error, _} = error -> {:reply, error, state}
-    end
+  def start_link(database_patterns: database_patterns, name: name) do
+    GenServer.start_link(__MODULE__, [database_patterns: database_patterns], name: name)
   end
 
-  def handle_call({:perform, path}, _, state) do
-    max_count = Configuration.get_recycle_threshold()
+  @doc """
+  Determine a file type.
+  """
+  @spec file(pid() | atom(), String.t()) :: {:ok, result()} | {:error, term()}
+  def file(pid, path) do
+    GenServer.call(pid, {:file, path})
+  end
 
-    case {run(path, state), state.count + 1} do
-      {{:error, :worker_failure} = reply, _} ->
-        {:reply, reply, stop(state)}
+  def init(database_patterns: database_patterns) do
+    {worker_path, worker_arguments} = Configuration.get_worker_command(database_patterns)
 
-      {reply, ^max_count} ->
-        {:reply, reply, stop(state)}
+    case File.stat(worker_path) do
+      {:ok, _} ->
+        port =
+          Port.open({:spawn_executable, to_charlist(worker_path)}, [
+            :stderr_to_stdout,
+            :binary,
+            :exit_status,
+            args: worker_arguments
+          ])
 
-      {reply, count} ->
-        {:reply, reply, %{state | count: count}}
-    end
-  end
+        {:ok, port, {:continue, :verify_port}}
 
-  def handle_info({:DOWN, _, :process, pid, :normal}, state) do
-    case state.pid do
-      ^pid -> {:noreply, %State{}}
-      _ -> {:noreply, state}
+      {:error, _} = e ->
+        {:stop, e}
     end
   end
 
-  defp start(%{started: false} = state) do
-    worker_command = Configuration.get_worker_command()
-    worker_options = [stdin: true, stdout: true, stderr: true, monitor: true]
-    worker_timeout = Configuration.get_worker_timeout()
-    {:ok, pid, ospid} = Exexec.run(worker_command, worker_options)
-    state = %{state | started: true, pid: pid, ospid: ospid}
-
+  # The process sends us an ack when it has read the databases
+  # and is ready to receive data.
+  # OTP-13019 Requires OTP 21
+  def handle_continue(:verify_port, port) do
     receive do
-      {:stdout, ^ospid, "ok\n"} -> {:ok, state}
-      {:stdout, ^ospid, "ok\r\n"} -> {:ok, state}
+      {_, {:data, "ok\n"}} ->
+        {:noreply, port}
     after
-      worker_timeout ->
-        {:error, :worker_failure}
+      10_000 ->
+        {:stop, :nok, port}
     end
   end
 
-  defp stop(%{started: true} = state) do
-    :normal = Exexec.stop_and_wait(state.ospid)
-    %State{}
-  end
-
-  defp run(path, %{pid: pid, ospid: ospid} = _state) do
-    worker_timeout = Configuration.get_worker_timeout()
-    :ok = Exexec.send(pid, "file; " <> path <> "\n")
+  def handle_call({:file, path}, _from, port) do
+    cmd = "file; " <> path <> "\n"
+    send(port, {self(), {:command, cmd}})
 
     receive do
-      {stream, ^ospid, message} ->
-        handle_response(stream, message)
+      {_, {:data, "ok; " <> message}} ->
+        {:reply, parse_response(message), port}
+
+      {_, {:data, "error; " <> message}} ->
+        {:reply, {:error, String.trim(message)}, port}
+
+      {_, {:data, _}} ->
+        {:stop, :shutdown, {:error, :malformed}, port}
     after
-      worker_timeout ->
-        {:error, :worker_failure}
+      @worker_timeout ->
+        {:stop, :shutdown, {:error, :worker_failure}, port}
     end
   end
 
-  defp handle_response(:stdout, "ok; " <> message) do
+  defp parse_response(message) do
     case message |> String.trim() |> String.split("\t") do
       [mime_type, encoding, content] ->
         {:ok, [mime_type: mime_type, encoding: encoding, content: content]}
@@ -91,16 +100,20 @@ defmodule GenMagic.ApprenticeServer do
     end
   end
 
-  defp handle_response(:stderr, "error; " <> message) do
-    {:error, String.trim(message)}
+  def terminate(_reason, port) do
+    case send(port, {self(), :close}) do
+      {_, :close} -> :ok
+      _ -> :ok
+    end
+  end
+
+  def handle_info({_, {:exit_status, 1}}, port) do
+    {:stop, :shutdown, port}
   end
 
-  # TODO handle late responses under load
-  # 17:13:47.808 [error] GenServer #PID<0.199.0> terminating
-  # ** (FunctionClauseError) no function clause matching in GenMagic.ApprenticeServer.handle_info/2
-  #     (gen_magic) lib/gen_magic/apprentice_server.ex:41: GenMagic.ApprenticeServer.handle_info({:stderr, 12304, "\n"}, %GenMagic.ApprenticeServer.State{count: 2, ospid: 12304, pid: #PID<0.243.0>, started: true})
-  #     (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
-  #     (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
-  #     (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3
-  # Last message: {:stderr, 12304, "\n"}
+  # Server is overloaded - late replies
+  # Normally caused only when previous call errors
+  def handle_info({_, {:data, _}}, port) do
+    {:stop, :shutdown, port}
+  end
 end
diff --git a/lib/gen_magic/configuration.ex b/lib/gen_magic/configuration.ex
index 15334e2..c78af34 100644
--- a/lib/gen_magic/configuration.ex
+++ b/lib/gen_magic/configuration.ex
@@ -5,11 +5,11 @@ defmodule GenMagic.Configuration do
 
   @otp_app Mix.Project.config()[:app]
 
-  def get_worker_command do
-    database_paths = get_database_paths()
+  def get_worker_command(patterns) do
+    database_paths = paths(patterns)
     worker_path = Path.join(:code.priv_dir(@otp_app), get_worker_name())
-    worker_arguments = Enum.map(database_paths, &("--file " <> &1))
-    Enum.join([worker_path | worker_arguments], " ")
+    worker_arguments = Enum.flat_map(database_paths, &["--file", &1])
+    {worker_path, worker_arguments}
   end
 
   def get_worker_name do
@@ -24,11 +24,18 @@ defmodule GenMagic.Configuration do
     get_env(:recycle_threshold)
   end
 
-  def get_database_paths do
-    get_env(:database_patterns) |> Enum.flat_map(&Path.wildcard/1)
+  def get_database_patterns do
+    case get_env(:database_patterns) do
+      nil -> []
+      l when is_list(l) -> l
+      s when is_binary(s) -> [s]
+    end
   end
 
   defp get_env(key) do
     Application.get_env(@otp_app, key)
   end
+
+  defp paths(patterns),
+    do: patterns |> Enum.flat_map(&Path.wildcard/1) |> Enum.filter(&File.exists?/1)
 end