diff --git a/README.md b/README.md index 5dd7728..0413c73 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ goes to another host. To turn them on, set `redirect: true` or `retry: :safe_tra - `:lens`: `:default`, `:programming`, `:forums`, `:pdfs`, `:non_commercial`, or `:world_news`. - `:sort`: `:recency`, `:website`, or `:ad_trackers`. - `:time`: `:day`, `:week`, `:month`, or `:year`. -- `:from` and `:to`: a `YYYY-MM-DD` date range. You cannot use them with `:time`. +- `:from` and `:to`: a `YYYY-MM-DD` date range. `:from` must not be after `:to`. You cannot use them with `:time`. - `:site`: adds a `site:` filter. - `:filetype`: adds a `filetype:` filter. - `:verbatim`: `true` turns off query expansion. diff --git a/lib/kagi/search.ex b/lib/kagi/search.ex index d4f0278..b7c38b1 100644 --- a/lib/kagi/search.ex +++ b/lib/kagi/search.ex @@ -79,8 +79,8 @@ defmodule Kagi.Search do @spec query_params(String.t() | [String.t()], keyword()) :: {:ok, keyword()} | {:error, Error.t()} defp query_params(query, options) do - with {:ok, query} <- build_query(query, options), - {:ok, options} <- validate_options(options) do + with {:ok, options} <- validate_options(options), + {:ok, query} <- build_query(query, options) do params = [ {:plain, :r, options[:region]}, {:mapped, :l, options[:lens], &lens_value/1}, @@ -97,13 +97,34 @@ defmodule Kagi.Search do @spec validate_options(keyword()) :: {:ok, keyword()} | {:error, Error.t()} defp validate_options(options) do - with :ok <- validate_time_range(options), - :ok <- validate_date(:from, options[:from]), - :ok <- validate_date(:to, options[:to]) do + with :ok <- validate_string(:site, options[:site]), + :ok <- validate_string(:filetype, options[:filetype]), + :ok <- validate_string(:region, options[:region]), + :ok <- validate_verbatim(options[:verbatim]), + :ok <- validate_time_range(options), + {:ok, from} <- parse_date(:from, options[:from]), + {:ok, to} <- parse_date(:to, options[:to]), + :ok <- validate_date_order(from, to) do {:ok, options} end end + @spec validate_string(:site | :filetype | :region, term()) :: :ok | {:error, Error.t()} + defp validate_string(_key, nil), do: :ok + defp validate_string(_key, value) when is_binary(value) and value != "", do: :ok + + defp validate_string(key, value) do + {:error, + Error.new(:invalid_option, ":#{key} must be a non-empty string, got: #{inspect(value)}")} + end + + @spec validate_verbatim(term()) :: :ok | {:error, Error.t()} + defp validate_verbatim(value) when is_nil(value) or is_boolean(value), do: :ok + + defp validate_verbatim(value) do + {:error, Error.new(:invalid_option, ":verbatim must be a boolean, got: #{inspect(value)}")} + end + @spec validate_time_range(keyword()) :: :ok | {:error, Error.t()} defp validate_time_range(options) do if options[:time] && (options[:from] || options[:to]) do @@ -113,21 +134,31 @@ defmodule Kagi.Search do end end - @spec validate_date(:from | :to, term()) :: :ok | {:error, Error.t()} - defp validate_date(_key, nil), do: :ok + @spec parse_date(:from | :to, term()) :: {:ok, Date.t() | nil} | {:error, Error.t()} + defp parse_date(_key, nil), do: {:ok, nil} - defp validate_date(key, date) when is_binary(date) do - if Regex.match?(~r/^\d{4}-\d{2}-\d{2}$/, date) do - :ok + defp parse_date(key, value) do + with true <- is_binary(value), + {:ok, date} <- Date.from_iso8601(value) do + {:ok, date} else - {:error, Error.new(:invalid_option, ":#{key} must use YYYY-MM-DD")} + _invalid -> + {:error, + Error.new(:invalid_option, ":#{key} must be a YYYY-MM-DD date, got: #{inspect(value)}")} end end - defp validate_date(key, _date) do - {:error, Error.new(:invalid_option, ":#{key} must use YYYY-MM-DD")} + @spec validate_date_order(Date.t() | nil, Date.t() | nil) :: :ok | {:error, Error.t()} + defp validate_date_order(%Date{} = from, %Date{} = to) do + if Date.compare(from, to) == :gt do + {:error, Error.new(:invalid_option, ":from must not be after :to")} + else + :ok + end end + defp validate_date_order(_from, _to), do: :ok + @spec build_query(String.t() | [String.t()], keyword()) :: {:ok, String.t()} | {:error, Error.t()} defp build_query(query, options) do diff --git a/lib/kagi/summary.ex b/lib/kagi/summary.ex index 2a5d34c..40f0f9a 100644 --- a/lib/kagi/summary.ex +++ b/lib/kagi/summary.ex @@ -94,17 +94,20 @@ defmodule Kagi.Summary do @spec query_params(String.t(), keyword()) :: {:ok, keyword()} | {:error, Error.t()} defp query_params(url, options) do - with {:ok, summary_type} <- summary_type(Keyword.get(options, :type, :summary)) do - {:ok, - [ - url: url, - stream: "1", - target_language: Keyword.get(options, :lang, "EN"), - summary_type: summary_type - ]} + with {:ok, summary_type} <- summary_type(Keyword.get(options, :type, :summary)), + {:ok, lang} <- target_language(Keyword.get(options, :lang, "EN")) do + {:ok, [url: url, stream: "1", target_language: lang, summary_type: summary_type]} end end + @spec target_language(term()) :: {:ok, String.t()} | {:error, Error.t()} + defp target_language(lang) when is_binary(lang) and lang != "", do: {:ok, lang} + + defp target_language(value) do + {:error, + Error.new(:invalid_option, ":lang must be a non-empty string, got: #{inspect(value)}")} + end + @spec summary_type(term()) :: {:ok, String.t()} | {:error, Error.t()} defp summary_type(:summary), do: {:ok, "summary"} defp summary_type(:takeaway), do: {:ok, "takeaway"} diff --git a/test/kagi/search_test.exs b/test/kagi/search_test.exs index fa85a78..45ebab9 100644 --- a/test/kagi/search_test.exs +++ b/test/kagi/search_test.exs @@ -96,6 +96,71 @@ defmodule Kagi.SearchTest do refute_received :network end + @spec client_capturing_url(pid()) :: Kagi.Client.t() + defp client_capturing_url(test_pid) do + adapter = + Kagi.FakeAdapter.put(Kagi.FakeAdapter, fn request -> + send(test_pid, {:network, request.url}) + {request, Req.Response.new(status: 200, body: ~s(
))} + end) + + %Kagi.Client{session_token: "token", req_options: [adapter: adapter]} + end + + test "rejects a :site, :filetype, or :region that is not a non-empty string before any request" do + client = client_capturing_url(self()) + + for key <- [:site, :filetype, :region], value <- [%{a: 1}, 42, ""] do + assert {:error, %Error{reason: :invalid_option, message: message}} = + Kagi.search(client, "elixir", [{key, value}]) + + assert message =~ ":#{key} must be a non-empty string, got: #{inspect(value)}" + end + + refute_received {:network, _url} + end + + test "rejects a :verbatim that is not a boolean" do + client = client_capturing_url(self()) + + for value <- ["false", 1] do + assert {:error, %Error{reason: :invalid_option, message: message}} = + Kagi.search(client, "elixir", verbatim: value) + + assert message =~ ":verbatim must be a boolean" + end + + refute_received {:network, _url} + end + + test "rejects :from with a trailing newline, an impossible date 2024-13-45, and a range where :from is after :to" do + client = client_capturing_url(self()) + + for options <- [[from: "2024-01-01\n"], [from: "2024-13-45"], [to: ~D[2024-01-01]]] do + assert {:error, %Error{reason: :invalid_option, message: message}} = + Kagi.search(client, "elixir", options) + + assert message =~ "must be a YYYY-MM-DD date" + end + + assert {:error, %Error{reason: :invalid_option, message: ":from must not be after :to"}} = + Kagi.search(client, "elixir", from: "2025-01-01", to: "2024-01-01") + + refute_received {:network, _url} + end + + test "accepts a valid :from/:to range and sends from_date and to_date unchanged" do + client = client_capturing_url(self()) + + assert {:ok, %Search{}} = + Kagi.search(client, "elixir", from: "2024-01-01", to: "2024-01-01", verbatim: true) + + assert_received {:network, url} + + assert %{"from_date" => "2024-01-01", "to_date" => "2024-01-01", "verbatim" => "1"} = + URI.decode_query(url.query) + end + test "rejects queries that are not strings or lists of strings" do client = %Kagi.Client{session_token: "token"} diff --git a/test/kagi/summary_test.exs b/test/kagi/summary_test.exs index c3f3d6d..3494b73 100644 --- a/test/kagi/summary_test.exs +++ b/test/kagi/summary_test.exs @@ -123,6 +123,19 @@ defmodule Kagi.SummaryTest do end end + test "rejects a :lang that is not a non-empty string" do + client = client_capturing_timeout(self(), []) + + for lang <- [%{}, :de, ""] do + assert {:error, %Error{reason: :invalid_option, message: message}} = + Kagi.summarize(client, "https://example.com", lang: lang) + + assert message =~ ":lang must be a non-empty string, got: #{inspect(lang)}" + end + + refute_received {:receive_timeout, _timeout} + end + defp fixture(name) do "test/fixtures/summary/#{name}" |> File.read!()