From 10f3958468e24ba49178a19435b189a6be0dabfb Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:47:50 +0000 Subject: [PATCH 1/6] object: return the deleted object as well --- lib/pleroma/object.ex | 2 +- lib/pleroma/web/activity_pub/activity_pub.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 8f96fd8fb..fddf38450 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -57,7 +57,7 @@ def delete(%Object{data: %{"id" => id}} = object) do with Repo.delete(object), Repo.delete_all(Activity.all_non_create_by_object_ap_id_q(id)), {:ok, true} <- Cachex.del(:user_cache, "object:#{id}") do - :ok + {:ok, object} end end end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 48ae36ebd..32c14995f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -273,7 +273,7 @@ def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, local \\ tru "to" => [user.follower_address, "https://www.w3.org/ns/activitystreams#Public"] } - with Object.delete(object), + with {:ok, _} <- Object.delete(object), {:ok, activity} <- insert(data, local), :ok <- maybe_federate(activity), {:ok, _actor} <- User.decrease_note_count(user) do From 2c3bfd7f76c2154ada70f1167023752e06ee595f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:52:58 +0000 Subject: [PATCH 2/6] user: delete user_info data in User.invalidate_cache() --- lib/pleroma/user.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index bb5b91c61..f724f8a5b 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -295,6 +295,7 @@ def update_and_set_cache(changeset) do def invalidate_cache(user) do Cachex.del(:user_cache, "ap_id:#{user.ap_id}") Cachex.del(:user_cache, "nickname:#{user.nickname}") + Cachex.del(:user_cache, "user_info:#{user.id}") end def get_cached_by_ap_id(ap_id) do From f584a603f95f95c7c8d2c1897b24b5c7399f4f74 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 07:56:21 +0000 Subject: [PATCH 3/6] user: make User.delete() return data consistent with Object.delete() --- lib/mix/tasks/rm_user.ex | 2 +- lib/pleroma/user.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mix/tasks/rm_user.ex b/lib/mix/tasks/rm_user.ex index 27521b745..b7c922d6c 100644 --- a/lib/mix/tasks/rm_user.ex +++ b/lib/mix/tasks/rm_user.ex @@ -7,7 +7,7 @@ def run([nickname]) do Mix.Task.run("app.start") with %User{local: true} = user <- User.get_by_nickname(nickname) do - User.delete(user) + {:ok, _} = User.delete(user) end end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index f724f8a5b..b2f59ab6b 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -657,7 +657,7 @@ def delete(%User{} = user) do end end) - :ok + {:ok, user} end def html_filter_policy(%User{info: %{"no_rich_text" => true}}) do From 21dafa7cd029870c3dc60846ead23f1866bc4cd3 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 08:09:51 +0000 Subject: [PATCH 4/6] tests: add tests for User + cache interactions --- test/user_test.exs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/user_test.exs b/test/user_test.exs index 05da24f8d..9b3519ece 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -551,4 +551,31 @@ test "html_filter_policy returns TwitterText scrubber when rich-text is disabled assert Pleroma.HTML.Scrubber.TwitterText == User.html_filter_policy(user) end end + + describe "caching" do + test "invalidate_cache works" do + user = insert(:user) + user_info = User.get_cached_user_info(user) + + User.invalidate_cache(user) + + {:ok, nil} = Cachex.get(:user_cache, "ap_id:#{user.ap_id}") + {:ok, nil} = Cachex.get(:user_cache, "nickname:#{user.nickname}") + {:ok, nil} = Cachex.get(:user_cache, "user_info:#{user.id}") + end + + test "User.delete() plugs any possible zombie objects" do + user = insert(:user) + + {:ok, _} = User.delete(user) + + {:ok, cached_user} = Cachex.get(:user_cache, "ap_id:#{user.ap_id}") + + assert cached_user != user + + {:ok, cached_user} = Cachex.get(:user_cache, "nickname:#{user.ap_id}") + + assert cached_user != user + end + end end From 2c092ed355872cd08bf4caaf85625245764ccf77 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 08:23:49 +0000 Subject: [PATCH 5/6] test: fixup test breakage caused by User.delete() harmonization --- test/user_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/user_test.exs b/test/user_test.exs index 9b3519ece..7dec3462f 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -511,7 +511,7 @@ test ".delete deactivates a user, all follow relationships and all create activi {:ok, _, _} = CommonAPI.favorite(activity.id, follower) {:ok, _, _} = CommonAPI.repeat(activity.id, follower) - :ok = User.delete(user) + {:ok, _} = User.delete(user) followed = Repo.get(User, followed.id) follower = Repo.get(User, follower.id) From 2b3a40d0383f2ea79c1704c7700ff4d3e5f3c17a Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Thu, 1 Nov 2018 08:30:10 +0000 Subject: [PATCH 6/6] object: split object_cache from user_cache --- lib/pleroma/application.ex | 30 +++++++++++++++++++++++------- lib/pleroma/object.ex | 4 ++-- test/object_test.exs | 2 +- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index a89728471..a6b921b45 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -16,14 +16,30 @@ def start(_type, _args) do supervisor(Pleroma.Web.Endpoint, []), # Start your own worker by calling: Pleroma.Worker.start_link(arg1, arg2, arg3) # worker(Pleroma.Worker, [arg1, arg2, arg3]), - worker(Cachex, [ - :user_cache, + worker( + Cachex, [ - default_ttl: 25000, - ttl_interval: 1000, - limit: 2500 - ] - ]), + :user_cache, + [ + default_ttl: 25000, + ttl_interval: 1000, + limit: 2500 + ] + ], + id: :cachex_user + ), + worker( + Cachex, + [ + :object_cache, + [ + default_ttl: 25000, + ttl_interval: 1000, + limit: 2500 + ] + ], + id: :cachex_object + ), worker( Cachex, [ diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index fddf38450..067ecfaf4 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -37,7 +37,7 @@ def get_cached_by_ap_id(ap_id) do else key = "object:#{ap_id}" - Cachex.fetch!(:user_cache, key, fn _ -> + Cachex.fetch!(:object_cache, key, fn _ -> object = get_by_ap_id(ap_id) if object do @@ -56,7 +56,7 @@ def context_mapping(context) do def delete(%Object{data: %{"id" => id}} = object) do with Repo.delete(object), Repo.delete_all(Activity.all_non_create_by_object_ap_id_q(id)), - {:ok, true} <- Cachex.del(:user_cache, "object:#{id}") do + {:ok, true} <- Cachex.del(:object_cache, "object:#{id}") do {:ok, object} end end diff --git a/test/object_test.exs b/test/object_test.exs index 3e398776c..909605560 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -42,7 +42,7 @@ test "ensures cache is cleared for the object" do Object.delete(cached_object) - {:ok, nil} = Cachex.get(:user_cache, "object:#{object.data["id"]}") + {:ok, nil} = Cachex.get(:object_cache, "object:#{object.data["id"]}") cached_object = Object.get_cached_by_ap_id(object.data["id"])