From 75ec6afffc5a79331014c2c279a39d2ea860758d Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 22 Oct 2019 10:25:28 +0200 Subject: [PATCH 1/3] Implement search engine & service in backend Signed-off-by: Thomas Citharel --- config/dev.exs | 3 +- lib/mobilizon/events/events.ex | 81 +++++++++++++------ lib/mobilizon_web/api/search.ex | 2 +- lib/service/search.ex | 59 ++++++++++++++ mix.exs | 1 + mix.lock | 1 + ...materialized_view_for_searching_events.exs | 65 +++++++++++++++ test/mobilizon/events/events_test.exs | 21 +++-- test/mobilizon_web/api/search_test.exs | 4 +- .../resolvers/search_resolver_test.exs | 34 +++++--- 10 files changed, 226 insertions(+), 45 deletions(-) create mode 100644 lib/service/search.ex create mode 100644 priv/repo/migrations/20191022083446_add_eager_materialized_view_for_searching_events.exs diff --git a/config/dev.exs b/config/dev.exs index c50f236af..ac5f5a921 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -71,4 +71,5 @@ config :mobilizon, Mobilizon.Storage.Repo, database: System.get_env("MOBILIZON_DATABASE_DBNAME") || "mobilizon_dev", hostname: System.get_env("MOBILIZON_DATABASE_HOST") || "localhost", port: System.get_env("MOBILIZON_DATABASE_PORT") || "5432", - pool_size: 10 + pool_size: 10, + show_sensitive_data_on_connection_error: true diff --git a/lib/mobilizon/events/events.ex b/lib/mobilizon/events/events.ex index 5458cc087..3737f229f 100644 --- a/lib/mobilizon/events/events.ex +++ b/lib/mobilizon/events/events.ex @@ -12,6 +12,7 @@ defmodule Mobilizon.Events do alias Mobilizon.Actors.Actor alias Mobilizon.Addresses.Address + alias Mobilizon.Service.Search alias Mobilizon.Events.{ Comment, @@ -246,6 +247,7 @@ defmodule Mobilizon.Events do role: :creator, event_id: event.id }) do + Search.insert_search_event(event) {:ok, event} else # We don't create a creator participant if the event is a draft @@ -259,20 +261,11 @@ defmodule Mobilizon.Events do with {:ok, %Event{} = event} <- %Event{} |> Event.changeset(attrs) + |> Ecto.Changeset.put_assoc(:tags, Map.get(attrs, "tags", [])) |> Repo.insert(), %Event{} = event <- - Repo.preload(event, [:tags, :organizer_actor, :physical_address, :picture]), - {:has_tags, true, _} <- {:has_tags, Map.has_key?(attrs, "tags"), event} do - event - |> Ecto.Changeset.change() - |> Ecto.Changeset.put_assoc(:tags, attrs["tags"]) - |> Repo.update() - else - {:has_tags, false, event} -> - {:ok, event} - - error -> - error + Repo.preload(event, [:tags, :organizer_actor, :physical_address, :picture]) do + {:ok, event} end end @@ -306,6 +299,8 @@ defmodule Mobilizon.Events do changes ) + Search.update_search_event(new_event) + {:ok, new_event} end end @@ -418,11 +413,14 @@ defmodule Mobilizon.Events do @doc """ Builds a page struct for events by their name. """ - @spec build_events_by_name(String.t(), integer | nil, integer | nil) :: Page.t() - def build_events_by_name(name, page \\ nil, limit \\ nil) do + @spec build_events_for_search(String.t(), integer | nil, integer | nil) :: Page.t() + def build_events_for_search(name, page \\ nil, limit \\ nil) + def build_events_for_search("", _page, _limit), do: %Page{total: 0, elements: []} + + def build_events_for_search(name, page, limit) do name - |> String.trim() - |> events_by_name_query() + |> normalize_search_string() + |> events_for_search_query() |> Page.build_page(page, limit) end @@ -1203,18 +1201,51 @@ defmodule Mobilizon.Events do ) end - @spec events_by_name_query(String.t()) :: Ecto.Query.t() - defp events_by_name_query(name) do - from( - e in Event, - where: - e.visibility == ^:public and - fragment("f_unaccent(?) %> f_unaccent(?)", e.title, ^name), - order_by: fragment("word_similarity(?, ?) desc", e.title, ^name), - preload: [:organizer_actor] + defmacro matching_event_ids_and_ranks(search_string) do + quote do + fragment( + """ + SELECT event_search.id AS id, + ts_rank( + event_search.document, plainto_tsquery(unaccent(?)) + ) AS rank + FROM event_search + WHERE event_search.document @@ plainto_tsquery(unaccent(?)) + OR event_search.title ILIKE ? + """, + ^unquote(search_string), + ^unquote(search_string), + ^"%#{unquote(search_string)}%" + ) + end + end + + @spec events_for_search_query(String.t()) :: Ecto.Query.t() + defp events_for_search_query(search_string) do + Event + |> where([e], e.visibility in ^@public_visibility) + |> do_event_for_search_query(search_string) + end + + @spec do_event_for_search_query(Ecto.Query.t(), String.t()) :: Ecto.Query.t() + defp do_event_for_search_query(query, search_string) do + from(event in query, + join: id_and_rank in matching_event_ids_and_ranks(search_string), + on: id_and_rank.id == event.id, + order_by: [desc: id_and_rank.rank] ) end + @spec normalize_search_string(String.t()) :: String.t() + defp normalize_search_string(search_string) do + search_string + |> String.downcase() + |> String.replace(~r/\n/, " ") + |> String.replace(~r/\t/, " ") + |> String.replace(~r/\s{2,}/, " ") + |> String.trim() + end + @spec events_by_tags_query([integer], integer) :: Ecto.Query.t() def events_by_tags_query(tags_ids, limit) do from( diff --git a/lib/mobilizon_web/api/search.ex b/lib/mobilizon_web/api/search.ex index 835bbce5a..0124afb5f 100644 --- a/lib/mobilizon_web/api/search.ex +++ b/lib/mobilizon_web/api/search.ex @@ -68,7 +68,7 @@ defmodule MobilizonWeb.API.Search do end true -> - {:ok, Events.build_events_by_name(search, page, limit)} + {:ok, Events.build_events_for_search(search, page, limit)} end end diff --git a/lib/service/search.ex b/lib/service/search.ex new file mode 100644 index 000000000..08d0ee9b4 --- /dev/null +++ b/lib/service/search.ex @@ -0,0 +1,59 @@ +defmodule Mobilizon.Service.Search do + @moduledoc """ + Module to handle search service + """ + + alias Mobilizon.Events.Event + alias Mobilizon.Storage.Repo + alias Ecto.Adapters.SQL + + def insert_search_event(%Event{} = event) do + SQL.query( + Repo, + """ + INSERT INTO event_search(id, title, document) VALUES ($1, $2, ( + SELECT + setweight(to_tsvector(unaccent($2)), 'A') || + setweight(to_tsvector(unaccent(coalesce($4, ' '))), 'B') || + setweight(to_tsvector(unaccent($3)), 'C') + ) + ); + """, + [ + event.id, + event.title, + HtmlSanitizeEx.strip_tags(event.description), + get_tags_string(event) + ] + ) + end + + def update_search_event(%Event{} = event) do + SQL.query( + Repo, + """ + UPDATE event_search + SET document = + (SELECT + setweight(to_tsvector(unaccent($2)), 'A') || + setweight(to_tsvector(unaccent(coalesce($4, ' '))), 'B') || + setweight(to_tsvector(unaccent($3)), 'C') + ), + title = $2 + WHERE id = $1; + """, + [ + event.id, + event.title, + HtmlSanitizeEx.strip_tags(event.description), + get_tags_string(event) + ] + ) + end + + defp get_tags_string(%Event{tags: tags}) do + tags + |> Enum.map(& &1.title) + |> Enum.join(" ") + end +end diff --git a/mix.exs b/mix.exs index f4a2bdff1..252157834 100644 --- a/mix.exs +++ b/mix.exs @@ -99,6 +99,7 @@ defmodule Mobilizon.Mixfile do {:html_sanitize_ex, "~> 1.3.0"}, {:ex_cldr_dates_times, "~> 2.0"}, {:ex_optimizer, "~> 0.1"}, + {:progress_bar, "~> 2.0"}, # Dev and test dependencies {:phoenix_live_reload, "~> 1.2", only: [:dev, :e2e]}, {:ex_machina, "~> 2.3", only: [:dev, :test]}, diff --git a/mix.lock b/mix.lock index 33b2eb293..6e765d0ae 100644 --- a/mix.lock +++ b/mix.lock @@ -97,6 +97,7 @@ "poison": {:hex, :poison, "4.0.1", "bcb755a16fac91cad79bfe9fc3585bb07b9331e50cfe3420a24bcc2d735709ae", [:mix], [], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm"}, "postgrex": {:hex, :postgrex, "0.15.1", "23ce3417de70f4c0e9e7419ad85bdabcc6860a6925fe2c6f3b1b5b1e8e47bf2f", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm"}, + "progress_bar": {:hex, :progress_bar, "2.0.0", "447285f533b4b8717881fdb7160c7360c2f2ab57276f8904ce6d40482857e573", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm"}, "rdf": {:hex, :rdf, "0.6.2", "1b85e37c135e232febeebda6b04ac4aba5f5e2bb1c3a2a6665ed4ccec19ade70", [:mix], [{:decimal, "~> 1.5", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, "rsa_ex": {:hex, :rsa_ex, "0.4.0", "e28dd7dc5236e156df434af0e4aa822384c8866c928e17b785d4edb7c253b558", [:mix], [], "hexpm"}, diff --git a/priv/repo/migrations/20191022083446_add_eager_materialized_view_for_searching_events.exs b/priv/repo/migrations/20191022083446_add_eager_materialized_view_for_searching_events.exs new file mode 100644 index 000000000..7d1c6f5f6 --- /dev/null +++ b/priv/repo/migrations/20191022083446_add_eager_materialized_view_for_searching_events.exs @@ -0,0 +1,65 @@ +defmodule Mobilizon.Storage.Repo.Migrations.AddEagerMaterializedViewForSearchingEvents do + use Ecto.Migration + import Ecto.Query + + alias Mobilizon.Storage.Repo + alias Mobilizon.Service.Search + alias Mobilizon.Events.Event + + require Logger + + def up do + create table(:event_search, primary_key: false) do + add(:id, references(:events, on_delete: :delete_all, on_update: :update_all), + primary_key: true + ) + + add(:title, :text, null: false) + add(:document, :tsvector) + end + + # to support full-text searches + create(index("event_search", [:document], using: :gin)) + + # to support substring title matches with ILIKE + execute( + "CREATE INDEX event_search_title_trgm_index ON event_search USING gin (title gin_trgm_ops)" + ) + + # to support updating CONCURRENTLY + create(unique_index("event_search", [:id])) + + flush() + + events = + Event + |> preload([e], :tags) + |> Repo.all() + + nb_events = length(events) + + IO.puts("\nStarting setting up search for #{nb_events} events, this can take a while…\n") + insert_search_event(events, nb_events) + end + + defp insert_search_event([%Event{url: url} = event | events], nb_events) do + with {:ok, _} <- Search.insert_search_event(event) do + Logger.debug("Added event #{url} to the search") + else + {:error, res} -> + Logger.error("Error while adding event #{url} to the search: #{inspect(res)}") + end + + ProgressBar.render(nb_events - length(events), nb_events) + + insert_search_event(events, nb_events) + end + + defp insert_search_event([], nb_events) do + IO.puts("\nFinished setting up search for #{nb_events} events!\n") + end + + def down do + drop(table(:event_search)) + end +end diff --git a/test/mobilizon/events/events_test.exs b/test/mobilizon/events/events_test.exs index 5aabd690f..f9de6244e 100644 --- a/test/mobilizon/events/events_test.exs +++ b/test/mobilizon/events/events_test.exs @@ -7,6 +7,7 @@ defmodule Mobilizon.EventsTest do alias Mobilizon.Events alias Mobilizon.Events.{Comment, Event, Participant, Session, Tag, TagRelation, Track} alias Mobilizon.Storage.Page + alias Mobilizon.Service.Search @event_valid_attrs %{ begins_on: "2010-04-17 14:00:00Z", @@ -22,6 +23,7 @@ defmodule Mobilizon.EventsTest do setup do actor = insert(:actor) event = insert(:event, organizer_actor: actor, visibility: :public) + Search.insert_search_event(event) {:ok, actor: actor, event: event} end @@ -55,22 +57,31 @@ defmodule Mobilizon.EventsTest do assert Events.get_event_with_preload!(event.id).participants == [] end - test "build_events_by_name/1 returns events for a given name", %{ + test "build_events_for_search/1 returns events for a given name", %{ event: %Event{title: title} = event } do - assert title == hd(Events.build_events_by_name(event.title).elements).title + assert title == hd(Events.build_events_for_search(event.title).elements).title %Event{} = event2 = insert(:event, title: "Special event") + Search.insert_search_event(event2) assert event2.title == - Events.build_events_by_name("Special").elements |> hd() |> Map.get(:title) + Events.build_events_for_search("Special").elements |> hd() |> Map.get(:title) assert event2.title == - Events.build_events_by_name(" Special ").elements + Events.build_events_for_search(" Spécïal ").elements |> hd() |> Map.get(:title) - assert %Page{elements: [], total: 0} == Events.build_events_by_name("") + tag1 = insert(:tag, title: "coucou") + tag2 = insert(:tag, title: "hola") + %Event{} = event3 = insert(:event, title: "Nothing like it", tags: [tag1, tag2]) + Search.insert_search_event(event3) + + assert event3.title == + Events.build_events_for_search("hola").elements |> hd() |> Map.get(:title) + + assert %Page{elements: [], total: 0} == Events.build_events_for_search("") end test "find_close_events/3 returns events in the area" do diff --git a/test/mobilizon_web/api/search_test.exs b/test/mobilizon_web/api/search_test.exs index c9acc25a8..ee19fdca3 100644 --- a/test/mobilizon_web/api/search_test.exs +++ b/test/mobilizon_web/api/search_test.exs @@ -46,13 +46,13 @@ defmodule MobilizonWeb.API.SearchTest do test "search events" do with_mock Events, - build_events_by_name: fn "toto", 1, 10 -> + build_events_for_search: fn "toto", 1, 10 -> %Page{total: 1, elements: [%Event{title: "super toto event"}]} end do assert {:ok, %{total: 1, elements: [%Event{title: "super toto event"}]}} = Search.search_events("toto", 1, 10) - assert_called(Events.build_events_by_name("toto", 1, 10)) + assert_called(Events.build_events_for_search("toto", 1, 10)) end end end diff --git a/test/mobilizon_web/resolvers/search_resolver_test.exs b/test/mobilizon_web/resolvers/search_resolver_test.exs index d7374474f..de37176ec 100644 --- a/test/mobilizon_web/resolvers/search_resolver_test.exs +++ b/test/mobilizon_web/resolvers/search_resolver_test.exs @@ -2,6 +2,7 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do use MobilizonWeb.ConnCase alias MobilizonWeb.AbsintheHelpers import Mobilizon.Factory + alias Mobilizon.Service.Search setup %{conn: conn} do user = insert(:user) @@ -16,6 +17,7 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do insert(:actor, user: user, preferred_username: "test_person") insert(:actor, type: :Group, preferred_username: "test_group") event = insert(:event, title: "test_event") + Search.insert_search_event(event) query = """ { @@ -48,7 +50,8 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do } do actor = insert(:actor, user: user, preferred_username: "test_person") insert(:actor, type: :Group, preferred_username: "test_group") - insert(:event, title: "test_event") + event = insert(:event, title: "test_event") + Search.insert_search_event(event) query = """ { @@ -80,7 +83,8 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do } do insert(:actor, user: user, preferred_username: "test_person") group = insert(:actor, type: :Group, preferred_username: "test_group") - insert(:event, title: "test_event") + event = insert(:event, title: "test_event") + Search.insert_search_event(event) query = """ { @@ -111,9 +115,12 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do user: user } do insert(:actor, user: user, preferred_username: "person", name: "I like pineapples") - insert(:event, title: "Pineapple fashion week") - insert(:event, title: "I love pineAPPLE") - insert(:event, title: "Hello") + event1 = insert(:event, title: "Pineapple fashion week") + event2 = insert(:event, title: "I love pineAPPLE") + event3 = insert(:event, title: "Hello") + Search.insert_search_event(event1) + Search.insert_search_event(event2) + Search.insert_search_event(event3) query = """ { @@ -140,8 +147,8 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do assert json_response(res, 200)["data"]["search_events"]["elements"] |> Enum.map(& &1["title"]) == [ - "I love pineAPPLE", - "Pineapple fashion week" + "Pineapple fashion week", + "I love pineAPPLE" ] end @@ -151,9 +158,12 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do } do actor = insert(:actor, user: user, preferred_username: "person", name: "I like pineapples") insert(:actor, preferred_username: "group", type: :Group, name: "pineapple group") - insert(:event, title: "Pineapple fashion week") - insert(:event, title: "I love pineAPPLE") - insert(:event, title: "Hello") + event1 = insert(:event, title: "Pineapple fashion week") + event2 = insert(:event, title: "I love pineAPPLE") + event3 = insert(:event, title: "Hello") + Search.insert_search_event(event1) + Search.insert_search_event(event2) + Search.insert_search_event(event3) query = """ { @@ -188,6 +198,7 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do insert(:actor, user: user, preferred_username: "person", name: "Torréfaction du Kafé") insert(:actor, type: :Group, preferred_username: "group", name: "Kafé group") event = insert(:event, title: "Tour du monde des Kafés") + Search.insert_search_event(event) # Elaborate query query = """ @@ -218,7 +229,8 @@ defmodule MobilizonWeb.Resolvers.SearchResolverTest do } do insert(:actor, user: user, preferred_username: "person", name: "Torréfaction du Kafé") group = insert(:actor, type: :Group, preferred_username: "group", name: "Kafé group") - insert(:event, title: "Tour du monde des Kafés") + event = insert(:event, title: "Tour du monde des Kafés") + Search.insert_search_event(event) # Elaborate query query = """ From 5ab8fe28886c161768e70e30f73f72e71f145cdc Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 23 Oct 2019 12:36:11 +0200 Subject: [PATCH 2/3] Make tags clickable Signed-off-by: Thomas Citharel --- js/src/router/event.ts | 7 +++++ js/src/views/Event/Event.vue | 52 +++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/js/src/router/event.ts b/js/src/router/event.ts index 8ea00b456..944580201 100644 --- a/js/src/router/event.ts +++ b/js/src/router/event.ts @@ -1,6 +1,7 @@ import EventList from '@/views/Event/EventList.vue'; import Location from '@/views/Location.vue'; import { RouteConfig } from 'vue-router'; +import { RouteName } from '@/router/index'; // tslint:disable:space-in-parens const participations = () => import(/* webpackChunkName: "participations" */ '@/views/Event/Participants.vue'); @@ -19,6 +20,7 @@ export enum EventRouteName { PARTICIPATIONS = 'Participations', EVENT = 'Event', LOCATION = 'Location', + TAG = 'Tag', } export const eventRoutes: RouteConfig[] = [ @@ -73,4 +75,9 @@ export const eventRoutes: RouteConfig[] = [ props: true, meta: { requiredAuth: false }, }, + { + path: '/tag/:tag', + name: EventRouteName.TAG, + redirect: '/search/:tag', + }, ]; diff --git a/js/src/views/Event/Event.vue b/js/src/views/Event/Event.vue index a0f4d4305..be089a1df 100644 --- a/js/src/views/Event/Event.vue +++ b/js/src/views/Event/Event.vue @@ -66,8 +66,14 @@ import {ParticipantRole} from "@/types/event.model"; {{ $t('Public event') }} {{ $t('Private event') }} - {{ tag.title }} - ⋅ + + {{ tag.title }} +

@@ -155,7 +161,7 @@ import {ParticipantRole} from "@/types/event.model"; {{ $t("The event organizer didn't add any description.") }}

-
+
@@ -242,6 +248,7 @@ import IdentityPicker from '@/views/Account/IdentityPicker.vue'; import ParticipationButton from '@/components/Event/ParticipationButton.vue'; import { GraphQLError } from 'graphql'; import { RouteName } from '@/router'; +import HTML = Mocha.reporters.HTML; @Component({ components: { @@ -329,6 +336,41 @@ export default class Event extends EventMixin { mounted() { this.identity = this.currentActor; + + this.$watch('eventDescription', function (eventDescription) { + if (!eventDescription) return; + const eventDescriptionElement = this.$refs['eventDescriptionElement'] as HTMLElement; + + eventDescriptionElement.addEventListener('click', ($event) => { + // TODO: Find the right type for target + let { target } : { target: any } = $event; + while (target && target.tagName !== 'A') target = target.parentNode; + // handle only links that occur inside the component and do not reference external resources + if (target && target.matches('.hashtag') && target.href) { + // some sanity checks taken from vue-router: + // https://github.com/vuejs/vue-router/blob/dev/src/components/link.js#L106 + const { altKey, ctrlKey, metaKey, shiftKey, button, defaultPrevented } = $event; + // don't handle with control keys + if (metaKey || altKey || ctrlKey || shiftKey) return; + // don't handle when preventDefault called + if (defaultPrevented) return; + // don't handle right clicks + if (button !== undefined && button !== 0) return; + // don't handle if `target="_blank"` + if (target && target.getAttribute) { + const linkTarget = target.getAttribute('target'); + if (/\b_blank\b/i.test(linkTarget)) return; + } + // don't handle same page links/anchors + const url = new URL(target.href); + const to = url.pathname; + if (window.location.pathname !== to && $event.preventDefault) { + $event.preventDefault(); + this.$router.push(to); + } + } + }); + }); } /** @@ -821,6 +863,10 @@ export default class Event extends EventMixin { padding: 0.3rem; background: $secondary; color: #111; + + &:empty { + display: none; + } } } } From c6ce7f6795306c1206475eb442c47c291fa400c2 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 23 Oct 2019 15:25:22 +0200 Subject: [PATCH 3/3] Fix unrelated warning Signed-off-by: Thomas Citharel --- lib/mobilizon_web/upload/filter/optimize.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mobilizon_web/upload/filter/optimize.ex b/lib/mobilizon_web/upload/filter/optimize.ex index 487c81496..de95b1250 100644 --- a/lib/mobilizon_web/upload/filter/optimize.ex +++ b/lib/mobilizon_web/upload/filter/optimize.ex @@ -20,7 +20,7 @@ defmodule MobilizonWeb.Upload.Filter.Optimize do optimizers = Config.get([__MODULE__, :optimizers], @default_optimizers) case ExOptimizer.optimize(file, deps: optimizers) do - {:ok, res} -> + {:ok, _res} -> :ok {:error, err} ->