From 3b1b631c2aedc8e359c296b11237fa4f6edd31e5 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 26 Aug 2019 18:59:57 +0700 Subject: [PATCH 01/18] Add validation in Pleroma.List.create/2 --- lib/pleroma/list.ex | 18 +++++++++++------- test/list_test.exs | 7 +++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/list.ex b/lib/pleroma/list.ex index 1d320206e..c572380c2 100644 --- a/lib/pleroma/list.ex +++ b/lib/pleroma/list.ex @@ -109,15 +109,19 @@ def rename(%Pleroma.List{} = list, title) do end def create(title, %User{} = creator) do - list = %Pleroma.List{user_id: creator.id, title: title} + changeset = title_changeset(%Pleroma.List{user_id: creator.id}, %{title: title}) - Repo.transaction(fn -> - list = Repo.insert!(list) + if changeset.valid? do + Repo.transaction(fn -> + list = Repo.insert!(changeset) - list - |> change(ap_id: "#{creator.ap_id}/lists/#{list.id}") - |> Repo.update!() - end) + list + |> change(ap_id: "#{creator.ap_id}/lists/#{list.id}") + |> Repo.update!() + end) + else + {:error, changeset} + end end def follow(%Pleroma.List{following: following} = list, %User{} = followed) do diff --git a/test/list_test.exs b/test/list_test.exs index f39033d02..8efba75ea 100644 --- a/test/list_test.exs +++ b/test/list_test.exs @@ -15,6 +15,13 @@ test "creating a list" do assert title == "title" end + test "validates title" do + user = insert(:user) + + assert {:error, changeset} = Pleroma.List.create("", user) + assert changeset.errors == [title: {"can't be blank", [validation: :required]}] + end + test "getting a list not belonging to the user" do user = insert(:user) other_user = insert(:user) From 4d82bc8b0b5a0b8b584b43330f902f8dc9637d3d Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 26 Aug 2019 19:16:40 +0700 Subject: [PATCH 02/18] Extract MastodonAPI.MastodonAPIController.errors/2 to MastodonAPI.FallbackController --- .../controllers/fallback_controller.ex | 34 +++++++++++++++++++ .../mastodon_api/mastodon_api_controller.ex | 31 +---------------- .../mastodon_api/subscription_controller.ex | 4 +-- 3 files changed, 36 insertions(+), 33 deletions(-) create mode 100644 lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex diff --git a/lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex b/lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex new file mode 100644 index 000000000..41243d5e7 --- /dev/null +++ b/lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex @@ -0,0 +1,34 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.FallbackController do + use Pleroma.Web, :controller + + def call(conn, {:error, %Ecto.Changeset{} = changeset}) do + error_message = + changeset + |> Ecto.Changeset.traverse_errors(fn {message, _opt} -> message end) + |> Enum.map_join(", ", fn {_k, v} -> v end) + + conn + |> put_status(:unprocessable_entity) + |> json(%{error: error_message}) + end + + def call(conn, {:error, :not_found}) do + render_error(conn, :not_found, "Record not found") + end + + def call(conn, {:error, error_message}) do + conn + |> put_status(:bad_request) + |> json(%{error: error_message}) + end + + def call(conn, _) do + conn + |> put_status(:internal_server_error) + |> json(dgettext("errors", "Something went wrong")) + end +end diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 53cf95fbb..e51b2d89c 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -83,7 +83,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do @local_mastodon_name "Mastodon-Local" - action_fallback(:errors) + action_fallback(Pleroma.Web.MastodonAPI.FallbackController) def create_app(conn, params) do scopes = Scopes.fetch_scopes(params, ["read"]) @@ -1587,35 +1587,6 @@ def delete_filter(%{assigns: %{user: user}} = conn, %{"id" => filter_id}) do json(conn, %{}) end - # fallback action - # - def errors(conn, {:error, %Changeset{} = changeset}) do - error_message = - changeset - |> Changeset.traverse_errors(fn {message, _opt} -> message end) - |> Enum.map_join(", ", fn {_k, v} -> v end) - - conn - |> put_status(:unprocessable_entity) - |> json(%{error: error_message}) - end - - def errors(conn, {:error, :not_found}) do - render_error(conn, :not_found, "Record not found") - end - - def errors(conn, {:error, error_message}) do - conn - |> put_status(:bad_request) - |> json(%{error: error_message}) - end - - def errors(conn, _) do - conn - |> put_status(:internal_server_error) - |> json(dgettext("errors", "Something went wrong")) - end - def suggestions(%{assigns: %{user: user}} = conn, _) do suggestions = Config.get(:suggestions) diff --git a/lib/pleroma/web/mastodon_api/subscription_controller.ex b/lib/pleroma/web/mastodon_api/subscription_controller.ex index 255ee2f18..e2b17aab1 100644 --- a/lib/pleroma/web/mastodon_api/subscription_controller.ex +++ b/lib/pleroma/web/mastodon_api/subscription_controller.ex @@ -64,8 +64,6 @@ def errors(conn, {:error, :not_found}) do end def errors(conn, _) do - conn - |> put_status(:internal_server_error) - |> json(dgettext("errors", "Something went wrong")) + Pleroma.Web.MastodonAPI.FallbackController.call(conn, nil) end end From 30510ade0e2f813413c5599245adc4dae8c7ffd8 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 26 Aug 2019 19:37:54 +0700 Subject: [PATCH 03/18] Extract MastodonAPIController's list actions into MastodonAPI.ListController; Add more tests --- .../controllers/list_controller.ex | 84 +++++++++ .../mastodon_api/mastodon_api_controller.ex | 76 -------- .../web/mastodon_api/views/list_view.ex | 6 +- lib/pleroma/web/router.ex | 16 +- .../controllers/list_controller_test.exs | 166 ++++++++++++++++++ .../mastodon_api_controller_test.exs | 101 +---------- .../{ => views}/list_view_test.exs | 14 +- 7 files changed, 274 insertions(+), 189 deletions(-) create mode 100644 lib/pleroma/web/mastodon_api/controllers/list_controller.ex create mode 100644 test/web/mastodon_api/controllers/list_controller_test.exs rename test/web/mastodon_api/{ => views}/list_view_test.exs (56%) diff --git a/lib/pleroma/web/mastodon_api/controllers/list_controller.ex b/lib/pleroma/web/mastodon_api/controllers/list_controller.ex new file mode 100644 index 000000000..2873deda8 --- /dev/null +++ b/lib/pleroma/web/mastodon_api/controllers/list_controller.ex @@ -0,0 +1,84 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.ListController do + use Pleroma.Web, :controller + + alias Pleroma.User + alias Pleroma.Web.MastodonAPI.AccountView + + plug(:list_by_id_and_user when action not in [:index, :create]) + + action_fallback(Pleroma.Web.MastodonAPI.FallbackController) + + # GET /api/v1/lists + def index(%{assigns: %{user: user}} = conn, opts) do + lists = Pleroma.List.for_user(user, opts) + render(conn, "index.json", lists: lists) + end + + # POST /api/v1/lists + def create(%{assigns: %{user: user}} = conn, %{"title" => title}) do + with {:ok, %Pleroma.List{} = list} <- Pleroma.List.create(title, user) do + render(conn, "show.json", list: list) + end + end + + # GET /api/v1/lists/:id + def show(%{assigns: %{list: list}} = conn, _) do + render(conn, "show.json", list: list) + end + + # PUT /api/v1/lists/:id + def update(%{assigns: %{list: list}} = conn, %{"title" => title}) do + with {:ok, list} <- Pleroma.List.rename(list, title) do + render(conn, "show.json", list: list) + end + end + + # DELETE /api/v1/lists/:id + def delete(%{assigns: %{list: list}} = conn, _) do + with {:ok, _list} <- Pleroma.List.delete(list) do + json(conn, %{}) + end + end + + # GET /api/v1/lists/:id/accounts + def list_accounts(%{assigns: %{user: user, list: list}} = conn, _) do + with {:ok, users} <- Pleroma.List.get_following(list) do + conn + |> put_view(AccountView) + |> render("accounts.json", for: user, users: users, as: :user) + end + end + + # POST /api/v1/lists/:id/accounts + def add_to_list(%{assigns: %{list: list}} = conn, %{"account_ids" => account_ids}) do + Enum.each(account_ids, fn account_id -> + with %User{} = followed <- User.get_cached_by_id(account_id) do + Pleroma.List.follow(list, followed) + end + end) + + json(conn, %{}) + end + + # DELETE /api/v1/lists/:id/accounts + def remove_from_list(%{assigns: %{list: list}} = conn, %{"account_ids" => account_ids}) do + Enum.each(account_ids, fn account_id -> + with %User{} = followed <- User.get_cached_by_id(account_id) do + Pleroma.List.unfollow(list, followed) + end + end) + + json(conn, %{}) + end + + defp list_by_id_and_user(%{assigns: %{user: user}, params: %{"id" => id}} = conn, _) do + case Pleroma.List.get(id, user) do + %Pleroma.List{} = list -> assign(conn, :list, list) + nil -> conn |> render_error(:not_found, "List not found") |> halt() + end + end +end diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index e51b2d89c..31b0aaca0 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -1205,88 +1205,12 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do |> render("index.json", %{activities: activities, for: user, as: :activity}) end - def get_lists(%{assigns: %{user: user}} = conn, opts) do - lists = Pleroma.List.for_user(user, opts) - res = ListView.render("lists.json", lists: lists) - json(conn, res) - end - - def get_list(%{assigns: %{user: user}} = conn, %{"id" => id}) do - with %Pleroma.List{} = list <- Pleroma.List.get(id, user) do - res = ListView.render("list.json", list: list) - json(conn, res) - else - _e -> render_error(conn, :not_found, "Record not found") - end - end - def account_lists(%{assigns: %{user: user}} = conn, %{"id" => account_id}) do lists = Pleroma.List.get_lists_account_belongs(user, account_id) res = ListView.render("lists.json", lists: lists) json(conn, res) end - def delete_list(%{assigns: %{user: user}} = conn, %{"id" => id}) do - with %Pleroma.List{} = list <- Pleroma.List.get(id, user), - {:ok, _list} <- Pleroma.List.delete(list) do - json(conn, %{}) - else - _e -> - json(conn, dgettext("errors", "error")) - end - end - - def create_list(%{assigns: %{user: user}} = conn, %{"title" => title}) do - with {:ok, %Pleroma.List{} = list} <- Pleroma.List.create(title, user) do - res = ListView.render("list.json", list: list) - json(conn, res) - end - end - - def add_to_list(%{assigns: %{user: user}} = conn, %{"id" => id, "account_ids" => accounts}) do - accounts - |> Enum.each(fn account_id -> - with %Pleroma.List{} = list <- Pleroma.List.get(id, user), - %User{} = followed <- User.get_cached_by_id(account_id) do - Pleroma.List.follow(list, followed) - end - end) - - json(conn, %{}) - end - - def remove_from_list(%{assigns: %{user: user}} = conn, %{"id" => id, "account_ids" => accounts}) do - accounts - |> Enum.each(fn account_id -> - with %Pleroma.List{} = list <- Pleroma.List.get(id, user), - %User{} = followed <- User.get_cached_by_id(account_id) do - Pleroma.List.unfollow(list, followed) - end - end) - - json(conn, %{}) - end - - def list_accounts(%{assigns: %{user: user}} = conn, %{"id" => id}) do - with %Pleroma.List{} = list <- Pleroma.List.get(id, user), - {:ok, users} = Pleroma.List.get_following(list) do - conn - |> put_view(AccountView) - |> render("accounts.json", %{for: user, users: users, as: :user}) - end - end - - def rename_list(%{assigns: %{user: user}} = conn, %{"id" => id, "title" => title}) do - with %Pleroma.List{} = list <- Pleroma.List.get(id, user), - {:ok, list} <- Pleroma.List.rename(list, title) do - res = ListView.render("list.json", list: list) - json(conn, res) - else - _e -> - json(conn, dgettext("errors", "error")) - end - end - def list_timeline(%{assigns: %{user: user}} = conn, %{"list_id" => id} = params) do with %Pleroma.List{title: _title, following: following} <- Pleroma.List.get(id, user) do params = diff --git a/lib/pleroma/web/mastodon_api/views/list_view.ex b/lib/pleroma/web/mastodon_api/views/list_view.ex index 0f86e2512..bfda6f5b3 100644 --- a/lib/pleroma/web/mastodon_api/views/list_view.ex +++ b/lib/pleroma/web/mastodon_api/views/list_view.ex @@ -6,11 +6,11 @@ defmodule Pleroma.Web.MastodonAPI.ListView do use Pleroma.Web, :view alias Pleroma.Web.MastodonAPI.ListView - def render("lists.json", %{lists: lists} = opts) do - render_many(lists, ListView, "list.json", opts) + def render("index.json", %{lists: lists} = opts) do + render_many(lists, ListView, "show.json", opts) end - def render("list.json", %{list: list}) do + def render("show.json", %{list: list}) do %{ id: to_string(list.id), title: list.title diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 1ad33630c..969dc66fd 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -312,9 +312,9 @@ defmodule Pleroma.Web.Router do get("/scheduled_statuses", MastodonAPIController, :scheduled_statuses) get("/scheduled_statuses/:id", MastodonAPIController, :show_scheduled_status) - get("/lists", MastodonAPIController, :get_lists) - get("/lists/:id", MastodonAPIController, :get_list) - get("/lists/:id/accounts", MastodonAPIController, :list_accounts) + get("/lists", ListController, :index) + get("/lists/:id", ListController, :show) + get("/lists/:id/accounts", ListController, :list_accounts) get("/domain_blocks", MastodonAPIController, :domain_blocks) @@ -355,12 +355,12 @@ defmodule Pleroma.Web.Router do post("/media", MastodonAPIController, :upload) put("/media/:id", MastodonAPIController, :update_media) - delete("/lists/:id", MastodonAPIController, :delete_list) - post("/lists", MastodonAPIController, :create_list) - put("/lists/:id", MastodonAPIController, :rename_list) + delete("/lists/:id", ListController, :delete) + post("/lists", ListController, :create) + put("/lists/:id", ListController, :update) - post("/lists/:id/accounts", MastodonAPIController, :add_to_list) - delete("/lists/:id/accounts", MastodonAPIController, :remove_from_list) + post("/lists/:id/accounts", ListController, :add_to_list) + delete("/lists/:id/accounts", ListController, :remove_from_list) post("/filters", MastodonAPIController, :create_filter) get("/filters/:id", MastodonAPIController, :get_filter) diff --git a/test/web/mastodon_api/controllers/list_controller_test.exs b/test/web/mastodon_api/controllers/list_controller_test.exs new file mode 100644 index 000000000..093506309 --- /dev/null +++ b/test/web/mastodon_api/controllers/list_controller_test.exs @@ -0,0 +1,166 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.ListControllerTest do + use Pleroma.Web.ConnCase + + alias Pleroma.Repo + + import Pleroma.Factory + + test "creating a list", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> post("/api/v1/lists", %{"title" => "cuties"}) + + assert %{"title" => title} = json_response(conn, 200) + assert title == "cuties" + end + + test "renders error for invalid params", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> post("/api/v1/lists", %{"title" => nil}) + + assert %{"error" => "can't be blank"} == json_response(conn, :unprocessable_entity) + end + + test "listing a user's lists", %{conn: conn} do + user = insert(:user) + + conn + |> assign(:user, user) + |> post("/api/v1/lists", %{"title" => "cuties"}) + + conn + |> assign(:user, user) + |> post("/api/v1/lists", %{"title" => "cofe"}) + + conn = + conn + |> assign(:user, user) + |> get("/api/v1/lists") + + assert [ + %{"id" => _, "title" => "cofe"}, + %{"id" => _, "title" => "cuties"} + ] = json_response(conn, :ok) + end + + test "adding users to a list", %{conn: conn} do + user = insert(:user) + other_user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + + conn = + conn + |> assign(:user, user) + |> post("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) + + assert %{} == json_response(conn, 200) + %Pleroma.List{following: following} = Pleroma.List.get(list.id, user) + assert following == [other_user.follower_address] + end + + test "removing users from a list", %{conn: conn} do + user = insert(:user) + other_user = insert(:user) + third_user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + {:ok, list} = Pleroma.List.follow(list, other_user) + {:ok, list} = Pleroma.List.follow(list, third_user) + + conn = + conn + |> assign(:user, user) + |> delete("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) + + assert %{} == json_response(conn, 200) + %Pleroma.List{following: following} = Pleroma.List.get(list.id, user) + assert following == [third_user.follower_address] + end + + test "listing users in a list", %{conn: conn} do + user = insert(:user) + other_user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + {:ok, list} = Pleroma.List.follow(list, other_user) + + conn = + conn + |> assign(:user, user) + |> get("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) + + assert [%{"id" => id}] = json_response(conn, 200) + assert id == to_string(other_user.id) + end + + test "retrieving a list", %{conn: conn} do + user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + + conn = + conn + |> assign(:user, user) + |> get("/api/v1/lists/#{list.id}") + + assert %{"id" => id} = json_response(conn, 200) + assert id == to_string(list.id) + end + + test "renders 404 if list is not found", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> get("/api/v1/lists/666") + + assert %{"error" => "List not found"} = json_response(conn, :not_found) + end + + test "renaming a list", %{conn: conn} do + user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + + conn = + conn + |> assign(:user, user) + |> put("/api/v1/lists/#{list.id}", %{"title" => "newname"}) + + assert %{"title" => name} = json_response(conn, 200) + assert name == "newname" + end + + test "validates title when renaming a list", %{conn: conn} do + user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + + conn = + conn + |> assign(:user, user) + |> put("/api/v1/lists/#{list.id}", %{"title" => " "}) + + assert %{"error" => "can't be blank"} == json_response(conn, :unprocessable_entity) + end + + test "deleting a list", %{conn: conn} do + user = insert(:user) + {:ok, list} = Pleroma.List.create("name", user) + + conn = + conn + |> assign(:user, user) + |> delete("/api/v1/lists/#{list.id}") + + assert %{} = json_response(conn, 200) + assert is_nil(Repo.get(Pleroma.List, list.id)) + end +end diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 6fcdc19aa..4fd0a5aeb 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -927,106 +927,7 @@ test "delete a filter", %{conn: conn} do end end - describe "lists" do - test "creating a list", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/lists", %{"title" => "cuties"}) - - assert %{"title" => title} = json_response(conn, 200) - assert title == "cuties" - end - - test "adding users to a list", %{conn: conn} do - user = insert(:user) - other_user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - - conn = - conn - |> assign(:user, user) - |> post("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) - - assert %{} == json_response(conn, 200) - %Pleroma.List{following: following} = Pleroma.List.get(list.id, user) - assert following == [other_user.follower_address] - end - - test "removing users from a list", %{conn: conn} do - user = insert(:user) - other_user = insert(:user) - third_user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - {:ok, list} = Pleroma.List.follow(list, other_user) - {:ok, list} = Pleroma.List.follow(list, third_user) - - conn = - conn - |> assign(:user, user) - |> delete("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) - - assert %{} == json_response(conn, 200) - %Pleroma.List{following: following} = Pleroma.List.get(list.id, user) - assert following == [third_user.follower_address] - end - - test "listing users in a list", %{conn: conn} do - user = insert(:user) - other_user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - {:ok, list} = Pleroma.List.follow(list, other_user) - - conn = - conn - |> assign(:user, user) - |> get("/api/v1/lists/#{list.id}/accounts", %{"account_ids" => [other_user.id]}) - - assert [%{"id" => id}] = json_response(conn, 200) - assert id == to_string(other_user.id) - end - - test "retrieving a list", %{conn: conn} do - user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - - conn = - conn - |> assign(:user, user) - |> get("/api/v1/lists/#{list.id}") - - assert %{"id" => id} = json_response(conn, 200) - assert id == to_string(list.id) - end - - test "renaming a list", %{conn: conn} do - user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - - conn = - conn - |> assign(:user, user) - |> put("/api/v1/lists/#{list.id}", %{"title" => "newname"}) - - assert %{"title" => name} = json_response(conn, 200) - assert name == "newname" - end - - test "deleting a list", %{conn: conn} do - user = insert(:user) - {:ok, list} = Pleroma.List.create("name", user) - - conn = - conn - |> assign(:user, user) - |> delete("/api/v1/lists/#{list.id}") - - assert %{} = json_response(conn, 200) - assert is_nil(Repo.get(Pleroma.List, list.id)) - end - + describe "list timelines" do test "list timeline", %{conn: conn} do user = insert(:user) other_user = insert(:user) diff --git a/test/web/mastodon_api/list_view_test.exs b/test/web/mastodon_api/views/list_view_test.exs similarity index 56% rename from test/web/mastodon_api/list_view_test.exs rename to test/web/mastodon_api/views/list_view_test.exs index 73143467f..fb00310b9 100644 --- a/test/web/mastodon_api/list_view_test.exs +++ b/test/web/mastodon_api/views/list_view_test.exs @@ -7,7 +7,7 @@ defmodule Pleroma.Web.MastodonAPI.ListViewTest do import Pleroma.Factory alias Pleroma.Web.MastodonAPI.ListView - test "Represent a list" do + test "show" do user = insert(:user) title = "mortal enemies" {:ok, list} = Pleroma.List.create(title, user) @@ -17,6 +17,16 @@ test "Represent a list" do title: title } - assert expected == ListView.render("list.json", %{list: list}) + assert expected == ListView.render("show.json", %{list: list}) + end + + test "index" do + user = insert(:user) + + {:ok, list} = Pleroma.List.create("my list", user) + {:ok, list2} = Pleroma.List.create("cofe", user) + + assert [%{id: _, title: "my list"}, %{id: _, title: "cofe"}] = + ListView.render("index.json", lists: [list, list2]) end end From 4194abbc8fbc8003d9923edaa491e798bea92107 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 26 Aug 2019 19:32:47 +0700 Subject: [PATCH 04/18] Move mastodon_api/*_controller.ex to mastodon_api/controllers/ --- .../mastodon_api_controller.ex | 20 +++++++++---------- .../{ => controllers}/search_controller.ex | 0 .../subscription_controller.ex | 0 3 files changed, 10 insertions(+), 10 deletions(-) rename lib/pleroma/web/mastodon_api/{ => controllers}/mastodon_api_controller.ex (98%) rename lib/pleroma/web/mastodon_api/{ => controllers}/search_controller.ex (100%) rename lib/pleroma/web/mastodon_api/{ => controllers}/subscription_controller.ex (100%) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex similarity index 98% rename from lib/pleroma/web/mastodon_api/mastodon_api_controller.ex rename to lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex index 31b0aaca0..83e877c0e 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex @@ -189,7 +189,7 @@ def update_credentials(%{assigns: %{user: user}} = conn, params) do info_cng = User.Info.profile_update(user.info, info_params) with changeset <- User.update_changeset(user, user_params), - changeset <- Ecto.Changeset.put_embed(changeset, :info, info_cng), + changeset <- Changeset.put_embed(changeset, :info, info_cng), {:ok, user} <- User.update_and_set_cache(changeset) do if original_user != user do CommonAPI.update(user) @@ -225,7 +225,7 @@ def update_avatar(%{assigns: %{user: user}} = conn, params) do def update_banner(%{assigns: %{user: user}} = conn, %{"banner" => ""}) do with new_info <- %{"banner" => %{}}, info_cng <- User.Info.profile_update(user.info, new_info), - changeset <- Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_cng), + changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng), {:ok, user} <- User.update_and_set_cache(changeset) do CommonAPI.update(user) @@ -237,7 +237,7 @@ def update_banner(%{assigns: %{user: user}} = conn, params) do with {:ok, object} <- ActivityPub.upload(%{"img" => params["banner"]}, type: :banner), new_info <- %{"banner" => object.data}, info_cng <- User.Info.profile_update(user.info, new_info), - changeset <- Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_cng), + changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng), {:ok, user} <- User.update_and_set_cache(changeset) do CommonAPI.update(user) %{"url" => [%{"href" => href} | _]} = object.data @@ -249,7 +249,7 @@ def update_banner(%{assigns: %{user: user}} = conn, params) do def update_background(%{assigns: %{user: user}} = conn, %{"img" => ""}) do with new_info <- %{"background" => %{}}, info_cng <- User.Info.profile_update(user.info, new_info), - changeset <- Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_cng), + changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng), {:ok, _user} <- User.update_and_set_cache(changeset) do json(conn, %{url: nil}) end @@ -259,7 +259,7 @@ def update_background(%{assigns: %{user: user}} = conn, params) do with {:ok, object} <- ActivityPub.upload(params, type: :background), new_info <- %{"background" => object.data}, info_cng <- User.Info.profile_update(user.info, new_info), - changeset <- Ecto.Changeset.change(user) |> Ecto.Changeset.put_embed(:info, info_cng), + changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_cng), {:ok, _user} <- User.update_and_set_cache(changeset) do %{"url" => [%{"href" => href} | _]} = object.data @@ -806,8 +806,8 @@ def set_mascot(%{assigns: %{user: user}} = conn, %{"file" => file}) do user_changeset = user - |> Ecto.Changeset.change() - |> Ecto.Changeset.put_embed(:info, info_changeset) + |> Changeset.change() + |> Changeset.put_embed(:info, info_changeset) {:ok, _user} = User.update_and_set_cache(user_changeset) @@ -1344,8 +1344,8 @@ def index(%{assigns: %{user: user}} = conn, _params) do def put_settings(%{assigns: %{user: user}} = conn, %{"data" => settings} = _params) do info_cng = User.Info.mastodon_settings_update(user.info, settings) - with changeset <- Ecto.Changeset.change(user), - changeset <- Ecto.Changeset.put_embed(changeset, :info, info_cng), + with changeset <- Changeset.change(user), + changeset <- Changeset.put_embed(changeset, :info, info_cng), {:ok, _user} <- User.update_and_set_cache(changeset) do json(conn, %{}) else @@ -1409,7 +1409,7 @@ defp get_or_make_app do {:ok, app} else app - |> Ecto.Changeset.change(%{scopes: scopes}) + |> Changeset.change(%{scopes: scopes}) |> Repo.update() end diff --git a/lib/pleroma/web/mastodon_api/search_controller.ex b/lib/pleroma/web/mastodon_api/controllers/search_controller.ex similarity index 100% rename from lib/pleroma/web/mastodon_api/search_controller.ex rename to lib/pleroma/web/mastodon_api/controllers/search_controller.ex diff --git a/lib/pleroma/web/mastodon_api/subscription_controller.ex b/lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex similarity index 100% rename from lib/pleroma/web/mastodon_api/subscription_controller.ex rename to lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex From 019ced055836b3d01ea95865549478dc5cdb3c0e Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 26 Aug 2019 19:34:43 +0700 Subject: [PATCH 05/18] Move test/web/mastodon_api/*_test.exs to test/web/mastodon_api/controllers and test/web/mastodon_api/views --- .../mastodon_api_controller/update_credentials_test.exs | 0 .../web/mastodon_api/{ => controllers}/search_controller_test.exs | 0 .../{ => controllers}/subscription_controller_test.exs | 0 test/web/mastodon_api/{ => views}/account_view_test.exs | 0 test/web/mastodon_api/{ => views}/conversation_view_test.exs | 0 test/web/mastodon_api/{ => views}/notification_view_test.exs | 0 test/web/mastodon_api/{ => views}/push_subscription_view_test.exs | 0 .../web/mastodon_api/{ => views}/scheduled_activity_view_test.exs | 0 test/web/mastodon_api/{ => views}/status_view_test.exs | 0 9 files changed, 0 insertions(+), 0 deletions(-) rename test/web/mastodon_api/{ => controllers}/mastodon_api_controller/update_credentials_test.exs (100%) rename test/web/mastodon_api/{ => controllers}/search_controller_test.exs (100%) rename test/web/mastodon_api/{ => controllers}/subscription_controller_test.exs (100%) rename test/web/mastodon_api/{ => views}/account_view_test.exs (100%) rename test/web/mastodon_api/{ => views}/conversation_view_test.exs (100%) rename test/web/mastodon_api/{ => views}/notification_view_test.exs (100%) rename test/web/mastodon_api/{ => views}/push_subscription_view_test.exs (100%) rename test/web/mastodon_api/{ => views}/scheduled_activity_view_test.exs (100%) rename test/web/mastodon_api/{ => views}/status_view_test.exs (100%) diff --git a/test/web/mastodon_api/mastodon_api_controller/update_credentials_test.exs b/test/web/mastodon_api/controllers/mastodon_api_controller/update_credentials_test.exs similarity index 100% rename from test/web/mastodon_api/mastodon_api_controller/update_credentials_test.exs rename to test/web/mastodon_api/controllers/mastodon_api_controller/update_credentials_test.exs diff --git a/test/web/mastodon_api/search_controller_test.exs b/test/web/mastodon_api/controllers/search_controller_test.exs similarity index 100% rename from test/web/mastodon_api/search_controller_test.exs rename to test/web/mastodon_api/controllers/search_controller_test.exs diff --git a/test/web/mastodon_api/subscription_controller_test.exs b/test/web/mastodon_api/controllers/subscription_controller_test.exs similarity index 100% rename from test/web/mastodon_api/subscription_controller_test.exs rename to test/web/mastodon_api/controllers/subscription_controller_test.exs diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/views/account_view_test.exs similarity index 100% rename from test/web/mastodon_api/account_view_test.exs rename to test/web/mastodon_api/views/account_view_test.exs diff --git a/test/web/mastodon_api/conversation_view_test.exs b/test/web/mastodon_api/views/conversation_view_test.exs similarity index 100% rename from test/web/mastodon_api/conversation_view_test.exs rename to test/web/mastodon_api/views/conversation_view_test.exs diff --git a/test/web/mastodon_api/notification_view_test.exs b/test/web/mastodon_api/views/notification_view_test.exs similarity index 100% rename from test/web/mastodon_api/notification_view_test.exs rename to test/web/mastodon_api/views/notification_view_test.exs diff --git a/test/web/mastodon_api/push_subscription_view_test.exs b/test/web/mastodon_api/views/push_subscription_view_test.exs similarity index 100% rename from test/web/mastodon_api/push_subscription_view_test.exs rename to test/web/mastodon_api/views/push_subscription_view_test.exs diff --git a/test/web/mastodon_api/scheduled_activity_view_test.exs b/test/web/mastodon_api/views/scheduled_activity_view_test.exs similarity index 100% rename from test/web/mastodon_api/scheduled_activity_view_test.exs rename to test/web/mastodon_api/views/scheduled_activity_view_test.exs diff --git a/test/web/mastodon_api/status_view_test.exs b/test/web/mastodon_api/views/status_view_test.exs similarity index 100% rename from test/web/mastodon_api/status_view_test.exs rename to test/web/mastodon_api/views/status_view_test.exs From 3da65292b389c1f1edeff03fd5097579721fb681 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 26 Aug 2019 14:34:52 -0500 Subject: [PATCH 06/18] Transmogrifier: Fix follow handling when the actor is an object. --- CHANGELOG.md | 1 + lib/pleroma/object.ex | 4 ++ .../web/activity_pub/transmogrifier.ex | 4 +- test/fixtures/osada-follow-activity.json | 56 +++++++++++++++++++ .../fixtures/tesla_mock/osada-user-indio.json | 1 + test/support/http_request_mock.ex | 5 ++ .../transmogrifier/follow_handling_test.exs | 19 +++++++ 7 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/osada-follow-activity.json create mode 100644 test/fixtures/tesla_mock/osada-user-indio.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fdcb014a..20af9badc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Improve digest email template ### Fixed +- Following from Osada - Not being able to pin unlisted posts - Objects being re-embedded to activities after being updated (e.g faved/reposted). Running 'mix pleroma.database prune_objects' again is advised. - Favorites timeline doing database-intensive queries diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index c8d339c19..468549c87 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -230,4 +230,8 @@ def increase_vote_count(ap_id, name) do _ -> :noop end end + + def get_ap_id(%{"id" => id}), do: id + def get_ap_id(id) when is_binary(id), do: id + def get_ap_id(_), do: {:error, "Object is not a string and has no id."} end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 36340a3a1..6c4259c02 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -464,8 +464,8 @@ def handle_incoming( %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = data, _options ) do - with %User{local: true} = followed <- User.get_cached_by_ap_id(followed), - {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower), + with %User{local: true} = followed <- User.get_cached_by_ap_id(Object.get_ap_id(followed)), + {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(Object.get_ap_id(follower)), {:ok, activity} <- ActivityPub.follow(follower, followed, id, false) do with deny_follow_blocked <- Pleroma.Config.get([:user, :deny_follow_blocked]), {_, false} <- {:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked}, diff --git a/test/fixtures/osada-follow-activity.json b/test/fixtures/osada-follow-activity.json new file mode 100644 index 000000000..b991eea36 --- /dev/null +++ b/test/fixtures/osada-follow-activity.json @@ -0,0 +1,56 @@ +{ + "@context":[ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + "https://apfed.club/apschema/v1.4" + ], + "id":"https://apfed.club/follow/9", + "type":"Follow", + "actor":{ + "type":"Person", + "id":"https://apfed.club/channel/indio", + "preferredUsername":"indio", + "name":"Indio", + "updated":"2019-08-20T23:52:34Z", + "icon":{ + "type":"Image", + "mediaType":"image/jpeg", + "updated":"2019-08-20T23:53:37Z", + "url":"https://apfed.club/photo/profile/l/2", + "height":300, + "width":300 + }, + "url":"https://apfed.club/channel/indio", + "inbox":"https://apfed.club/inbox/indio", + "outbox":"https://apfed.club/outbox/indio", + "followers":"https://apfed.club/followers/indio", + "following":"https://apfed.club/following/indio", + "endpoints":{ + "sharedInbox":"https://apfed.club/inbox" + }, + "publicKey":{ + "id":"https://apfed.club/channel/indio", + "owner":"https://apfed.club/channel/indio", + "publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA77TIR1VuSYFnmDRFGHHb\n4vaGdx9ranzRX4bfOKAqa++Ch5L4EqJpPy08RuM+NrYCYiYl4QQFDSSDXAEgb5g9\nC1TgWTfI7q/E0UBX2Vr0mU6X4i1ztv0tuQvegRjcSJ7l1AvoBs8Ip4MEJ3OPEQhB\ngJqAACB3Gnps4zi2I0yavkxUfGVKr6zKT3BxWh5hTpKC7Do+ChIrVZC2EwxND9K6 +\nsAnQHThcb5EQuvuzUQZKeS7IEOsd0JpZDmJjbfMGrAWE81pLIfEeeA2joCJiBBTO\nglDsW+juvZ+lWqJpMr2hMWpvfrFjJeUawNJCIzsLdVIZR+aKj5yy6yqoS8hkN9Ha\n1MljZpsXl+EmwcwAIqim1YeLwERCEAQ/JWbSt8pQTQbzZ6ibwQ4mchCxacrRbIVR +\nnL59fWMBassJcbY0VwrTugm2SBsYbDjESd55UZV03Rwr8qseGTyi+hH8O7w2SIaY\nzjN6AdZiPmsh00YflzlCk8MSLOHMol1vqIUzXxU8CdXn9+KsuQdZGrTz0YKN/db4\naVwUGJatz2Tsvf7R1tJBjJfeQWOWbbn3pycLVH86LjZ83qngp9ZVnAveUnUqz0yS +\nhe+buZ6UMsfGzbIYon2bKNlz6gYTH0YPcr+cLe+29drtt0GZiXha1agbpo4RB8zE +\naNL2fucF5YT0yNpbd/5WoV0CAwEAAQ==\n-----END PUBLIC KEY-----\n" + } + }, + "object":"https://pleroma.site/users/kaniini", + "to":[ + "https://pleroma.site/users/kaniini" + ], + "signature":{ + "@context":[ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1" + ], + "type":"RsaSignature2017", + "nonce":"52c035e0a9e81dce8b486159204e97c22637e91f75cdfad5378de91de68e9117", + "creator":"https://apfed.club/channel/indio/public_key_pem", + "created":"2019-08-22T03:38:02Z", + "signatureValue":"oVliRCIqNIh6yUp851dYrF0y21aHp3Rz6VkIpW1pFMWfXuzExyWSfcELpyLseeRmsw5bUu9zJkH44B4G2LiJQKA9UoEQDjrDMZBmbeUpiQqq3DVUzkrBOI8bHZ7xyJ/CjSZcNHHh0MHhSKxswyxWMGi4zIqzkAZG3vRRgoPVHdjPm00sR3B8jBLw1cjoffv+KKeM/zEUpe13gqX9qHAWHHqZepxgSWmq+EKOkRvHUPBXiEJZfXzc5uW+vZ09F3WBYmaRoy8Y0e1P29fnRLqSy7EEINdrHaGclRqoUZyiawpkgy3lWWlynesV/HiLBR7EXT79eKstxf4wfTDaPKBCfTCsOWuMWHr7Genu37ew2/t7eiBGqCwwW12ylhml/OLHgNK3LOhmRABhtfpaFZSxfDVnlXfaLpY1xekVOj2oC0FpBtnoxVKLpIcyLw6dkfSil5ANd+hl59W/bpPA8KT90ii1fSNCo3+FcwQVx0YsPznJNA60XfFuVsme7zNcOst6393e1WriZxBanFpfB63zVQc9u1fjyfktx/yiUNxIlre+sz9OCc0AACn94iRhBYh4bbzdleUOTnM7lnD4Dj2FP+xeDIP8CA8wXUeq5+9kopSp2kAmlUEyFUdg4no7naIeu1SZnopfUg56PsVCp9JHiUK1SYAyWbdC+FbUECu5CvI=" + } +} diff --git a/test/fixtures/tesla_mock/osada-user-indio.json b/test/fixtures/tesla_mock/osada-user-indio.json new file mode 100644 index 000000000..c1d52c92a --- /dev/null +++ b/test/fixtures/tesla_mock/osada-user-indio.json @@ -0,0 +1 @@ +{"@context":["https://www.w3.org/ns/activitystreams","https://w3id.org/security/v1"],"type":"Person","id":"https://apfed.club/channel/indio","preferredUsername":"indio","name":"Indio","updated":"2019-08-20T23:52:34Z","icon":{"type":"Image","mediaType":"image/jpeg","updated":"2019-08-20T23:53:37Z","url":"https://apfed.club/photo/profile/l/2","height":300,"width":300},"url":"https://apfed.club/channel/indio","inbox":"https://apfed.club/inbox/indio","outbox":"https://apfed.club/outbox/indio","followers":"https://apfed.club/followers/indio","following":"https://apfed.club/following/indio","endpoints":{"sharedInbox":"https://apfed.club/inbox"},"publicKey":{"id":"https://apfed.club/channel/indio","owner":"https://apfed.club/channel/indio","publicKeyPem":"-----BEGIN PUBLIC KEY-----\nMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA77TIR1VuSYFnmDRFGHHb\n4vaGdx9ranzRX4bfOKAqa++Ch5L4EqJpPy08RuM+NrYCYiYl4QQFDSSDXAEgb5g9\nC1TgWTfI7q/E0UBX2Vr0mU6X4i1ztv0tuQvegRjcSJ7l1AvoBs8Ip4MEJ3OPEQhB\ngJqAACB3Gnps4zi2I0yavkxUfGVKr6zKT3BxWh5hTpKC7Do+ChIrVZC2EwxND9K6\nsAnQHThcb5EQuvuzUQZKeS7IEOsd0JpZDmJjbfMGrAWE81pLIfEeeA2joCJiBBTO\nglDsW+juvZ+lWqJpMr2hMWpvfrFjJeUawNJCIzsLdVIZR+aKj5yy6yqoS8hkN9Ha\n1MljZpsXl+EmwcwAIqim1YeLwERCEAQ/JWbSt8pQTQbzZ6ibwQ4mchCxacrRbIVR\nnL59fWMBassJcbY0VwrTugm2SBsYbDjESd55UZV03Rwr8qseGTyi+hH8O7w2SIaY\nzjN6AdZiPmsh00YflzlCk8MSLOHMol1vqIUzXxU8CdXn9+KsuQdZGrTz0YKN/db4\naVwUGJatz2Tsvf7R1tJBjJfeQWOWbbn3pycLVH86LjZ83qngp9ZVnAveUnUqz0yS\nhe+buZ6UMsfGzbIYon2bKNlz6gYTH0YPcr+cLe+29drtt0GZiXha1agbpo4RB8zE\naNL2fucF5YT0yNpbd/5WoV0CAwEAAQ==\n-----END PUBLIC KEY-----\n"},"signature":{"@context":["https://www.w3.org/ns/activitystreams","https://w3id.org/security/v1"],"type":"RsaSignature2017","nonce":"c672a408d2e88b322b36a61bf0c25f586be9245d30293c55b8d653dcc867aaf7","creator":"https://apfed.club/channel/indio/public_key_pem","created":"2019-08-26T07:24:03Z","signatureValue":"MyAv5gnedu6L/DYFaE1TUYvp4LjI9ZUU0axwGYOhgD7qsjivMgwbOrjX/iH32xlcfF8nWOMh/ogu3+Qwr5sqLHkS2AimWmw1+Ubf2KccE58b8vI8zWfyu8QJnMuE92jtBPv8UTQUHw8ZebbExk3L99oXaeyVihKiMBmd63NpVTpGXZTg6m+H+KfWchVajPoyNKZtKMd3nH99x5j54Cqkz0BN5CSTwCSG0wP95G0VtZHtmhX+tsAPM3oAj0d+gtCZSCd8Nu8fvFAwCyTg1oKSfRqKb27EKHlskqK9X57x0jURH77CTAIQSejgGcKJ5GGLtvofubJkafadjagqrtqz6Mz6BZ642ssJ2KGkRAn79Q4F08goI6cfU5lLk2Tooe5A55XERnmE3SkYGyTvLpacZplxJdU0sa+deX9D7+alSGFJZSziaxpCxzrO6lEApe4b9kHXAzn9VaZt9trijkHq/kkq0i3NRcP7n8JG9q+Vv8jY9ddY6HcH89RNCBIA6MKLtAqc+vSc5G24qeZlw2MzlQWBp0KGuVG8DQR00AL6cXLBzF1WY8JZeEg6zqm+DMznbuNzgiS34BP+AehBSHlQ4MZebwDnK3ZPPqGSwioIWMxIFfZDaVDX9Pp1pXAARQMw0c/y4sDcf9FMzsr8jteEa7ZQcoqq5kXQTSCP56TEHnI="}} \ No newline at end of file diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 55b141dd8..05eebbe9b 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -775,6 +775,11 @@ def get("https://mastodon.social/users/lambadalambda", _, _, _) do {:ok, %Tesla.Env{status: 200, body: File.read!("test/fixtures/lambadalambda.json")}} end + def get("https://apfed.club/channel/indio", _, _, _) do + {:ok, + %Tesla.Env{status: 200, body: File.read!("test/fixtures/tesla_mock/osada-user-indio.json")}} + end + def get("https://social.heldscal.la/user/23211", _, _, Accept: "application/activity+json") do {:ok, Tesla.Mock.json(%{"id" => "https://social.heldscal.la/user/23211"}, status: 200)} end diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs index 857d65564..fe89f7cb0 100644 --- a/test/web/activity_pub/transmogrifier/follow_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/follow_handling_test.exs @@ -19,6 +19,25 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do end describe "handle_incoming" do + test "it works for osada follow request" do + user = insert(:user) + + data = + File.read!("test/fixtures/osada-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + + {:ok, %Activity{data: data, local: false} = activity} = Transmogrifier.handle_incoming(data) + + assert data["actor"] == "https://apfed.club/channel/indio" + assert data["type"] == "Follow" + assert data["id"] == "https://apfed.club/follow/9" + + activity = Repo.get(Activity, activity.id) + assert activity.data["state"] == "accept" + assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) + end + test "it works for incoming follow requests" do user = insert(:user) From 00abe099cd85b03b880908eef1e469e656d56365 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Tue, 27 Aug 2019 16:21:03 +0300 Subject: [PATCH 07/18] added tests for ActivityPub.like\unlike --- lib/pleroma/activity/queries.ex | 49 ++++++ lib/pleroma/object.ex | 2 - lib/pleroma/web/activity_pub/activity_pub.ex | 9 +- .../activity_pub/activity_pub_controller.ex | 59 +++---- lib/pleroma/web/activity_pub/utils.ex | 150 ++++++++---------- test/support/factory.ex | 16 +- test/web/activity_pub/activity_pub_test.exs | 44 +++++ test/web/activity_pub/utils_test.exs | 102 ++++++++++++ 8 files changed, 304 insertions(+), 127 deletions(-) create mode 100644 lib/pleroma/activity/queries.ex diff --git a/lib/pleroma/activity/queries.ex b/lib/pleroma/activity/queries.ex new file mode 100644 index 000000000..aa5b29566 --- /dev/null +++ b/lib/pleroma/activity/queries.ex @@ -0,0 +1,49 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2018 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Activity.Queries do + @moduledoc """ + Contains queries for Activity. + """ + + import Ecto.Query, only: [from: 2] + + @type query :: Ecto.Queryable.t() | Activity.t() + + alias Pleroma.Activity + + @spec by_actor(query, String.t()) :: query + def by_actor(query \\ Activity, actor) do + from( + activity in query, + where: fragment("(?)->>'actor' = ?", activity.data, ^actor) + ) + end + + @spec by_object_id(query, String.t()) :: query + def by_object_id(query \\ Activity, object_id) do + from(activity in query, + where: + fragment( + "coalesce((?)->'object'->>'id', (?)->>'object') = ?", + activity.data, + activity.data, + ^object_id + ) + ) + end + + @spec by_type(query, String.t()) :: query + def by_type(query \\ Activity, activity_type) do + from( + activity in query, + where: fragment("(?)->>'type' = ?", activity.data, ^activity_type) + ) + end + + @spec limit(query, pos_integer()) :: query + def limit(query \\ Activity, limit) do + from(activity in query, limit: ^limit) + end +end diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index c8d339c19..d58eb7f7d 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -150,8 +150,6 @@ def set_cache(%Object{data: %{"id" => ap_id}} = object) do def update_and_set_cache(changeset) do with {:ok, object} <- Repo.update(changeset) do set_cache(object) - else - e -> e end end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 172c952d4..eeb826814 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -139,7 +139,7 @@ def insert(map, local \\ true, fake \\ false, bypass_actor_check \\ false) when # Splice in the child object if we have one. activity = - if !is_nil(object) do + if not is_nil(object) do Map.put(activity, :object, object) else activity @@ -331,12 +331,7 @@ def like( end end - def unlike( - %User{} = actor, - %Object{} = object, - activity_id \\ nil, - local \\ true - ) do + def unlike(%User{} = actor, %Object{} = object, activity_id \\ nil, local \\ true) do with %Activity{} = like_activity <- get_existing_like(actor.ap_id, object), unlike_data <- make_unlike_data(actor, like_activity, activity_id), {:ok, unlike_activity} <- insert(unlike_data, local), diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index ed801a7ae..5c73fc9f3 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -309,42 +309,43 @@ def handle_user_activity(_, _) do end def update_outbox( - %{assigns: %{user: user}} = conn, + %{assigns: %{user: %User{nickname: user_nickname} = user}} = conn, %{"nickname" => nickname} = params - ) do - if nickname == user.nickname do - actor = user.ap_id() + ) + when user_nickname == nickname do + actor = user.ap_id() - params = - params - |> Map.drop(["id"]) - |> Map.put("actor", actor) - |> Transmogrifier.fix_addressing() - - with {:ok, %Activity{} = activity} <- handle_user_activity(user, params) do - conn - |> put_status(:created) - |> put_resp_header("location", activity.data["id"]) - |> json(activity.data) - else - {:error, message} -> - conn - |> put_status(:bad_request) - |> json(message) - end - else - err = - dgettext("errors", "can't update outbox of %{nickname} as %{as_nickname}", - nickname: nickname, - as_nickname: user.nickname - ) + params = + params + |> Map.drop(["id"]) + |> Map.put("actor", actor) + |> Transmogrifier.fix_addressing() + with {:ok, %Activity{} = activity} <- handle_user_activity(user, params) do conn - |> put_status(:forbidden) - |> json(err) + |> put_status(:created) + |> put_resp_header("location", activity.data["id"]) + |> json(activity.data) + else + {:error, message} -> + conn + |> put_status(:bad_request) + |> json(message) end end + def update_outbox(%{assigns: %{user: user}} = conn, %{"nickname" => nickname} = _) do + err = + dgettext("errors", "can't update outbox of %{nickname} as %{as_nickname}", + nickname: nickname, + as_nickname: user.nickname + ) + + conn + |> put_status(:forbidden) + |> json(err) + end + def errors(conn, {:error, :not_found}) do conn |> put_status(:not_found) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 1c3058658..c9c0c3763 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -166,6 +166,7 @@ def create_context(context) do @doc """ Enqueues an activity for federation if it's local """ + @spec maybe_federate(any()) :: :ok def maybe_federate(%Activity{local: true} = activity) do if Pleroma.Config.get!([:instance, :federating]) do priority = @@ -256,46 +257,27 @@ def insert_full_object(map), do: {:ok, map, nil} @doc """ Returns an existing like if a user already liked an object """ + @spec get_existing_like(String.t(), map()) :: Activity.t() | nil def get_existing_like(actor, %{data: %{"id" => id}}) do - query = - from( - activity in Activity, - where: fragment("(?)->>'actor' = ?", activity.data, ^actor), - # this is to use the index - where: - fragment( - "coalesce((?)->'object'->>'id', (?)->>'object') = ?", - activity.data, - activity.data, - ^id - ), - where: fragment("(?)->>'type' = 'Like'", activity.data) - ) - - Repo.one(query) + actor + |> Activity.Queries.by_actor() + |> Activity.Queries.by_object_id(id) + |> Activity.Queries.by_type("Like") + |> Activity.Queries.limit(1) + |> Repo.one() end @doc """ Returns like activities targeting an object """ def get_object_likes(%{data: %{"id" => id}}) do - query = - from( - activity in Activity, - # this is to use the index - where: - fragment( - "coalesce((?)->'object'->>'id', (?)->>'object') = ?", - activity.data, - activity.data, - ^id - ), - where: fragment("(?)->>'type' = 'Like'", activity.data) - ) - - Repo.all(query) + id + |> Activity.Queries.by_object_id() + |> Activity.Queries.by_type("Like") + |> Repo.all() end + @spec make_like_data(User.t(), map(), String.t()) :: map() def make_like_data( %User{ap_id: ap_id} = actor, %{data: %{"actor" => object_actor_id, "id" => id}} = object, @@ -315,7 +297,7 @@ def make_like_data( |> List.delete(actor.ap_id) |> List.delete(object_actor.follower_address) - data = %{ + %{ "type" => "Like", "actor" => ap_id, "object" => id, @@ -323,38 +305,49 @@ def make_like_data( "cc" => cc, "context" => object.data["context"] } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end + @spec update_element_in_object(String.t(), list(any), Object.t()) :: + {:ok, Object.t()} | {:error, Ecto.Changeset.t()} def update_element_in_object(property, element, object) do - with new_data <- - object.data - |> Map.put("#{property}_count", length(element)) - |> Map.put("#{property}s", element), - changeset <- Changeset.change(object, data: new_data), - {:ok, object} <- Object.update_and_set_cache(changeset) do - {:ok, object} - end + data = + Map.merge( + object.data, + %{"#{property}_count" => length(element), "#{property}s" => element} + ) + + object + |> Changeset.change(data: data) + |> Object.update_and_set_cache() end - def update_likes_in_object(likes, object) do + @spec add_like_to_object(Activity.t(), Object.t()) :: + {:ok, Object.t()} | {:error, Ecto.Changeset.t()} + def add_like_to_object(%Activity{data: %{"actor" => actor}}, object) do + [actor | fetch_likes(object)] + |> Enum.uniq() + |> update_likes_in_object(object) + end + + @spec remove_like_from_object(Activity.t(), Object.t()) :: + {:ok, Object.t()} | {:error, Ecto.Changeset.t()} + def remove_like_from_object(%Activity{data: %{"actor" => actor}}, object) do + object + |> fetch_likes() + |> List.delete(actor) + |> update_likes_in_object(object) + end + + defp update_likes_in_object(likes, object) do update_element_in_object("like", likes, object) end - def add_like_to_object(%Activity{data: %{"actor" => actor}}, object) do - likes = if is_list(object.data["likes"]), do: object.data["likes"], else: [] - - with likes <- [actor | likes] |> Enum.uniq() do - update_likes_in_object(likes, object) - end - end - - def remove_like_from_object(%Activity{data: %{"actor" => actor}}, object) do - likes = if is_list(object.data["likes"]), do: object.data["likes"], else: [] - - with likes <- likes |> List.delete(actor) do - update_likes_in_object(likes, object) + defp fetch_likes(object) do + if is_list(object.data["likes"]) do + object.data["likes"] + else + [] end end @@ -405,7 +398,7 @@ def make_follow_data( %User{ap_id: followed_id} = _followed, activity_id ) do - data = %{ + %{ "type" => "Follow", "actor" => follower_id, "to" => [followed_id], @@ -413,10 +406,7 @@ def make_follow_data( "object" => followed_id, "state" => "pending" } - - data = if activity_id, do: Map.put(data, "id", activity_id), else: data - - data + |> maybe_put("id", activity_id) end def fetch_latest_follow(%User{ap_id: follower_id}, %User{ap_id: followed_id}) do @@ -478,7 +468,7 @@ def make_announce_data( activity_id, false ) do - data = %{ + %{ "type" => "Announce", "actor" => ap_id, "object" => id, @@ -486,8 +476,7 @@ def make_announce_data( "cc" => [], "context" => object.data["context"] } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end def make_announce_data( @@ -496,7 +485,7 @@ def make_announce_data( activity_id, true ) do - data = %{ + %{ "type" => "Announce", "actor" => ap_id, "object" => id, @@ -504,8 +493,7 @@ def make_announce_data( "cc" => [Pleroma.Constants.as_public()], "context" => object.data["context"] } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end @doc """ @@ -516,7 +504,7 @@ def make_unannounce_data( %Activity{data: %{"context" => context}} = activity, activity_id ) do - data = %{ + %{ "type" => "Undo", "actor" => ap_id, "object" => activity.data, @@ -524,8 +512,7 @@ def make_unannounce_data( "cc" => [Pleroma.Constants.as_public()], "context" => context } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end def make_unlike_data( @@ -533,7 +520,7 @@ def make_unlike_data( %Activity{data: %{"context" => context}} = activity, activity_id ) do - data = %{ + %{ "type" => "Undo", "actor" => ap_id, "object" => activity.data, @@ -541,8 +528,7 @@ def make_unlike_data( "cc" => [Pleroma.Constants.as_public()], "context" => context } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end def add_announce_to_object( @@ -573,14 +559,13 @@ def remove_announce_from_object(%Activity{data: %{"actor" => actor}}, object) do #### Unfollow-related helpers def make_unfollow_data(follower, followed, follow_activity, activity_id) do - data = %{ + %{ "type" => "Undo", "actor" => follower.ap_id, "to" => [followed.ap_id], "object" => follow_activity.data } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end #### Block-related helpers @@ -610,25 +595,23 @@ def fetch_latest_block(%User{ap_id: blocker_id}, %User{ap_id: blocked_id}) do end def make_block_data(blocker, blocked, activity_id) do - data = %{ + %{ "type" => "Block", "actor" => blocker.ap_id, "to" => [blocked.ap_id], "object" => blocked.ap_id } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end def make_unblock_data(blocker, blocked, block_activity, activity_id) do - data = %{ + %{ "type" => "Undo", "actor" => blocker.ap_id, "to" => [blocked.ap_id], "object" => block_activity.data } - - if activity_id, do: Map.put(data, "id", activity_id), else: data + |> maybe_put("id", activity_id) end #### Create-related helpers @@ -799,4 +782,7 @@ def get_existing_votes(actor, %{data: %{"id" => id}}) do Repo.all(query) end + + defp maybe_put(map, _key, nil), do: map + defp maybe_put(map, key, value), do: Map.put(map, key, value) end diff --git a/test/support/factory.ex b/test/support/factory.ex index 62d1de717..719115003 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -207,13 +207,15 @@ def like_activity_factory(attrs \\ %{}) do object = Object.normalize(note_activity) user = insert(:user) - data = %{ - "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), - "actor" => user.ap_id, - "type" => "Like", - "object" => object.data["id"], - "published_at" => DateTime.utc_now() |> DateTime.to_iso8601() - } + data = + %{ + "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), + "actor" => user.ap_id, + "type" => "Like", + "object" => object.data["id"], + "published_at" => DateTime.utc_now() |> DateTime.to_iso8601() + } + |> Map.merge(attrs[:data_attrs] || %{}) %Pleroma.Activity{ data: data diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 1515f4eb6..f72b44aed 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -21,6 +21,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do :ok end + clear_config([:instance, :federating]) + describe "streaming out participations" do test "it streams them out" do user = insert(:user) @@ -676,6 +678,29 @@ test "returns reblogs for users for whom reblogs have not been muted" do end describe "like an object" do + test_with_mock "sends an activity to federation", Pleroma.Web.Federator, [:passthrough], [] do + Pleroma.Config.put([:instance, :federating], true) + note_activity = insert(:note_activity) + assert object_activity = Object.normalize(note_activity) + + user = insert(:user) + + {:ok, like_activity, _object} = ActivityPub.like(user, object_activity) + assert called(Pleroma.Web.Federator.publish(like_activity, 5)) + end + + test "returns exist activity if object already liked" do + note_activity = insert(:note_activity) + assert object_activity = Object.normalize(note_activity) + + user = insert(:user) + + {:ok, like_activity, _object} = ActivityPub.like(user, object_activity) + + {:ok, like_activity_exist, _object} = ActivityPub.like(user, object_activity) + assert like_activity == like_activity_exist + end + test "adds a like activity to the db" do note_activity = insert(:note_activity) assert object = Object.normalize(note_activity) @@ -706,6 +731,25 @@ test "adds a like activity to the db" do end describe "unliking" do + test_with_mock "sends an activity to federation", Pleroma.Web.Federator, [:passthrough], [] do + Pleroma.Config.put([:instance, :federating], true) + + note_activity = insert(:note_activity) + object = Object.normalize(note_activity) + user = insert(:user) + + {:ok, object} = ActivityPub.unlike(user, object) + refute called(Pleroma.Web.Federator.publish()) + + {:ok, _like_activity, object} = ActivityPub.like(user, object) + assert object.data["like_count"] == 1 + + {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) + assert object.data["like_count"] == 0 + + assert called(Pleroma.Web.Federator.publish(unlike_activity, 5)) + end + test "unliking a previously liked object" do note_activity = insert(:note_activity) object = Object.normalize(note_activity) diff --git a/test/web/activity_pub/utils_test.exs b/test/web/activity_pub/utils_test.exs index ca5f057a7..eb429b2c4 100644 --- a/test/web/activity_pub/utils_test.exs +++ b/test/web/activity_pub/utils_test.exs @@ -14,6 +14,8 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do import Pleroma.Factory + require Pleroma.Constants + describe "fetch the latest Follow" do test "fetches the latest Follow activity" do %Activity{data: %{"type" => "Follow"}} = activity = insert(:follow_activity) @@ -87,6 +89,32 @@ test "works with an object that has only IR tags" do end end + describe "make_unlike_data/3" do + test "returns data for unlike activity" do + user = insert(:user) + like_activity = insert(:like_activity, data_attrs: %{"context" => "test context"}) + + assert Utils.make_unlike_data(user, like_activity, nil) == %{ + "type" => "Undo", + "actor" => user.ap_id, + "object" => like_activity.data, + "to" => [user.follower_address, like_activity.data["actor"]], + "cc" => [Pleroma.Constants.as_public()], + "context" => like_activity.data["context"] + } + + assert Utils.make_unlike_data(user, like_activity, "9mJEZK0tky1w2xD2vY") == %{ + "type" => "Undo", + "actor" => user.ap_id, + "object" => like_activity.data, + "to" => [user.follower_address, like_activity.data["actor"]], + "cc" => [Pleroma.Constants.as_public()], + "context" => like_activity.data["context"], + "id" => "9mJEZK0tky1w2xD2vY" + } + end + end + describe "make_like_data" do setup do user = insert(:user) @@ -299,4 +327,78 @@ test "updates the state of the given follow activity" do assert Repo.get(Activity, follow_activity_two.id).data["state"] == "reject" end end + + describe "update_element_in_object/3" do + test "updates likes" do + user = insert(:user) + activity = insert(:note_activity) + object = Object.normalize(activity) + + assert {:ok, updated_object} = + Utils.update_element_in_object( + "like", + [user.ap_id], + object + ) + + assert updated_object.data["likes"] == [user.ap_id] + assert updated_object.data["like_count"] == 1 + end + end + + describe "add_like_to_object/2" do + test "add actor to likes" do + user = insert(:user) + user2 = insert(:user) + object = insert(:note) + + assert {:ok, updated_object} = + Utils.add_like_to_object( + %Activity{data: %{"actor" => user.ap_id}}, + object + ) + + assert updated_object.data["likes"] == [user.ap_id] + assert updated_object.data["like_count"] == 1 + + assert {:ok, updated_object2} = + Utils.add_like_to_object( + %Activity{data: %{"actor" => user2.ap_id}}, + updated_object + ) + + assert updated_object2.data["likes"] == [user2.ap_id, user.ap_id] + assert updated_object2.data["like_count"] == 2 + end + end + + describe "remove_like_from_object/2" do + test "removes ap_id from likes" do + user = insert(:user) + user2 = insert(:user) + object = insert(:note, data: %{"likes" => [user.ap_id, user2.ap_id], "like_count" => 2}) + + assert {:ok, updated_object} = + Utils.remove_like_from_object( + %Activity{data: %{"actor" => user.ap_id}}, + object + ) + + assert updated_object.data["likes"] == [user2.ap_id] + assert updated_object.data["like_count"] == 1 + end + end + + describe "get_existing_like/2" do + test "fetches existing like" do + note_activity = insert(:note_activity) + assert object = Object.normalize(note_activity) + + user = insert(:user) + refute Utils.get_existing_like(user.ap_id, object) + {:ok, like_activity, _object} = ActivityPub.like(user, object) + + assert ^like_activity = Utils.get_existing_like(user.ap_id, object) + end + end end From c30cc039e423e8f31d0222747e301514b7d0dd9e Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 27 Aug 2019 12:22:30 -0500 Subject: [PATCH 08/18] Transmogrifier: Use Containment.get_actor to get actors. --- lib/pleroma/object.ex | 4 ---- lib/pleroma/web/activity_pub/transmogrifier.ex | 6 ++++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 468549c87..c8d339c19 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -230,8 +230,4 @@ def increase_vote_count(ap_id, name) do _ -> :noop end end - - def get_ap_id(%{"id" => id}), do: id - def get_ap_id(id) when is_binary(id), do: id - def get_ap_id(_), do: {:error, "Object is not a string and has no id."} end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 6c4259c02..468961bd0 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -464,8 +464,10 @@ def handle_incoming( %{"type" => "Follow", "object" => followed, "actor" => follower, "id" => id} = data, _options ) do - with %User{local: true} = followed <- User.get_cached_by_ap_id(Object.get_ap_id(followed)), - {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(Object.get_ap_id(follower)), + with %User{local: true} = followed <- + User.get_cached_by_ap_id(Containment.get_actor(%{"actor" => followed})), + {:ok, %User{} = follower} <- + User.get_or_fetch_by_ap_id(Containment.get_actor(%{"actor" => follower})), {:ok, activity} <- ActivityPub.follow(follower, followed, id, false) do with deny_follow_blocked <- Pleroma.Config.get([:user, :deny_follow_blocked]), {_, false} <- {:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked}, From ffcd742aa0797b5bb872e58c1e605f22c8652250 Mon Sep 17 00:00:00 2001 From: Maksim Date: Tue, 27 Aug 2019 17:37:19 +0000 Subject: [PATCH 09/18] Apply suggestion to lib/pleroma/web/activity_pub/activity_pub_controller.ex --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 5c73fc9f3..08bf1c752 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -309,10 +309,9 @@ def handle_user_activity(_, _) do end def update_outbox( - %{assigns: %{user: %User{nickname: user_nickname} = user}} = conn, + %{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{"nickname" => nickname} = params - ) - when user_nickname == nickname do + ) do actor = user.ap_id() params = From 7853b3f17d3b57d7ac91bc909a57143674f57272 Mon Sep 17 00:00:00 2001 From: feld Date: Fri, 30 Aug 2019 00:38:03 +0000 Subject: [PATCH 10/18] Fix AntiFollowbotPolicy when trying to follow a relay --- CHANGELOG.md | 1 + .../web/activity_pub/mrf/anti_followbot_policy.ex | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20af9badc..4acb749ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Reverse Proxy limiting `max_body_length` was incorrectly defined and only checked `Content-Length` headers which may not be sufficient in some circumstances - MRF: fix use of unserializable keyword lists in describe() implementations - ActivityPub: Deactivated user deletion +- MRF: fix ability to follow a relay when AntiFollowbotPolicy was enabled ### Added - Expiring/ephemeral activites. All activities can have expires_at value set, which controls when they should be deleted automatically. diff --git a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex index de1eb4aa5..b3547ecd4 100644 --- a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex @@ -25,11 +25,15 @@ defp score_displayname("fedibot"), do: 1.0 defp score_displayname(_), do: 0.0 defp determine_if_followbot(%User{nickname: nickname, name: displayname}) do - # nickname will always be a binary string because it's generated by Pleroma. + # nickname will be a binary string except when following a relay nick_score = - nickname - |> String.downcase() - |> score_nickname() + if is_binary(nickname) do + nickname + |> String.downcase() + |> score_nickname() + else + 0.0 + end # displayname will either be a binary string or nil, if a displayname isn't set. name_score = From 99b4847da3244a0d023ae25b2669afb07a4eda4f Mon Sep 17 00:00:00 2001 From: kPherox Date: Fri, 30 Aug 2019 21:00:50 +0900 Subject: [PATCH 11/18] Fix missing changes in pleroma/pleroma!1197 --- installation/pleroma.nginx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/installation/pleroma.nginx b/installation/pleroma.nginx index e3c70de54..4da9918ca 100644 --- a/installation/pleroma.nginx +++ b/installation/pleroma.nginx @@ -71,26 +71,26 @@ server { proxy_set_header Connection "upgrade"; proxy_set_header Host $http_host; - # this is explicitly IPv4 since Pleroma.Web.Endpoint binds on IPv4 only - # and `localhost.` resolves to [::0] on some systems: see issue #930 + # this is explicitly IPv4 since Pleroma.Web.Endpoint binds on IPv4 only + # and `localhost.` resolves to [::0] on some systems: see issue #930 proxy_pass http://127.0.0.1:4000; client_max_body_size 16m; } location ~ ^/(media|proxy) { - proxy_cache pleroma_media_cache; + proxy_cache pleroma_media_cache; slice 1m; proxy_cache_key $host$uri$is_args$args$slice_range; proxy_set_header Range $slice_range; proxy_http_version 1.1; proxy_cache_valid 200 206 301 304 1h; - proxy_cache_lock on; + proxy_cache_lock on; proxy_ignore_client_abort on; - proxy_buffering on; + proxy_buffering on; chunked_transfer_encoding on; proxy_ignore_headers Cache-Control; - proxy_hide_header Cache-Control; - proxy_pass http://localhost:4000; + proxy_hide_header Cache-Control; + proxy_pass http://127.0.0.1:4000; } } From a4c5f71e933c905433b80c90bcd626e7da703669 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Mon, 2 Sep 2019 22:48:52 +0300 Subject: [PATCH 12/18] Return total from pagination + tests --- CHANGELOG.md | 1 + lib/pleroma/activity/search.ex | 1 + lib/pleroma/conversation/participation.ex | 1 + lib/pleroma/notification.ex | 1 + lib/pleroma/pagination.ex | 24 ++++-- lib/pleroma/user/search.ex | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 3 + .../controllers/mastodon_api_controller.ex | 2 + lib/pleroma/web/mastodon_api/mastodon_api.ex | 4 + test/pagination_test.exs | 78 +++++++++++++++++++ 10 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 test/pagination_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4acb749ac..06ad303de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Unsubscribe followers when they unfollow a user - AdminAPI: Add "godmode" while fetching user statuses (i.e. admin can see private statuses) - Improve digest email template +– Pagination: return `total` alongside with `items` when paginating ### Fixed - Following from Osada diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex index f847ac238..f7156c81c 100644 --- a/lib/pleroma/activity/search.ex +++ b/lib/pleroma/activity/search.ex @@ -27,6 +27,7 @@ def search(user, search_query, options \\ []) do |> maybe_restrict_local(user) |> maybe_restrict_author(author) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => limit}, :offset) + |> Map.get(:items) |> maybe_fetch(user, search_query) end diff --git a/lib/pleroma/conversation/participation.ex b/lib/pleroma/conversation/participation.ex index ea5b9fe17..5fd8d3d41 100644 --- a/lib/pleroma/conversation/participation.ex +++ b/lib/pleroma/conversation/participation.ex @@ -67,6 +67,7 @@ def for_user(user, params \\ %{}) do preload: [conversation: [:users]] ) |> Pleroma.Pagination.fetch_paginated(params) + |> Map.get(:items) end def for_user_and_conversation(user, conversation) do diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 5d29af853..3e4ddd2ba 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -75,6 +75,7 @@ def for_user(user, opts \\ %{}) do user |> for_user_query(opts) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) end @doc """ diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 2b869ccdc..d21ecf628 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -18,19 +18,29 @@ def fetch_paginated(query, params, type \\ :keyset) def fetch_paginated(query, params, :keyset) do options = cast_params(params) + total = Repo.aggregate(query, :count, :id) - query - |> paginate(options, :keyset) - |> Repo.all() - |> enforce_order(options) + %{ + total: total, + items: + query + |> paginate(options, :keyset) + |> Repo.all() + |> enforce_order(options) + } end def fetch_paginated(query, params, :offset) do options = cast_params(params) + total = Repo.aggregate(query, :count, :id) - query - |> paginate(options, :offset) - |> Repo.all() + %{ + total: total, + items: + query + |> paginate(options, :offset) + |> Repo.all() + } end def paginate(query, options, method \\ :keyset) diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index 6fb2c2352..bc05639b5 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -34,6 +34,7 @@ def search(query_string, opts \\ []) do query_string |> search_query(for_user, following) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset) + |> Map.get(:items) end) results diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index eeb826814..8f07638cd 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -556,6 +556,7 @@ def fetch_public_activities(opts \\ %{}) do q |> restrict_unlisted() |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() end @@ -953,6 +954,7 @@ def fetch_activities(recipients, opts \\ %{}) do fetch_activities_query(recipients ++ list_memberships, opts) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() |> maybe_update_cc(list_memberships, opts["user"]) end @@ -987,6 +989,7 @@ def fetch_activities_bounded(recipients, recipients_with_public, opts \\ %{}) do fetch_activities_query([], opts) |> fetch_activities_bounded_query(recipients, recipients_with_public) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() end diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex index 83e877c0e..d532ba685 100644 --- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex @@ -420,6 +420,7 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do [user.ap_id] |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) + |> Map.get(:items) conn |> add_link_headers(:dm_timeline, activities) @@ -1194,6 +1195,7 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do bookmarks = Bookmark.for_user_query(user.id) |> Pagination.fetch_paginated(params) + |> Map.get(:items) activities = bookmarks diff --git a/lib/pleroma/web/mastodon_api/mastodon_api.ex b/lib/pleroma/web/mastodon_api/mastodon_api.ex index ac01d1ff3..cf3962944 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api.ex @@ -45,12 +45,14 @@ def get_followers(user, params \\ %{}) do user |> User.get_followers_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_friends(user, params \\ %{}) do user |> User.get_friends_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_notifications(user, params \\ %{}) do @@ -60,12 +62,14 @@ def get_notifications(user, params \\ %{}) do |> Notification.for_user_query(options) |> restrict(:exclude_types, options) |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_scheduled_activities(user, params \\ %{}) do user |> ScheduledActivity.for_user_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end defp cast_params(params) do diff --git a/test/pagination_test.exs b/test/pagination_test.exs new file mode 100644 index 000000000..048ab6f3c --- /dev/null +++ b/test/pagination_test.exs @@ -0,0 +1,78 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.PaginationTest do + use Pleroma.DataCase + + import Pleroma.Factory + + alias Pleroma.Object + alias Pleroma.Pagination + + describe "keyset" do + setup do + notes = insert_list(5, :note) + + %{notes: notes} + end + + test "paginates by min_id", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"min_id" => id}) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by since_id", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"since_id" => id}) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by max_id", %{notes: notes} do + id = Enum.at(notes, 1).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"max_id" => id}) + + assert length(paginated) == 1 + assert total == 5 + end + + test "paginates by min_id & limit", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"min_id" => id, "limit" => 1}) + + assert length(paginated) == 1 + assert total == 5 + end + end + + describe "offset" do + setup do + notes = insert_list(5, :note) + + %{notes: notes} + end + + test "paginates by limit" do + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"limit" => 2}, :offset) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by limit & offset" do + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"limit" => 2, "offset" => 4}, :offset) + + assert length(paginated) == 1 + assert total == 5 + end + end +end From b15cfd80ef5d5bc971f78a53dfa3d37dec4499a5 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Tue, 3 Sep 2019 13:58:27 +0300 Subject: [PATCH 13/18] Return "total" optionally --- CHANGELOG.md | 2 +- lib/pleroma/activity/search.ex | 1 - lib/pleroma/conversation/participation.ex | 1 - lib/pleroma/notification.ex | 1 - lib/pleroma/pagination.ex | 38 +++++++++++-------- lib/pleroma/user/search.ex | 1 - lib/pleroma/web/activity_pub/activity_pub.ex | 3 -- .../controllers/mastodon_api_controller.ex | 2 - lib/pleroma/web/mastodon_api/mastodon_api.ex | 4 -- test/pagination_test.exs | 24 ++++++------ 10 files changed, 36 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06ad303de..8264688d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Unsubscribe followers when they unfollow a user - AdminAPI: Add "godmode" while fetching user statuses (i.e. admin can see private statuses) - Improve digest email template -– Pagination: return `total` alongside with `items` when paginating +– Pagination: (optional) return `total` alongside with `items` when paginating ### Fixed - Following from Osada diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex index f7156c81c..f847ac238 100644 --- a/lib/pleroma/activity/search.ex +++ b/lib/pleroma/activity/search.ex @@ -27,7 +27,6 @@ def search(user, search_query, options \\ []) do |> maybe_restrict_local(user) |> maybe_restrict_author(author) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => limit}, :offset) - |> Map.get(:items) |> maybe_fetch(user, search_query) end diff --git a/lib/pleroma/conversation/participation.ex b/lib/pleroma/conversation/participation.ex index 5fd8d3d41..ea5b9fe17 100644 --- a/lib/pleroma/conversation/participation.ex +++ b/lib/pleroma/conversation/participation.ex @@ -67,7 +67,6 @@ def for_user(user, params \\ %{}) do preload: [conversation: [:users]] ) |> Pleroma.Pagination.fetch_paginated(params) - |> Map.get(:items) end def for_user_and_conversation(user, conversation) do diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 3e4ddd2ba..5d29af853 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -75,7 +75,6 @@ def for_user(user, opts \\ %{}) do user |> for_user_query(opts) |> Pagination.fetch_paginated(opts) - |> Map.get(:items) end @doc """ diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index d21ecf628..b55379c4a 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -16,31 +16,39 @@ defmodule Pleroma.Pagination do def fetch_paginated(query, params, type \\ :keyset) - def fetch_paginated(query, params, :keyset) do - options = cast_params(params) + def fetch_paginated(query, %{"total" => true} = params, :keyset) do total = Repo.aggregate(query, :count, :id) %{ total: total, - items: - query - |> paginate(options, :keyset) - |> Repo.all() - |> enforce_order(options) + items: fetch_paginated(query, Map.drop(params, ["total"]), :keyset) + } + end + + def fetch_paginated(query, params, :keyset) do + options = cast_params(params) + + query + |> paginate(options, :keyset) + |> Repo.all() + |> enforce_order(options) + end + + def fetch_paginated(query, %{"total" => true} = params, :offset) do + total = Repo.aggregate(query, :count, :id) + + %{ + total: total, + items: fetch_paginated(query, Map.drop(params, ["total"]), :offset) } end def fetch_paginated(query, params, :offset) do options = cast_params(params) - total = Repo.aggregate(query, :count, :id) - %{ - total: total, - items: - query - |> paginate(options, :offset) - |> Repo.all() - } + query + |> paginate(options, :offset) + |> Repo.all() end def paginate(query, options, method \\ :keyset) diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index bc05639b5..6fb2c2352 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -34,7 +34,6 @@ def search(query_string, opts \\ []) do query_string |> search_query(for_user, following) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset) - |> Map.get(:items) end) results diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 8f07638cd..eeb826814 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -556,7 +556,6 @@ def fetch_public_activities(opts \\ %{}) do q |> restrict_unlisted() |> Pagination.fetch_paginated(opts) - |> Map.get(:items) |> Enum.reverse() end @@ -954,7 +953,6 @@ def fetch_activities(recipients, opts \\ %{}) do fetch_activities_query(recipients ++ list_memberships, opts) |> Pagination.fetch_paginated(opts) - |> Map.get(:items) |> Enum.reverse() |> maybe_update_cc(list_memberships, opts["user"]) end @@ -989,7 +987,6 @@ def fetch_activities_bounded(recipients, recipients_with_public, opts \\ %{}) do fetch_activities_query([], opts) |> fetch_activities_bounded_query(recipients, recipients_with_public) |> Pagination.fetch_paginated(opts) - |> Map.get(:items) |> Enum.reverse() end diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex index d532ba685..83e877c0e 100644 --- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex @@ -420,7 +420,6 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do [user.ap_id] |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) - |> Map.get(:items) conn |> add_link_headers(:dm_timeline, activities) @@ -1195,7 +1194,6 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do bookmarks = Bookmark.for_user_query(user.id) |> Pagination.fetch_paginated(params) - |> Map.get(:items) activities = bookmarks diff --git a/lib/pleroma/web/mastodon_api/mastodon_api.ex b/lib/pleroma/web/mastodon_api/mastodon_api.ex index cf3962944..ac01d1ff3 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api.ex @@ -45,14 +45,12 @@ def get_followers(user, params \\ %{}) do user |> User.get_followers_query() |> Pagination.fetch_paginated(params) - |> Map.get(:items) end def get_friends(user, params \\ %{}) do user |> User.get_friends_query() |> Pagination.fetch_paginated(params) - |> Map.get(:items) end def get_notifications(user, params \\ %{}) do @@ -62,14 +60,12 @@ def get_notifications(user, params \\ %{}) do |> Notification.for_user_query(options) |> restrict(:exclude_types, options) |> Pagination.fetch_paginated(params) - |> Map.get(:items) end def get_scheduled_activities(user, params \\ %{}) do user |> ScheduledActivity.for_user_query() |> Pagination.fetch_paginated(params) - |> Map.get(:items) end defp cast_params(params) do diff --git a/test/pagination_test.exs b/test/pagination_test.exs index 048ab6f3c..c0fbe7933 100644 --- a/test/pagination_test.exs +++ b/test/pagination_test.exs @@ -19,7 +19,9 @@ defmodule Pleroma.PaginationTest do test "paginates by min_id", %{notes: notes} do id = Enum.at(notes, 2).id |> Integer.to_string() - %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"min_id" => id}) + + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"min_id" => id, "total" => true}) assert length(paginated) == 2 assert total == 5 @@ -27,7 +29,9 @@ test "paginates by min_id", %{notes: notes} do test "paginates by since_id", %{notes: notes} do id = Enum.at(notes, 2).id |> Integer.to_string() - %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"since_id" => id}) + + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"since_id" => id, "total" => true}) assert length(paginated) == 2 assert total == 5 @@ -35,7 +39,9 @@ test "paginates by since_id", %{notes: notes} do test "paginates by max_id", %{notes: notes} do id = Enum.at(notes, 1).id |> Integer.to_string() - %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"max_id" => id}) + + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"max_id" => id, "total" => true}) assert length(paginated) == 1 assert total == 5 @@ -44,11 +50,9 @@ test "paginates by max_id", %{notes: notes} do test "paginates by min_id & limit", %{notes: notes} do id = Enum.at(notes, 2).id |> Integer.to_string() - %{total: total, items: paginated} = - Pagination.fetch_paginated(Object, %{"min_id" => id, "limit" => 1}) + paginated = Pagination.fetch_paginated(Object, %{"min_id" => id, "limit" => 1}) assert length(paginated) == 1 - assert total == 5 end end @@ -60,19 +64,15 @@ test "paginates by min_id & limit", %{notes: notes} do end test "paginates by limit" do - %{total: total, items: paginated} = - Pagination.fetch_paginated(Object, %{"limit" => 2}, :offset) + paginated = Pagination.fetch_paginated(Object, %{"limit" => 2}, :offset) assert length(paginated) == 2 - assert total == 5 end test "paginates by limit & offset" do - %{total: total, items: paginated} = - Pagination.fetch_paginated(Object, %{"limit" => 2, "offset" => 4}, :offset) + paginated = Pagination.fetch_paginated(Object, %{"limit" => 2, "offset" => 4}, :offset) assert length(paginated) == 1 - assert total == 5 end end end From c2b6c1b089a813cf8c7cbd54c0f80bee4985522c Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 4 Sep 2019 11:33:08 +0300 Subject: [PATCH 14/18] Extend `/api/pleroma/notifications/read` to mark multiple notifications as read and make it respond with Mastoapi entities --- CHANGELOG.md | 1 + docs/api/pleroma_api.md | 11 ++-- lib/pleroma/notification.ex | 21 ++++++- .../web/pleroma_api/pleroma_api_controller.ex | 25 +++++++++ lib/pleroma/web/router.ex | 7 +-- .../pleroma_api_controller_test.exs | 56 +++++++++++++++++++ test/web/twitter_api/util_controller_test.exs | 32 ----------- 7 files changed, 108 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8264688d6..40f4580f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - **Breaking:** Configuration: A setting to explicitly disable the mailer was added, defaulting to true, if you are using a mailer add `config :pleroma, Pleroma.Emails.Mailer, enabled: true` to your config - **Breaking:** Configuration: `/media/` is now removed when `base_url` is configured, append `/media/` to your `base_url` config to keep the old behaviour if desired +- **Breaking:** `/api/pleroma/notifications/read` is moved to `/api/v1/pleroma/notifications/read` and now supports `max_id` and responds with Mastodon API entities. - Configuration: OpenGraph and TwitterCard providers enabled by default - Configuration: Filter.AnonymizeFilename added ability to retain file extension with custom text - Federation: Return 403 errors when trying to request pages from a user's follower/following collections if they have `hide_followers`/`hide_follows` set diff --git a/docs/api/pleroma_api.md b/docs/api/pleroma_api.md index b134b31a8..e76a35b3b 100644 --- a/docs/api/pleroma_api.md +++ b/docs/api/pleroma_api.md @@ -126,13 +126,14 @@ Request parameters can be passed via [query strings](https://en.wikipedia.org/wi ## `/api/pleroma/admin/`… See [Admin-API](Admin-API.md) -## `/api/pleroma/notifications/read` -### Mark a single notification as read +## `/api/pleroma/v1/notifications/read` +### Mark notifications as read * Method `POST` * Authentication: required -* Params: - * `id`: notification's id -* Response: JSON. Returns `{"status": "success"}` if the reading was successful, otherwise returns `{"error": "error_msg"}` +* Params (mutually exclusive): + * `id`: a single notification id to read + * `max_id`: read all notifications up to this id +* Response: Notification entity/Array of Notification entities. In case of `max_id`, only the first 80 notifications will be returned. ## `/api/v1/pleroma/accounts/:id/subscribe` ### Subscribe to receive notifications for all statuses posted by a user diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 5d29af853..d7e232992 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -102,15 +102,32 @@ def set_read_up_to(%{id: user_id} = _user, id) do n in Notification, where: n.user_id == ^user_id, where: n.id <= ^id, + where: n.seen == false, update: [ set: [ seen: true, updated_at: ^NaiveDateTime.utc_now() ] - ] + ], + # Ideally we would preload object and activities here + # but Ecto does not support preloads in update_all + select: n.id ) - Repo.update_all(query, []) + {_, notification_ids} = Repo.update_all(query, []) + + from(n in Notification, where: n.id in ^notification_ids) + |> join(:inner, [n], activity in assoc(n, :activity)) + |> join(:left, [n, a], object in Object, + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + object.data, + a.data + ) + ) + |> preload([n, a, o], activity: {a, object: o}) + |> Repo.all() end def read_one(%User{} = user, notification_id) do diff --git a/lib/pleroma/web/pleroma_api/pleroma_api_controller.ex b/lib/pleroma/web/pleroma_api/pleroma_api_controller.ex index b6d2bf86b..f4df3b024 100644 --- a/lib/pleroma/web/pleroma_api/pleroma_api_controller.ex +++ b/lib/pleroma/web/pleroma_api/pleroma_api_controller.ex @@ -8,8 +8,10 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIController do import Pleroma.Web.ControllerHelper, only: [add_link_headers: 7] alias Pleroma.Conversation.Participation + alias Pleroma.Notification alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.MastodonAPI.ConversationView + alias Pleroma.Web.MastodonAPI.NotificationView alias Pleroma.Web.MastodonAPI.StatusView def conversation(%{assigns: %{user: user}} = conn, %{"id" => participation_id}) do @@ -70,4 +72,27 @@ def update_conversation( |> render("participation.json", %{participation: participation, for: user}) end end + + def read_notification(%{assigns: %{user: user}} = conn, %{"id" => notification_id}) do + with {:ok, notification} <- Notification.read_one(user, notification_id) do + conn + |> put_view(NotificationView) + |> render("show.json", %{notification: notification, for: user}) + else + {:error, message} -> + conn + |> put_status(:bad_request) + |> json(%{"error" => message}) + end + end + + def read_notification(%{assigns: %{user: user}} = conn, %{"max_id" => max_id}) do + with notifications <- Notification.set_read_up_to(user, max_id) do + notifications = Enum.take(notifications, 80) + + conn + |> put_view(NotificationView) + |> render("index.json", %{notifications: notifications, for: user}) + end + end end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 969dc66fd..44a4279f7 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -236,12 +236,6 @@ defmodule Pleroma.Web.Router do post("/blocks_import", UtilController, :blocks_import) post("/follow_import", UtilController, :follow_import) end - - scope [] do - pipe_through(:oauth_read) - - post("/notifications/read", UtilController, :notifications_read) - end end scope "/oauth", Pleroma.Web.OAuth do @@ -277,6 +271,7 @@ defmodule Pleroma.Web.Router do scope [] do pipe_through(:oauth_write) patch("/conversations/:id", PleromaAPIController, :update_conversation) + post("/notifications/read", PleromaAPIController, :read_notification) end end diff --git a/test/web/pleroma_api/pleroma_api_controller_test.exs b/test/web/pleroma_api/pleroma_api_controller_test.exs index ed6b79727..7eaeda4a0 100644 --- a/test/web/pleroma_api/pleroma_api_controller_test.exs +++ b/test/web/pleroma_api/pleroma_api_controller_test.exs @@ -6,6 +6,7 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIControllerTest do use Pleroma.Web.ConnCase alias Pleroma.Conversation.Participation + alias Pleroma.Notification alias Pleroma.Repo alias Pleroma.Web.CommonAPI @@ -91,4 +92,59 @@ test "PATCH /api/v1/pleroma/conversations/:id", %{conn: conn} do assert user in participation.recipients assert other_user in participation.recipients end + + describe "POST /api/v1/pleroma/notifications/read" do + test "it marks a single notification as read", %{conn: conn} do + user1 = insert(:user) + user2 = insert(:user) + {:ok, activity1} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) + {:ok, activity2} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) + {:ok, [notification1]} = Notification.create_notifications(activity1) + {:ok, [notification2]} = Notification.create_notifications(activity2) + + response = + conn + |> assign(:user, user1) + |> post("/api/v1/pleroma/notifications/read", %{"id" => "#{notification1.id}"}) + |> json_response(:ok) + + assert %{"pleroma" => %{"is_seen" => true}} = response + assert Repo.get(Notification, notification1.id).seen + refute Repo.get(Notification, notification2.id).seen + end + + test "it marks multiple notifications as read", %{conn: conn} do + user1 = insert(:user) + user2 = insert(:user) + {:ok, _activity1} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) + {:ok, _activity2} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) + {:ok, _activity3} = CommonAPI.post(user2, %{"status" => "HIE @#{user1.nickname}"}) + + [notification3, notification2, notification1] = Notification.for_user(user1, %{limit: 3}) + + [response1, response2] = + conn + |> assign(:user, user1) + |> post("/api/v1/pleroma/notifications/read", %{"max_id" => "#{notification2.id}"}) + |> json_response(:ok) + + assert %{"pleroma" => %{"is_seen" => true}} = response1 + assert %{"pleroma" => %{"is_seen" => true}} = response2 + assert Repo.get(Notification, notification1.id).seen + assert Repo.get(Notification, notification2.id).seen + refute Repo.get(Notification, notification3.id).seen + end + + test "it returns error when notification not found", %{conn: conn} do + user1 = insert(:user) + + response = + conn + |> assign(:user, user1) + |> post("/api/v1/pleroma/notifications/read", %{"id" => "22222222222222"}) + |> json_response(:bad_request) + + assert response == %{"error" => "Cannot get notification"} + end + end end diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index fe4ffdb59..cf8e69d2b 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -5,7 +5,6 @@ defmodule Pleroma.Web.TwitterAPI.UtilControllerTest do use Pleroma.Web.ConnCase - alias Pleroma.Notification alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web.CommonAPI @@ -141,37 +140,6 @@ test "it imports blocks users from file", %{conn: conn} do end end - describe "POST /api/pleroma/notifications/read" do - test "it marks a single notification as read", %{conn: conn} do - user1 = insert(:user) - user2 = insert(:user) - {:ok, activity1} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) - {:ok, activity2} = CommonAPI.post(user2, %{"status" => "hi @#{user1.nickname}"}) - {:ok, [notification1]} = Notification.create_notifications(activity1) - {:ok, [notification2]} = Notification.create_notifications(activity2) - - conn - |> assign(:user, user1) - |> post("/api/pleroma/notifications/read", %{"id" => "#{notification1.id}"}) - |> json_response(:ok) - - assert Repo.get(Notification, notification1.id).seen - refute Repo.get(Notification, notification2.id).seen - end - - test "it returns error when notification not found", %{conn: conn} do - user1 = insert(:user) - - response = - conn - |> assign(:user, user1) - |> post("/api/pleroma/notifications/read", %{"id" => "22222222222222"}) - |> json_response(403) - - assert response == %{"error" => "Cannot get notification"} - end - end - describe "PUT /api/pleroma/notification_settings" do test "it updates notification settings", %{conn: conn} do user = insert(:user) From 7c3838090f86fbfdbf4e45fcfbabc21c19f26924 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 4 Sep 2019 10:14:15 +0000 Subject: [PATCH 15/18] Apply suggestion to lib/pleroma/notification.ex --- lib/pleroma/notification.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index d7e232992..b7c880c51 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -116,7 +116,8 @@ def set_read_up_to(%{id: user_id} = _user, id) do {_, notification_ids} = Repo.update_all(query, []) - from(n in Notification, where: n.id in ^notification_ids) + Notification + |> where([n], n.id in ^notification_ids) |> join(:inner, [n], activity in assoc(n, :activity)) |> join(:left, [n, a], object in Object, on: From 377aa9fb90ff1c8537112f23bfc329f1ac0696b4 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 4 Sep 2019 10:37:43 +0000 Subject: [PATCH 16/18] Apply suggestion to docs/api/pleroma_api.md --- docs/api/pleroma_api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/pleroma_api.md b/docs/api/pleroma_api.md index e76a35b3b..c08ee9ecd 100644 --- a/docs/api/pleroma_api.md +++ b/docs/api/pleroma_api.md @@ -126,7 +126,7 @@ Request parameters can be passed via [query strings](https://en.wikipedia.org/wi ## `/api/pleroma/admin/`… See [Admin-API](Admin-API.md) -## `/api/pleroma/v1/notifications/read` +## `/api/v1/pleroma/notifications/read` ### Mark notifications as read * Method `POST` * Authentication: required From 328b2612cd957aa3ad810101a20037e4e9843bb0 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 4 Sep 2019 13:39:39 +0300 Subject: [PATCH 17/18] Clarify that read notifications are returned --- docs/api/pleroma_api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/pleroma_api.md b/docs/api/pleroma_api.md index c08ee9ecd..7d343e97a 100644 --- a/docs/api/pleroma_api.md +++ b/docs/api/pleroma_api.md @@ -133,7 +133,7 @@ See [Admin-API](Admin-API.md) * Params (mutually exclusive): * `id`: a single notification id to read * `max_id`: read all notifications up to this id -* Response: Notification entity/Array of Notification entities. In case of `max_id`, only the first 80 notifications will be returned. +* Response: Notification entity/Array of Notification entities that were read. In case of `max_id`, only the first 80 read notifications will be returned. ## `/api/v1/pleroma/accounts/:id/subscribe` ### Subscribe to receive notifications for all statuses posted by a user From 3face454671bfdf2b850daf9dcb05468eb909e95 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 4 Sep 2019 14:16:56 +0300 Subject: [PATCH 18/18] Mastodon API: Add `pleroma.thread_muted` to Status entity Needed for pleroma-fe!941 --- CHANGELOG.md | 1 + docs/api/differences_in_mastoapi_responses.md | 1 + .../web/mastodon_api/views/status_view.ex | 3 ++- .../mastodon_api/views/status_view_test.exs | 21 ++++++++++++++++++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40f4580f7..a414ba5e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - **Breaking:** `/api/pleroma/notifications/read` is moved to `/api/v1/pleroma/notifications/read` and now supports `max_id` and responds with Mastodon API entities. - Configuration: OpenGraph and TwitterCard providers enabled by default - Configuration: Filter.AnonymizeFilename added ability to retain file extension with custom text +- Mastodon API: `pleroma.thread_muted` key in the Status entity - Federation: Return 403 errors when trying to request pages from a user's follower/following collections if they have `hide_followers`/`hide_follows` set - NodeInfo: Return `skipThreadContainment` in `metadata` for the `skip_thread_containment` option - NodeInfo: Return `mailerEnabled` in `metadata` diff --git a/docs/api/differences_in_mastoapi_responses.md b/docs/api/differences_in_mastoapi_responses.md index f34e3dd72..02f90f3e8 100644 --- a/docs/api/differences_in_mastoapi_responses.md +++ b/docs/api/differences_in_mastoapi_responses.md @@ -26,6 +26,7 @@ Has these additional fields under the `pleroma` object: - `content`: a map consisting of alternate representations of the `content` property with the key being it's mimetype. Currently the only alternate representation supported is `text/plain` - `spoiler_text`: a map consisting of alternate representations of the `spoiler_text` property with the key being it's mimetype. Currently the only alternate representation supported is `text/plain` - `expires_at`: a datetime (iso8601) that states when the post will expire (be deleted automatically), or empty if the post won't expire +- `thread_muted`: true if the thread the post belongs to is muted ## Attachments diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index a4ee0b5dd..4c3c8c564 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -299,7 +299,8 @@ def render("status.json", %{activity: %{data: %{"object" => _object}} = activity content: %{"text/plain" => content_plaintext}, spoiler_text: %{"text/plain" => summary_plaintext}, expires_at: expires_at, - direct_conversation_id: direct_conversation_id + direct_conversation_id: direct_conversation_id, + thread_muted: thread_muted? } } end diff --git a/test/web/mastodon_api/views/status_view_test.exs b/test/web/mastodon_api/views/status_view_test.exs index 1b6beb6d2..90451cbdc 100644 --- a/test/web/mastodon_api/views/status_view_test.exs +++ b/test/web/mastodon_api/views/status_view_test.exs @@ -150,7 +150,8 @@ test "a note activity" do content: %{"text/plain" => HtmlSanitizeEx.strip_tags(object_data["content"])}, spoiler_text: %{"text/plain" => HtmlSanitizeEx.strip_tags(object_data["summary"])}, expires_at: nil, - direct_conversation_id: nil + direct_conversation_id: nil, + thread_muted: false } } @@ -173,6 +174,24 @@ test "tells if the message is muted for some reason" do assert status.muted == true end + test "tells if the message is thread muted" do + user = insert(:user) + other_user = insert(:user) + + {:ok, user} = User.mute(user, other_user) + + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "test"}) + status = StatusView.render("status.json", %{activity: activity, for: user}) + + assert status.pleroma.thread_muted == false + + {:ok, activity} = CommonAPI.add_mute(user, activity) + + status = StatusView.render("status.json", %{activity: activity, for: user}) + + assert status.pleroma.thread_muted == true + end + test "tells if the status is bookmarked" do user = insert(:user)