Refactor option validation and command execution helpers

lib/open_jtalk.ex

@@ -69,54 +69,7 @@ defmodule OpenJTalk do
   Returns the original `opts` on success.
   """
   @spec validate_options!(keyword) :: keyword
-  def validate_options!(opts) when is_list(opts) do
-    check_known_keys!(opts)
-    validate_playback_mode!(opts)
-    validate_timeout!(opts)
-    opts
-  end
-
-  defp check_known_keys!(opts) do
-    allowed = [
-      :timbre,
-      :pitch_shift,
-      :rate,
-      :gain,
-      :voice,
-      :dictionary,
-      :timeout,
-      :playback_mode,
-      :out
-    ]
-
-    unknown =
-      opts
-      |> Keyword.keys()
-      |> Enum.uniq()
-      |> Enum.reject(&(&1 in allowed))
-
-    if unknown != [] do
-      raise ArgumentError, "unknown option(s) for OpenJTalk: #{inspect(unknown)}"
-    end
-
-    :ok
-  end
-
-  defp validate_playback_mode!(opts) do
-    case Keyword.fetch(opts, :playback_mode) do
-      :error -> :ok
-      {:ok, mode} when mode in [:auto, :file, :stdin] -> :ok
-      {:ok, bad} -> raise ArgumentError, "invalid value for :playback_mode: #{inspect(bad)}"
-    end
-  end
-
-  defp validate_timeout!(opts) do
-    case Keyword.fetch(opts, :timeout) do
-      :error -> :ok
-      {:ok, t} when is_integer(t) and t >= 0 -> :ok
-      {:ok, bad} -> raise ArgumentError, "invalid value for :timeout : #{inspect(bad)}"
-    end
-  end
+  def validate_options!(opts), do: OpenJTalk.Options.validate!(opts)
 
   @doc """
   Synthesize `text` to a WAV file.
@@ -183,7 +136,7 @@ defmodule OpenJTalk do
   @spec say(binary, [say_option()]) :: :ok | {:error, term}
   def say(text, opts \\ []) do
     opts = validate_options!(opts)
-    mode = Keyword.get(opts, :playback_mode, :auto)
+    mode = OpenJTalk.Options.playback_mode(opts)
     do_say(text, mode, opts)
   end
 

lib/open_jtalk/command.ex

@@ -0,0 +1,26 @@
+defmodule OpenJTalk.Command do
+  @moduledoc false
+  # Small command-runner seam around MuonTrap.
+  #
+  # Tests can replace the runner with:
+  #
+  #     Application.put_env(:open_jtalk_elixir, :command_runner, MyRunner)
+  #
+  # where `MyRunner.cmd/3` has the same shape as `MuonTrap.cmd/3`.
+
+  @spec run(binary(), [binary()], keyword()) :: {binary(), non_neg_integer()}
+  def run(command, args, opts \\ [])
+      when is_binary(command) and is_list(args) and is_list(opts) do
+    command_runner().cmd(command, args, opts)
+  end
+
+  defp command_runner() do
+    Application.get_env(:open_jtalk_elixir, :command_runner, OpenJTalk.Command.MuonTrap)
+  end
+end
+
+defmodule OpenJTalk.Command.MuonTrap do
+  @moduledoc false
+
+  def cmd(command, args, opts), do: MuonTrap.cmd(command, args, opts)
+end

lib/open_jtalk/options.ex

@@ -0,0 +1,83 @@
+defmodule OpenJTalk.Options do
+  @moduledoc false
+  # Shared option validation and normalization for synthesis and playback.
+
+  @allowed_keys [
+    :timbre,
+    :pitch_shift,
+    :rate,
+    :gain,
+    :voice,
+    :dictionary,
+    :timeout,
+    :playback_mode,
+    :out
+  ]
+
+  @playback_modes [:auto, :file, :stdin]
+  @default_timeout 20_000
+
+  @doc "Validate options for synthesis and playback. Returns the original options."
+  @spec validate!(keyword()) :: keyword()
+  def validate!(opts) when is_list(opts) do
+    if Keyword.keyword?(opts) do
+      check_known_keys!(opts)
+      validate_playback_mode!(opts)
+      validate_timeout!(opts)
+      opts
+    else
+      raise ArgumentError, "OpenJTalk options must be a keyword list"
+    end
+  end
+
+  def validate!(_opts) do
+    raise ArgumentError, "OpenJTalk options must be a keyword list"
+  end
+
+  @doc "Return the requested playback mode, defaulting to `:auto`."
+  @spec playback_mode(keyword()) :: OpenJTalk.playback_mode()
+  def playback_mode(opts), do: Keyword.get(opts, :playback_mode, :auto)
+
+  @doc "Normalize a timeout value to the default when it is absent or invalid."
+  @spec normalize_timeout(term()) :: non_neg_integer()
+  def normalize_timeout(nil), do: @default_timeout
+  def normalize_timeout(value) when is_integer(value) and value >= 0, do: value
+  def normalize_timeout(_value), do: @default_timeout
+
+  @doc "Clamp a numeric value between lower and upper bounds."
+  @spec clamp(number(), number(), number()) :: number()
+  def clamp(value, lower, upper)
+      when is_number(value) and is_number(lower) and is_number(upper) do
+    value |> min(upper) |> max(lower)
+  end
+
+  defp check_known_keys!(opts) do
+    unknown =
+      opts
+      |> Keyword.keys()
+      |> Enum.uniq()
+      |> Enum.reject(&(&1 in @allowed_keys))
+
+    if unknown != [] do
+      raise ArgumentError, "unknown option(s) for OpenJTalk: #{inspect(unknown)}"
+    end
+
+    :ok
+  end
+
+  defp validate_playback_mode!(opts) do
+    case Keyword.fetch(opts, :playback_mode) do
+      :error -> :ok
+      {:ok, mode} when mode in @playback_modes -> :ok
+      {:ok, bad} -> raise ArgumentError, "invalid value for :playback_mode: #{inspect(bad)}"
+    end
+  end
+
+  defp validate_timeout!(opts) do
+    case Keyword.fetch(opts, :timeout) do
+      :error -> :ok
+      {:ok, timeout} when is_integer(timeout) and timeout >= 0 -> :ok
+      {:ok, bad} -> raise ArgumentError, "invalid value for :timeout: #{inspect(bad)}"
+    end
+  end
+end

lib/open_jtalk/synth.ex

@@ -2,13 +2,12 @@ defmodule OpenJTalk.Synth do
   @moduledoc false
   # Build and run the `open_jtalk` CLI for synthesis.
 
-  alias OpenJTalk.Assets
+  alias OpenJTalk.{Assets, Command, Options}
 
   @typedoc "Use the canonical top-level synth option type."
   @type option :: OpenJTalk.synth_option()
 
   @base_alpha 0.55
-  @default_timeout 20_000
 
   @doc """
   Build the `open_jtalk` argv to synthesize into `wav_out`.
@@ -22,10 +21,10 @@ defmodule OpenJTalk.Synth do
     with {:ok, bin} <- Assets.resolve_bin(),
          {:ok, dic} <- Assets.resolve_dictionary(opts[:dictionary]),
          {:ok, voice} <- Assets.resolve_voice(opts[:voice]) do
-      alpha = clamp(@base_alpha + (opts[:timbre] || 0.0), 0.0, 1.0)
-      rate = clamp(opts[:rate] || 1.0, 0.5, 2.0)
-      fm = clamp(opts[:pitch_shift] || 0, -24, 24)
-      gain = clamp(opts[:gain] || 0, -20, 20)
+      alpha = Options.clamp(@base_alpha + (opts[:timbre] || 0.0), 0.0, 1.0)
+      rate = Options.clamp(opts[:rate] || 1.0, 0.5, 2.0)
+      fm = Options.clamp(opts[:pitch_shift] || 0, -24, 24)
+      gain = Options.clamp(opts[:gain] || 0, -20, 20)
 
       args =
         [
@@ -49,16 +48,16 @@ defmodule OpenJTalk.Synth do
   end
 
   @doc """
-  Run the `open_jtalk` command via MuonTrap.
+  Run the `open_jtalk` command via the configured command runner.
 
   Returns `{:ok, stdout}` or `{:error, {:open_jtalk_exit, status, trimmed_output}}`.
   """
   @spec run([binary], non_neg_integer() | nil) :: {:ok, binary} | {:error, term}
   def run([bin | args], timeout_ms) do
     env = [{"LC_ALL", "C"}] ++ ld_path_env()
-    timeout = normalize_timeout(timeout_ms)
+    timeout = Options.normalize_timeout(timeout_ms)
 
-    case MuonTrap.cmd(bin, args, env: env, stderr_to_stdout: true, timeout: timeout) do
+    case Command.run(bin, args, env: env, stderr_to_stdout: true, timeout: timeout) do
       {out, 0} -> {:ok, out}
       {out, status} -> {:error, {:open_jtalk_exit, status, String.trim(out)}}
     end
@@ -82,12 +81,4 @@ defmodule OpenJTalk.Synth do
       []
     end
   end
-
-  defp normalize_timeout(nil), do: @default_timeout
-  defp normalize_timeout(int) when is_integer(int) and int >= 0, do: int
-  defp normalize_timeout(_), do: @default_timeout
-
-  defp clamp(x, lo, hi) when is_number(x) and is_number(lo) and is_number(hi) do
-    x |> min(hi) |> max(lo)
-  end
 end

test/open_jtalk/command_test.exs

@@ -0,0 +1,32 @@
+defmodule OpenJTalk.CommandTest do
+  use ExUnit.Case, async: false
+
+  alias OpenJTalk.Command
+
+  setup do
+    original_runner = Application.get_env(:open_jtalk_elixir, :command_runner)
+    Application.put_env(:open_jtalk_elixir, :command_runner, __MODULE__.Runner)
+
+    on_exit(fn ->
+      if original_runner do
+        Application.put_env(:open_jtalk_elixir, :command_runner, original_runner)
+      else
+        Application.delete_env(:open_jtalk_elixir, :command_runner)
+      end
+    end)
+  end
+
+  test "run/3 delegates to configured command runner" do
+    assert {"ok", 0} = Command.run("echo", ["hello"], timeout: 123)
+
+    assert_received {:cmd, "echo", ["hello"], [timeout: 123]}
+  end
+
+  defmodule Runner do
+    def cmd(command, args, opts) do
+      send(self(), {:cmd, command, args, opts})
+
+      {"ok", 0}
+    end
+  end
+end

test/open_jtalk/options_test.exs

@@ -0,0 +1,54 @@
+defmodule OpenJTalk.OptionsTest do
+  use ExUnit.Case, async: true
+
+  alias OpenJTalk.Options
+
+  test "validate!/1 returns valid options unchanged" do
+    opts = [timbre: 0.1, pitch_shift: -2, rate: 1.2, gain: 3, playback_mode: :file]
+
+    assert Options.validate!(opts) == opts
+    assert OpenJTalk.validate_options!(opts) == opts
+  end
+
+  test "validate!/1 rejects unknown options" do
+    assert_raise ArgumentError, "unknown option(s) for OpenJTalk: [:bogus]", fn ->
+      Options.validate!(bogus: true)
+    end
+  end
+
+  test "validate!/1 rejects invalid playback mode" do
+    assert_raise ArgumentError, "invalid value for :playback_mode: :bogus", fn ->
+      Options.validate!(playback_mode: :bogus)
+    end
+  end
+
+  test "validate!/1 rejects invalid timeout" do
+    assert_raise ArgumentError, "invalid value for :timeout: -1", fn ->
+      Options.validate!(timeout: -1)
+    end
+  end
+
+  test "validate!/1 rejects non-keyword options" do
+    assert_raise ArgumentError, "OpenJTalk options must be a keyword list", fn ->
+      Options.validate!([:not_a_keyword])
+    end
+  end
+
+  test "playback_mode/1 defaults to auto" do
+    assert Options.playback_mode([]) == :auto
+    assert Options.playback_mode(playback_mode: :stdin) == :stdin
+  end
+
+  test "normalize_timeout/1 defaults absent or invalid values" do
+    assert Options.normalize_timeout(nil) == 20_000
+    assert Options.normalize_timeout(:bad) == 20_000
+    assert Options.normalize_timeout(0) == 0
+    assert Options.normalize_timeout(123) == 123
+  end
+
+  test "clamp/3 limits numeric values" do
+    assert Options.clamp(-1.0, 0.0, 1.0) == 0.0
+    assert Options.clamp(0.5, 0.0, 1.0) == 0.5
+    assert Options.clamp(2.0, 0.0, 1.0) == 1.0
+  end
+end