From a23a51b17bf3f3048f382d571f03bf09cb70de19 Mon Sep 17 00:00:00 2001 From: Stefan Breunig Date: Mon, 3 Jun 2024 12:19:45 +0200 Subject: [PATCH] handle slow search response times + boost results from Hamburg --- data/articles/static/suche.ex | 43 +++++++-------- data/settings.ex | 2 + lib/article/decorators.ex | 2 +- lib/article/renderer.ex | 2 +- lib/basemap/nominatim.ex | 12 ++++- lib/search/meilisearch/api.ex | 15 +++--- lib/search/meilisearch/nominatim.ex | 30 +++++++---- lib/search/meilisearch/runner.ex | 52 ++++++++++-------- lib/veloroute_web/live/frame_live.ex | 58 +++++++++++++-------- lib/veloroute_web/live/frame_live.html.heex | 2 +- test/article/article_test.exs | 2 +- 11 files changed, 129 insertions(+), 91 deletions(-) diff --git a/data/articles/static/suche.ex b/data/articles/static/suche.ex index 07ed7392..c492815c 100644 --- a/data/articles/static/suche.ex +++ b/data/articles/static/suche.ex @@ -9,8 +9,6 @@ defmodule Data.Article.Static.Suche do def tags(), do: [] def text(assigns) do - assigns = assign(assigns, :search_results, search(assigns)) - ~H"""

@@ -18,30 +16,33 @@ defmodule Data.Article.Static.Suche do
<.noindex> - <%= if @search_results == [] && @search_query != "" do %> -

Leider keine Ergebnisse.

- <% else %> - - <% end %> + <.async_result :let={search_results} assign={@search_results}> + <:loading>

Lädt…

+ <:failed :let={error}> +

Fehler in der Suchfunktion:

+ <%= inspect(error) %> + + <.results results={search_results} query={@search_query}/> + """ end - defp search(%{search_query: query, search_bounds: bbox}) do - bbox = Geo.BoundingBox.parse(bbox) || Settings.initial() + defp results(%{results: sr, query: sq} = assigns) when sr == [] and sq != "" do + ~H{

Leider keine Ergebnisse.

} + end - Search.Meilisearch.Runner.query(query, bbox) - |> Enum.reject(&is_nil/1) - |> Search.Result.merge_same() - |> Search.Result.sort() - |> Enum.take(15) + defp results(%{results: sr} = assigns) when is_list(sr) do + ~H""" + + """ end defp debug?() do diff --git a/data/settings.ex b/data/settings.ex index bdd89e41..624fa22c 100644 --- a/data/settings.ex +++ b/data/settings.ex @@ -78,6 +78,8 @@ defmodule Settings do position: %{lat: 53.55044, lon: 9.99440} } + def boost_search_results_within, do: "Hamburg" + # no trailing slash def url, do: "https://veloroute.hamburg" diff --git a/lib/article/decorators.ex b/lib/article/decorators.ex index 1535ffa4..03e6583b 100644 --- a/lib/article/decorators.ex +++ b/lib/article/decorators.ex @@ -54,7 +54,7 @@ defmodule Article.Decorators do default = %{ __changed__: %{}, search_query: nil, - search_bounds: nil, + search_results: Phoenix.LiveView.AsyncResult.ok([]), limit_to_map_bounds: false, show_map_image: false, enable_drawing_tools: false, diff --git a/lib/article/renderer.ex b/lib/article/renderer.ex index 2b6b8042..045567e5 100644 --- a/lib/article/renderer.ex +++ b/lib/article/renderer.ex @@ -10,7 +10,7 @@ defmodule Article.Renderer do attr :map_bounds, :any attr :lang, :string attr :search_query, :string - attr :search_bounds, :any + attr :search_results, :any attr :enable_drawing_tools, :boolean, default: false attr :limit_to_map_bounds, :boolean, default: false attr :show_map_image, :boolean, default: false diff --git a/lib/basemap/nominatim.ex b/lib/basemap/nominatim.ex index 1f6eddd2..97e74fc2 100644 --- a/lib/basemap/nominatim.ex +++ b/lib/basemap/nominatim.ex @@ -360,7 +360,11 @@ defmodule Basemap.Nominatim do ST_YMax(ST_Envelope(ST_UNION(geometry))) ) AS bbox, combo.parents_name, - combo.parents_postcode + combo.parents_postcode, + CASE + WHEN '#{Settings.boost_search_results_within()}' = ANY(combo.parents_name) THEN 1 + ELSE 30 + END AS rank_boosted_areas FROM combo GROUP BY combo.class, @@ -407,7 +411,11 @@ defmodule Basemap.Nominatim do ST_Y(interpol.centroid) ) AS bbox, combo.parents_name, - combo.parents_postcode + combo.parents_postcode, + CASE + WHEN '#{Settings.boost_search_results_within()}' = ANY(combo.parents_name) THEN 1 + ELSE 30 + END AS rank_boosted_areas FROM combo INNER JOIN interpol ON interpol.parent_place_id = combo.place_id WHERE combo.name->'name' IS NOT NULL diff --git a/lib/search/meilisearch/api.ex b/lib/search/meilisearch/api.ex index e73efda6..c93176c6 100644 --- a/lib/search/meilisearch/api.ex +++ b/lib/search/meilisearch/api.ex @@ -3,7 +3,7 @@ defmodule Search.Meilisearch.API do use Tesla @index_timeout_ms 5 * 60 * 1000 - @general_timeout_ms 1500 + @general_timeout_ms 10_000 plug Tesla.Middleware.BaseUrl, "http://localhost:7700/" plug Tesla.Middleware.JSON @@ -101,7 +101,7 @@ defmodule Search.Meilisearch.API do end end - @spec multi_search(%{atom() => map()}) :: %{atom() => list()} + @spec multi_search(%{atom() => map()}) :: {:ok, %{atom() => list()}} | {:error, binary()} def multi_search(queries) do payload = %{ "queries" => @@ -114,14 +114,13 @@ defmodule Search.Meilisearch.API do with {:ok, %{body: %{"results" => results}}} <- post("/multi-search", payload, opts: @adapter_opts_general) do - Enum.into(results, %{}, fn %{"indexUid" => index, "hits" => hits} -> - {String.to_existing_atom(index), hits} - end) + {:ok, + Enum.into(results, %{}, fn %{"indexUid" => index, "hits" => hits} -> + {String.to_existing_atom(index), hits} + end)} else other -> - Logger.warning("failed to multi-query for #{inspect(payload)}. Result: #{inspect(other)}") - - [] + {:error, "failed to multi-query for #{inspect(payload)}. Result: #{inspect(other)}"} end end diff --git a/lib/search/meilisearch/nominatim.ex b/lib/search/meilisearch/nominatim.ex index 2108f5bf..20c4079e 100644 --- a/lib/search/meilisearch/nominatim.ex +++ b/lib/search/meilisearch/nominatim.ex @@ -17,13 +17,7 @@ defmodule Search.Meilisearch.Nominatim do %{ q: query, limit: 10, - sort: [ - "rank_search:asc", - "rank_address:asc", - "importance:desc", - "_geoPoint(#{lat}, #{lon}):asc", - "admin_level:asc" - ] + sort: ["_geoPoint(#{lat}, #{lon}):asc"] } end @@ -81,7 +75,7 @@ defmodule Search.Meilisearch.Nominatim do bounds: bbox, center: Geo.CheapRuler.center(bbox), name: name, - relevance: f.("_rankingScore"), + relevance: f.("_rankingScore") + (30 - f.("rank_boosted_areas")) / 1000, type: if(f.("class") in ["place"], do: "poi", else: ""), subtext: subtext } @@ -212,11 +206,25 @@ defmodule Search.Meilisearch.Nominatim do %{ displayedAttributes: - ~w(id class type name address parents_name parents_postcode extratags bbox boost), + ~w(id class type name address parents_name parents_postcode extratags bbox boost rank_boosted_areas rank_search rank_address importance), # order is from most important to least important searchableAttributes: ~w(name boost address type parents_name type extratags), - sortableAttributes: ~w(importance rank_search rank_address _geo admin_level), - synonyms: synonyms + sortableAttributes: + ~w(importance rank_search rank_address rank_boosted_areas _geo admin_level), + synonyms: synonyms, + proximityPrecision: "byAttribute", + rankingRules: ~w(words + typo + proximity + attribute + rank_boosted_areas:asc + rank_search:asc + rank_address:asc + importance:desc + admin_level:asc + sort + exactness + ) } end diff --git a/lib/search/meilisearch/runner.ex b/lib/search/meilisearch/runner.ex index 2f81371f..03445563 100644 --- a/lib/search/meilisearch/runner.ex +++ b/lib/search/meilisearch/runner.ex @@ -21,17 +21,16 @@ defmodule Search.Meilisearch.Runner do queue: [queued_task()] } - @spec query(binary() | nil, Geo.BoundingBox.like()) :: [Search.Result.t()] - def query(nil, _bbox), do: [] - def query("", _bbox), do: [] + @spec query(binary() | nil, Geo.BoundingBox.like()) :: + {:ok, [Search.Result.t()]} | {:error, binary()} + def query(nil, _bbox), do: {:ok, []} + def query("", _bbox), do: {:ok, []} def query(query, bbox) do try do - GenServer.call(__MODULE__, {:search, query, bbox}, 1000) + GenServer.call(__MODULE__, {:search, query, bbox}, :infinity) catch - :exit, err -> - Logger.warning(inspect(err)) - [] + :exit, err -> {:error, inspect(err)} end end @@ -299,30 +298,39 @@ defmodule Search.Meilisearch.Runner do defp search(query, bbox) do %{lat: lat, lon: lon} = Geo.CheapRuler.center(bbox) - lookup = Enum.into(@indexers, %{}, &{&1.id(), &1}) - @indexers |> Enum.into(%{}, fn indexer -> {indexer.id(), indexer.params(query, lat, lon)} end) |> Search.Meilisearch.API.multi_search() - |> Enum.flat_map(fn {index, results} -> - indexer = lookup[index] + |> post_process() + end - results = - if function_exported?(indexer, :maybe_merge, 1), - do: indexer.maybe_merge(results), - else: results + defp post_process({:ok, list}) do + lookup = Enum.into(@indexers, %{}, &{&1.id(), &1}) - Enum.map(results, fn result -> - sr = indexer.format(result) - if sr, do: Map.put(sr, :source, inspect(result, pretty: true)) - end) - end) - |> Util.compact() - |> Enum.reject(fn %{relevance: rel} -> rel < @min_relevance end) + {:ok, + Enum.flat_map(list, fn {index, results} -> + indexer = lookup[index] + + results = + if function_exported?(indexer, :maybe_merge, 1), + do: indexer.maybe_merge(results), + else: results + + Enum.map(results, fn result -> + sr = indexer.format(result) + if sr, do: Map.put(sr, :source, inspect(result, pretty: true)) + end) + end) + |> Util.compact() + |> Enum.reject(fn %{relevance: rel} -> rel < @min_relevance end) + |> Search.Result.merge_same() + |> Search.Result.sort()} end + defp post_process({:error, reason}), do: {:error, reason} + @spec index(state()) :: state() defp index(%{indexers: []} = state), do: state diff --git a/lib/veloroute_web/live/frame_live.ex b/lib/veloroute_web/live/frame_live.ex index db79ab04..a2569b92 100644 --- a/lib/veloroute_web/live/frame_live.ex +++ b/lib/veloroute_web/live/frame_live.ex @@ -3,6 +3,9 @@ defmodule VelorouteWeb.FrameLive do require Logger import Guards + @search_page Data.Article.Static.Suche + @search_page_name @search_page.name() + @default_bounds struct(Geo.BoundingBox, Settings.initial()) @initial_state [ prev_page: nil, @@ -12,8 +15,8 @@ defmodule VelorouteWeb.FrameLive do article_date: nil, article_title: nil, article_summary: nil, - search_query: nil, - search_bounds: nil, + search_query: "", + search_results: nil, tmp_last_article_set: nil, limit_to_map_bounds: false, show_map_image: false, @@ -36,7 +39,12 @@ defmodule VelorouteWeb.FrameLive do map_bounds: Geo.BoundingBox.parse(params["bounds"]) || @default_bounds ) - socket = socket |> assign(state) |> maybe_run_events_from_url(params) + socket = + socket + |> assign(state) + |> maybe_run_events_from_url(params) + |> search(params["search_query"]) + {:ok, socket} end @@ -89,27 +97,12 @@ defmodule VelorouteWeb.FrameLive do {:noreply, socket} end - def handle_event("search", %{"search_query" => value}, socket) do - handle_event("search", %{"value" => value}, socket) - end - - def handle_event("search", %{"value" => ""}, socket) do - {:noreply, socket} + def handle_event("search", %{"search_query" => query}, socket) do + {:noreply, search(socket, query) |> show_search_page()} end - @search_page "suche" def handle_event("search", %{"value" => query}, socket) do - query = if query && query != "", do: String.trim(query), else: socket.assigns.search_query - search_article = Article.List.find_exact(@search_page) - - socket = - socket - |> assign(:search_query, query || "") - |> assign(:search_bounds, socket.assigns[:map_bounds]) - - socket = push_patch(socket, to: article_path(socket, search_article)) - - {:noreply, socket} + {:noreply, search(socket, query) |> show_search_page()} end def handle_event("video-reverse", attr, socket) do @@ -267,9 +260,9 @@ defmodule VelorouteWeb.FrameLive do {:noreply, socket} end - def handle_params(%{"page" => @search_page, "search_query" => query} = params, nil, socket) do + def handle_params(%{"page" => @search_page_name, "search_query" => query} = params, nil, socket) do params - |> Map.put("article", @search_page) + |> Map.put("article", @search_page_name) |> handle_params(nil, assign(socket, :search_query, query)) end @@ -329,6 +322,25 @@ defmodule VelorouteWeb.FrameLive do |> push_patch(to: ~p"/?#{url_query(socket)}") end + defp search(socket, query) do + query = if query && query != "", do: String.trim(query), else: socket.assigns.search_query + bbox = Geo.BoundingBox.parse(socket.assigns[:map_bounds]) || Settings.initial() + + querier = fn -> + with {:ok, results} <- Search.Meilisearch.Runner.query(query, bbox) do + {:ok, %{search_results: results}} + end + end + + socket + |> assign(:search_query, query) + |> assign_async(:search_results, querier) + end + + defp show_search_page(socket) do + push_patch(socket, to: article_path(socket, @search_page)) + end + defp set_bounds(socket, article, bounds_param) defp set_bounds(%{assigns: %{map_bounds: @default_bounds}} = socket, article, bounds_param) diff --git a/lib/veloroute_web/live/frame_live.html.heex b/lib/veloroute_web/live/frame_live.html.heex index 448e3d1a..37696f3e 100644 --- a/lib/veloroute_web/live/frame_live.html.heex +++ b/lib/veloroute_web/live/frame_live.html.heex @@ -89,7 +89,7 @@ video_hash={@video_hash} video_start={@video_start} search_query={@search_query} - search_bounds={@search_bounds} + search_results={@search_results} map_bounds={@map_bounds} limit_to_map_bounds={@limit_to_map_bounds} lang={@lang} diff --git a/test/article/article_test.exs b/test/article/article_test.exs index 1a368062..de6821b8 100644 --- a/test/article/article_test.exs +++ b/test/article/article_test.exs @@ -22,7 +22,7 @@ defmodule ArticleTest do Article.Decorators.html(art, %{ __changed__: %{}, search_query: nil, - search_bounds: nil, + search_results: Phoenix.LiveView.AsyncResult.ok([]), limit_to_map_bounds: false, show_map_image: false })