From 73d01857e3ff1737a4ea733a3f6c6419379ce8e8 Mon Sep 17 00:00:00 2001 From: Alex S Date: Sun, 14 Apr 2019 19:45:56 +0700 Subject: [PATCH 01/10] bookmarks in separate table --- lib/pleroma/bookmark.ex | 53 +++++++++++++++++++ lib/pleroma/user.ex | 19 +------ .../mastodon_api/mastodon_api_controller.ex | 33 +++++++++--- .../web/mastodon_api/views/status_view.ex | 4 +- .../20190413082658_create_bookmarks.exs | 14 +++++ test/bookmark_test.exs | 37 +++++++++++++ test/user_test.exs | 27 ---------- 7 files changed, 135 insertions(+), 52 deletions(-) create mode 100644 lib/pleroma/bookmark.ex create mode 100644 priv/repo/migrations/20190413082658_create_bookmarks.exs create mode 100644 test/bookmark_test.exs diff --git a/lib/pleroma/bookmark.ex b/lib/pleroma/bookmark.ex new file mode 100644 index 000000000..c5c3e078b --- /dev/null +++ b/lib/pleroma/bookmark.ex @@ -0,0 +1,53 @@ +defmodule Pleroma.Bookmark do + use Ecto.Schema + + import Ecto.Changeset + import Ecto.Query + + alias Pleroma.Activity + alias Pleroma.Bookmark + alias Pleroma.FlakeId + alias Pleroma.Repo + alias Pleroma.User + + @type t :: %__MODULE__{} + + schema "bookmarks" do + belongs_to(:user, User, type: FlakeId) + belongs_to(:activity, Activity, type: FlakeId) + + timestamps() + end + + @spec create(FlakeId.t(), FlakeId.t()) :: {:ok, Bookmark.t()} | {:error, Changeset.t()} + def create(user_id, activity_id) do + attrs = %{ + user_id: user_id, + activity_id: activity_id + } + + %Bookmark{} + |> cast(attrs, [:user_id, :activity_id]) + |> validate_required([:user_id, :activity_id]) + |> unique_constraint(:activity_id, name: :bookmarks_user_id_activity_id_index) + |> Repo.insert() + end + + @spec for_user_query(FlakeId.t()) :: Ecto.Query.t() + def for_user_query(user_id) do + Bookmark + |> where(user_id: ^user_id) + |> join(:inner, [b], activity in assoc(b, :activity)) + |> preload([b, a], activity: a) + end + + @spec destroy(FlakeId.t(), FlakeId.t()) :: {:ok, Bookmark.t()} | {:error, Changeset.t()} + def destroy(user_id, activity_id) do + from(b in Bookmark, + where: b.user_id == ^user_id, + where: b.activity_id == ^activity_id + ) + |> Repo.one() + |> Repo.delete() + end +end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index f1feab279..c5b1ddc5d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -10,6 +10,7 @@ defmodule Pleroma.User do alias Comeonin.Pbkdf2 alias Pleroma.Activity + alias Pleroma.Bookmark alias Pleroma.Formatter alias Pleroma.Notification alias Pleroma.Object @@ -53,8 +54,8 @@ defmodule Pleroma.User do field(:search_rank, :float, virtual: true) field(:search_type, :integer, virtual: true) field(:tags, {:array, :string}, default: []) - field(:bookmarks, {:array, :string}, default: []) field(:last_refreshed_at, :naive_datetime_usec) + has_many(:bookmarks, Bookmark) has_many(:notifications, Notification) has_many(:registrations, Registration) embeds_one(:info, Pleroma.User.Info) @@ -1379,22 +1380,6 @@ defp update_tags(%User{} = user, new_tags) do updated_user end - def bookmark(%User{} = user, status_id) do - bookmarks = Enum.uniq(user.bookmarks ++ [status_id]) - update_bookmarks(user, bookmarks) - end - - def unbookmark(%User{} = user, status_id) do - bookmarks = Enum.uniq(user.bookmarks -- [status_id]) - update_bookmarks(user, bookmarks) - end - - def update_bookmarks(%User{} = user, bookmarks) do - user - |> change(%{bookmarks: bookmarks}) - |> update_and_set_cache - end - defp normalize_tags(tags) do [tags] |> List.flatten() diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 0ba8d9eea..a93aa6ad5 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -6,6 +6,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do use Pleroma.Web, :controller alias Ecto.Changeset alias Pleroma.Activity + alias Pleroma.Bookmark alias Pleroma.Config alias Pleroma.Filter alias Pleroma.Notification @@ -279,6 +280,8 @@ def home_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.contain_timeline(user) |> Enum.reverse() + user = Repo.preload(user, :bookmarks) + conn |> add_link_headers(:home_timeline, activities) |> put_view(StatusView) @@ -297,6 +300,8 @@ def public_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_public_activities() |> Enum.reverse() + user = Repo.preload(user, :bookmarks) + conn |> add_link_headers(:public_timeline, activities, false, %{"local" => local_only}) |> put_view(StatusView) @@ -304,7 +309,8 @@ def public_timeline(%{assigns: %{user: user}} = conn, params) do end def user_statuses(%{assigns: %{user: reading_user}} = conn, params) do - with %User{} = user <- User.get_cached_by_id(params["id"]) do + with %User{} = user <- User.get_cached_by_id(params["id"]), + reading_user <- Repo.preload(reading_user, :bookmarks) do activities = ActivityPub.fetch_user_activities(user, reading_user, params) conn @@ -331,6 +337,8 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) + user = Repo.preload(user, :bookmarks) + conn |> add_link_headers(:dm_timeline, activities) |> put_view(StatusView) @@ -548,7 +556,9 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %Object{} = object <- Object.normalize(activity), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), - {:ok, user} <- User.bookmark(user, object.data["id"]) do + {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do + user = Repo.preload(user, :bookmarks) + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -560,7 +570,9 @@ def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %Object{} = object <- Object.normalize(activity), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), - {:ok, user} <- User.unbookmark(user, object.data["id"]) do + {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do + user = Repo.preload(user, :bookmarks) + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -1124,15 +1136,20 @@ def user_favourites(%{assigns: %{user: for_user}} = conn, %{"id" => id} = params end end - def bookmarks(%{assigns: %{user: user}} = conn, _) do + def bookmarks(%{assigns: %{user: user}} = conn, params) do user = User.get_cached_by_id(user.id) + user = Repo.preload(user, :bookmarks) + + bookmarks = + Bookmark.for_user_query(user.id) + |> Pagination.fetch_paginated(params) activities = - user.bookmarks - |> Enum.map(fn id -> Activity.get_create_by_object_ap_id(id) end) - |> Enum.reverse() + bookmarks + |> Enum.map(fn b -> b.activity end) conn + |> add_link_headers(:bookmarks, bookmarks) |> put_view(StatusView) |> render("index.json", %{activities: activities, for: user, as: :activity}) end @@ -1238,6 +1255,8 @@ def list_timeline(%{assigns: %{user: user}} = conn, %{"list_id" => id} = params) |> ActivityPub.fetch_activities_bounded(following, params) |> Enum.reverse() + user = Repo.preload(user, :bookmarks) + conn |> put_view(StatusView) |> render("index.json", %{activities: activities, for: user, as: :activity}) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index 7dd80d708..b2ed023dc 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -148,7 +148,9 @@ def render("status.json", %{activity: %{data: %{"object" => _object}} = activity favorited = opts[:for] && opts[:for].ap_id in (object.data["likes"] || []) - bookmarked = opts[:for] && object.data["id"] in opts[:for].bookmarks + bookmarked = + opts[:for] && Ecto.assoc_loaded?(opts[:for].bookmarks) && + Enum.any?(opts[:for].bookmarks, fn b -> b.activity_id == activity.id end) attachment_data = object.data["attachment"] || [] attachments = render_many(attachment_data, StatusView, "attachment.json", as: :attachment) diff --git a/priv/repo/migrations/20190413082658_create_bookmarks.exs b/priv/repo/migrations/20190413082658_create_bookmarks.exs new file mode 100644 index 000000000..38b108158 --- /dev/null +++ b/priv/repo/migrations/20190413082658_create_bookmarks.exs @@ -0,0 +1,14 @@ +defmodule Pleroma.Repo.Migrations.CreateBookmarks do + use Ecto.Migration + + def change do + create table(:bookmarks) do + add(:user_id, references(:users, type: :uuid, on_delete: :delete_all)) + add(:activity_id, references(:activities, type: :uuid, on_delete: :delete_all)) + + timestamps() + end + + create(unique_index(:bookmarks, [:user_id, :activity_id])) + end +end diff --git a/test/bookmark_test.exs b/test/bookmark_test.exs new file mode 100644 index 000000000..3be148023 --- /dev/null +++ b/test/bookmark_test.exs @@ -0,0 +1,37 @@ +defmodule Pleroma.BookmarkTest do + use Pleroma.DataCase + import Pleroma.Factory + alias Pleroma.Bookmark + alias Pleroma.Web.CommonAPI + + describe "create/2" do + test "with valid params" do + user = insert(:user) + {:ok, activity} = CommonAPI.post(user, %{"status" => "Some cool information"}) + {:ok, bookmark} = Bookmark.create(user.id, activity.id) + assert bookmark.user_id == user.id + assert bookmark.activity_id == activity.id + end + + test "with invalid params" do + {:error, changeset} = Bookmark.create(nil, "") + refute changeset.valid? + + assert changeset.errors == [ + user_id: {"can't be blank", [validation: :required]}, + activity_id: {"can't be blank", [validation: :required]} + ] + end + end + + describe "destroy/2" do + test "with valid params" do + user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "Some cool information"}) + {:ok, _bookmark} = Bookmark.create(user.id, activity.id) + + {:ok, _deleted_bookmark} = Bookmark.destroy(user.id, activity.id) + end + end +end diff --git a/test/user_test.exs b/test/user_test.exs index 42d570c50..7be47e5fb 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -1125,33 +1125,6 @@ test "Adds rel=me on linkbacked urls" do end end - test "bookmarks" do - user = insert(:user) - - {:ok, activity1} = - CommonAPI.post(user, %{ - "status" => "heweoo!" - }) - - id1 = Object.normalize(activity1).data["id"] - - {:ok, activity2} = - CommonAPI.post(user, %{ - "status" => "heweoo!" - }) - - id2 = Object.normalize(activity2).data["id"] - - assert {:ok, user_state1} = User.bookmark(user, id1) - assert user_state1.bookmarks == [id1] - - assert {:ok, user_state2} = User.unbookmark(user, id1) - assert user_state2.bookmarks == [] - - assert {:ok, user_state3} = User.bookmark(user, id2) - assert user_state3.bookmarks == [id2] - end - test "follower count is updated when a follower is blocked" do user = insert(:user) follower = insert(:user) From 324c9c8ab54b7df59534f6a8160d4a20d5b79722 Mon Sep 17 00:00:00 2001 From: Alex S Date: Sun, 14 Apr 2019 21:37:05 +0700 Subject: [PATCH 02/10] migrate user.bookmarks to separate table --- lib/pleroma/user.ex | 2 ++ .../20190414125034_migrate_old_bookmarks.exs | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index c5b1ddc5d..3831588f9 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -55,6 +55,8 @@ defmodule Pleroma.User do field(:search_type, :integer, virtual: true) field(:tags, {:array, :string}, default: []) field(:last_refreshed_at, :naive_datetime_usec) + # TODO: add migration to delete `bookmarks` field from DB + field(:old_bookmarks, {:array, :string}, default: [], source: :bookmarks) has_many(:bookmarks, Bookmark) has_many(:notifications, Notification) has_many(:registrations, Registration) diff --git a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs new file mode 100644 index 000000000..1930fc3cf --- /dev/null +++ b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs @@ -0,0 +1,21 @@ +defmodule Pleroma.Repo.Migrations.MigrateOldBookmarks do + use Ecto.Migration + alias Pleroma.Activity + alias Pleroma.Bookmark + alias Pleroma.User + alias Pleroma.Repo + + def up do + Repo.all(User) + |> Enum.each(fn user -> + Enum.each(user.old_bookmarks, fn id -> + activity = Activity.get_create_by_object_ap_id(id) + {:ok, _} = Bookmark.create(user.id, activity.id) + end) + end) + end + + def down do + execute("TRUNCATE TABLE bookmarks") + end +end From 1258128f4fede8122a3950051e52595e36ab0511 Mon Sep 17 00:00:00 2001 From: Alex S Date: Sun, 14 Apr 2019 21:42:18 +0700 Subject: [PATCH 03/10] favourites add bookmark display --- lib/pleroma/web/mastodon_api/mastodon_api_controller.ex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index a93aa6ad5..4f8af32c3 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -1093,6 +1093,8 @@ def favourites(%{assigns: %{user: user}} = conn, params) do ActivityPub.fetch_activities([], params) |> Enum.reverse() + user = Repo.preload(user, :bookmarks) + conn |> add_link_headers(:favourites, activities) |> put_view(StatusView) From 17b5b78737c4412676a7199248943c23c0be23df Mon Sep 17 00:00:00 2001 From: Alex S Date: Sun, 14 Apr 2019 21:54:49 +0700 Subject: [PATCH 04/10] changelog file update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70381f382..c538dd9f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Deps: Updated Cowboy to 2.6 - Deps: Updated Ecto to 3.0.7 - Don't ship finmoji by default, they can be installed as an emoji pack +- `User.bookmarks` in separate table, added support max_id & since_id for bookmark timeline endpoints. ### Fixed - Followers counter not being updated when a follower is blocked From 6322c1e123c102d603bd9f6e8bd443568c2fd1cb Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 15 Apr 2019 11:43:02 +0700 Subject: [PATCH 05/10] migration optimization changelog wording --- CHANGELOG.md | 2 +- .../20190414125034_migrate_old_bookmarks.exs | 20 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c538dd9f0..24d6456b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Deps: Updated Cowboy to 2.6 - Deps: Updated Ecto to 3.0.7 - Don't ship finmoji by default, they can be installed as an emoji pack -- `User.bookmarks` in separate table, added support max_id & since_id for bookmark timeline endpoints. +- Mastodon API: Added support max_id & since_id for bookmark timeline endpoints. ### Fixed - Followers counter not being updated when a follower is blocked diff --git a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs index 1930fc3cf..c30c302f7 100644 --- a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs +++ b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs @@ -1,16 +1,26 @@ defmodule Pleroma.Repo.Migrations.MigrateOldBookmarks do use Ecto.Migration + import Ecto.Query alias Pleroma.Activity alias Pleroma.Bookmark alias Pleroma.User alias Pleroma.Repo def up do - Repo.all(User) - |> Enum.each(fn user -> - Enum.each(user.old_bookmarks, fn id -> - activity = Activity.get_create_by_object_ap_id(id) - {:ok, _} = Bookmark.create(user.id, activity.id) + query = + from(u in User, + where: u.local == true, + where: fragment("array_length(?, 1)", u.old_bookmarks) > 0, + select: %{id: u.id, old_bookmarks: u.old_bookmarks} + ) + + Repo.transaction(fn -> + Repo.stream(query) + |> Enum.each(fn user -> + Enum.each(user.old_bookmarks, fn id -> + activity = Activity.get_create_by_object_ap_id(id) + {:ok, _} = Bookmark.create(user.id, activity.id) + end) end) end) end From 58711a79d0cc58b9a19c15b7abd2e6ec1b8b5c41 Mon Sep 17 00:00:00 2001 From: Alex S Date: Wed, 17 Apr 2019 14:24:11 +0700 Subject: [PATCH 06/10] removing useless transaction --- .../20190414125034_migrate_old_bookmarks.exs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs index c30c302f7..09f02c223 100644 --- a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs +++ b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs @@ -14,13 +14,11 @@ def up do select: %{id: u.id, old_bookmarks: u.old_bookmarks} ) - Repo.transaction(fn -> - Repo.stream(query) - |> Enum.each(fn user -> - Enum.each(user.old_bookmarks, fn id -> - activity = Activity.get_create_by_object_ap_id(id) - {:ok, _} = Bookmark.create(user.id, activity.id) - end) + Repo.stream(query) + |> Enum.each(fn user -> + Enum.each(user.old_bookmarks, fn id -> + activity = Activity.get_create_by_object_ap_id(id) + {:ok, _} = Bookmark.create(user.id, activity.id) end) end) end From 3c2ae800082f34206f95cee5fe23b5bf79ed7361 Mon Sep 17 00:00:00 2001 From: Alex S Date: Sat, 20 Apr 2019 14:41:42 +0700 Subject: [PATCH 07/10] unused --- lib/pleroma/web/mastodon_api/mastodon_api_controller.ex | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 4f8af32c3..859cf9524 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -553,7 +553,6 @@ def unpin_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), - %Object{} = object <- Object.normalize(activity), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do @@ -567,7 +566,6 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), - %Object{} = object <- Object.normalize(activity), %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do From 229ce6abbc1873c35a56450942c8aee0a027b6a8 Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 22 Apr 2019 12:45:45 +0700 Subject: [PATCH 08/10] migration without using old field name removing old field from db, after bookmarks migration --- lib/pleroma/user.ex | 2 -- .../20190414125034_migrate_old_bookmarks.exs | 20 +++++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 3831588f9..c5b1ddc5d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -55,8 +55,6 @@ defmodule Pleroma.User do field(:search_type, :integer, virtual: true) field(:tags, {:array, :string}, default: []) field(:last_refreshed_at, :naive_datetime_usec) - # TODO: add migration to delete `bookmarks` field from DB - field(:old_bookmarks, {:array, :string}, default: [], source: :bookmarks) has_many(:bookmarks, Bookmark) has_many(:notifications, Notification) has_many(:registrations, Registration) diff --git a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs index 09f02c223..ebe69696e 100644 --- a/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs +++ b/priv/repo/migrations/20190414125034_migrate_old_bookmarks.exs @@ -6,24 +6,24 @@ defmodule Pleroma.Repo.Migrations.MigrateOldBookmarks do alias Pleroma.User alias Pleroma.Repo - def up do + def change do query = from(u in User, where: u.local == true, - where: fragment("array_length(?, 1)", u.old_bookmarks) > 0, - select: %{id: u.id, old_bookmarks: u.old_bookmarks} + where: fragment("array_length(bookmarks, 1)") > 0, + select: %{id: u.id, bookmarks: fragment("bookmarks")} ) Repo.stream(query) - |> Enum.each(fn user -> - Enum.each(user.old_bookmarks, fn id -> - activity = Activity.get_create_by_object_ap_id(id) - {:ok, _} = Bookmark.create(user.id, activity.id) + |> Enum.each(fn %{id: user_id, bookmarks: bookmarks} -> + Enum.each(bookmarks, fn ap_id -> + activity = Activity.get_create_by_object_ap_id(ap_id) + {:ok, _} = Bookmark.create(user_id, activity.id) end) end) - end - def down do - execute("TRUNCATE TABLE bookmarks") + alter table(:users) do + remove(:bookmarks) + end end end From 85953c0836c84550c8f61218d0a5b284c0716d34 Mon Sep 17 00:00:00 2001 From: Alex S Date: Mon, 22 Apr 2019 16:16:19 +0700 Subject: [PATCH 09/10] fixes for tests --- .../web/mastodon_api/mastodon_api_controller.ex | 2 +- lib/pleroma/web/mastodon_api/views/status_view.ex | 11 +++++++++-- .../web/mastodon_api/mastodon_api_controller_test.exs | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 859cf9524..f5067e17e 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -280,7 +280,7 @@ def home_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.contain_timeline(user) |> Enum.reverse() - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> add_link_headers(:home_timeline, activities) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index b2ed023dc..57cb9fdcc 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -85,7 +85,12 @@ def render( activity_object = Object.normalize(activity) favorited = opts[:for] && opts[:for].ap_id in (activity_object.data["likes"] || []) - bookmarked = opts[:for] && activity_object.data["id"] in opts[:for].bookmarks + + bookmarked = + opts[:for] && Ecto.assoc_loaded?(opts[:for].bookmarks) && + Enum.any?(opts[:for].bookmarks, fn b -> + b.activity_id == activity.id or b.activity.data["object"]["id"] == object + end) mentions = activity.recipients @@ -150,7 +155,9 @@ def render("status.json", %{activity: %{data: %{"object" => _object}} = activity bookmarked = opts[:for] && Ecto.assoc_loaded?(opts[:for].bookmarks) && - Enum.any?(opts[:for].bookmarks, fn b -> b.activity_id == activity.id end) + Enum.any?(opts[:for].bookmarks, fn b -> + b.activity_id == activity.id + end) attachment_data = object.data["attachment"] || [] attachments = render_many(attachment_data, StatusView, "attachment.json", as: :attachment) diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index a22944088..15bb02297 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -1022,7 +1022,7 @@ test "reblogged status for another user", %{conn: conn} do user2 = insert(:user) user3 = insert(:user) CommonAPI.favorite(activity.id, user2) - {:ok, user2} = User.bookmark(user2, activity.data["object"]["id"]) + {:ok, _bookmark} = Pleroma.Bookmark.create(user2.id, activity.id) {:ok, reblog_activity1, _object} = CommonAPI.repeat(activity.id, user1) {:ok, _, _object} = CommonAPI.repeat(activity.id, user2) From a825056d4d9ef823fe45d49df01062d199518829 Mon Sep 17 00:00:00 2001 From: Alex S Date: Thu, 25 Apr 2019 14:09:57 +0700 Subject: [PATCH 10/10] test fixes --- .../mastodon_api/mastodon_api_controller.ex | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index f5067e17e..fb11abf2d 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -300,7 +300,7 @@ def public_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_public_activities() |> Enum.reverse() - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> add_link_headers(:public_timeline, activities, false, %{"local" => local_only}) @@ -337,7 +337,7 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> add_link_headers(:dm_timeline, activities) @@ -348,6 +348,8 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do def get_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do with %Activity{} = activity <- Activity.get_by_id_with_object(id), true <- Visibility.visible_for_user?(activity, user) do + user = Repo.preload(user, bookmarks: :activity) + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user}) @@ -497,6 +499,8 @@ def delete_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do def reblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do with {:ok, announce, _activity} <- CommonAPI.repeat(ap_id_or_id, user), %Activity{} = announce <- Activity.normalize(announce.data) do + user = Repo.preload(user, bookmarks: :activity) + conn |> put_view(StatusView) |> try_render("status.json", %{activity: announce, for: user, as: :activity}) @@ -506,6 +510,8 @@ def reblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do def unreblog_status(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do with {:ok, _unannounce, %{data: %{"id" => id}}} <- CommonAPI.unrepeat(ap_id_or_id, user), %Activity{} = activity <- Activity.get_create_by_object_ap_id_with_object(id) do + user = Repo.preload(user, bookmarks: :activity) + conn |> put_view(StatusView) |> try_render("status.json", %{activity: activity, for: user, as: :activity}) @@ -556,7 +562,7 @@ def bookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.create(user.id, activity.id) do - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> put_view(StatusView) @@ -569,7 +575,7 @@ def unbookmark_status(%{assigns: %{user: user}} = conn, %{"id" => id}) do %User{} = user <- User.get_cached_by_nickname(user.nickname), true <- Visibility.visible_for_user?(activity, user), {:ok, _bookmark} <- Bookmark.destroy(user.id, activity.id) do - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> put_view(StatusView) @@ -1091,7 +1097,7 @@ def favourites(%{assigns: %{user: user}} = conn, params) do ActivityPub.fetch_activities([], params) |> Enum.reverse() - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> add_link_headers(:favourites, activities) @@ -1138,7 +1144,7 @@ def user_favourites(%{assigns: %{user: for_user}} = conn, %{"id" => id} = params def bookmarks(%{assigns: %{user: user}} = conn, params) do user = User.get_cached_by_id(user.id) - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) bookmarks = Bookmark.for_user_query(user.id) @@ -1255,7 +1261,7 @@ def list_timeline(%{assigns: %{user: user}} = conn, %{"list_id" => id} = params) |> ActivityPub.fetch_activities_bounded(following, params) |> Enum.reverse() - user = Repo.preload(user, :bookmarks) + user = Repo.preload(user, bookmarks: :activity) conn |> put_view(StatusView)