From a115b49b4c21cf54755a2e5fed25dddfa9cee90f Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 17 Nov 2020 15:42:03 +0100 Subject: [PATCH 1/3] Only load all locales in prod mode Signed-off-by: Thomas Citharel --- config/config.exs | 20 +------------------- config/dev.exs | 7 ------- config/prod.exs | 25 +++++++++++++++++++++++++ lib/mobilizon/cldr.ex | 6 +++++- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/config/config.exs b/config/config.exs index 01cd4e484..6145a5c89 100644 --- a/config/config.exs +++ b/config/config.exs @@ -181,26 +181,8 @@ config :http_signatures, config :mobilizon, :cldr, locales: [ - "ar", - "be", - "ca", - "cs", - "de", - "en", - "es", - "fi", "fr", - "gl", - "hu", - "it", - "ja", - "nl", - "nn", - "oc", - "pl", - "pt", - "ru", - "sv" + "en" ] config :mobilizon, :activitypub, diff --git a/config/dev.exs b/config/dev.exs index 265761685..a58b0c969 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -92,13 +92,6 @@ config :mobilizon, :instance, # config :mobilizon, :activitypub, sign_object_fetches: false -# No need to compile every locale in development environment -config :mobilizon, :cldr, - locales: [ - "fr", - "en" - ] - config :mobilizon, :anonymous, reports: [ allowed: true diff --git a/config/prod.exs b/config/prod.exs index 5dd0e3b32..5107dc6bd 100644 --- a/config/prod.exs +++ b/config/prod.exs @@ -13,6 +13,31 @@ config :mobilizon, Mobilizon.Web.Endpoint, # Do not print debug messages in production config :logger, level: :info +# Load all locales in production +config :mobilizon, :cldr, + locales: [ + "ar", + "be", + "ca", + "cs", + "de", + "en", + "es", + "fi", + "fr", + "gl", + "hu", + "it", + "ja", + "nl", + "nn", + "oc", + "pl", + "pt", + "ru", + "sv" + ] + cond do System.get_env("INSTANCE_CONFIG") && File.exists?("./config/#{System.get_env("INSTANCE_CONFIG")}") -> diff --git a/lib/mobilizon/cldr.ex b/lib/mobilizon/cldr.ex index 0188b8708..3759211f7 100644 --- a/lib/mobilizon/cldr.ex +++ b/lib/mobilizon/cldr.ex @@ -5,6 +5,10 @@ defmodule Mobilizon.Cldr do use Cldr, locales: Application.get_env(:mobilizon, :cldr)[:locales], - gettext: Mobilizon.Web.Gettext, + gettext: + if(Application.fetch_env!(:mobilizon, :env) == :prod, + do: Mobilizon.Web.Gettext, + else: nil + ), providers: [Cldr.Number, Cldr.Calendar, Cldr.DateTime, Cldr.Language] end From 15a82c7bce196bed1ddc89794bbe890420678ec1 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 17 Nov 2020 15:45:08 +0100 Subject: [PATCH 2/3] [Metadata] Fix actors not sanitizing their description and refactor Signed-off-by: Thomas Citharel --- lib/service/formatter/html.ex | 2 +- lib/service/metadata/actor.ex | 30 +++++++++++------ lib/service/metadata/instance.ex | 15 ++++----- lib/service/metadata/metadata.ex | 10 ++++-- lib/service/metadata/utils.ex | 55 ++++++++++++++++++++++++++------ 5 files changed, 80 insertions(+), 32 deletions(-) diff --git a/lib/service/formatter/html.ex b/lib/service/formatter/html.ex index 926d3fee2..ef4cdd6a4 100644 --- a/lib/service/formatter/html.ex +++ b/lib/service/formatter/html.ex @@ -32,7 +32,7 @@ defmodule Mobilizon.Service.Formatter.HTML do @spec strip_tags_and_insert_spaces(String.t()) :: String.t() def strip_tags_and_insert_spaces(html) when is_binary(html) do html - |> String.replace(" String.replace("><", "> <") |> strip_tags() end diff --git a/lib/service/metadata/actor.ex b/lib/service/metadata/actor.ex index 5677edf8e..fd3988ddb 100644 --- a/lib/service/metadata/actor.ex +++ b/lib/service/metadata/actor.ex @@ -4,20 +4,32 @@ defimpl Mobilizon.Service.Metadata, for: Mobilizon.Actors.Actor do alias Mobilizon.Actors.Actor alias Mobilizon.Web.JsonLD.ObjectView alias Mobilizon.Web.MediaProxy + import Mobilizon.Service.Metadata.Utils, only: [process_description: 2, default_description: 1] + + def build_tags(_actor, _locale \\ "en") + + def build_tags(%Actor{type: :Group} = group, locale) do + default_desc = default_description(locale) + + group = + Map.update(group, :summary, default_desc, fn summary -> + process_description(summary, locale) + end) - def build_tags(%Actor{} = actor, _locale \\ "en") do [ - Tag.tag(:meta, property: "og:title", content: Actor.display_name_and_username(actor)), - Tag.tag(:meta, property: "og:url", content: actor.url), - Tag.tag(:meta, property: "og:description", content: actor.summary), + Tag.tag(:meta, property: "og:title", content: Actor.display_name_and_username(group)), + Tag.tag(:meta, property: "og:url", content: group.url), + Tag.tag(:meta, property: "og:description", content: group.summary), Tag.tag(:meta, property: "og:type", content: "profile"), - Tag.tag(:meta, property: "profile:username", content: actor.preferred_username), + Tag.tag(:meta, property: "profile:username", content: group.preferred_username), Tag.tag(:meta, property: "twitter:card", content: "summary") ] - |> maybe_add_avatar(actor) - |> maybe_add_group_schema(actor) + |> maybe_add_avatar(group) + |> add_group_schema(group) end + def build_tags(%Actor{} = _actor, _locale), do: [] + @spec maybe_add_avatar(list(Tag.t()), Actor.t()) :: list(Tag.t()) defp maybe_add_avatar(tags, actor) do if is_nil(actor.avatar) do @@ -28,12 +40,10 @@ defimpl Mobilizon.Service.Metadata, for: Mobilizon.Actors.Actor do end end - defp maybe_add_group_schema(tags, %Actor{type: :Group} = group) do + defp add_group_schema(tags, %Actor{} = group) do tags ++ [~s{} |> HTML.raw()] end - defp maybe_add_group_schema(tags, _), do: tags - # Insert JSON-LD schema by hand because Tag.content_tag wants to escape it defp json(%Actor{} = group) do "group.json" diff --git a/lib/service/metadata/instance.ex b/lib/service/metadata/instance.ex index aef15f862..5c88e4478 100644 --- a/lib/service/metadata/instance.ex +++ b/lib/service/metadata/instance.ex @@ -7,11 +7,15 @@ defmodule Mobilizon.Service.Metadata.Instance do alias Phoenix.HTML.Tag alias Mobilizon.Config - alias Mobilizon.Service.Formatter.HTML, as: HTMLFormatter + alias Mobilizon.Service.Metadata.Utils alias Mobilizon.Web.Endpoint + @doc """ + Build the list of tags for the instance + """ + @spec build_tags() :: list(Phoenix.HTML.safe()) def build_tags do - description = process_description(Config.instance_description()) + description = Utils.process_description(Config.instance_description()) title = "#{Config.instance_name()} - Mobilizon" instance_json_ld = """ @@ -38,11 +42,4 @@ defmodule Mobilizon.Service.Metadata.Instance do HTML.raw(instance_json_ld) ] end - - defp process_description(description) do - description - |> HTMLFormatter.strip_tags() - |> String.slice(0..200) - |> (&"#{&1}…").() - end end diff --git a/lib/service/metadata/metadata.ex b/lib/service/metadata/metadata.ex index eb58edb70..01def810f 100644 --- a/lib/service/metadata/metadata.ex +++ b/lib/service/metadata/metadata.ex @@ -1,7 +1,13 @@ defprotocol Mobilizon.Service.Metadata do - @doc """ - Build tags + @moduledoc """ + Service that allows producing metadata HTML tags about content """ + @doc """ + Build tags for an entity. Returns a list of `t:Phoenix.HTML.safe/0` tags. + + Locale can be provided to generate fallback descriptions. + """ + @spec build_tags(any(), String.t()) :: list(Phoenix.HTML.safe()) def build_tags(entity, locale \\ "en") end diff --git a/lib/service/metadata/utils.ex b/lib/service/metadata/utils.ex index f2c25bdf4..1fddcba1f 100644 --- a/lib/service/metadata/utils.ex +++ b/lib/service/metadata/utils.ex @@ -9,28 +9,63 @@ defmodule Mobilizon.Service.Metadata.Utils do @slice_limit 200 - @spec stringify_tags(Enum.t()) :: String.t() + @doc """ + Converts list of tags, containing either `t:Phoenix.HTML.safe/0` or strings, to a concatenated string listing the tags + """ + @spec stringify_tags(list(Phoenix.HTML.safe() | String.t())) :: String.t() def stringify_tags(tags), do: Enum.reduce(tags, "", &stringify_tag/2) - defp stringify_tag(tag, acc) when is_tuple(tag), do: acc <> HTML.safe_to_string(tag) - defp stringify_tag(tag, acc) when is_binary(tag), do: acc <> tag - + @doc """ + Removes the HTML tags from a text + """ @spec strip_tags(String.t()) :: String.t() - def strip_tags(text), do: HTMLFormatter.strip_tags(text) + def strip_tags(text), do: HTMLFormatter.strip_tags_and_insert_spaces(text) + @doc """ + Processes a text and limits it. + + * Removes the HTML tags from a text + * Slices it to a limit and add an ellipsis character + * Returns a default description if text is empty + """ @spec process_description(String.t(), String.t(), integer()) :: String.t() def process_description(description, locale \\ "en", limit \\ @slice_limit) def process_description(nil, locale, limit), do: process_description("", locale, limit) def process_description("", locale, _limit) do - Gettext.put_locale(locale) - gettext("The event organizer didn't add any description.") + default_description(locale) end def process_description(description, _locale, limit) do description - |> HTMLFormatter.strip_tags() - |> String.slice(0..limit) - |> (&"#{&1}…").() + |> HTMLFormatter.strip_tags_and_insert_spaces() + |> String.trim() + |> maybe_slice(limit) end + + @doc """ + Returns the default description for a text + """ + @spec default_description(String.t()) :: String.t() + def default_description(locale \\ "en") do + Gettext.put_locale(locale) + gettext("The event organizer didn't add any description.") + end + + defp maybe_slice(description, limit) do + if String.length(description) > limit do + description + |> String.slice(0..limit) + |> String.trim() + |> (&"#{&1}…").() + else + description + end + end + + @spec stringify_tag(Phoenix.HTML.safe(), String.t()) :: String.t() + defp stringify_tag(tag, acc) when is_tuple(tag), do: acc <> HTML.safe_to_string(tag) + + @spec stringify_tag(String.t(), String.t()) :: String.t() + defp stringify_tag(tag, acc) when is_binary(tag), do: acc <> tag end From 72cd3e688d705e17c5dfed9a10c45156c7d502a0 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 17 Nov 2020 15:45:42 +0100 Subject: [PATCH 3/3] Add tests for metadata Signed-off-by: Thomas Citharel --- test/service/metadata/instance_test.exs | 24 ++++ test/service/metadata/metadata_test.exs | 143 ++++++++++++++++++++++++ test/service/metadata/utils_test.exs | 60 ++++++++++ 3 files changed, 227 insertions(+) create mode 100644 test/service/metadata/instance_test.exs create mode 100644 test/service/metadata/metadata_test.exs create mode 100644 test/service/metadata/utils_test.exs diff --git a/test/service/metadata/instance_test.exs b/test/service/metadata/instance_test.exs new file mode 100644 index 000000000..8e7033e80 --- /dev/null +++ b/test/service/metadata/instance_test.exs @@ -0,0 +1,24 @@ +defmodule Mobilizon.Service.Metadata.InstanceTest do + alias Mobilizon.Config + alias Mobilizon.Service.Metadata.{Instance, Utils} + alias Mobilizon.Web.Endpoint + use Mobilizon.DataCase + + describe "build_tags/0 for the instance" do + test "gives tags" do + title = "#{Config.instance_name()} - Mobilizon" + description = Utils.process_description(Config.instance_description()) + + assert Instance.build_tags() |> Utils.stringify_tags() == + "#{title}\n" + end + end +end diff --git a/test/service/metadata/metadata_test.exs b/test/service/metadata/metadata_test.exs new file mode 100644 index 000000000..478d2f0a5 --- /dev/null +++ b/test/service/metadata/metadata_test.exs @@ -0,0 +1,143 @@ +defmodule Mobilizon.Service.MetadataTest do + alias Mobilizon.Actors.Actor + alias Mobilizon.Discussions.Comment + alias Mobilizon.Events.Event + alias Mobilizon.Posts.Post + alias Mobilizon.Service.Metadata + alias Mobilizon.Tombstone + use Mobilizon.DataCase + import Mobilizon.Factory + + describe "build_tags/2 for an actor" do + test "that is a group gives tags" do + %Actor{} = group = insert(:group, name: "My group") + + assert group |> Metadata.build_tags() |> Metadata.Utils.stringify_tags() == + "" + + assert group + |> Map.put(:avatar, nil) + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == + "" + end + + test "that is not a group doesn't give anything" do + %Actor{} = person = insert(:actor) + + assert person |> Metadata.build_tags() |> Metadata.Utils.stringify_tags() == "" + assert person |> Metadata.build_tags("fr") |> Metadata.Utils.stringify_tags() == "" + end + end + + describe "build_tags/2 for an event" do + test "gives tags" do + alias Mobilizon.Web.Endpoint + + %Event{} = event = insert(:event) + + # Because the description in Schema.org data is double-escaped + a = "\n" + b = "\\n" + + assert event + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == + "#{event.title} - Mobilizon" + + assert event + |> Map.put(:picture, nil) + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == + "#{event.title} - Mobilizon" + end + end + + describe "build_tags/2 for a post" do + test "gives tags" do + %Post{} = post = insert(:post) + + assert post + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == + "" + end + end + + describe "build_tags/2 for a comment" do + test "gives tags" do + %Comment{} = comment = insert(:comment) + + assert comment + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == + "" + end + end + + describe "build_tags/2 for a tombstone" do + test "gives nothing" do + %Tombstone{} = tombstone = insert(:tombstone) + + assert tombstone + |> Metadata.build_tags() + |> Metadata.Utils.stringify_tags() == "" + end + end +end diff --git a/test/service/metadata/utils_test.exs b/test/service/metadata/utils_test.exs new file mode 100644 index 000000000..9ba6742ae --- /dev/null +++ b/test/service/metadata/utils_test.exs @@ -0,0 +1,60 @@ +defmodule Mobilizon.Service.Metadata.UtilsTest do + alias Mobilizon.Service.Metadata.Utils + use Mobilizon.DataCase + + describe "process_description/3" do + test "process_description/3 strip tags" do + assert Utils.process_description("

This is my biography

") == "This is my biography" + end + + test "process_description/3 cuts after a limit" do + assert Utils.process_description("

This is my biography

", "fr", 10) == + "This is my…" + end + + test "process_description/3 cuts after the default limit" do + assert Utils.process_description( + "

Biography

It all started when someone wanted a very long string to be cut. However it's difficult to invent things to write when you've got nothing to say. Anyway, what's the deal here. We just need to reach 200 characters.", + "fr" + ) == + "Biography It all started when someone wanted a very long string to be cut. However it's difficult to invent things to write when you've got nothing to say. Anyway, what's the deal here. We…" + end + + test "process_description/3 returns default if no description is provided" do + assert Utils.process_description(nil) == + "The event organizer didn't add any description." + + assert Utils.process_description("", "en") == + "The event organizer didn't add any description." + end + end + + describe "default_description/1" do + test "returns default description with a correct locale" do + assert Utils.default_description("en") == "The event organizer didn't add any description." + end + + test "returns default description with no locale provided" do + assert Utils.default_description() == "The event organizer didn't add any description." + end + end + + describe "stringify_tags/1" do + test "converts tags to string" do + alias Phoenix.HTML.Tag + + tag_1 = Tag.tag(:meta, property: "og:url", content: "one") + tag_2 = "" + tag_3 = Tag.tag(:meta, property: "og:url", content: "three") + + assert Utils.stringify_tags([tag_1, tag_2, tag_3]) == + "" + end + end + + describe "strip_tags/1" do + test "removes tags from string" do + assert Utils.strip_tags("

Hello

How are you

") == "Hello How are you" + end + end +end