From d57e3429e4533cbb695c06db4b20d2a4b1ff229a Mon Sep 17 00:00:00 2001 From: MareStare Date: Mon, 12 May 2025 08:26:34 +0000 Subject: [PATCH 01/26] Start parser snapshot test --- .formatter.exs | 2 +- lib/philomena/images/query.ex | 4 ++ lib/philomena_query/parse/parser.ex | 77 +++++++++++++++++++--------- mix.exs | 10 +++- mix.lock | 1 + test/philomena/images/query-test.yml | 2 + test/philomena/images/query_test.exs | 14 +++++ 7 files changed, 83 insertions(+), 27 deletions(-) create mode 100644 test/philomena/images/query-test.yml create mode 100644 test/philomena/images/query_test.exs diff --git a/.formatter.exs b/.formatter.exs index 8a6391c6a..4b3ce1a7c 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,5 +1,5 @@ [ - import_deps: [:ecto, :phoenix], + import_deps: [:assert_value, :ecto, :phoenix], inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"], subdirectories: ["priv/*/migrations"] ] diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 4d41252a4..2f006a629 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -197,6 +197,9 @@ defmodule Philomena.Images.Query do end end + @type options :: [user: %{role: String.t()}, watch: boolean()] + + @spec parse(Parser.options(), options(), String.t()) :: Parser.result() defp parse(fields, context, query_string) do case prepare_context(context, query_string) do {:ok, context} -> @@ -214,6 +217,7 @@ defmodule Philomena.Images.Query do defp fields_for(%{role: role}) when role in ~W(moderator admin), do: moderator_fields() defp fields_for(_), do: raise(ArgumentError, "Unknown user role.") + @spec compile(String.t(), options()) :: Parser.result() def compile(query_string, opts \\ []) do user = Keyword.get(opts, :user) watch = Keyword.get(opts, :watch, false) diff --git a/lib/philomena_query/parse/parser.ex b/lib/philomena_query/parse/parser.ex index ab1870055..9dbb80b80 100644 --- a/lib/philomena_query/parse/parser.ex +++ b/lib/philomena_query/parse/parser.ex @@ -53,6 +53,9 @@ defmodule PhilomenaQuery.Parse.Parser do @typedoc "Query in the search engine JSON query language." @type query :: map() + @typedoc "Result of calling the `parse/3` function." + @type result :: {:ok, query()} | {:error, String.t()} + @typedoc "Whether the default field is `:term` (not analyzed) or `:ngram` (analyzed)." @type default_field_type :: :term | :ngram @@ -100,9 +103,7 @@ defmodule PhilomenaQuery.Parse.Parser do transforms: %{String.t() => transform()}, aliases: %{String.t() => String.t()}, no_downcase_fields: [String.t()], - normalizations: %{String.t() => normalizer()}, - __fields__: map(), - __data__: context() + normalizations: %{String.t() => normalizer()} } defstruct [ @@ -126,25 +127,55 @@ defmodule PhilomenaQuery.Parse.Parser do @max_clause_count 512 + @typedoc "Options for the new/1 function." + @type options :: [ + # Booleans + bool_fields: [String.t()], + + # Dates + date_fields: [String.t()], + + # Floating point numbers + float_fields: [String.t()], + + # Signed integers + int_fields: [String.t()], + + # Numeric values (unsigned integers without fuzzing + # or range queries) + numeric_fields: [String.t()], + + # IP CIDR masks + ip_fields: [String.t()], + + # Wildcardable fields which are searched as the exact value + literal_fields: [String.t()], + + # Wildcardable fields which are searched as stemmed values + ngram_fields: [String.t()], + + # Fields which do not exist on the document and are created by a callback + custom_fields: [String.t()], + + # A map of custom field names to transform functions + transforms: %{String.t() => transform()}, + + # A map of field names to the names they should have in the search engine + aliases: %{String.t() => String.t()}, + + # A list of field names which do not have string downcasing applied + no_downcase_fields: [String.t()], + + # a map of field names to normalization functions (see `t:normalizer/0`) + normalizations: %{String.t() => normalizer()} + ] + @doc """ Creates a `Parser` suitable for safely parsing user-input queries. - Fields refer to attributes of the indexed document which will be searchable with - `m:PhilomenaQuery.Search`. - - Available options: - - `bool_fields` - a list of field names parsed as booleans - - `float_fields` - a list of field names parsed as floats - - `int_fields` - a list of field names parsed as signed integers - - `numeric_fields` - a list of field names parsed as numeric values (unsigned integers without fuzzing or range queries) - - `ip_fields` - a list of field names parsed as IP CIDR masks - - `literal_fields` - wildcardable fields which are searched as the exact value - - `ngram_fields` - wildcardable fields which are searched as stemmed values - - `custom_fields` - fields which do not exist on the document and are created by a callback - - `transforms` - a map of custom field names to transform functions - - `aliases` - a map of field names to the names they should have in the search engine - - `no_downcase_fields` - a list of field names which do not have string downcasing applied - - `normalizations` - a map of field names to normalization functions (see `t:normalizer/0`) + Fields refer to attributes of the indexed document which will be searchable + with `m:PhilomenaQuery.Search`. See the available options described in the + `t:options/0` typespec. ## Example @@ -160,7 +191,7 @@ defmodule PhilomenaQuery.Parse.Parser do Parser.new(options) """ - @spec new(keyword()) :: t() + @spec new(options()) :: t() def new(options) do parser = struct(Parser, options) @@ -205,10 +236,8 @@ defmodule PhilomenaQuery.Parse.Parser do {:error, "Imbalanced parentheses."} """ - @spec parse(t(), String.t(), context()) :: {:ok, query()} | {:error, String.t()} - def parse(parser, input, context \\ nil) - - def parse(%Parser{} = parser, input, context) do + @spec parse(t(), String.t(), context()) :: result() + def parse(%Parser{} = parser, input, context \\ nil) do parser = %{parser | __data__: context} with {:ok, input} <- coerce_string(input), diff --git a/mix.exs b/mix.exs index 53b58c28d..0278c6a64 100644 --- a/mix.exs +++ b/mix.exs @@ -44,7 +44,6 @@ defmodule Philomena.MixProject do {:phoenix_view, "~> 2.0"}, {:phoenix_live_reload, "~> 1.4", only: :dev}, {:gettext, "~> 0.22"}, - {:jason, "~> 1.4"}, {:bandit, "~> 1.2"}, {:slime, "~> 1.3.1"}, {:phoenix_slime, "~> 0.13", @@ -70,6 +69,10 @@ defmodule Philomena.MixProject do {:sweet_xml, "~> 0.7"}, {:inet_cidr, "~> 1.0"}, + # Serialization + {:jason, "~> 1.4"}, + {:yaml_elixir, "~> 2.11", only: [:test]}, + # SMTP {:swoosh, "~> 1.17"}, {:mua, "~> 0.2.0"}, @@ -93,7 +96,10 @@ defmodule Philomena.MixProject do # Fixes for Elixir v1.15+ {:canary, "~> 1.1", - github: "marcinkoziej/canary", ref: "704debde7a2c0600f78c687807884bf37c45bd79"} + github: "marcinkoziej/canary", ref: "704debde7a2c0600f78c687807884bf37c45bd79"}, + + # Automated testing + {:assert_value, "~> 0.10.4", only: [:dev, :test]} ] end diff --git a/mix.lock b/mix.lock index c62f7bf0c..fdc68fbdf 100644 --- a/mix.lock +++ b/mix.lock @@ -1,4 +1,5 @@ %{ + "assert_value": {:hex, :assert_value, "0.10.4", "0f0e528f048734e1b9c7ed947696a18d387cb052b68c20d973ed28ee03623b5a", [:mix], [], "hexpm", "802784272bbff2c257b04b264c02fddc072a625502d615f111dd4769f6b1d2c0"}, "bandit": {:hex, :bandit, "1.6.11", "2fbadd60c95310eefb4ba7f1e58810aa8956e18c664a3b2029d57edb7d28d410", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "543f3f06b4721619a1220bed743aa77bf7ecc9c093ba9fab9229ff6b99eacc65"}, "bcrypt_elixir": {:hex, :bcrypt_elixir, "3.3.1", "9f2e7e00f661a6acfae1431f1bc590e28698aaecc962c2a7b33150dfe9289c3d", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "9f539e9d3828fad4ffc8152dadc0d27c6d78cb2853a9a1d6518cfe8a5adb7f50"}, "briefly": {:hex, :briefly, "0.5.1", "ee10d48da7f79ed2aebdc3e536d5f9a0c3e36ff76c0ad0d4254653a152b13a8a", [:mix], [], "hexpm", "bd684aa92ad8b7b4e0d92c31200993c4bc1469fc68cd6d5f15144041bd15cb57"}, diff --git a/test/philomena/images/query-test.yml b/test/philomena/images/query-test.yml new file mode 100644 index 000000000..80d87f4e7 --- /dev/null +++ b/test/philomena/images/query-test.yml @@ -0,0 +1,2 @@ +tests: + - input: '*' diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs new file mode 100644 index 000000000..a510ca72a --- /dev/null +++ b/test/philomena/images/query_test.exs @@ -0,0 +1,14 @@ +defmodule Philomena.Images.UsersTest do + alias Philomena.Images.Query + + use ExUnit.Case, async: true + import AssertValue + + test "query" do + queries = File.read!("query-tests.json") + + tree = Query.compile("*") + + assert_value tree == {:ok, %{match_all: %{}}} + end +end From 93e61d461f7438ab8709eb09d0825e7bb196741d Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:14:50 +0000 Subject: [PATCH 02/26] Begin Philomena Query Language parser snapshot tests --- .formatter.exs | 2 +- docker-compose.yml | 2 + mix.exs | 15 ++- mix.lock | 3 +- scripts/philomena.sh | 12 +++ test/philomena/images/queries.json | 124 ++++++++++++++++++++++ test/philomena/images/query-test.yml | 2 - test/philomena/images/query_test.exs | 41 ++++++-- test/support/labeled.ex | 16 +++ test/support/phiql_case.ex | 147 +++++++++++++++++++++++++++ 10 files changed, 349 insertions(+), 15 deletions(-) create mode 100644 test/philomena/images/queries.json delete mode 100644 test/philomena/images/query-test.yml create mode 100644 test/support/labeled.ex create mode 100644 test/support/phiql_case.ex diff --git a/.formatter.exs b/.formatter.exs index 4b3ce1a7c..8a6391c6a 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,5 +1,5 @@ [ - import_deps: [:assert_value, :ecto, :phoenix], + import_deps: [:ecto, :phoenix], inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"], subdirectories: ["priv/*/migrations"] ] diff --git a/docker-compose.yml b/docker-compose.yml index 0281aa4a4..cc0760670 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -20,6 +20,8 @@ services: # be a bare `level` which will be used as a catch-all for all other log # events that do not match any of the previous filters. - PHILOMENA_LOG=${PHILOMENA_LOG-Ecto=debug,Exq=none,PhilomenaMedia.Objects=info,debug} + # Set this env var to `y` to regenerate the test snapshots + - ASSERT_VALUE_ACCEPT_DIFFS - MIX_ENV=dev - PGPASSWORD=postgres - ANONYMOUS_NAME_SALT=2fmJRo0OgMFe65kyAJBxPT0QtkVes/jnKDdtP21fexsRqiw8TlSY7yO+uFyMZycp diff --git a/mix.exs b/mix.exs index 0278c6a64..43783256b 100644 --- a/mix.exs +++ b/mix.exs @@ -71,7 +71,6 @@ defmodule Philomena.MixProject do # Serialization {:jason, "~> 1.4"}, - {:yaml_elixir, "~> 2.11", only: [:test]}, # SMTP {:swoosh, "~> 1.17"}, @@ -99,7 +98,19 @@ defmodule Philomena.MixProject do github: "marcinkoziej/canary", ref: "704debde7a2c0600f78c687807884bf37c45bd79"}, # Automated testing - {:assert_value, "~> 0.10.4", only: [:dev, :test]} + { + :assert_value, + "~> 0.10.4", + # This is really stupid but Elixir just doesn't have any mechanism to + # suppress warnings at all, even from code from third-party libraries. + # So we are using the version from + # https://github.com/assert-value/assert_value_elixir/pull/18 which + # fixes the compiler warning "the following clause will never match" + # when using `assert_value`. + only: [:dev, :test], + github: "assert-value/assert_value_elixir", + ref: "4f4e86f0c340d3c29756e68f385ebec85113ba0f" + } ] end diff --git a/mix.lock b/mix.lock index fdc68fbdf..2be25801b 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,5 @@ %{ - "assert_value": {:hex, :assert_value, "0.10.4", "0f0e528f048734e1b9c7ed947696a18d387cb052b68c20d973ed28ee03623b5a", [:mix], [], "hexpm", "802784272bbff2c257b04b264c02fddc072a625502d615f111dd4769f6b1d2c0"}, + "assert_value": {:git, "https://github.com/assert-value/assert_value_elixir.git", "4f4e86f0c340d3c29756e68f385ebec85113ba0f", [ref: "4f4e86f0c340d3c29756e68f385ebec85113ba0f"]}, "bandit": {:hex, :bandit, "1.6.11", "2fbadd60c95310eefb4ba7f1e58810aa8956e18c664a3b2029d57edb7d28d410", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "543f3f06b4721619a1220bed743aa77bf7ecc9c093ba9fab9229ff6b99eacc65"}, "bcrypt_elixir": {:hex, :bcrypt_elixir, "3.3.1", "9f2e7e00f661a6acfae1431f1bc590e28698aaecc962c2a7b33150dfe9289c3d", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "9f539e9d3828fad4ffc8152dadc0d27c6d78cb2853a9a1d6518cfe8a5adb7f50"}, "briefly": {:hex, :briefly, "0.5.1", "ee10d48da7f79ed2aebdc3e536d5f9a0c3e36ff76c0ad0d4254653a152b13a8a", [:mix], [], "hexpm", "bd684aa92ad8b7b4e0d92c31200993c4bc1469fc68cd6d5f15144041bd15cb57"}, @@ -79,4 +79,5 @@ "websock_adapter": {:hex, :websock_adapter, "0.5.8", "3b97dc94e407e2d1fc666b2fb9acf6be81a1798a2602294aac000260a7c4a47d", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "315b9a1865552212b5f35140ad194e67ce31af45bcee443d4ecb96b5fd3f3782"}, "yamerl": {:hex, :yamerl, "0.10.0", "4ff81fee2f1f6a46f1700c0d880b24d193ddb74bd14ef42cb0bcf46e81ef2f8e", [:rebar3], [], "hexpm", "346adb2963f1051dc837a2364e4acf6eb7d80097c0f53cbdc3046ec8ec4b4e6e"}, "yaml_elixir": {:hex, :yaml_elixir, "2.11.0", "9e9ccd134e861c66b84825a3542a1c22ba33f338d82c07282f4f1f52d847bd50", [:mix], [{:yamerl, "~> 0.10", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "53cc28357ee7eb952344995787f4bb8cc3cecbf189652236e9b163e8ce1bc242"}, + "ymlr": {:hex, :ymlr, "5.1.3", "a8061add5a378e20272a31905be70209a5680fdbe0ad51f40cb1af4bdd0a010b", [:mix], [], "hexpm", "8663444fa85101a117887c170204d4c5a2182567e5f84767f0071cf15f2efb1e"}, } diff --git a/scripts/philomena.sh b/scripts/philomena.sh index d14c3dec1..3200340a6 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -101,6 +101,17 @@ function init { "$(dirname "${BASH_SOURCE[0]}")/init.sh" } +# Update the `queries.json` test snapshots after the implementation changes. +function update_query_tests { + ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ + --remove-orphans \ + app run-test 'test/philomena/images/query_test.exs' + + step docker compose down + + step npx prettier --write test/philomena/images/queries.json +} + subcommand="${1:-}" shift || true @@ -118,6 +129,7 @@ case "$subcommand" in # Shortcut for `philomena exec docker compose` compose) docker compose "$@" ;; + update-query-tests) update_query_tests "$@" ;; *) die "See the available sub-commands in ${BASH_SOURCE[0]}" ;; diff --git a/test/philomena/images/queries.json b/test/philomena/images/queries.json new file mode 100644 index 000000000..1c579c4a1 --- /dev/null +++ b/test/philomena/images/queries.json @@ -0,0 +1,124 @@ +{ + "wildcard": [ + { + "philomena": "*", + "opensearch": { + "match_all": {} + } + }, + { + "philomena": "artist:*", + "opensearch": { + "wildcard": { + "namespaced_tags.name": "artist:*" + } + } + }, + { + "philomena": "artist:mare", + "opensearch": { + "term": { + "namespaced_tags.name": "artist:mare" + } + } + } + ], + "operators": [ + { + "philomena": "safe OR pony", + "opensearch": { + "bool": { + "should": [ + { + "term": { + "namespaced_tags.name": "safe" + } + }, + { + "term": { + "namespaced_tags.name": "pony" + } + } + ] + } + } + }, + { + "philomena": "safe AND pony", + "opensearch": { + "bool": { + "must": [ + { + "term": { + "namespaced_tags.name": "safe" + } + }, + { + "term": { + "namespaced_tags.name": "pony" + } + } + ] + } + } + }, + { + "philomena": "safe AND (solo OR pony)", + "opensearch": { + "bool": { + "must": [ + { + "term": { + "namespaced_tags.name": "safe" + } + }, + { + "bool": { + "should": [ + { + "term": { + "namespaced_tags.name": "solo" + } + }, + { + "term": { + "namespaced_tags.name": "pony" + } + } + ] + } + } + ] + } + } + } + ], + "authenticated_user": [ + { + "contexts": [ + { + "user": ["user", "moderator", "admin", "assistant"] + } + ], + "philomena": "my:faves", + "opensearch": { + "term": { + "favourited_by_user_ids": "{user_id}" + } + } + }, + { + "contexts": [ + { + "user": ["anon"] + } + ], + "philomena": "my:faves", + "opensearch": { + "term": { + "namespaced_tags.name": "my:faves" + } + } + } + ] +} diff --git a/test/philomena/images/query-test.yml b/test/philomena/images/query-test.yml deleted file mode 100644 index 80d87f4e7..000000000 --- a/test/philomena/images/query-test.yml +++ /dev/null @@ -1,2 +0,0 @@ -tests: - - input: '*' diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index a510ca72a..4f7b52a7f 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -1,14 +1,37 @@ defmodule Philomena.Images.UsersTest do - alias Philomena.Images.Query + alias Philomena.Labeled - use ExUnit.Case, async: true - import AssertValue + use Philomena.QueryCase - test "query" do - queries = File.read!("query-tests.json") - - tree = Query.compile("*") - - assert_value tree == {:ok, %{match_all: %{}}} + test "phiql" do + QueryCase.assert_phiql(%{ + compile: &Philomena.Images.Query.compile/2, + snapshot: "#{__DIR__}/queries.json", + contexts: %{ + user: [ + Labeled.new(:anon, nil), + Labeled.new(:user, %{id: "{user_id}", role: "user"}), + Labeled.new(:assistant, %{id: "{user_id}", role: "assistant"}), + Labeled.new(:moderator, %{id: "{user_id}", role: "moderator"}), + Labeled.new(:admin, %{id: "{user_id}", role: "admin"}) + ], + watch: [true, false] + }, + test_cases: [ + wildcard: [ + "*", + "artist:*", + "artist:mare" + ], + operators: [ + "safe OR pony", + "safe AND pony", + "safe AND (solo OR pony)" + ], + authenticated_user: [ + "my:faves" + ] + ] + }) end end diff --git a/test/support/labeled.ex b/test/support/labeled.ex new file mode 100644 index 000000000..605ebb05a --- /dev/null +++ b/test/support/labeled.ex @@ -0,0 +1,16 @@ +defmodule Philomena.Labeled do + @enforce_keys [:label, :value] + defstruct [:label, :value] + @type t(v) :: %__MODULE__{label: String.t(), value: v} + + @spec new(String.t(), a) :: t(a) when a: any() + def new(label, value) do + %__MODULE__{label: label, value: value} + end + + def prefer_label(%__MODULE__{label: label}), do: label + def prefer_label(unlabeled), do: unlabeled + + def prefer_value(%__MODULE__{value: value}), do: value + def prefer_value(unlabeled), do: unlabeled +end diff --git a/test/support/phiql_case.ex b/test/support/phiql_case.ex new file mode 100644 index 000000000..d5f6fe4aa --- /dev/null +++ b/test/support/phiql_case.ex @@ -0,0 +1,147 @@ +defmodule Philomena.QueryCase do + @moduledoc """ + This module defines the setup for testing the PhiQL (Philomena Query + Language) parsing. + """ + + alias PhilomenaQuery.Parse.Parser + alias Philomena.Labeled + import AssertValue + + use ExUnit.CaseTemplate + + using do + quote do + alias Philomena.Labeled + alias Philomena.QueryCase + end + end + + # Context combinations multimap. The number of keys in this map should be kept + # small, because the test calculates a multi-cartesian product of values + # between the keys of this map. + @type contexts_schema :: %{String.t() => [Labeled.t(any()) | any()]} + + @type phiql_test :: %{ + # The so-called "system under test". This function accepts a PhiQL + # string, a context and returns the compiled OpenSearch query. + compile: (String.t(), keyword([any()]) -> Parser.result()), + + # Defines the combinations of contexts to test with. + contexts: contexts_schema(), + + # Path to the file where to store the snapshot of the test results. + snapshot: String.t(), + + # The test cases with input PhiQL strings arbitrarily grouped for + # readability. + test_cases: keyword([String.t()]) + } + + @spec assert_phiql(phiql_test()) :: :ok + def assert_phiql(test) do + actual = + test.test_cases + |> map_values(fn inputs -> + inputs + |> Enum.map(&compile_input(test, &1)) + |> List.flatten() + end) + |> Jason.OrderedObject.new() + |> Jason.encode!(pretty: true) + + assert_value(actual == File.read!(test.snapshot)) + end + + @spec compile_input(phiql_test(), String.t()) :: map() + defp compile_input(test, input) do + test.contexts + |> IO.inspect(label: "Contexts 1") + |> multimap_cartesian_product() + |> IO.inspect(label: "Contexts 2") + |> Enum.group_by(fn ctx -> + ctx = map_values(ctx, &Labeled.prefer_value/1) + + case test.compile.(input, ctx) do + {:ok, output} -> output + {:error, error} -> "Error: #{error}" + end + end) + |> Enum.map(fn {output, contexts} -> + contexts = + contexts + |> Enum.map(fn ctx -> map_values(ctx, &Labeled.prefer_label/1) |> Map.new() end) + + contexts = + case normalize_contexts(test.contexts, contexts) do + [context] when map_size(context) == 0 -> [] + contexts -> [contexts: contexts] + end + + Jason.OrderedObject.new(contexts ++ [philomena: input, opensearch: output]) + end) + end + + @spec map_values([{k, v}], (v -> new_v)) :: [{k, new_v}] + when k: any(), v: any(), new_v: any() + defp map_values(key_values, map_value) do + Enum.map(key_values, fn {key, value} -> {key, map_value.(value)} end) + end + + defp multimap_cartesian_product(map) when map_size(map) == 0, do: [%{}] + + defp multimap_cartesian_product(map) do + IO.inspect(map, label: "iter 0") + + {key, values} = map |> Enum.at(0) + + IO.inspect({key, values}, label: "iter 1") + + rest = map |> Map.delete(key) + + IO.inspect(rest, label: "iter 1.1") + + for value <- values, + rest <- multimap_cartesian_product(rest) |> IO.inspect(label: "Return") do + IO.inspect({key, value, rest}, label: "iter 2") + Map.put_new(rest, key, value) + end + end + + @spec normalize_contexts([contexts_schema()], [map()]) :: map() + defp normalize_contexts(schema, contexts) + + defp normalize_contexts(schema, contexts) when map_size(schema) == 0 do + contexts + end + + defp normalize_contexts(schema, contexts) do + {key, possible_values} = schema |> Enum.at(0) + + schema = schema |> Map.delete(key) + + groups = + contexts + |> Enum.group_by( + fn ctx -> ctx[key] end, + fn ctx -> Map.delete(ctx, key) end + ) + + groups + |> Map.values() + |> Enum.uniq() + |> case do + [other] when map_size(groups) == length(possible_values) -> + normalize_contexts(schema, other) + + [other] -> + values = Map.keys(groups) + + normalize_contexts(schema, other) + |> Enum.map(&Map.merge(&1, %{key => values})) + + _ -> + normalize_contexts(schema, contexts) + end + end +end From d65a2324145ed61ce30eee0c9a9fecde1e194470 Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:19:11 +0000 Subject: [PATCH 03/26] Rename "query" to "phiql" --- scripts/philomena.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index 3200340a6..72158fcd4 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -102,7 +102,7 @@ function init { } # Update the `queries.json` test snapshots after the implementation changes. -function update_query_tests { +function update_phiql_tests { ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ --remove-orphans \ app run-test 'test/philomena/images/query_test.exs' @@ -129,7 +129,7 @@ case "$subcommand" in # Shortcut for `philomena exec docker compose` compose) docker compose "$@" ;; - update-query-tests) update_query_tests "$@" ;; + update-phiql-tests) update_phiql_tests "$@" ;; *) die "See the available sub-commands in ${BASH_SOURCE[0]}" ;; From 9eddec06af0ef9c1ed8a3dda52c845821050bdbd Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:32:31 +0000 Subject: [PATCH 04/26] Remove debugging remnants and typespec fixes --- test/support/phiql_case.ex | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/test/support/phiql_case.ex b/test/support/phiql_case.ex index d5f6fe4aa..8310f0b04 100644 --- a/test/support/phiql_case.ex +++ b/test/support/phiql_case.ex @@ -53,12 +53,10 @@ defmodule Philomena.QueryCase do assert_value(actual == File.read!(test.snapshot)) end - @spec compile_input(phiql_test(), String.t()) :: map() + @spec compile_input(phiql_test(), String.t()) :: [map()] defp compile_input(test, input) do test.contexts - |> IO.inspect(label: "Contexts 1") |> multimap_cartesian_product() - |> IO.inspect(label: "Contexts 2") |> Enum.group_by(fn ctx -> ctx = map_values(ctx, &Labeled.prefer_value/1) @@ -91,24 +89,17 @@ defmodule Philomena.QueryCase do defp multimap_cartesian_product(map) when map_size(map) == 0, do: [%{}] defp multimap_cartesian_product(map) do - IO.inspect(map, label: "iter 0") - {key, values} = map |> Enum.at(0) - IO.inspect({key, values}, label: "iter 1") - rest = map |> Map.delete(key) - IO.inspect(rest, label: "iter 1.1") - for value <- values, - rest <- multimap_cartesian_product(rest) |> IO.inspect(label: "Return") do - IO.inspect({key, value, rest}, label: "iter 2") + rest <- multimap_cartesian_product(rest) do Map.put_new(rest, key, value) end end - @spec normalize_contexts([contexts_schema()], [map()]) :: map() + @spec normalize_contexts([contexts_schema()], [map()]) :: [map()] defp normalize_contexts(schema, contexts) defp normalize_contexts(schema, contexts) when map_size(schema) == 0 do From 67685331c1f683b52d876bc9f88d7f15138017aa Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:33:35 +0000 Subject: [PATCH 05/26] Rename options -> context --- lib/philomena/images/query.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 2f006a629..49807ef54 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -197,9 +197,9 @@ defmodule Philomena.Images.Query do end end - @type options :: [user: %{role: String.t()}, watch: boolean()] + @type context :: [user: %{role: String.t()}, watch: boolean()] - @spec parse(Parser.options(), options(), String.t()) :: Parser.result() + @spec parse(Parser.options(), context(), String.t()) :: Parser.result() defp parse(fields, context, query_string) do case prepare_context(context, query_string) do {:ok, context} -> From 99d4610133afb175ccae838ed62746a35bfc620c Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:36:16 +0000 Subject: [PATCH 06/26] Revert some unnecessary changes --- mix.exs | 4 +--- mix.lock | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/mix.exs b/mix.exs index 43783256b..67b15440b 100644 --- a/mix.exs +++ b/mix.exs @@ -44,6 +44,7 @@ defmodule Philomena.MixProject do {:phoenix_view, "~> 2.0"}, {:phoenix_live_reload, "~> 1.4", only: :dev}, {:gettext, "~> 0.22"}, + {:jason, "~> 1.4"}, {:bandit, "~> 1.2"}, {:slime, "~> 1.3.1"}, {:phoenix_slime, "~> 0.13", @@ -69,9 +70,6 @@ defmodule Philomena.MixProject do {:sweet_xml, "~> 0.7"}, {:inet_cidr, "~> 1.0"}, - # Serialization - {:jason, "~> 1.4"}, - # SMTP {:swoosh, "~> 1.17"}, {:mua, "~> 0.2.0"}, diff --git a/mix.lock b/mix.lock index 2be25801b..57fc3506a 100644 --- a/mix.lock +++ b/mix.lock @@ -79,5 +79,4 @@ "websock_adapter": {:hex, :websock_adapter, "0.5.8", "3b97dc94e407e2d1fc666b2fb9acf6be81a1798a2602294aac000260a7c4a47d", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "315b9a1865552212b5f35140ad194e67ce31af45bcee443d4ecb96b5fd3f3782"}, "yamerl": {:hex, :yamerl, "0.10.0", "4ff81fee2f1f6a46f1700c0d880b24d193ddb74bd14ef42cb0bcf46e81ef2f8e", [:rebar3], [], "hexpm", "346adb2963f1051dc837a2364e4acf6eb7d80097c0f53cbdc3046ec8ec4b4e6e"}, "yaml_elixir": {:hex, :yaml_elixir, "2.11.0", "9e9ccd134e861c66b84825a3542a1c22ba33f338d82c07282f4f1f52d847bd50", [:mix], [{:yamerl, "~> 0.10", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "53cc28357ee7eb952344995787f4bb8cc3cecbf189652236e9b163e8ce1bc242"}, - "ymlr": {:hex, :ymlr, "5.1.3", "a8061add5a378e20272a31905be70209a5680fdbe0ad51f40cb1af4bdd0a010b", [:mix], [], "hexpm", "8663444fa85101a117887c170204d4c5a2182567e5f84767f0071cf15f2efb1e"}, } From 4fbf75dd46aaa7a8a3bff2fb713a88f609812a2e Mon Sep 17 00:00:00 2001 From: MareStare Date: Tue, 13 May 2025 02:37:46 +0000 Subject: [PATCH 07/26] Improve `run-test` script a bit --- docker/app/run-test | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/docker/app/run-test b/docker/app/run-test index f845890f1..3f9d8ade1 100755 --- a/docker/app/run-test +++ b/docker/app/run-test @@ -1,35 +1,37 @@ -#!/usr/bin/env sh -set -e +#!/usr/bin/env bash + +set -euo pipefail + +. "$(dirname "${BASH_SOURCE[0]}")/../../scripts/lib.sh" export MIX_ENV=test # Always install mix dependencies -(cd /srv/philomena && mix deps.get) +cd /srv/philomena + +step mix deps.get # Run formatting check -mix format --check-formatted +step mix format --check-formatted # Sleep to allow OpenSearch to finish initializing # if it's not done doing whatever it does yet -echo -n "Waiting for OpenSearch" +step echo -n "Waiting for OpenSearch" -until wget -qO - opensearch:9200; do - echo -n "." +until step wget -qO - http://opensearch:9200; do sleep 2 done -echo - # Create the database -mix ecto.create -mix ecto.load +step mix ecto.create +step mix ecto.load # Test the application -mix test +step mix test "$@" # Security lint -mix sobelow --config -mix deps.audit +step mix sobelow --config +step mix deps.audit # Static analysis exec mix dialyzer From 5510239b178523c559cb011b01f9e03747e4298d Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 00:10:44 +0000 Subject: [PATCH 08/26] Change naming "PhiQL" -> "Philomena Search Syntax" --- scripts/philomena.sh | 6 ++--- test/philomena/images/query_test.exs | 10 ++++----- .../{queries.json => search-syntax.json} | 0 .../{phiql_case.ex => search_syntax_case.ex} | 22 +++++++++---------- 4 files changed, 19 insertions(+), 19 deletions(-) rename test/philomena/images/{queries.json => search-syntax.json} (100%) rename test/support/{phiql_case.ex => search_syntax_case.ex} (85%) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index 72158fcd4..c3ab3e9d9 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -102,14 +102,14 @@ function init { } # Update the `queries.json` test snapshots after the implementation changes. -function update_phiql_tests { +function update_search_syntax_tests { ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ --remove-orphans \ app run-test 'test/philomena/images/query_test.exs' step docker compose down - step npx prettier --write test/philomena/images/queries.json + step npx prettier --write test/philomena/images/search-syntax.json } subcommand="${1:-}" @@ -129,7 +129,7 @@ case "$subcommand" in # Shortcut for `philomena exec docker compose` compose) docker compose "$@" ;; - update-phiql-tests) update_phiql_tests "$@" ;; + update-search-syntax-tests) update_search_syntax_tests "$@" ;; *) die "See the available sub-commands in ${BASH_SOURCE[0]}" ;; diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index 4f7b52a7f..352a52423 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -1,12 +1,12 @@ -defmodule Philomena.Images.UsersTest do +defmodule Philomena.Images.QueryTest do alias Philomena.Labeled - use Philomena.QueryCase + use Philomena.SearchSyntaxCase - test "phiql" do - QueryCase.assert_phiql(%{ + test "search syntax" do + assert_search_syntax(%{ compile: &Philomena.Images.Query.compile/2, - snapshot: "#{__DIR__}/queries.json", + snapshot: "#{__DIR__}/search-syntax.json", contexts: %{ user: [ Labeled.new(:anon, nil), diff --git a/test/philomena/images/queries.json b/test/philomena/images/search-syntax.json similarity index 100% rename from test/philomena/images/queries.json rename to test/philomena/images/search-syntax.json diff --git a/test/support/phiql_case.ex b/test/support/search_syntax_case.ex similarity index 85% rename from test/support/phiql_case.ex rename to test/support/search_syntax_case.ex index 8310f0b04..f693c40bc 100644 --- a/test/support/phiql_case.ex +++ b/test/support/search_syntax_case.ex @@ -1,7 +1,6 @@ -defmodule Philomena.QueryCase do +defmodule Philomena.SearchSyntaxCase do @moduledoc """ - This module defines the setup for testing the PhiQL (Philomena Query - Language) parsing. + This module defines the setup for testing the Philomena Search Syntax parser. """ alias PhilomenaQuery.Parse.Parser @@ -13,7 +12,7 @@ defmodule Philomena.QueryCase do using do quote do alias Philomena.Labeled - alias Philomena.QueryCase + import Philomena.SearchSyntaxCase, only: [assert_search_syntax: 1] end end @@ -22,9 +21,10 @@ defmodule Philomena.QueryCase do # between the keys of this map. @type contexts_schema :: %{String.t() => [Labeled.t(any()) | any()]} - @type phiql_test :: %{ - # The so-called "system under test". This function accepts a PhiQL - # string, a context and returns the compiled OpenSearch query. + @type search_syntax_test :: %{ + # The so-called "system under test". This function accepts a Philomena + # Search Syntax string, a context and returns the compiled OpenSearch + # query. compile: (String.t(), keyword([any()]) -> Parser.result()), # Defines the combinations of contexts to test with. @@ -33,13 +33,13 @@ defmodule Philomena.QueryCase do # Path to the file where to store the snapshot of the test results. snapshot: String.t(), - # The test cases with input PhiQL strings arbitrarily grouped for + # The test cases with input Philomena Search Syntax strings arbitrarily grouped for # readability. test_cases: keyword([String.t()]) } - @spec assert_phiql(phiql_test()) :: :ok - def assert_phiql(test) do + @spec assert_search_syntax(search_syntax_test()) :: :ok + def assert_search_syntax(test) do actual = test.test_cases |> map_values(fn inputs -> @@ -53,7 +53,7 @@ defmodule Philomena.QueryCase do assert_value(actual == File.read!(test.snapshot)) end - @spec compile_input(phiql_test(), String.t()) :: [map()] + @spec compile_input(search_syntax_test(), String.t()) :: [map()] defp compile_input(test, input) do test.contexts |> multimap_cartesian_product() From 72164a05a0f99c165b4df033d2b8a2b0c1637ac3 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 00:12:44 +0000 Subject: [PATCH 09/26] Fix context typespec --- lib/philomena/images/query.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 49807ef54..8e0c451b6 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -197,7 +197,11 @@ defmodule Philomena.Images.Query do end end - @type context :: [user: %{role: String.t()}, watch: boolean()] + @type context :: [ + user: %{role: String.t(), id: integer()}, + watch: boolean(), + filter: boolean() + ] @spec parse(Parser.options(), context(), String.t()) :: Parser.result() defp parse(fields, context, query_string) do @@ -217,7 +221,7 @@ defmodule Philomena.Images.Query do defp fields_for(%{role: role}) when role in ~W(moderator admin), do: moderator_fields() defp fields_for(_), do: raise(ArgumentError, "Unknown user role.") - @spec compile(String.t(), options()) :: Parser.result() + @spec compile(String.t(), context()) :: Parser.result() def compile(query_string, opts \\ []) do user = Keyword.get(opts, :user) watch = Keyword.get(opts, :watch, false) From 2e37dfce7ac250c0711ebf14b601b97b088a2bae Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 02:50:16 +0000 Subject: [PATCH 10/26] Add some test cases for `filter_id` --- lib/philomena/images/query.ex | 13 +- test/philomena/images/query_test.exs | 55 +++++-- test/philomena/images/search-syntax.json | 178 +++++++++++++++++++++- test/support/fixtures/filters_fixtures.ex | 18 +++ test/support/fixtures/tags_fixtures.ex | 18 +++ test/support/fixtures/users_fixtures.ex | 42 ++++- test/support/search_syntax_case.ex | 6 + 7 files changed, 311 insertions(+), 19 deletions(-) create mode 100644 test/support/fixtures/filters_fixtures.ex create mode 100644 test/support/fixtures/tags_fixtures.ex diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 8e0c451b6..545961309 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -198,7 +198,18 @@ defmodule Philomena.Images.Query do end @type context :: [ - user: %{role: String.t(), id: integer()}, + user: %{ + role: String.t(), + id: integer(), + watched_tag_ids: [integer()], + watched_images_query_str: String.t(), + watched_images_exclude_str: String.t(), + no_spoilered_in_watched: boolean(), + current_filter: %{ + spoilered_tag_ids: [integer()], + spoilered_complex_str: String.t() + } + }, watch: boolean(), filter: boolean() ] diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index 352a52423..04a790cb0 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -1,21 +1,49 @@ defmodule Philomena.Images.QueryTest do alias Philomena.Labeled + import Philomena.UsersFixtures + import Philomena.FiltersFixtures + import Philomena.TagsFixtures use Philomena.SearchSyntaxCase + defp make_user(attrs) do + # Set the user ID to a dummy value to make sure it's consistent between + # the test cases of different users. This way we avoid generating extra + # test cases in the snapshot that just differ by user ID. + # |> Map.put(:id, 1) + attrs |> user_fixture() + end + test "search syntax" do + users = [ + Labeled.new(:anon, nil), + Labeled.new(:user, make_user(%{confirmed: true})), + Labeled.new(:assistant, make_user(%{role: "assistant"})), + Labeled.new(:moderator, make_user(%{role: "moderator"})), + Labeled.new(:admin, make_user(%{role: "admin"})) + ] + + for id <- 10..14 do + tag_fixture(%{id: id, name: "tag#{id}"}) + end + |> Enum.to_list() + + system_filter = + system_filter_fixture(%{ + id: 100, + name: "System Filter", + spoilered_tag_list: "tag10,tag11", + hidden_tag_list: "tag12,tag13", + hidden_complex_str: "truly AND complex" + }) + assert_search_syntax(%{ compile: &Philomena.Images.Query.compile/2, snapshot: "#{__DIR__}/search-syntax.json", contexts: %{ - user: [ - Labeled.new(:anon, nil), - Labeled.new(:user, %{id: "{user_id}", role: "user"}), - Labeled.new(:assistant, %{id: "{user_id}", role: "assistant"}), - Labeled.new(:moderator, %{id: "{user_id}", role: "moderator"}), - Labeled.new(:admin, %{id: "{user_id}", role: "admin"}) - ], - watch: [true, false] + user: users, + watch: [true, false], + filter: [true, false] }, test_cases: [ wildcard: [ @@ -29,7 +57,16 @@ defmodule Philomena.Images.QueryTest do "safe AND (solo OR pony)" ], authenticated_user: [ - "my:faves" + "my:faves", + "my:watched" + ], + system_filter: [ + "filter_id:#{system_filter.id}" + ], + invalid_filters: [ + "filter_id:invalid_id", + # non-existent filter + "filter_id:9999" ] ] }) diff --git a/test/philomena/images/search-syntax.json b/test/philomena/images/search-syntax.json index 1c579c4a1..395376a64 100644 --- a/test/philomena/images/search-syntax.json +++ b/test/philomena/images/search-syntax.json @@ -97,13 +97,52 @@ { "contexts": [ { - "user": ["user", "moderator", "admin", "assistant"] + "user": ["user"] } ], "philomena": "my:faves", "opensearch": { "term": { - "favourited_by_user_ids": "{user_id}" + "favourited_by_user_ids": 504 + } + } + }, + { + "contexts": [ + { + "user": ["assistant"] + } + ], + "philomena": "my:faves", + "opensearch": { + "term": { + "favourited_by_user_ids": 505 + } + } + }, + { + "contexts": [ + { + "user": ["moderator"] + } + ], + "philomena": "my:faves", + "opensearch": { + "term": { + "favourited_by_user_ids": 506 + } + } + }, + { + "contexts": [ + { + "user": ["admin"] + } + ], + "philomena": "my:faves", + "opensearch": { + "term": { + "favourited_by_user_ids": 507 } } }, @@ -119,6 +158,141 @@ "namespaced_tags.name": "my:faves" } } + }, + { + "contexts": [ + { + "user": ["user", "moderator", "admin", "assistant"], + "watch": [false] + } + ], + "philomena": "my:watched", + "opensearch": { + "bool": { + "should": [ + { + "terms": { + "tag_ids": [] + } + }, + { + "match_none": {} + } + ], + "must_not": [ + { + "match_none": {} + } + ] + } + } + }, + { + "contexts": [ + { + "user": ["anon"] + } + ], + "philomena": "my:watched", + "opensearch": { + "term": { + "namespaced_tags.name": "my:watched" + } + } + }, + { + "contexts": [ + { + "user": ["user", "moderator", "admin", "assistant"], + "watch": [true] + } + ], + "philomena": "my:watched", + "opensearch": "Error: Recursive watchlists are not allowed." + } + ], + "system_filter": [ + { + "contexts": [ + { + "filter": [false] + } + ], + "philomena": "filter_id:100", + "opensearch": { + "bool": { + "must_not": [ + { + "terms": { + "tag_ids": [12, 13] + } + }, + { + "bool": { + "must": [ + { + "term": { + "namespaced_tags.name": "truly" + } + }, + { + "term": { + "namespaced_tags.name": "complex" + } + } + ] + } + } + ] + } + } + }, + { + "contexts": [ + { + "filter": [true] + } + ], + "philomena": "filter_id:100", + "opensearch": "Error: Filter queries inside filters are not allowed." + } + ], + "invalid_filters": [ + { + "contexts": [ + { + "filter": [true] + } + ], + "philomena": "filter_id:invalid_id", + "opensearch": "Error: Filter queries inside filters are not allowed." + }, + { + "contexts": [ + { + "filter": [false] + } + ], + "philomena": "filter_id:invalid_id", + "opensearch": "Error: Invalid filter `invalid_id`." + }, + { + "contexts": [ + { + "filter": [true] + } + ], + "philomena": "filter_id:9999", + "opensearch": "Error: Filter queries inside filters are not allowed." + }, + { + "contexts": [ + { + "filter": [false] + } + ], + "philomena": "filter_id:9999", + "opensearch": "Error: Invalid filter `9999`." } ] } diff --git a/test/support/fixtures/filters_fixtures.ex b/test/support/fixtures/filters_fixtures.ex new file mode 100644 index 000000000..810d0ec95 --- /dev/null +++ b/test/support/fixtures/filters_fixtures.ex @@ -0,0 +1,18 @@ +defmodule Philomena.FiltersFixtures do + @moduledoc """ + This module defines test helpers for creating entities via the + `Philomena.Filters` context. + """ + + alias Philomena.Filters.Filter + alias Philomena.Repo + + def system_filter_fixture(attrs \\ %{}) do + {:ok, filter} = + %Filter{id: attrs.id, system: true} + |> Filter.changeset(attrs) + |> Repo.insert() + + filter + end +end diff --git a/test/support/fixtures/tags_fixtures.ex b/test/support/fixtures/tags_fixtures.ex new file mode 100644 index 000000000..e25935a3c --- /dev/null +++ b/test/support/fixtures/tags_fixtures.ex @@ -0,0 +1,18 @@ +defmodule Philomena.TagsFixtures do + @moduledoc """ + This module defines test helpers for creating entities via the + `Philomena.Tags` context. + """ + + alias Philomena.Tags.Tag + alias Philomena.Repo + + def tag_fixture(attrs \\ %{}) do + {:ok, tag} = + %Tag{id: attrs.id} + |> Tag.creation_changeset(attrs) + |> Repo.insert() + + tag + end +end diff --git a/test/support/fixtures/users_fixtures.ex b/test/support/fixtures/users_fixtures.ex index 76a2ff917..ddafca093 100644 --- a/test/support/fixtures/users_fixtures.ex +++ b/test/support/fixtures/users_fixtures.ex @@ -5,6 +5,7 @@ defmodule Philomena.UsersFixtures do """ alias Philomena.Users + alias Philomena.Users.User alias Philomena.Repo def unique_user_email, do: "user#{System.unique_integer()}@example.com" @@ -13,6 +14,15 @@ defmodule Philomena.UsersFixtures do def user_fixture(attrs \\ %{}) do email = unique_user_email() + {role, attrs} = Map.pop(attrs, :role) + role = role || "user" + + {confirmed, attrs} = Map.pop(attrs, :confirmed) + confirmed = confirmed || role != "user" + + {locked, attrs} = Map.pop(attrs, :locked) + locked = locked || false + {:ok, user} = attrs |> Enum.into(%{ @@ -22,19 +32,37 @@ defmodule Philomena.UsersFixtures do }) |> Users.register_user() - user + updates = + [ + if role != "user" do + fn user -> + user + |> Repo.preload(:roles) + |> User.update_changeset(%{role: role}, []) + end + end, + if(confirmed, do: &User.confirm_changeset/1), + if(locked, do: &User.lock_changeset/1) + ] + |> Enum.reject(&is_nil/1) + + case updates do + [] -> + user + + _ -> + updates + |> Enum.reduce(user, fn update, user -> update.(user) end) + |> Repo.update!() + end end def confirmed_user_fixture(attrs \\ %{}) do - user_fixture(attrs) - |> Users.User.confirm_changeset() - |> Repo.update!() + user_fixture(Map.put(attrs, :confirmed, true)) end def locked_user_fixture(attrs \\ %{}) do - user_fixture(attrs) - |> Users.User.lock_changeset() - |> Repo.update!() + user_fixture(Map.put(attrs, :locked, true)) end def extract_user_token(fun) do diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index f693c40bc..80511fa28 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -16,6 +16,12 @@ defmodule Philomena.SearchSyntaxCase do end end + setup tags do + pid = Ecto.Adapters.SQL.Sandbox.start_owner!(Philomena.Repo, shared: not tags[:async]) + on_exit(fn -> Ecto.Adapters.SQL.Sandbox.stop_owner(pid) end) + :ok + end + # Context combinations multimap. The number of keys in this map should be kept # small, because the test calculates a multi-cartesian product of values # between the keys of this map. From 7621387bc4e2f609e8b13c157ca1d9a0bf769cb5 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 02:50:25 +0000 Subject: [PATCH 11/26] Improve the update script --- docker/app/run-test | 12 ++++++++---- scripts/philomena.sh | 33 ++++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/docker/app/run-test b/docker/app/run-test index 3f9d8ade1..713a3abb6 100755 --- a/docker/app/run-test +++ b/docker/app/run-test @@ -16,15 +16,19 @@ step mix format --check-formatted # Sleep to allow OpenSearch to finish initializing # if it's not done doing whatever it does yet -step echo -n "Waiting for OpenSearch" +info "Waiting for OpenSearch" until step wget -qO - http://opensearch:9200; do sleep 2 done -# Create the database -step mix ecto.create -step mix ecto.load +# Create the database, or migrate if one already exists from a previous run +if step createdb -h postgres -U postgres philomena_test; then + step mix ecto.create + step mix ecto.load +elif [[ "$(mix ecto.migrations)" == *" down "* ]]; then + step mix ecto.migrate --all +fi # Test the application step mix test "$@" diff --git a/scripts/philomena.sh b/scripts/philomena.sh index c3ab3e9d9..c4a9e38fe 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -103,13 +103,36 @@ function init { # Update the `queries.json` test snapshots after the implementation changes. function update_search_syntax_tests { - ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ - --remove-orphans \ - app run-test 'test/philomena/images/query_test.exs' + # shellcheck disable=SC2016 + test=' + set -euo pipefail - step docker compose down + . scripts/lib.sh + + export MIX_ENV=test + + if step createdb -h postgres -U postgres philomena_test; then + step mix ecto.create + step mix ecto.load + elif [[ "$(mix ecto.migrations)" == *" down "* ]]; then + step mix ecto.migrate --all + fi - step npx prettier --write test/philomena/images/search-syntax.json + step mix test test/philomena/images/query_test.exs + step npx prettier --write test/philomena/images/search-syntax.json + ' + + ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ + --remove-orphans \ + app bash -c "$test" + + # See the git diff for the updated snapshot in vscode if it's available. + if command -v code &>/dev/null; then + step git difftool \ + --no-prompt \ + --extcmd "code --wait --diff" \ + -- test/philomena/images/search-syntax.json + fi } subcommand="${1:-}" From 68cec44a33e7f477e628f0ebb18726fed5a1431e Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 02:56:04 +0000 Subject: [PATCH 12/26] Deponify the test data and fix user ID cardinality problem --- test/philomena/images/query_test.exs | 18 +++---- test/philomena/images/search-syntax.json | 67 +++++------------------- 2 files changed, 22 insertions(+), 63 deletions(-) diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index 04a790cb0..b9e76e03b 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -7,17 +7,15 @@ defmodule Philomena.Images.QueryTest do use Philomena.SearchSyntaxCase defp make_user(attrs) do - # Set the user ID to a dummy value to make sure it's consistent between - # the test cases of different users. This way we avoid generating extra - # test cases in the snapshot that just differ by user ID. - # |> Map.put(:id, 1) - attrs |> user_fixture() + # Pretend that all users have the same ID. This doesn't influence the parser + # logic because it doesn't load the users from the DB. + attrs |> user_fixture() |> Map.put(:id, 1) end test "search syntax" do users = [ Labeled.new(:anon, nil), - Labeled.new(:user, make_user(%{confirmed: true})), + Labeled.new(:user, make_user(%{role: "user"})), Labeled.new(:assistant, make_user(%{role: "assistant"})), Labeled.new(:moderator, make_user(%{role: "moderator"})), Labeled.new(:admin, make_user(%{role: "admin"})) @@ -49,12 +47,12 @@ defmodule Philomena.Images.QueryTest do wildcard: [ "*", "artist:*", - "artist:mare" + "artist:artist1" ], operators: [ - "safe OR pony", - "safe AND pony", - "safe AND (solo OR pony)" + "tag1 OR tag2", + "tag1 AND tag2", + "tag1 AND (tag2 OR tag3)" ], authenticated_user: [ "my:faves", diff --git a/test/philomena/images/search-syntax.json b/test/philomena/images/search-syntax.json index 395376a64..354bc5981 100644 --- a/test/philomena/images/search-syntax.json +++ b/test/philomena/images/search-syntax.json @@ -15,28 +15,28 @@ } }, { - "philomena": "artist:mare", + "philomena": "artist:artist1", "opensearch": { "term": { - "namespaced_tags.name": "artist:mare" + "namespaced_tags.name": "artist:artist1" } } } ], "operators": [ { - "philomena": "safe OR pony", + "philomena": "tag1 OR tag2", "opensearch": { "bool": { "should": [ { "term": { - "namespaced_tags.name": "safe" + "namespaced_tags.name": "tag1" } }, { "term": { - "namespaced_tags.name": "pony" + "namespaced_tags.name": "tag2" } } ] @@ -44,18 +44,18 @@ } }, { - "philomena": "safe AND pony", + "philomena": "tag1 AND tag2", "opensearch": { "bool": { "must": [ { "term": { - "namespaced_tags.name": "safe" + "namespaced_tags.name": "tag1" } }, { "term": { - "namespaced_tags.name": "pony" + "namespaced_tags.name": "tag2" } } ] @@ -63,13 +63,13 @@ } }, { - "philomena": "safe AND (solo OR pony)", + "philomena": "tag1 AND (tag2 OR tag3)", "opensearch": { "bool": { "must": [ { "term": { - "namespaced_tags.name": "safe" + "namespaced_tags.name": "tag1" } }, { @@ -77,12 +77,12 @@ "should": [ { "term": { - "namespaced_tags.name": "solo" + "namespaced_tags.name": "tag2" } }, { "term": { - "namespaced_tags.name": "pony" + "namespaced_tags.name": "tag3" } } ] @@ -97,52 +97,13 @@ { "contexts": [ { - "user": ["user"] + "user": ["user", "moderator", "admin", "assistant"] } ], "philomena": "my:faves", "opensearch": { "term": { - "favourited_by_user_ids": 504 - } - } - }, - { - "contexts": [ - { - "user": ["assistant"] - } - ], - "philomena": "my:faves", - "opensearch": { - "term": { - "favourited_by_user_ids": 505 - } - } - }, - { - "contexts": [ - { - "user": ["moderator"] - } - ], - "philomena": "my:faves", - "opensearch": { - "term": { - "favourited_by_user_ids": 506 - } - } - }, - { - "contexts": [ - { - "user": ["admin"] - } - ], - "philomena": "my:faves", - "opensearch": { - "term": { - "favourited_by_user_ids": 507 + "favourited_by_user_ids": 1 } } }, From 73429a7d57b4a4ac8dc62eb9ff1b222164ad7c9f Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 02:58:06 +0000 Subject: [PATCH 13/26] Don't wait in diftool --- scripts/philomena.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index c4a9e38fe..e494718ed 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -130,7 +130,7 @@ function update_search_syntax_tests { if command -v code &>/dev/null; then step git difftool \ --no-prompt \ - --extcmd "code --wait --diff" \ + --extcmd "code --diff" \ -- test/philomena/images/search-syntax.json fi } From 9c6d70c39206d4124116cea6cee104cdd09cb30a Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 03:06:16 +0000 Subject: [PATCH 14/26] Ensure the stability of context value lists by sorting them. --- test/philomena/images/search-syntax.json | 6 +++--- test/support/search_syntax_case.ex | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/philomena/images/search-syntax.json b/test/philomena/images/search-syntax.json index 354bc5981..3bc02fabb 100644 --- a/test/philomena/images/search-syntax.json +++ b/test/philomena/images/search-syntax.json @@ -97,7 +97,7 @@ { "contexts": [ { - "user": ["user", "moderator", "admin", "assistant"] + "user": ["admin", "assistant", "moderator", "user"] } ], "philomena": "my:faves", @@ -123,7 +123,7 @@ { "contexts": [ { - "user": ["user", "moderator", "admin", "assistant"], + "user": ["admin", "assistant", "moderator", "user"], "watch": [false] } ], @@ -164,7 +164,7 @@ { "contexts": [ { - "user": ["user", "moderator", "admin", "assistant"], + "user": ["admin", "assistant", "moderator", "user"], "watch": [true] } ], diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index 80511fa28..bd34cd4d3 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -132,7 +132,7 @@ defmodule Philomena.SearchSyntaxCase do normalize_contexts(schema, other) [other] -> - values = Map.keys(groups) + values = Map.keys(groups) |> Enum.sort() normalize_contexts(schema, other) |> Enum.map(&Map.merge(&1, %{key => values})) From cd0085108f4a6cb4fd1039d72301d0e2a7f72e92 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 03:06:23 +0000 Subject: [PATCH 15/26] Fix the diftool. The `--wait` is required --- scripts/philomena.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index e494718ed..c4a9e38fe 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -130,7 +130,7 @@ function update_search_syntax_tests { if command -v code &>/dev/null; then step git difftool \ --no-prompt \ - --extcmd "code --diff" \ + --extcmd "code --wait --diff" \ -- test/philomena/images/search-syntax.json fi } From 062be42c03da6402148f9a00efc58773fe679357 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 03:27:43 +0000 Subject: [PATCH 16/26] Add a comment about refactoring in philomena.sh --- scripts/philomena.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index c4a9e38fe..476bd1ee7 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -103,6 +103,8 @@ function init { # Update the `queries.json` test snapshots after the implementation changes. function update_search_syntax_tests { + # TODO: refactor this after the devcontainer PR is merged: + # https://github.com/philomena-dev/philomena/pull/528 # shellcheck disable=SC2016 test=' set -euo pipefail @@ -111,6 +113,8 @@ function update_search_syntax_tests { export MIX_ENV=test + step mix deps.get + if step createdb -h postgres -U postgres philomena_test; then step mix ecto.create step mix ecto.load @@ -119,7 +123,7 @@ function update_search_syntax_tests { fi step mix test test/philomena/images/query_test.exs - step npx prettier --write test/philomena/images/search-syntax.json + step npx prettier --write test/philomena/images/search-syntax.yml ' ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ From 6b298605cd81a5853a2af4a91e0e92c87f5b399e Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 03:38:42 +0000 Subject: [PATCH 17/26] Don't store the users in DB unnecessarily in tests --- scripts/philomena.sh | 2 +- test/philomena/images/query_test.exs | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/philomena.sh b/scripts/philomena.sh index 476bd1ee7..ffa0ddb02 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -123,7 +123,7 @@ function update_search_syntax_tests { fi step mix test test/philomena/images/query_test.exs - step npx prettier --write test/philomena/images/search-syntax.yml + step npx prettier --write test/philomena/images/search-syntax.json ' ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index b9e76e03b..09fa996bb 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -1,15 +1,24 @@ defmodule Philomena.Images.QueryTest do alias Philomena.Labeled - import Philomena.UsersFixtures + alias Philomena.Users.User + alias Philomena.Filters.Filter import Philomena.FiltersFixtures import Philomena.TagsFixtures use Philomena.SearchSyntaxCase defp make_user(attrs) do - # Pretend that all users have the same ID. This doesn't influence the parser - # logic because it doesn't load the users from the DB. - attrs |> user_fixture() |> Map.put(:id, 1) + %User{ + # Pretend that all users have the same ID. This doesn't influence the parser + # logic because it doesn't load the users from the DB. + id: 1, + watched_tag_ids: [], + watched_images_query_str: "", + watched_images_exclude_str: "", + no_spoilered_in_watched: false, + current_filter: %Filter{spoilered_tag_ids: [], spoilered_complex_str: ""} + } + |> Map.merge(attrs) end test "search syntax" do From f3cebccbcd923e5a5a494a7b5d4e57f51486563d Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 03:49:38 +0000 Subject: [PATCH 18/26] Self-review --- docker/app/run-test | 2 +- lib/philomena/images/query.ex | 13 +------------ lib/philomena_query/parse/parser.ex | 4 +++- test/philomena/images/query_test.exs | 1 - test/support/labeled.ex | 4 ++++ 5 files changed, 9 insertions(+), 15 deletions(-) diff --git a/docker/app/run-test b/docker/app/run-test index 713a3abb6..6a3a84198 100755 --- a/docker/app/run-test +++ b/docker/app/run-test @@ -6,9 +6,9 @@ set -euo pipefail export MIX_ENV=test -# Always install mix dependencies cd /srv/philomena +# Always install mix dependencies step mix deps.get # Run formatting check diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 545961309..3dd433865 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -198,18 +198,7 @@ defmodule Philomena.Images.Query do end @type context :: [ - user: %{ - role: String.t(), - id: integer(), - watched_tag_ids: [integer()], - watched_images_query_str: String.t(), - watched_images_exclude_str: String.t(), - no_spoilered_in_watched: boolean(), - current_filter: %{ - spoilered_tag_ids: [integer()], - spoilered_complex_str: String.t() - } - }, + user: map(), watch: boolean(), filter: boolean() ] diff --git a/lib/philomena_query/parse/parser.ex b/lib/philomena_query/parse/parser.ex index 9dbb80b80..4921472bf 100644 --- a/lib/philomena_query/parse/parser.ex +++ b/lib/philomena_query/parse/parser.ex @@ -103,7 +103,9 @@ defmodule PhilomenaQuery.Parse.Parser do transforms: %{String.t() => transform()}, aliases: %{String.t() => String.t()}, no_downcase_fields: [String.t()], - normalizations: %{String.t() => normalizer()} + normalizations: %{String.t() => normalizer()}, + __fields__: map(), + __data__: context() } defstruct [ diff --git a/test/philomena/images/query_test.exs b/test/philomena/images/query_test.exs index 09fa996bb..67d8ef113 100644 --- a/test/philomena/images/query_test.exs +++ b/test/philomena/images/query_test.exs @@ -33,7 +33,6 @@ defmodule Philomena.Images.QueryTest do for id <- 10..14 do tag_fixture(%{id: id, name: "tag#{id}"}) end - |> Enum.to_list() system_filter = system_filter_fixture(%{ diff --git a/test/support/labeled.ex b/test/support/labeled.ex index 605ebb05a..d428c12a7 100644 --- a/test/support/labeled.ex +++ b/test/support/labeled.ex @@ -1,4 +1,8 @@ defmodule Philomena.Labeled do + # Defines a `label, value` pair. The callers can use the `label` to render + # the value in a user-friendly way instead of using the `value` directly. + # The `prefer*` methods can be used to extract the label/value out of the + # value in case if it is a `Labeled` struct instance. @enforce_keys [:label, :value] defstruct [:label, :value] @type t(v) :: %__MODULE__{label: String.t(), value: v} From eab364e23af48d147e92ab54f227b4f804711462 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 14 May 2025 04:05:34 +0000 Subject: [PATCH 19/26] Fix CI --- docker/app/run-test | 3 +++ scripts/philomena.sh | 1 - test/support/search_syntax_case.ex | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docker/app/run-test b/docker/app/run-test index 6a3a84198..9a2284edd 100755 --- a/docker/app/run-test +++ b/docker/app/run-test @@ -11,6 +11,9 @@ cd /srv/philomena # Always install mix dependencies step mix deps.get +# Some tests depend on prettier +step npm ci --no-scripts + # Run formatting check step mix format --check-formatted diff --git a/scripts/philomena.sh b/scripts/philomena.sh index ffa0ddb02..5c3a81075 100755 --- a/scripts/philomena.sh +++ b/scripts/philomena.sh @@ -123,7 +123,6 @@ function update_search_syntax_tests { fi step mix test test/philomena/images/query_test.exs - step npx prettier --write test/philomena/images/search-syntax.json ' ASSERT_VALUE_ACCEPT_DIFFS=y step docker compose run \ diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index bd34cd4d3..6a81b839a 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -56,6 +56,20 @@ defmodule Philomena.SearchSyntaxCase do |> Jason.OrderedObject.new() |> Jason.encode!(pretty: true) + # This is really dumb, but Elixir's subprocess API doesn't support + # passing custom payload via stdin for a command. + {actual, 0} = + System.cmd( + "bash", + [ + "-c", + "echo \"$1\" | npx prettier --stdin-filepath \"$2\" --parser json", + "--", + actual, + test.snapshot + ] + ) + assert_value(actual == File.read!(test.snapshot)) end From f9137152969fb6748f110c57e20ae0ec999f7705 Mon Sep 17 00:00:00 2001 From: MareStare Date: Thu, 15 May 2025 00:49:45 +0000 Subject: [PATCH 20/26] Fix dialyzer warnings --- test/support/search_syntax_case.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index 6a81b839a..364c09a3f 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -71,6 +71,7 @@ defmodule Philomena.SearchSyntaxCase do ) assert_value(actual == File.read!(test.snapshot)) + :ok end @spec compile_input(search_syntax_test(), String.t()) :: [map()] @@ -119,7 +120,7 @@ defmodule Philomena.SearchSyntaxCase do end end - @spec normalize_contexts([contexts_schema()], [map()]) :: [map()] + @spec normalize_contexts(contexts_schema(), [map()]) :: [map()] defp normalize_contexts(schema, contexts) defp normalize_contexts(schema, contexts) when map_size(schema) == 0 do From e400b94fb65967d440baaf07bed700ddda60f14d Mon Sep 17 00:00:00 2001 From: MareStare Date: Thu, 15 May 2025 00:59:54 +0000 Subject: [PATCH 21/26] Add more comments --- test/support/search_syntax_case.ex | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index 364c09a3f..642635f6c 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -120,6 +120,22 @@ defmodule Philomena.SearchSyntaxCase do end end + # Reduces all the combinations of contexts that produce the same output. For + # example the sequence like this: + # ```ex + # [%{ a: "a1", b: "bar1" }, %{ a: "a2", b: "bar2" }] + # ``` + # will be reduced to: + # ```ex + # [%{ a: ["a1", "a2"] }] + # ``` + # only if `bar1` and `bar2` cover the set of all possible values for `b`, and + # `a1` and `a2` don't cover the set of all possible values for `a`. + # + # In other words the value of `b` doesn't influence the output at all, and + # thus can be omitted in the normalized contexts list, and the values of `a` + # are just collected into a list in a single map instead of being several maps + # with a single value in each of them. @spec normalize_contexts(contexts_schema(), [map()]) :: [map()] defp normalize_contexts(schema, contexts) From 78fe2b5c36da67d3f8a774a40dbf736c2871ae51 Mon Sep 17 00:00:00 2001 From: MareStare Date: Thu, 15 May 2025 01:09:05 +0000 Subject: [PATCH 22/26] Fux type mismatch --- lib/philomena/images/query.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/philomena/images/query.ex b/lib/philomena/images/query.ex index 3dd433865..78ed2b7eb 100644 --- a/lib/philomena/images/query.ex +++ b/lib/philomena/images/query.ex @@ -203,7 +203,7 @@ defmodule Philomena.Images.Query do filter: boolean() ] - @spec parse(Parser.options(), context(), String.t()) :: Parser.result() + @spec parse(Parser.options(), map(), String.t()) :: Parser.result() defp parse(fields, context, query_string) do case prepare_context(context, query_string) do {:ok, context} -> From 4f90db5bbeb5061bec03836569753b47653cf785 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 21 May 2025 11:13:57 +0000 Subject: [PATCH 23/26] Use the pipeline operator --- test/support/fixtures/users_fixtures.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/support/fixtures/users_fixtures.ex b/test/support/fixtures/users_fixtures.ex index ddafca093..5be0bc32f 100644 --- a/test/support/fixtures/users_fixtures.ex +++ b/test/support/fixtures/users_fixtures.ex @@ -58,11 +58,15 @@ defmodule Philomena.UsersFixtures do end def confirmed_user_fixture(attrs \\ %{}) do - user_fixture(Map.put(attrs, :confirmed, true)) + attrs + |> Map.put(:confirmed, true) + |> user_fixture() end def locked_user_fixture(attrs \\ %{}) do - user_fixture(Map.put(attrs, :locked, true)) + attrs + |> Map.put(:locked, true) + |> user_fixture() end def extract_user_token(fun) do From 8baeb95336691b2bb4c6e7d8f83d6228da6eb91f Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 21 May 2025 11:21:24 +0000 Subject: [PATCH 24/26] Use the fixed 0.10.5 version of assert_value --- mix.exs | 14 +------------- mix.lock | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/mix.exs b/mix.exs index 67b15440b..706e39930 100644 --- a/mix.exs +++ b/mix.exs @@ -96,19 +96,7 @@ defmodule Philomena.MixProject do github: "marcinkoziej/canary", ref: "704debde7a2c0600f78c687807884bf37c45bd79"}, # Automated testing - { - :assert_value, - "~> 0.10.4", - # This is really stupid but Elixir just doesn't have any mechanism to - # suppress warnings at all, even from code from third-party libraries. - # So we are using the version from - # https://github.com/assert-value/assert_value_elixir/pull/18 which - # fixes the compiler warning "the following clause will never match" - # when using `assert_value`. - only: [:dev, :test], - github: "assert-value/assert_value_elixir", - ref: "4f4e86f0c340d3c29756e68f385ebec85113ba0f" - } + {:assert_value, "~> 0.10.5", only: [:dev, :test]} ] end diff --git a/mix.lock b/mix.lock index 57fc3506a..e7ea9fb8f 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,5 @@ %{ - "assert_value": {:git, "https://github.com/assert-value/assert_value_elixir.git", "4f4e86f0c340d3c29756e68f385ebec85113ba0f", [ref: "4f4e86f0c340d3c29756e68f385ebec85113ba0f"]}, + "assert_value": {:hex, :assert_value, "0.10.5", "b1d68243194ba3da490674ff53a9d9b8dfff411d5dec9fc113c9b82be76eeddf", [:mix], [], "hexpm", "ba89aecb2e886e55b2c7ef9973a153838976b2abd10a931fa2d41b74dfb27de6"}, "bandit": {:hex, :bandit, "1.6.11", "2fbadd60c95310eefb4ba7f1e58810aa8956e18c664a3b2029d57edb7d28d410", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "543f3f06b4721619a1220bed743aa77bf7ecc9c093ba9fab9229ff6b99eacc65"}, "bcrypt_elixir": {:hex, :bcrypt_elixir, "3.3.1", "9f2e7e00f661a6acfae1431f1bc590e28698aaecc962c2a7b33150dfe9289c3d", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "9f539e9d3828fad4ffc8152dadc0d27c6d78cb2853a9a1d6518cfe8a5adb7f50"}, "briefly": {:hex, :briefly, "0.5.1", "ee10d48da7f79ed2aebdc3e536d5f9a0c3e36ff76c0ad0d4254653a152b13a8a", [:mix], [], "hexpm", "bd684aa92ad8b7b4e0d92c31200993c4bc1469fc68cd6d5f15144041bd15cb57"}, From bdcf8483741b650712c4f8d5cc6c03d847ada443 Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 21 May 2025 11:49:01 +0000 Subject: [PATCH 25/26] Link to the Port API hexdocs.pm/elixir/1.18.3/Port.html#module-example --- test/support/search_syntax_case.ex | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index 642635f6c..4709c8b6d 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -56,8 +56,11 @@ defmodule Philomena.SearchSyntaxCase do |> Jason.OrderedObject.new() |> Jason.encode!(pretty: true) - # This is really dumb, but Elixir's subprocess API doesn't support - # passing custom payload via stdin for a command. + # Elixir's `System.cmd` API doesn't support passing custom payload via stdin + # for a command. As a simple workaround we use a bash wrapper that translates + # a CLI parameter into the stdin for `prettier`. An alternative way to do + # that could be with the Port API, but bash solution is a bit simpler: + # hexdocs.pm/elixir/1.18.3/Port.html#module-example {actual, 0} = System.cmd( "bash", From d0314f8a5ec91f3790dbc2fd40d796c872cf51ad Mon Sep 17 00:00:00 2001 From: MareStare Date: Wed, 21 May 2025 11:52:40 +0000 Subject: [PATCH 26/26] Add https:// --- test/support/search_syntax_case.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/search_syntax_case.ex b/test/support/search_syntax_case.ex index 4709c8b6d..796a61e79 100644 --- a/test/support/search_syntax_case.ex +++ b/test/support/search_syntax_case.ex @@ -60,7 +60,7 @@ defmodule Philomena.SearchSyntaxCase do # for a command. As a simple workaround we use a bash wrapper that translates # a CLI parameter into the stdin for `prettier`. An alternative way to do # that could be with the Port API, but bash solution is a bit simpler: - # hexdocs.pm/elixir/1.18.3/Port.html#module-example + # https://hexdocs.pm/elixir/1.18.3/Port.html#module-example {actual, 0} = System.cmd( "bash",