From 77f3da035894e2add911101466bfe41b99ee481e Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 23 Feb 2021 13:52:28 +0300 Subject: [PATCH] [#3213] Misc. tweaks: proper upsert in Hashtag, better feature toggle management. --- config/config.exs | 2 ++ config/description.exs | 9 +++++---- docs/configuration/cheatsheet.md | 2 +- lib/pleroma/config.ex | 4 ++++ lib/pleroma/hashtag.ex | 20 ++++++++----------- .../migrators/hashtags_table_migrator.ex | 18 +++++++---------- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- .../web/activity_pub/activity_pub_test.exs | 4 ++-- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/config/config.exs b/config/config.exs index c371c397c..05acbf169 100644 --- a/config/config.exs +++ b/config/config.exs @@ -657,6 +657,8 @@ config :pleroma, :database, rum_enabled: false +config :pleroma, :features, improved_hashtag_timeline: :auto + config :pleroma, :populate_hashtags_table, fault_rate_allowance: 0.01 config :pleroma, :env, Mix.env() diff --git a/config/description.exs b/config/description.exs index e280ed8cf..41e5e4056 100644 --- a/config/description.exs +++ b/config/description.exs @@ -461,15 +461,16 @@ }, %{ group: :pleroma, - key: :database, + key: :features, type: :group, - description: "Database-related settings", + description: "Customizable features", children: [ %{ key: :improved_hashtag_timeline, - type: :keyword, + type: {:dropdown, :atom}, description: - "If `true`, hashtags will be fetched from `hashtags` table for hashtags timeline. When `false`, object-embedded hashtags will be used (slower). Is auto-set to `true` (unless overridden) when HashtagsTableMigrator completes." + "Setting to force toggle / force disable improved hashtags timeline. `:enabled` forces hashtags to be fetched from `hashtags` table for hashtags timeline. `:disabled` forces object-embedded hashtags to be used (slower). Keep it `:auto` for automatic behaviour (it is auto-set to `:enabled` [unless overridden] when HashtagsTableMigrator completes).", + suggestions: [:auto, :enabled, :disabled] } ] }, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 6a1031f15..db1deb665 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -66,7 +66,7 @@ To add configuration to your config file, you can copy it from the base config. * `password_reset_token_validity`: The time after which reset tokens aren't accepted anymore, in seconds (default: one day). ## :database -* `improved_hashtag_timeline`: If `true`, hashtags will be fetched from `hashtags` table for hashtags timeline. When `false`, object-embedded hashtags will be used (slower). Is auto-set to `true` (unless overridden) when `HashtagsTableMigrator` completes. +* `improved_hashtag_timeline`: Setting to force toggle / force disable improved hashtags timeline. `:enabled` forces hashtags to be fetched from `hashtags` table for hashtags timeline. `:disabled` forces object-embedded hashtags to be used (slower). Keep it `:auto` for automatic behaviour (it is auto-set to `:enabled` [unless overridden] when HashtagsTableMigrator completes). ## Background migrations * `populate_hashtags_table/sleep_interval_ms`: Sleep interval between each chunk of processed records in order to decrease the load on the system (defaults to 0 and should be keep default on most instances). diff --git a/lib/pleroma/config.ex b/lib/pleroma/config.ex index f17e14128..e057d8c02 100644 --- a/lib/pleroma/config.ex +++ b/lib/pleroma/config.ex @@ -111,4 +111,8 @@ def oauth_admin_scopes(scopes) when is_list(scopes) do end ) end + + def feature_enabled?(feature_name) do + get([:features, feature_name]) not in [nil, false, :disabled, :auto] + end end diff --git a/lib/pleroma/hashtag.ex b/lib/pleroma/hashtag.ex index e9d143fb1..53e2e9c89 100644 --- a/lib/pleroma/hashtag.ex +++ b/lib/pleroma/hashtag.ex @@ -27,19 +27,15 @@ def normalize_name(name) do |> String.trim() end - def get_by_name(name) do - Repo.get_by(Hashtag, name: normalize_name(name)) - end + def get_or_create_by_name(name) do + changeset = changeset(%Hashtag{}, %{name: name}) - def get_or_create_by_name(name) when is_bitstring(name) do - with %Hashtag{} = hashtag <- get_by_name(name) do - {:ok, hashtag} - else - _ -> - %Hashtag{} - |> changeset(%{name: name}) - |> Repo.insert() - end + Repo.insert( + changeset, + on_conflict: [set: [name: get_field(changeset, :name)]], + conflict_target: :name, + returning: true + ) end def get_or_create_by_names(names) when is_list(names) do diff --git a/lib/pleroma/migrators/hashtags_table_migrator.ex b/lib/pleroma/migrators/hashtags_table_migrator.ex index 07bb9aeb2..6123c88e0 100644 --- a/lib/pleroma/migrators/hashtags_table_migrator.ex +++ b/lib/pleroma/migrators/hashtags_table_migrator.ex @@ -24,7 +24,7 @@ defmodule Pleroma.Migrators.HashtagsTableMigrator do defdelegate put_stat(key, value), to: State, as: :put_data_key defdelegate increment_stat(key, increment), to: State, as: :increment_data_key - @feature_config_path [:database, :improved_hashtag_timeline] + @feature_config_path [:features, :improved_hashtag_timeline] @reg_name {:global, __MODULE__} def whereis, do: GenServer.whereis(@reg_name) @@ -296,16 +296,12 @@ def count(force \\ false, timeout \\ :infinity) do end defp on_complete(data_migration) do - cond do - data_migration.feature_lock -> - :noop - - not is_nil(feature_state()) -> - :noop - - true -> - Config.put(@feature_config_path, true) - :ok + if data_migration.feature_lock || feature_state() == :disabled do + Logger.warn("#{__MODULE__}: migration complete but feature is locked; consider enabling.") + :noop + else + Config.put(@feature_config_path, :enabled) + :ok end end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 8182bc205..9d557c2cd 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1273,7 +1273,7 @@ def fetch_activities_query(recipients, opts \\ %{}) do |> exclude_invisible_actors(opts) |> exclude_visibility(opts) - if Config.get([:database, :improved_hashtag_timeline]) do + if Config.feature_enabled?(:improved_hashtag_timeline) do query |> restrict_hashtag_any(opts) |> restrict_hashtag_all(opts) diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index c41c8a5dd..f92323abe 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -220,8 +220,8 @@ test "it fetches the appropriate tag-restricted posts" do {:ok, status_four} = CommonAPI.post(user, %{status: ". #Any1 #any2"}) {:ok, status_five} = CommonAPI.post(user, %{status: ". #Any2 #any1"}) - for hashtag_timeline_strategy <- [true, false] do - clear_config([:database, :improved_hashtag_timeline], hashtag_timeline_strategy) + for hashtag_timeline_strategy <- [:eanbled, :disabled] do + clear_config([:features, :improved_hashtag_timeline], hashtag_timeline_strategy) fetch_one = ActivityPub.fetch_activities([], %{type: "Create", tag: "test"})