From 9b6c7843d669c80bd062214e598d0f5d9738de8e Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Tue, 3 Mar 2020 04:58:12 +0100 Subject: [PATCH 01/34] debian_based_*.md: Use erlang-nox metapackage --- docs/installation/debian_based_en.md | 10 +++------- docs/installation/debian_based_jp.md | 22 +++++++++------------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/docs/installation/debian_based_en.md b/docs/installation/debian_based_en.md index fe2dbb92d..a900ec61d 100644 --- a/docs/installation/debian_based_en.md +++ b/docs/installation/debian_based_en.md @@ -7,13 +7,9 @@ This guide will assume you are on Debian Stretch. This guide should also work wi * `postgresql` (9.6+, Ubuntu 16.04 comes with 9.5, you can get a newer version from [here](https://www.postgresql.org/download/linux/ubuntu/)) * `postgresql-contrib` (9.6+, same situtation as above) -* `elixir` (1.5+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) +* `elixir` (1.8+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) * `erlang-dev` -* `erlang-tools` -* `erlang-parsetools` -* `erlang-eldap`, if you want to enable ldap authenticator -* `erlang-ssh` -* `erlang-xmerl` +* `erlang-nox` * `git` * `build-essential` @@ -50,7 +46,7 @@ sudo dpkg -i /tmp/erlang-solutions_1.0_all.deb ```shell sudo apt update -sudo apt install elixir erlang-dev erlang-parsetools erlang-xmerl erlang-tools erlang-ssh +sudo apt install elixir erlang-dev erlang-nox ``` ### Install PleromaBE diff --git a/docs/installation/debian_based_jp.md b/docs/installation/debian_based_jp.md index 7aa0bcc24..a3c4621d8 100644 --- a/docs/installation/debian_based_jp.md +++ b/docs/installation/debian_based_jp.md @@ -10,21 +10,17 @@ ### 必要なソフトウェア - PostgreSQL 9.6以上 (Ubuntu16.04では9.5しか提供されていないので，[](https://www.postgresql.org/download/linux/ubuntu/)こちらから新しいバージョンを入手してください) -- postgresql-contrib 9.6以上 (同上) -- Elixir 1.5 以上 ([Debianのリポジトリからインストールしないこと！！！ ここからインストールすること！](https://elixir-lang.org/install.html#unix-and-unix-like)。または [asdf](https://github.com/asdf-vm/asdf) をpleromaユーザーでインストールしてください) - - erlang-dev -- erlang-tools -- erlang-parsetools -- erlang-eldap (LDAP認証を有効化するときのみ必要) -- erlang-ssh -- erlang-xmerl -- git -- build-essential +- `postgresql-contrib` 9.6以上 (同上) +- Elixir 1.8 以上 ([Debianのリポジトリからインストールしないこと！！！ ここからインストールすること！](https://elixir-lang.org/install.html#unix-and-unix-like)。または [asdf](https://github.com/asdf-vm/asdf) をpleromaユーザーでインストールしてください) +- `erlang-dev` +- `erlang-nox` +- `git` +- `build-essential` #### このガイドで利用している追加パッケージ -- nginx (おすすめです。他のリバースプロキシを使う場合は、参考となる設定をこのリポジトリから探してください) -- certbot (または何らかのLet's Encrypt向けACMEクライアント) +- `nginx` (おすすめです。他のリバースプロキシを使う場合は、参考となる設定をこのリポジトリから探してください) +- `certbot` (または何らかのLet's Encrypt向けACMEクライアント) ### システムを準備する @@ -51,7 +47,7 @@ sudo dpkg -i /tmp/erlang-solutions_1.0_all.deb * ElixirとErlangをインストールします、 ``` sudo apt update -sudo apt install elixir erlang-dev erlang-parsetools erlang-xmerl erlang-tools erlang-ssh +sudo apt install elixir erlang-dev erlang-nox ``` ### Pleroma BE (バックエンド) をインストールします From c28aaf9d82a781508eba886bd455767a110d1b7c Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 13 Apr 2020 21:21:04 +0400 Subject: [PATCH 02/34] Add OpenAPI spec for CustomEmojiController --- .../operations/custom_emoji_operation.ex | 25 +++++++++++ .../web/api_spec/schemas/custom_emoji.ex | 30 +++++++++++++ .../schemas/custom_emojis_response.ex | 42 +++++++++++++++++++ .../controllers/custom_emoji_controller.ex | 4 ++ .../custom_emoji_controller_test.exs | 27 ++++++++++-- 5 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex create mode 100644 lib/pleroma/web/api_spec/schemas/custom_emoji.ex create mode 100644 lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex diff --git a/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex new file mode 100644 index 000000000..cf2215823 --- /dev/null +++ b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex @@ -0,0 +1,25 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.CustomEmojiOperation do + alias OpenApiSpex.Operation + alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse + + def open_api_operation(action) do + operation = String.to_existing_atom("#{action}_operation") + apply(__MODULE__, operation, []) + end + + def index_operation do + %Operation{ + tags: ["custom_emojis"], + summary: "List custom custom emojis", + description: "Returns custom emojis that are available on the server.", + operationId: "CustomEmojiController.index", + responses: %{ + 200 => Operation.response("Custom Emojis", "application/json", CustomEmojisResponse) + } + } + end +end diff --git a/lib/pleroma/web/api_spec/schemas/custom_emoji.ex b/lib/pleroma/web/api_spec/schemas/custom_emoji.ex new file mode 100644 index 000000000..5531b2081 --- /dev/null +++ b/lib/pleroma/web/api_spec/schemas/custom_emoji.ex @@ -0,0 +1,30 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.Schemas.CustomEmoji do + alias OpenApiSpex.Schema + + require OpenApiSpex + + OpenApiSpex.schema(%{ + title: "CustomEmoji", + description: "Response schema for an CustomEmoji", + type: :object, + properties: %{ + shortcode: %Schema{type: :string}, + url: %Schema{type: :string}, + static_url: %Schema{type: :string}, + visible_in_picker: %Schema{type: :boolean}, + category: %Schema{type: :string}, + tags: %Schema{type: :array} + }, + example: %{ + "shortcode" => "aaaa", + "url" => "https://files.mastodon.social/custom_emojis/images/000/007/118/original/aaaa.png", + "static_url" => + "https://files.mastodon.social/custom_emojis/images/000/007/118/static/aaaa.png", + "visible_in_picker" => true + } + }) +end diff --git a/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex b/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex new file mode 100644 index 000000000..01582a63d --- /dev/null +++ b/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex @@ -0,0 +1,42 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse do + alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji + + require OpenApiSpex + + OpenApiSpex.schema(%{ + title: "CustomEmojisResponse", + description: "Response schema for custom emojis", + type: :array, + items: CustomEmoji, + example: [ + %{ + "category" => "Fun", + "shortcode" => "blank", + "static_url" => "https://lain.com/emoji/blank.png", + "tags" => ["Fun"], + "url" => "https://lain.com/emoji/blank.png", + "visible_in_picker" => true + }, + %{ + "category" => "Gif,Fun", + "shortcode" => "firefox", + "static_url" => "https://lain.com/emoji/Firefox.gif", + "tags" => ["Gif", "Fun"], + "url" => "https://lain.com/emoji/Firefox.gif", + "visible_in_picker" => true + }, + %{ + "category" => "pack:mixed", + "shortcode" => "sadcat", + "static_url" => "https://lain.com/emoji/mixed/sadcat.png", + "tags" => ["pack:mixed"], + "url" => "https://lain.com/emoji/mixed/sadcat.png", + "visible_in_picker" => true + } + ] + }) +end diff --git a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex index d82de1db5..3bfebef8b 100644 --- a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex @@ -5,6 +5,10 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiController do use Pleroma.Web, :controller + plug(OpenApiSpex.Plug.CastAndValidate) + + defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.CustomEmojiOperation + def index(conn, _params) do render(conn, "index.json", custom_emojis: Pleroma.Emoji.get_all()) end diff --git a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs index 6567a0667..0b2ffa470 100644 --- a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs +++ b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs @@ -4,13 +4,18 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiControllerTest do use Pleroma.Web.ConnCase, async: true + alias Pleroma.Web.ApiSpec + alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji + alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse + import OpenApiSpex.TestAssertions test "with tags", %{conn: conn} do - [emoji | _body] = - conn - |> get("/api/v1/custom_emojis") - |> json_response(200) + assert resp = + conn + |> get("/api/v1/custom_emojis") + |> json_response(200) + assert [emoji | _body] = resp assert Map.has_key?(emoji, "shortcode") assert Map.has_key?(emoji, "static_url") assert Map.has_key?(emoji, "tags") @@ -18,5 +23,19 @@ test "with tags", %{conn: conn} do assert Map.has_key?(emoji, "category") assert Map.has_key?(emoji, "url") assert Map.has_key?(emoji, "visible_in_picker") + assert_schema(resp, "CustomEmojisResponse", ApiSpec.spec()) + assert_schema(emoji, "CustomEmoji", ApiSpec.spec()) + end + + test "CustomEmoji example matches schema" do + api_spec = ApiSpec.spec() + schema = CustomEmoji.schema() + assert_schema(schema.example, "CustomEmoji", api_spec) + end + + test "CustomEmojisResponse example matches schema" do + api_spec = ApiSpec.spec() + schema = CustomEmojisResponse.schema() + assert_schema(schema.example, "CustomEmojisResponse", api_spec) end end From 6cda360fea8a42168b5835ef903cf3bf89c8151a Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Thu, 16 Apr 2020 10:36:37 +0300 Subject: [PATCH 03/34] don't restart postgrex --- lib/pleroma/config/loader.ex | 2 +- lib/pleroma/config/transfer_task.ex | 20 +++++++++++--------- test/config/transfer_task_test.exs | 9 +++++++++ test/fixtures/config/temp.secret.exs | 2 ++ test/tasks/config_test.exs | 3 ++- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/config/loader.ex b/lib/pleroma/config/loader.ex index 6ca6550bd..0f3ecf1ed 100644 --- a/lib/pleroma/config/loader.ex +++ b/lib/pleroma/config/loader.ex @@ -47,7 +47,7 @@ defp filter(configs) do @spec filter_group(atom(), keyword()) :: keyword() def filter_group(group, configs) do Enum.reject(configs[group], fn {key, _v} -> - key in @reject_keys or (group == :phoenix and key == :serve_endpoints) + key in @reject_keys or (group == :phoenix and key == :serve_endpoints) or group == :postgrex end) end end diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index f4722f99d..c02b70e96 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -46,14 +46,6 @@ def load_and_update_env(deleted_settings \\ [], restart_pleroma? \\ true) do with {_, true} <- {:configurable, Config.get(:configurable_from_database)} do # We need to restart applications for loaded settings take effect - # TODO: some problem with prometheus after restart! - reject_restart = - if restart_pleroma? do - [nil, :prometheus] - else - [:pleroma, nil, :prometheus] - end - {logger, other} = (Repo.all(ConfigDB) ++ deleted_settings) |> Enum.map(&transform_and_merge/1) @@ -65,10 +57,20 @@ def load_and_update_env(deleted_settings \\ [], restart_pleroma? \\ true) do started_applications = Application.started_applications() + # TODO: some problem with prometheus after restart! + reject = [nil, :prometheus, :postgrex] + + reject = + if restart_pleroma? do + reject + else + [:pleroma | reject] + end + other |> Enum.map(&update/1) |> Enum.uniq() - |> Enum.reject(&(&1 in reject_restart)) + |> Enum.reject(&(&1 in reject)) |> maybe_set_pleroma_last() |> Enum.each(&restart(started_applications, &1, Config.get(:env))) diff --git a/test/config/transfer_task_test.exs b/test/config/transfer_task_test.exs index 00db0b686..473899d1d 100644 --- a/test/config/transfer_task_test.exs +++ b/test/config/transfer_task_test.exs @@ -16,6 +16,7 @@ test "transfer config values from db to env" do refute Application.get_env(:pleroma, :test_key) refute Application.get_env(:idna, :test_key) refute Application.get_env(:quack, :test_key) + refute Application.get_env(:postgrex, :test_key) initial = Application.get_env(:logger, :level) ConfigDB.create(%{ @@ -36,6 +37,12 @@ test "transfer config values from db to env" do value: [:test_value1, :test_value2] }) + ConfigDB.create(%{ + group: ":postgrex", + key: ":test_key", + value: :value + }) + ConfigDB.create(%{group: ":logger", key: ":level", value: :debug}) TransferTask.start_link([]) @@ -44,11 +51,13 @@ test "transfer config values from db to env" do assert Application.get_env(:idna, :test_key) == [live: 15, com: 35] assert Application.get_env(:quack, :test_key) == [:test_value1, :test_value2] assert Application.get_env(:logger, :level) == :debug + assert Application.get_env(:postgrex, :test_key) == :value on_exit(fn -> Application.delete_env(:pleroma, :test_key) Application.delete_env(:idna, :test_key) Application.delete_env(:quack, :test_key) + Application.delete_env(:postgrex, :test_key) Application.put_env(:logger, :level, initial) end) end diff --git a/test/fixtures/config/temp.secret.exs b/test/fixtures/config/temp.secret.exs index f4686c101..dc950ca30 100644 --- a/test/fixtures/config/temp.secret.exs +++ b/test/fixtures/config/temp.secret.exs @@ -7,3 +7,5 @@ config :quack, level: :info config :pleroma, Pleroma.Repo, pool: Ecto.Adapters.SQL.Sandbox + +config :postgrex, :json_library, Poison diff --git a/test/tasks/config_test.exs b/test/tasks/config_test.exs index 3dee4f082..04bc947a9 100644 --- a/test/tasks/config_test.exs +++ b/test/tasks/config_test.exs @@ -38,7 +38,7 @@ test "error if file with custom settings doesn't exist" do on_exit(fn -> Application.put_env(:quack, :level, initial) end) end - test "settings are migrated to db" do + test "filtered settings are migrated to db" do assert Repo.all(ConfigDB) == [] Mix.Tasks.Pleroma.Config.migrate_to_db("test/fixtures/config/temp.secret.exs") @@ -47,6 +47,7 @@ test "settings are migrated to db" do config2 = ConfigDB.get_by_params(%{group: ":pleroma", key: ":second_setting"}) config3 = ConfigDB.get_by_params(%{group: ":quack", key: ":level"}) refute ConfigDB.get_by_params(%{group: ":pleroma", key: "Pleroma.Repo"}) + refute ConfigDB.get_by_params(%{group: ":postgrex", key: ":json_library"}) assert ConfigDB.from_binary(config1.value) == [key: "value", key2: [Repo]] assert ConfigDB.from_binary(config2.value) == [key: "value2", key2: ["Activity"]] From f719a5b23a9bec4ed94f36c07e24aa1413654bae Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 13:28:34 +0200 Subject: [PATCH 04/34] WebPush: Return proper values for jobs. --- lib/pleroma/web/push/impl.ex | 3 ++- test/web/push/impl_test.exs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex index f1740a6e0..a9f893f7b 100644 --- a/lib/pleroma/web/push/impl.ex +++ b/lib/pleroma/web/push/impl.ex @@ -55,11 +55,12 @@ def perform( |> Jason.encode!() |> push_message(build_sub(subscription), gcm_api_key, subscription) end + |> (&{:ok, &1}).() end def perform(_) do Logger.warn("Unknown notification type") - :error + {:error, :unknown_type} end @doc "Push message to web" diff --git a/test/web/push/impl_test.exs b/test/web/push/impl_test.exs index 9121d90e7..b2664bf28 100644 --- a/test/web/push/impl_test.exs +++ b/test/web/push/impl_test.exs @@ -63,12 +63,12 @@ test "performs sending notifications" do activity: activity ) - assert Impl.perform(notif) == [:ok, :ok] + assert Impl.perform(notif) == {:ok, [:ok, :ok]} end @tag capture_log: true test "returns error if notif does not match " do - assert Impl.perform(%{}) == :error + assert Impl.perform(%{}) == {:error, :unknown_type} end test "successful message sending" do From 923513b6417973f700a80ee969c6c92ed2c9faee Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 13:28:52 +0200 Subject: [PATCH 05/34] Federator: Return proper values for jobs --- lib/pleroma/web/federator/federator.ex | 13 +++++++++---- test/web/federator_test.exs | 7 +++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex index fd904ef0a..f5803578d 100644 --- a/lib/pleroma/web/federator/federator.ex +++ b/lib/pleroma/web/federator/federator.ex @@ -72,19 +72,24 @@ def perform(:incoming_ap_doc, params) do # actor shouldn't be acting on objects outside their own AP server. with {:ok, _user} <- ap_enabled_actor(params["actor"]), nil <- Activity.normalize(params["id"]), - :ok <- Containment.contain_origin_from_id(params["actor"], params), + {_, :ok} <- + {:correct_origin?, Containment.contain_origin_from_id(params["actor"], params)}, {:ok, activity} <- Transmogrifier.handle_incoming(params) do {:ok, activity} else + {:correct_origin?, _} -> + Logger.debug("Origin containment failure for #{params["id"]}") + {:error, :origin_containment_failed} + %Activity{} -> Logger.debug("Already had #{params["id"]}") - :error + {:error, :already_present} - _e -> + e -> # Just drop those for now Logger.debug("Unhandled activity") Logger.debug(Jason.encode!(params, pretty: true)) - :error + {:error, e} end end diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index 59e53bb03..261518ef0 100644 --- a/test/web/federator_test.exs +++ b/test/web/federator_test.exs @@ -130,6 +130,9 @@ test "successfully processes incoming AP docs with correct origin" do assert {:ok, job} = Federator.incoming_ap_doc(params) assert {:ok, _activity} = ObanHelpers.perform(job) + + assert {:ok, job} = Federator.incoming_ap_doc(params) + assert {:error, :already_present} = ObanHelpers.perform(job) end test "rejects incoming AP docs with incorrect origin" do @@ -148,7 +151,7 @@ test "rejects incoming AP docs with incorrect origin" do } assert {:ok, job} = Federator.incoming_ap_doc(params) - assert :error = ObanHelpers.perform(job) + assert {:error, :origin_containment_failed} = ObanHelpers.perform(job) end test "it does not crash if MRF rejects the post" do @@ -164,7 +167,7 @@ test "it does not crash if MRF rejects the post" do |> Poison.decode!() assert {:ok, job} = Federator.incoming_ap_doc(params) - assert :error = ObanHelpers.perform(job) + assert {:error, _} = ObanHelpers.perform(job) end end end From 5102468d0f8067548ef233b5947da4bc517f5774 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 14:06:39 +0200 Subject: [PATCH 06/34] Polls: Persist and show voters' count --- lib/pleroma/object.ex | 5 ++++- lib/pleroma/web/activity_pub/activity_pub.ex | 5 +++-- lib/pleroma/web/mastodon_api/views/poll_view.ex | 7 +++++++ test/web/mastodon_api/views/poll_view_test.exs | 16 ++++++++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 9574432f0..e678fd415 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -261,7 +261,7 @@ def decrease_replies_count(ap_id) do end end - def increase_vote_count(ap_id, name) do + def increase_vote_count(ap_id, name, actor) do with %Object{} = object <- Object.normalize(ap_id), "Question" <- object.data["type"] do multiple = Map.has_key?(object.data, "anyOf") @@ -276,12 +276,15 @@ def increase_vote_count(ap_id, name) do option end) + voters = [actor | object.data["voters"] || []] |> Enum.uniq() + data = if multiple do Map.put(object.data, "anyOf", options) else Map.put(object.data, "oneOf", options) end + |> Map.put("voters", voters) object |> Object.change(%{data: data}) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index eedea08a2..4a133498e 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -118,9 +118,10 @@ def decrease_replies_count_if_reply(_object), do: :noop def increase_poll_votes_if_vote(%{ "object" => %{"inReplyTo" => reply_ap_id, "name" => name}, - "type" => "Create" + "type" => "Create", + "actor" => actor }) do - Object.increase_vote_count(reply_ap_id, name) + Object.increase_vote_count(reply_ap_id, name, actor) end def increase_poll_votes_if_vote(_create_data), do: :noop diff --git a/lib/pleroma/web/mastodon_api/views/poll_view.ex b/lib/pleroma/web/mastodon_api/views/poll_view.ex index 40edbb213..59a5deb28 100644 --- a/lib/pleroma/web/mastodon_api/views/poll_view.ex +++ b/lib/pleroma/web/mastodon_api/views/poll_view.ex @@ -19,6 +19,7 @@ def render("show.json", %{object: object, multiple: multiple, options: options} expired: expired, multiple: multiple, votes_count: votes_count, + voters_count: (multiple || nil) && voters_count(object), options: options, voted: voted?(params), emojis: Pleroma.Web.MastodonAPI.StatusView.build_emojis(object.data["emoji"]) @@ -62,6 +63,12 @@ defp options_and_votes_count(options) do end) end + defp voters_count(%{data: %{"voters" => [_ | _] = voters}}) do + length(voters) + end + + defp voters_count(_), do: 0 + defp voted?(%{object: object} = opts) do if opts[:for] do existing_votes = Pleroma.Web.ActivityPub.Utils.get_existing_votes(opts[:for].ap_id, object) diff --git a/test/web/mastodon_api/views/poll_view_test.exs b/test/web/mastodon_api/views/poll_view_test.exs index 6211fa888..63b204387 100644 --- a/test/web/mastodon_api/views/poll_view_test.exs +++ b/test/web/mastodon_api/views/poll_view_test.exs @@ -43,7 +43,8 @@ test "renders a poll" do %{title: "why are you even asking?", votes_count: 0} ], voted: false, - votes_count: 0 + votes_count: 0, + voters_count: nil } result = PollView.render("show.json", %{object: object}) @@ -69,9 +70,20 @@ test "detects if it is multiple choice" do } }) + voter = insert(:user) + object = Object.normalize(activity) - assert %{multiple: true} = PollView.render("show.json", %{object: object}) + {:ok, _votes, object} = CommonAPI.vote(voter, object, [0, 1]) + + assert match?( + %{ + multiple: true, + voters_count: 1, + votes_count: 2 + }, + PollView.render("show.json", %{object: object}) + ) end test "detects emoji" do From 568f48435e7db4582c54a443b2d543cc004f7f9b Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 12:10:20 +0000 Subject: [PATCH 07/34] Apply suggestion to docs/installation/debian_based_en.md --- docs/installation/debian_based_en.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/installation/debian_based_en.md b/docs/installation/debian_based_en.md index a900ec61d..62d8733f7 100644 --- a/docs/installation/debian_based_en.md +++ b/docs/installation/debian_based_en.md @@ -7,7 +7,7 @@ This guide will assume you are on Debian Stretch. This guide should also work wi * `postgresql` (9.6+, Ubuntu 16.04 comes with 9.5, you can get a newer version from [here](https://www.postgresql.org/download/linux/ubuntu/)) * `postgresql-contrib` (9.6+, same situtation as above) -* `elixir` (1.8+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) +* `elixir` (1.8+, Follow the guide to install from the Erlang Solutions repo or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) * `erlang-dev` * `erlang-nox` * `git` From c10485db163d56acd7206980f91f0e51153ef36a Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 14:26:19 +0200 Subject: [PATCH 08/34] StatusController: Ignore nil scheduled_at parameters. --- .../web/mastodon_api/controllers/status_controller.ex | 3 ++- .../controllers/status_controller_test.exs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 397dd10e3..f6e4f7d66 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -127,7 +127,8 @@ def index(%{assigns: %{user: user}} = conn, %{"ids" => ids} = params) do def create( %{assigns: %{user: user}} = conn, %{"status" => _, "scheduled_at" => scheduled_at} = params - ) do + ) + when not is_nil(scheduled_at) do params = Map.put(params, "in_reply_to_status_id", params["in_reply_to_id"]) with {:far_enough, true} <- {:far_enough, ScheduledActivity.far_enough?(scheduled_at)}, diff --git a/test/web/mastodon_api/controllers/status_controller_test.exs b/test/web/mastodon_api/controllers/status_controller_test.exs index 162f7b1b2..85068edd0 100644 --- a/test/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/web/mastodon_api/controllers/status_controller_test.exs @@ -302,6 +302,17 @@ test "creates a scheduled activity", %{conn: conn} do assert [] == Repo.all(Activity) end + test "ignores nil values", %{conn: conn} do + conn = + post(conn, "/api/v1/statuses", %{ + "status" => "not scheduled", + "scheduled_at" => nil + }) + + assert result = json_response(conn, 200) + assert Activity.get_by_id(result["id"]) + end + test "creates a scheduled activity with a media attachment", %{user: user, conn: conn} do scheduled_at = NaiveDateTime.add(NaiveDateTime.utc_now(), :timer.minutes(120), :millisecond) From 5b3952619818d38f8fdba9a64b050ce3f24394ff Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 15:04:26 +0200 Subject: [PATCH 09/34] AccountController: Use code 400 for self-follow. --- .../controllers/account_controller.ex | 21 ++++++++++--------- .../controllers/account_controller_test.exs | 6 +++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index e8e59ac66..5a92cebd8 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -293,7 +293,7 @@ def lists(%{assigns: %{user: user, account: account}} = conn, _params) do @doc "POST /api/v1/accounts/:id/follow" def follow(%{assigns: %{user: %{id: id}, account: %{id: id}}}, _params) do - {:error, :not_found} + {:error, "Can not follow yourself"} end def follow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @@ -306,7 +306,7 @@ def follow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @doc "POST /api/v1/accounts/:id/unfollow" def unfollow(%{assigns: %{user: %{id: id}, account: %{id: id}}}, _params) do - {:error, :not_found} + {:error, "Can not unfollow yourself"} end def unfollow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @@ -356,14 +356,15 @@ def unblock(%{assigns: %{user: blocker, account: blocked}} = conn, _params) do end @doc "POST /api/v1/follows" - def follows(%{assigns: %{user: follower}} = conn, %{"uri" => uri}) do - with {_, %User{} = followed} <- {:followed, User.get_cached_by_nickname(uri)}, - {_, true} <- {:followed, follower.id != followed.id}, - {:ok, follower, followed, _} <- CommonAPI.follow(follower, followed) do - render(conn, "show.json", user: followed, for: follower) - else - {:followed, _} -> {:error, :not_found} - {:error, message} -> json_response(conn, :forbidden, %{error: message}) + def follows(conn, %{"uri" => uri}) do + case User.get_cached_by_nickname(uri) do + %User{} = user -> + conn + |> assign(:account, user) + |> follow(%{}) + + nil -> + {:error, :not_found} end end diff --git a/test/web/mastodon_api/controllers/account_controller_test.exs b/test/web/mastodon_api/controllers/account_controller_test.exs index 61c2697b2..8c428efee 100644 --- a/test/web/mastodon_api/controllers/account_controller_test.exs +++ b/test/web/mastodon_api/controllers/account_controller_test.exs @@ -681,17 +681,17 @@ test "following without reblogs" do test "following / unfollowing errors", %{user: user, conn: conn} do # self follow conn_res = post(conn, "/api/v1/accounts/#{user.id}/follow") - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not follow yourself"} = json_response(conn_res, 400) # self unfollow user = User.get_cached_by_id(user.id) conn_res = post(conn, "/api/v1/accounts/#{user.id}/unfollow") - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not unfollow yourself"} = json_response(conn_res, 400) # self follow via uri user = User.get_cached_by_id(user.id) conn_res = post(conn, "/api/v1/follows", %{"uri" => user.nickname}) - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not follow yourself"} = json_response(conn_res, 400) # follow non existing user conn_res = post(conn, "/api/v1/accounts/doesntexist/follow") From 8b88e2a6e2b3a777ca99bf94676ab47f2d4cc0ea Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 15:31:37 +0200 Subject: [PATCH 10/34] Stats: Ignore internal users for user count. --- lib/pleroma/stats.ex | 19 ++++++++++++++----- test/{stat_test.exs => stats_test.exs} | 13 ++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) rename test/{stat_test.exs => stats_test.exs} (84%) diff --git a/lib/pleroma/stats.ex b/lib/pleroma/stats.ex index 4446562ac..6763786a7 100644 --- a/lib/pleroma/stats.ex +++ b/lib/pleroma/stats.ex @@ -45,11 +45,11 @@ def get_peers do end def init(_args) do - {:ok, get_stat_data()} + {:ok, calculate_stat_data()} end def handle_call(:force_update, _from, _state) do - new_stats = get_stat_data() + new_stats = calculate_stat_data() {:reply, new_stats, new_stats} end @@ -58,12 +58,12 @@ def handle_call(:get_state, _from, state) do end def handle_cast(:run_update, _state) do - new_stats = get_stat_data() + new_stats = calculate_stat_data() {:noreply, new_stats} end - defp get_stat_data do + def calculate_stat_data do peers = from( u in User, @@ -77,7 +77,16 @@ defp get_stat_data do status_count = Repo.aggregate(User.Query.build(%{local: true}), :sum, :note_count) - user_count = Repo.aggregate(User.Query.build(%{local: true, active: true}), :count, :id) + users_query = + from(u in User, + where: u.deactivated != true, + where: u.local == true, + where: not is_nil(u.nickname), + where: fragment("? not like 'internal.%'", u.nickname), + where: fragment("? not like '%/relay'", u.ap_id) + ) + + user_count = Repo.aggregate(users_query, :count, :id) %{ peers: peers, diff --git a/test/stat_test.exs b/test/stats_test.exs similarity index 84% rename from test/stat_test.exs rename to test/stats_test.exs index bccc1c8d0..73c7c1495 100644 --- a/test/stat_test.exs +++ b/test/stats_test.exs @@ -2,11 +2,22 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.StateTest do +defmodule Pleroma.StatsTest do use Pleroma.DataCase import Pleroma.Factory alias Pleroma.Web.CommonAPI + describe "user count" do + test "it ignores internal users" do + _user = insert(:user, local: true) + _internal = insert(:user, local: true, nickname: nil) + _internal = insert(:user, local: true, nickname: "internal.dude") + _internal = Pleroma.Web.ActivityPub.Relay.get_actor() + + assert match?(%{stats: %{user_count: 1}}, Pleroma.Stats.calculate_stat_data()) + end + end + describe "status visibility count" do test "on new status" do user = insert(:user) From 452072ec95781214df262962b71c60b7d771b7b1 Mon Sep 17 00:00:00 2001 From: Karol Kosek Date: Wed, 22 Apr 2020 16:02:40 +0200 Subject: [PATCH 11/34] static_fe: Add microformats2 classes --- .../templates/static_fe/static_fe/_attachment.html.eex | 6 +++--- .../web/templates/static_fe/static_fe/_notice.html.eex | 10 +++++++--- .../templates/static_fe/static_fe/_user_card.html.eex | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex index 7e04e9550..4853e7f4b 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex @@ -1,8 +1,8 @@ <%= case @mediaType do %> <% "audio" -> %> - + <% "video" -> %> - + <% _ -> %> -<%= @name %> +<%= @name %> <% end %> diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex index df5e5eedd..df0244795 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex @@ -1,12 +1,16 @@ -
id="selected" <% end %>> +
id="selected" <% end %>>

- <%= link format_date(@published), to: @link, class: "activity-link" %> + + +

<%= render("_user_card.html", %{user: @user}) %>
<%= if @title != "" do %>
open<% end %>> - <%= raw @title %> + <%= raw @title %>
<%= raw @content %>
<% else %> diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex index 56f3a1524..977b894d3 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex @@ -1,10 +1,10 @@
- +
- <%= raw Formatter.emojify(@user.name, @user.emoji) %> + <%= raw Formatter.emojify(@user.name, @user.emoji) %> <%= @user.nickname %> From 7a3a88a13ef526fba18bb6aeadc93f5da934dc5b Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 17:21:13 +0200 Subject: [PATCH 12/34] Streamer: Stream boosts to the boosting user. --- lib/pleroma/user.ex | 4 +++- lib/pleroma/web/streamer/worker.ex | 18 --------------- test/user_test.exs | 12 ++++++++++ test/web/streamer/streamer_test.exs | 36 +++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index bef4679cb..477237756 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1180,7 +1180,9 @@ def get_users_from_set(ap_ids, local_only \\ true) do end @spec get_recipients_from_activity(Activity.t()) :: [User.t()] - def get_recipients_from_activity(%Activity{recipients: to}) do + def get_recipients_from_activity(%Activity{recipients: to, actor: actor}) do + to = [actor | to] + User.Query.build(%{recipients_from_activity: to, local: true, deactivated: false}) |> Repo.all() end diff --git a/lib/pleroma/web/streamer/worker.ex b/lib/pleroma/web/streamer/worker.ex index abfed21c8..f6160fa4d 100644 --- a/lib/pleroma/web/streamer/worker.ex +++ b/lib/pleroma/web/streamer/worker.ex @@ -158,24 +158,6 @@ defp should_send?(%User{} = user, %Notification{activity: activity}) do should_send?(user, activity) end - def push_to_socket(topics, topic, %Activity{data: %{"type" => "Announce"}} = item) do - Enum.each(topics[topic] || [], fn %StreamerSocket{ - transport_pid: transport_pid, - user: socket_user - } -> - # Get the current user so we have up-to-date blocks etc. - if socket_user do - user = User.get_cached_by_ap_id(socket_user.ap_id) - - if should_send?(user, item) do - send(transport_pid, {:text, StreamerView.render("update.json", item, user)}) - end - else - send(transport_pid, {:text, StreamerView.render("update.json", item)}) - end - end) - end - def push_to_socket(topics, topic, %Participation{} = participation) do Enum.each(topics[topic] || [], fn %StreamerSocket{transport_pid: transport_pid} -> send(transport_pid, {:text, StreamerView.render("conversation.json", participation)}) diff --git a/test/user_test.exs b/test/user_test.exs index 65e118d6d..cd4041673 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -987,6 +987,18 @@ test "it imports user blocks from list" do end describe "get_recipients_from_activity" do + test "works for announces" do + actor = insert(:user) + user = insert(:user, local: true) + + {:ok, activity} = CommonAPI.post(actor, %{"status" => "hello"}) + {:ok, announce, _} = CommonAPI.repeat(activity.id, user) + + recipients = User.get_recipients_from_activity(announce) + + assert user in recipients + end + test "get recipients" do actor = insert(:user) user = insert(:user, local: true) diff --git a/test/web/streamer/streamer_test.exs b/test/web/streamer/streamer_test.exs index eb082b79f..8b8d8af6c 100644 --- a/test/web/streamer/streamer_test.exs +++ b/test/web/streamer/streamer_test.exs @@ -28,6 +28,42 @@ defmodule Pleroma.Web.StreamerTest do {:ok, %{user: user, notify: notify}} end + test "it streams the user's post in the 'user' stream", %{user: user} do + task = + Task.async(fn -> + assert_receive {:text, _}, @streamer_timeout + end) + + Streamer.add_socket( + "user", + %{transport_pid: task.pid, assigns: %{user: user}} + ) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey"}) + + Streamer.stream("user", activity) + Task.await(task) + end + + test "it streams boosts of the user in the 'user' stream", %{user: user} do + task = + Task.async(fn -> + assert_receive {:text, _}, @streamer_timeout + end) + + Streamer.add_socket( + "user", + %{transport_pid: task.pid, assigns: %{user: user}} + ) + + other_user = insert(:user) + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "hey"}) + {:ok, announce, _} = CommonAPI.repeat(activity.id, user) + + Streamer.stream("user", announce) + Task.await(task) + end + test "it sends notify to in the 'user' stream", %{user: user, notify: notify} do task = Task.async(fn -> From 88b82e5c3edae649f1caa45c6ef805828e4b8b1e Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 21 Apr 2020 20:05:25 +0400 Subject: [PATCH 13/34] Fix follow import --- .../controllers/util_controller.ex | 25 +++++++++---------- test/web/twitter_api/util_controller_test.exs | 24 ++++++++++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 537f9f778..824951d59 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -199,15 +199,16 @@ def follow_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def follow_import(%{assigns: %{user: follower}} = conn, %{"list" => list}) do - with lines <- String.split(list, "\n"), - followed_identifiers <- - Enum.map(lines, fn line -> - String.split(line, ",") |> List.first() - end) - |> List.delete("Account address") do - User.follow_import(follower, followed_identifiers) - json(conn, "job started") - end + followed_identifiers = + list + |> String.split("\n") + |> Enum.map(&(&1 |> String.split(",") |> List.first())) + |> List.delete("Account address") + |> Enum.map(&(&1 |> String.trim() |> String.trim_leading("@"))) + |> Enum.reject(&(&1 == "")) + + User.follow_import(follower, followed_identifiers) + json(conn, "job started") end def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do @@ -215,10 +216,8 @@ def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def blocks_import(%{assigns: %{user: blocker}} = conn, %{"list" => list}) do - with blocked_identifiers <- String.split(list) do - User.blocks_import(blocker, blocked_identifiers) - json(conn, "job started") - end + User.blocks_import(blocker, _blocked_identifiers = String.split(list)) + json(conn, "job started") end def change_password(%{assigns: %{user: user}} = conn, params) do diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 30e54bebd..85aaab19b 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -95,6 +95,30 @@ test "requires 'follow' or 'write:follows' permissions" do end end end + + test "it imports with different nickname variations", %{conn: conn} do + [user2, user3, user4, user5, user6] = insert_list(5, :user) + + identifiers = + [ + user2.ap_id, + user3.nickname, + " ", + "@" <> user4.nickname, + user5.nickname <> "@localhost", + "@" <> user6.nickname <> "@localhost" + ] + |> Enum.join("\n") + + response = + conn + |> post("/api/pleroma/follow_import", %{"list" => identifiers}) + |> json_response(:ok) + + assert response == "job started" + assert [job_result] = ObanHelpers.perform_all() + assert job_result == [user2, user3, user4, user5, user6] + end end describe "POST /api/pleroma/blocks_import" do From e7771424a895572626ec36a98bf862952dc6ff07 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 18:13:13 +0400 Subject: [PATCH 14/34] Fix blocks import --- .../controllers/util_controller.ex | 3 ++- test/web/twitter_api/util_controller_test.exs | 25 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 824951d59..d5d5ce08f 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -216,7 +216,8 @@ def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def blocks_import(%{assigns: %{user: blocker}} = conn, %{"list" => list}) do - User.blocks_import(blocker, _blocked_identifiers = String.split(list)) + blocked_identifiers = list |> String.split() |> Enum.map(&String.trim_leading(&1, "@")) + User.blocks_import(blocker, blocked_identifiers) json(conn, "job started") end diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 85aaab19b..d835331ae 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -96,7 +96,7 @@ test "requires 'follow' or 'write:follows' permissions" do end end - test "it imports with different nickname variations", %{conn: conn} do + test "it imports follows with different nickname variations", %{conn: conn} do [user2, user3, user4, user5, user6] = insert_list(5, :user) identifiers = @@ -160,6 +160,29 @@ test "it imports blocks users from file", %{user: user1, conn: conn} do ) end end + + test "it imports blocks with different nickname variations", %{conn: conn} do + [user2, user3, user4, user5, user6] = insert_list(5, :user) + + identifiers = + [ + user2.ap_id, + user3.nickname, + "@" <> user4.nickname, + user5.nickname <> "@localhost", + "@" <> user6.nickname <> "@localhost" + ] + |> Enum.join(" ") + + response = + conn + |> post("/api/pleroma/blocks_import", %{"list" => identifiers}) + |> json_response(:ok) + + assert response == "job started" + assert [job_result] = ObanHelpers.perform_all() + assert job_result == [user2, user3, user4, user5, user6] + end end describe "PUT /api/pleroma/notification_settings" do From 6db52c3b3680efbdb56d53d84d5dec0f4b6e34f0 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 19:00:08 +0400 Subject: [PATCH 15/34] Fix Oban warning Warning example: [warn] Expected Elixir.Pleroma.Workers.BackgroundWorker.perform/2 to return :ok, {:ok, value}, or {:error, reason}. Instead received: [error: "not found @user@server.party", error: "not found "] The job will be considered a success. --- lib/pleroma/workers/background_worker.ex | 4 ++-- test/user_test.exs | 4 ++-- test/web/twitter_api/util_controller_test.exs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/workers/background_worker.ex b/lib/pleroma/workers/background_worker.ex index 0f8ece2c4..57c3a9c3a 100644 --- a/lib/pleroma/workers/background_worker.ex +++ b/lib/pleroma/workers/background_worker.ex @@ -35,7 +35,7 @@ def perform( _job ) do blocker = User.get_cached_by_id(blocker_id) - User.perform(:blocks_import, blocker, blocked_identifiers) + {:ok, User.perform(:blocks_import, blocker, blocked_identifiers)} end def perform( @@ -47,7 +47,7 @@ def perform( _job ) do follower = User.get_cached_by_id(follower_id) - User.perform(:follow_import, follower, followed_identifiers) + {:ok, User.perform(:follow_import, follower, followed_identifiers)} end def perform(%{"op" => "media_proxy_preload", "message" => message}, _job) do diff --git a/test/user_test.exs b/test/user_test.exs index 65e118d6d..23e7cf6e3 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -756,8 +756,8 @@ test "it imports user followings from list" do ] {:ok, job} = User.follow_import(user1, identifiers) - result = ObanHelpers.perform(job) + assert {:ok, result} = ObanHelpers.perform(job) assert is_list(result) assert result == [user2, user3] end @@ -979,8 +979,8 @@ test "it imports user blocks from list" do ] {:ok, job} = User.blocks_import(user1, identifiers) - result = ObanHelpers.perform(job) + assert {:ok, result} = ObanHelpers.perform(job) assert is_list(result) assert result == [user2, user3] end diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index d835331ae..b701239a0 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -116,7 +116,7 @@ test "it imports follows with different nickname variations", %{conn: conn} do |> json_response(:ok) assert response == "job started" - assert [job_result] = ObanHelpers.perform_all() + assert [{:ok, job_result}] = ObanHelpers.perform_all() assert job_result == [user2, user3, user4, user5, user6] end end @@ -180,7 +180,7 @@ test "it imports blocks with different nickname variations", %{conn: conn} do |> json_response(:ok) assert response == "job started" - assert [job_result] = ObanHelpers.perform_all() + assert [{:ok, job_result}] = ObanHelpers.perform_all() assert job_result == [user2, user3, user4, user5, user6] end end From 771c1ad735eb06842278e823a29acb94fd9acafb Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 19:31:03 +0400 Subject: [PATCH 16/34] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d8e7efc3..702c58180 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Support pagination in conversations API - **Breaking**: SimplePolicy `:reject` and `:accept` allow deletions again +- Fix follower/blocks import when nicknames starts with @ ## [unreleased-patch] ### Fixed From e62173dfc8b739508345b7ab97477ae04fcdb457 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 18:40:53 +0200 Subject: [PATCH 17/34] SideEffects: Run in transaction. This fixes race conditions. --- lib/pleroma/web/activity_pub/side_effects.ex | 11 +++++--- test/web/common_api/common_api_test.exs | 27 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 6a8f1af96..a0f71fd88 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -15,12 +15,15 @@ def handle(object, meta \\ []) # - Add like to object # - Set up notification def handle(%{data: %{"type" => "Like"}} = object, meta) do - liked_object = Object.get_by_ap_id(object.data["object"]) - Utils.add_like_to_object(object, liked_object) + Pleroma.Repo.transaction(fn -> + liked_object = Object.get_by_ap_id(object.data["object"]) + Utils.add_like_to_object(object, liked_object) - Notification.create_notifications(object) + Notification.create_notifications(object) - {:ok, object, meta} + {:ok, object, meta} + end) + |> (fn {:ok, res} -> res end).() end # Nothing to do diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index e130736ec..68a29108a 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -21,6 +21,33 @@ defmodule Pleroma.Web.CommonAPITest do setup do: clear_config([:instance, :limit]) setup do: clear_config([:instance, :max_pinned_statuses]) + test "favoriting race condition" do + user = insert(:user) + users_serial = insert_list(10, :user) + users = insert_list(10, :user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "."}) + + users_serial + |> Enum.map(fn user -> + CommonAPI.favorite(user, activity.id) + end) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["like_count"] == 10 + + users + |> Enum.map(fn user -> + Task.async(fn -> + CommonAPI.favorite(user, activity.id) + end) + end) + |> Enum.map(&Task.await/1) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["like_count"] == 20 + end + test "when replying to a conversation / participation, it will set the correct context id even if no explicit reply_to is given" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => ".", "visibility" => "direct"}) From f5bda09de648a6de3151c8614005ebc70447facb Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 19:02:22 +0200 Subject: [PATCH 18/34] Stats: Use `invisible` property for filtering. --- lib/pleroma/stats.ex | 3 +-- test/stats_test.exs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/pleroma/stats.ex b/lib/pleroma/stats.ex index 6763786a7..8d2809bbb 100644 --- a/lib/pleroma/stats.ex +++ b/lib/pleroma/stats.ex @@ -82,8 +82,7 @@ def calculate_stat_data do where: u.deactivated != true, where: u.local == true, where: not is_nil(u.nickname), - where: fragment("? not like 'internal.%'", u.nickname), - where: fragment("? not like '%/relay'", u.ap_id) + where: not u.invisible ) user_count = Repo.aggregate(users_query, :count, :id) diff --git a/test/stats_test.exs b/test/stats_test.exs index 73c7c1495..c1aeb2c7f 100644 --- a/test/stats_test.exs +++ b/test/stats_test.exs @@ -11,7 +11,6 @@ defmodule Pleroma.StatsTest do test "it ignores internal users" do _user = insert(:user, local: true) _internal = insert(:user, local: true, nickname: nil) - _internal = insert(:user, local: true, nickname: "internal.dude") _internal = Pleroma.Web.ActivityPub.Relay.get_actor() assert match?(%{stats: %{user_count: 1}}, Pleroma.Stats.calculate_stat_data()) From 1bcbdc7a9f5df35f42d1ab331bc4b6785e180284 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 21:21:21 +0200 Subject: [PATCH 19/34] SideEffects: Use less cryptic syntax. --- lib/pleroma/web/activity_pub/side_effects.ex | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index a0f71fd88..5981e7545 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -15,15 +15,17 @@ def handle(object, meta \\ []) # - Add like to object # - Set up notification def handle(%{data: %{"type" => "Like"}} = object, meta) do - Pleroma.Repo.transaction(fn -> - liked_object = Object.get_by_ap_id(object.data["object"]) - Utils.add_like_to_object(object, liked_object) + {:ok, result} = + Pleroma.Repo.transaction(fn -> + liked_object = Object.get_by_ap_id(object.data["object"]) + Utils.add_like_to_object(object, liked_object) - Notification.create_notifications(object) + Notification.create_notifications(object) - {:ok, object, meta} - end) - |> (fn {:ok, res} -> res end).() + {:ok, object, meta} + end) + + result end # Nothing to do From 7d38197894692306c940b55045b91d563e138284 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 23 Apr 2020 13:33:30 +0200 Subject: [PATCH 20/34] CommonAPI: Don't make repeating announces possible --- lib/pleroma/web/common_api/common_api.ex | 23 +++++++++++-------- lib/pleroma/web/common_api/utils.ex | 18 --------------- test/web/common_api/common_api_test.exs | 18 ++++++++++++--- test/web/common_api/common_api_utils_test.exs | 20 ---------------- 4 files changed, 28 insertions(+), 51 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index f50a909aa..d1efe0c36 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -86,8 +86,9 @@ def delete(activity_id, user) do end end - def repeat(id_or_ap_id, user, params \\ %{}) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)}, + def repeat(id, user, params \\ %{}) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)}, object <- Object.normalize(activity), announce_activity <- Utils.get_existing_announce(user.ap_id, object), public <- public_announce?(object, params) do @@ -102,8 +103,9 @@ def repeat(id_or_ap_id, user, params \\ %{}) do end end - def unrepeat(id_or_ap_id, user) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do + def unrepeat(id, user) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)} do object = Object.normalize(activity) ActivityPub.unannounce(user, object) else @@ -160,8 +162,9 @@ def favorite_helper(user, id) do end end - def unfavorite(id_or_ap_id, user) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do + def unfavorite(id, user) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)} do object = Object.normalize(activity) ActivityPub.unlike(user, object) else @@ -332,12 +335,12 @@ defp maybe_create_activity_expiration({:ok, activity}, %NaiveDateTime{} = expire defp maybe_create_activity_expiration(result, _), do: result - def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do + def pin(id, %{ap_id: user_ap_id} = user) do with %Activity{ actor: ^user_ap_id, data: %{"type" => "Create"}, object: %Object{data: %{"type" => object_type}} - } = activity <- get_by_id_or_ap_id(id_or_ap_id), + } = activity <- Activity.get_by_id_with_object(id), true <- object_type in ["Note", "Article", "Question"], true <- Visibility.is_public?(activity), {:ok, _user} <- User.add_pinnned_activity(user, activity) do @@ -348,8 +351,8 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do end end - def unpin(id_or_ap_id, user) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), + def unpin(id, user) do + with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id(id), {:ok, _user} <- User.remove_pinnned_activity(user, activity) do {:ok, activity} else diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 7eec5aa09..945e63e22 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -22,24 +22,6 @@ defmodule Pleroma.Web.CommonAPI.Utils do require Logger require Pleroma.Constants - # This is a hack for twidere. - def get_by_id_or_ap_id(id) do - activity = - with true <- FlakeId.flake_id?(id), - %Activity{} = activity <- Activity.get_by_id_with_object(id) do - activity - else - _ -> Activity.get_create_by_object_ap_id_with_object(id) - end - - activity && - if activity.data["type"] == "Create" do - activity - else - Activity.get_create_by_object_ap_id_with_object(activity.data["object"]) - end - end - def attachments_from_ids(%{"media_ids" => ids, "descriptions" => desc} = _) do attachments_from_ids_descs(ids, desc) end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 68a29108a..e87193c83 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -283,6 +283,16 @@ test "repeating a status" do {:ok, %Activity{}, _} = CommonAPI.repeat(activity.id, user) end + test "can't repeat a repeat" do + user = insert(:user) + other_user = insert(:user) + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"}) + + {:ok, %Activity{} = announce, _} = CommonAPI.repeat(activity.id, other_user) + + refute match?({:ok, %Activity{}, _}, CommonAPI.repeat(announce.id, user)) + end + test "repeating a status privately" do user = insert(:user) other_user = insert(:user) @@ -312,8 +322,8 @@ test "retweeting a status twice returns the status" do other_user = insert(:user) {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"}) - {:ok, %Activity{} = activity, object} = CommonAPI.repeat(activity.id, user) - {:ok, ^activity, ^object} = CommonAPI.repeat(activity.id, user) + {:ok, %Activity{} = announce, object} = CommonAPI.repeat(activity.id, user) + {:ok, ^announce, ^object} = CommonAPI.repeat(activity.id, user) end test "favoriting a status twice returns ok, but without the like activity" do @@ -387,7 +397,9 @@ test "unpin status", %{user: user, activity: activity} do user = refresh_record(user) - assert {:ok, ^activity} = CommonAPI.unpin(activity.id, user) + id = activity.id + + assert match?({:ok, %{id: ^id}}, CommonAPI.unpin(activity.id, user)) user = refresh_record(user) diff --git a/test/web/common_api/common_api_utils_test.exs b/test/web/common_api/common_api_utils_test.exs index b21445fe9..18a3b3b87 100644 --- a/test/web/common_api/common_api_utils_test.exs +++ b/test/web/common_api/common_api_utils_test.exs @@ -335,26 +335,6 @@ test "for direct posts, a reply" do end end - describe "get_by_id_or_ap_id/1" do - test "get activity by id" do - activity = insert(:note_activity) - %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.id) - assert note.id == activity.id - end - - test "get activity by ap_id" do - activity = insert(:note_activity) - %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.data["object"]) - assert note.id == activity.id - end - - test "get activity by object when type isn't `Create` " do - activity = insert(:like_activity) - %Pleroma.Activity{} = like = Utils.get_by_id_or_ap_id(activity.id) - assert like.data["object"] == activity.data["object"] - end - end - describe "to_master_date/1" do test "removes microseconds from date (NaiveDateTime)" do assert Utils.to_masto_date(~N[2015-01-23 23:50:07.123]) == "2015-01-23T23:50:07.000Z" From f362836742aabd5b60b92c1296f2bbb6d83a3d59 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Fri, 24 Apr 2020 14:46:59 +0400 Subject: [PATCH 21/34] Support validation for inline OpenAPI schema and automatic tests for examples --- .../web/api_spec/operations/app_operation.ex | 60 +++++++++++++++++-- .../operations/custom_emoji_operation.ex | 40 ++++++++++++- .../operations/domain_block_operation.ex | 31 ++++++++-- .../api_spec/schemas/app_create_request.ex | 33 ---------- .../api_spec/schemas/app_create_response.ex | 33 ---------- .../schemas/custom_emojis_response.ex | 42 ------------- .../api_spec/schemas/domain_block_request.ex | 20 ------- .../schemas/domain_blocks_response.ex | 16 ----- lib/pleroma/web/oauth/scopes.ex | 6 +- test/support/api_spec_helpers.ex | 57 ++++++++++++++++++ test/support/conn_case.ex | 36 +++++++++++ test/web/api_spec/app_operation_test.exs | 45 -------------- test/web/api_spec/schema_examples_test.exs | 43 +++++++++++++ .../controllers/app_controller_test.exs | 4 +- .../custom_emoji_controller_test.exs | 17 +----- .../domain_block_controller_test.exs | 28 +++------ 16 files changed, 267 insertions(+), 244 deletions(-) delete mode 100644 lib/pleroma/web/api_spec/schemas/app_create_request.ex delete mode 100644 lib/pleroma/web/api_spec/schemas/app_create_response.ex delete mode 100644 lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex delete mode 100644 lib/pleroma/web/api_spec/schemas/domain_block_request.ex delete mode 100644 lib/pleroma/web/api_spec/schemas/domain_blocks_response.ex create mode 100644 test/support/api_spec_helpers.ex delete mode 100644 test/web/api_spec/app_operation_test.exs create mode 100644 test/web/api_spec/schema_examples_test.exs diff --git a/lib/pleroma/web/api_spec/operations/app_operation.ex b/lib/pleroma/web/api_spec/operations/app_operation.ex index 26d8dbd42..035ef2470 100644 --- a/lib/pleroma/web/api_spec/operations/app_operation.ex +++ b/lib/pleroma/web/api_spec/operations/app_operation.ex @@ -6,8 +6,6 @@ defmodule Pleroma.Web.ApiSpec.AppOperation do alias OpenApiSpex.Operation alias OpenApiSpex.Schema alias Pleroma.Web.ApiSpec.Helpers - alias Pleroma.Web.ApiSpec.Schemas.AppCreateRequest - alias Pleroma.Web.ApiSpec.Schemas.AppCreateResponse @spec open_api_operation(atom) :: Operation.t() def open_api_operation(action) do @@ -22,9 +20,9 @@ def create_operation do summary: "Create an application", description: "Create a new application to obtain OAuth2 credentials", operationId: "AppController.create", - requestBody: Helpers.request_body("Parameters", AppCreateRequest, required: true), + requestBody: Helpers.request_body("Parameters", create_request(), required: true), responses: %{ - 200 => Operation.response("App", "application/json", AppCreateResponse), + 200 => Operation.response("App", "application/json", create_response()), 422 => Operation.response( "Unprocessable Entity", @@ -93,4 +91,58 @@ def verify_credentials_operation do } } end + + defp create_request do + %Schema{ + title: "AppCreateRequest", + description: "POST body for creating an app", + type: :object, + properties: %{ + client_name: %Schema{type: :string, description: "A name for your application."}, + redirect_uris: %Schema{ + type: :string, + description: + "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter." + }, + scopes: %Schema{ + type: :string, + description: "Space separated list of scopes", + default: "read" + }, + website: %Schema{type: :string, description: "A URL to the homepage of your app"} + }, + required: [:client_name, :redirect_uris], + example: %{ + "client_name" => "My App", + "redirect_uris" => "https://myapp.com/auth/callback", + "website" => "https://myapp.com/" + } + } + end + + defp create_response do + %Schema{ + title: "AppCreateResponse", + description: "Response schema for an app", + type: :object, + properties: %{ + id: %Schema{type: :string}, + name: %Schema{type: :string}, + client_id: %Schema{type: :string}, + client_secret: %Schema{type: :string}, + redirect_uri: %Schema{type: :string}, + vapid_key: %Schema{type: :string}, + website: %Schema{type: :string, nullable: true} + }, + example: %{ + "id" => "123", + "name" => "My App", + "client_id" => "TWhM-tNSuncnqN7DBJmoyeLnk6K3iJJ71KKXxgL1hPM", + "client_secret" => "ZEaFUFmF0umgBX1qKJDjaU99Q31lDkOU8NutzTOoliw", + "vapid_key" => + "BCk-QqERU0q-CfYZjcuB6lnyyOYfJ2AifKqfeGIm7Z-HiTU5T9eTG5GxVA0_OH5mMlI4UkkDTpaZwozy0TzdZ2M=", + "website" => "https://myapp.com/" + } + } + end end diff --git a/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex index cf2215823..a117fe460 100644 --- a/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex +++ b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex @@ -4,7 +4,8 @@ defmodule Pleroma.Web.ApiSpec.CustomEmojiOperation do alias OpenApiSpex.Operation - alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse + alias OpenApiSpex.Schema + alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji def open_api_operation(action) do operation = String.to_existing_atom("#{action}_operation") @@ -18,8 +19,43 @@ def index_operation do description: "Returns custom emojis that are available on the server.", operationId: "CustomEmojiController.index", responses: %{ - 200 => Operation.response("Custom Emojis", "application/json", CustomEmojisResponse) + 200 => Operation.response("Custom Emojis", "application/json", custom_emojis_resposnse()) } } end + + defp custom_emojis_resposnse do + %Schema{ + title: "CustomEmojisResponse", + description: "Response schema for custom emojis", + type: :array, + items: CustomEmoji, + example: [ + %{ + "category" => "Fun", + "shortcode" => "blank", + "static_url" => "https://lain.com/emoji/blank.png", + "tags" => ["Fun"], + "url" => "https://lain.com/emoji/blank.png", + "visible_in_picker" => false + }, + %{ + "category" => "Gif,Fun", + "shortcode" => "firefox", + "static_url" => "https://lain.com/emoji/Firefox.gif", + "tags" => ["Gif", "Fun"], + "url" => "https://lain.com/emoji/Firefox.gif", + "visible_in_picker" => true + }, + %{ + "category" => "pack:mixed", + "shortcode" => "sadcat", + "static_url" => "https://lain.com/emoji/mixed/sadcat.png", + "tags" => ["pack:mixed"], + "url" => "https://lain.com/emoji/mixed/sadcat.png", + "visible_in_picker" => true + } + ] + } + end end diff --git a/lib/pleroma/web/api_spec/operations/domain_block_operation.ex b/lib/pleroma/web/api_spec/operations/domain_block_operation.ex index dd14837c3..3b7f51ceb 100644 --- a/lib/pleroma/web/api_spec/operations/domain_block_operation.ex +++ b/lib/pleroma/web/api_spec/operations/domain_block_operation.ex @@ -6,8 +6,6 @@ defmodule Pleroma.Web.ApiSpec.DomainBlockOperation do alias OpenApiSpex.Operation alias OpenApiSpex.Schema alias Pleroma.Web.ApiSpec.Helpers - alias Pleroma.Web.ApiSpec.Schemas.DomainBlockRequest - alias Pleroma.Web.ApiSpec.Schemas.DomainBlocksResponse def open_api_operation(action) do operation = String.to_existing_atom("#{action}_operation") @@ -22,7 +20,13 @@ def index_operation do security: [%{"oAuth" => ["follow", "read:blocks"]}], operationId: "DomainBlockController.index", responses: %{ - 200 => Operation.response("Domain blocks", "application/json", DomainBlocksResponse) + 200 => + Operation.response("Domain blocks", "application/json", %Schema{ + description: "Response schema for domain blocks", + type: :array, + items: %Schema{type: :string}, + example: ["google.com", "facebook.com"] + }) } } end @@ -40,7 +44,7 @@ def create_operation do - prevent following new users from it (but does not remove existing follows) """, operationId: "DomainBlockController.create", - requestBody: Helpers.request_body("Parameters", DomainBlockRequest, required: true), + requestBody: domain_block_request(), security: [%{"oAuth" => ["follow", "write:blocks"]}], responses: %{ 200 => Operation.response("Empty object", "application/json", %Schema{type: :object}) @@ -54,11 +58,28 @@ def delete_operation do summary: "Unblock a domain", description: "Remove a domain block, if it exists in the user's array of blocked domains.", operationId: "DomainBlockController.delete", - requestBody: Helpers.request_body("Parameters", DomainBlockRequest, required: true), + requestBody: domain_block_request(), security: [%{"oAuth" => ["follow", "write:blocks"]}], responses: %{ 200 => Operation.response("Empty object", "application/json", %Schema{type: :object}) } } end + + defp domain_block_request do + Helpers.request_body( + "Parameters", + %Schema{ + type: :object, + properties: %{ + domain: %Schema{type: :string} + }, + required: [:domain] + }, + required: true, + example: %{ + "domain" => "facebook.com" + } + ) + end end diff --git a/lib/pleroma/web/api_spec/schemas/app_create_request.ex b/lib/pleroma/web/api_spec/schemas/app_create_request.ex deleted file mode 100644 index 8a83abef3..000000000 --- a/lib/pleroma/web/api_spec/schemas/app_create_request.ex +++ /dev/null @@ -1,33 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.Schemas.AppCreateRequest do - alias OpenApiSpex.Schema - require OpenApiSpex - - OpenApiSpex.schema(%{ - title: "AppCreateRequest", - description: "POST body for creating an app", - type: :object, - properties: %{ - client_name: %Schema{type: :string, description: "A name for your application."}, - redirect_uris: %Schema{ - type: :string, - description: - "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter." - }, - scopes: %Schema{ - type: :string, - description: "Space separated list of scopes. If none is provided, defaults to `read`." - }, - website: %Schema{type: :string, description: "A URL to the homepage of your app"} - }, - required: [:client_name, :redirect_uris], - example: %{ - "client_name" => "My App", - "redirect_uris" => "https://myapp.com/auth/callback", - "website" => "https://myapp.com/" - } - }) -end diff --git a/lib/pleroma/web/api_spec/schemas/app_create_response.ex b/lib/pleroma/web/api_spec/schemas/app_create_response.ex deleted file mode 100644 index f290fb031..000000000 --- a/lib/pleroma/web/api_spec/schemas/app_create_response.ex +++ /dev/null @@ -1,33 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.Schemas.AppCreateResponse do - alias OpenApiSpex.Schema - - require OpenApiSpex - - OpenApiSpex.schema(%{ - title: "AppCreateResponse", - description: "Response schema for an app", - type: :object, - properties: %{ - id: %Schema{type: :string}, - name: %Schema{type: :string}, - client_id: %Schema{type: :string}, - client_secret: %Schema{type: :string}, - redirect_uri: %Schema{type: :string}, - vapid_key: %Schema{type: :string}, - website: %Schema{type: :string, nullable: true} - }, - example: %{ - "id" => "123", - "name" => "My App", - "client_id" => "TWhM-tNSuncnqN7DBJmoyeLnk6K3iJJ71KKXxgL1hPM", - "client_secret" => "ZEaFUFmF0umgBX1qKJDjaU99Q31lDkOU8NutzTOoliw", - "vapid_key" => - "BCk-QqERU0q-CfYZjcuB6lnyyOYfJ2AifKqfeGIm7Z-HiTU5T9eTG5GxVA0_OH5mMlI4UkkDTpaZwozy0TzdZ2M=", - "website" => "https://myapp.com/" - } - }) -end diff --git a/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex b/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex deleted file mode 100644 index 01582a63d..000000000 --- a/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex +++ /dev/null @@ -1,42 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse do - alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji - - require OpenApiSpex - - OpenApiSpex.schema(%{ - title: "CustomEmojisResponse", - description: "Response schema for custom emojis", - type: :array, - items: CustomEmoji, - example: [ - %{ - "category" => "Fun", - "shortcode" => "blank", - "static_url" => "https://lain.com/emoji/blank.png", - "tags" => ["Fun"], - "url" => "https://lain.com/emoji/blank.png", - "visible_in_picker" => true - }, - %{ - "category" => "Gif,Fun", - "shortcode" => "firefox", - "static_url" => "https://lain.com/emoji/Firefox.gif", - "tags" => ["Gif", "Fun"], - "url" => "https://lain.com/emoji/Firefox.gif", - "visible_in_picker" => true - }, - %{ - "category" => "pack:mixed", - "shortcode" => "sadcat", - "static_url" => "https://lain.com/emoji/mixed/sadcat.png", - "tags" => ["pack:mixed"], - "url" => "https://lain.com/emoji/mixed/sadcat.png", - "visible_in_picker" => true - } - ] - }) -end diff --git a/lib/pleroma/web/api_spec/schemas/domain_block_request.ex b/lib/pleroma/web/api_spec/schemas/domain_block_request.ex deleted file mode 100644 index ee9238361..000000000 --- a/lib/pleroma/web/api_spec/schemas/domain_block_request.ex +++ /dev/null @@ -1,20 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.Schemas.DomainBlockRequest do - alias OpenApiSpex.Schema - require OpenApiSpex - - OpenApiSpex.schema(%{ - title: "DomainBlockRequest", - type: :object, - properties: %{ - domain: %Schema{type: :string} - }, - required: [:domain], - example: %{ - "domain" => "facebook.com" - } - }) -end diff --git a/lib/pleroma/web/api_spec/schemas/domain_blocks_response.ex b/lib/pleroma/web/api_spec/schemas/domain_blocks_response.ex deleted file mode 100644 index d895aca4e..000000000 --- a/lib/pleroma/web/api_spec/schemas/domain_blocks_response.ex +++ /dev/null @@ -1,16 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.Schemas.DomainBlocksResponse do - require OpenApiSpex - alias OpenApiSpex.Schema - - OpenApiSpex.schema(%{ - title: "DomainBlocksResponse", - description: "Response schema for domain blocks", - type: :array, - items: %Schema{type: :string}, - example: ["google.com", "facebook.com"] - }) -end diff --git a/lib/pleroma/web/oauth/scopes.ex b/lib/pleroma/web/oauth/scopes.ex index 1023f16d4..6f06f1431 100644 --- a/lib/pleroma/web/oauth/scopes.ex +++ b/lib/pleroma/web/oauth/scopes.ex @@ -17,12 +17,8 @@ defmodule Pleroma.Web.OAuth.Scopes do """ @spec fetch_scopes(map() | struct(), list()) :: list() - def fetch_scopes(%Pleroma.Web.ApiSpec.Schemas.AppCreateRequest{scopes: scopes}, default) do - parse_scopes(scopes, default) - end - def fetch_scopes(params, default) do - parse_scopes(params["scope"] || params["scopes"], default) + parse_scopes(params["scope"] || params["scopes"] || params[:scopes], default) end def parse_scopes(scopes, _default) when is_list(scopes) do diff --git a/test/support/api_spec_helpers.ex b/test/support/api_spec_helpers.ex new file mode 100644 index 000000000..80c69c788 --- /dev/null +++ b/test/support/api_spec_helpers.ex @@ -0,0 +1,57 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Tests.ApiSpecHelpers do + @moduledoc """ + OpenAPI spec test helpers + """ + + import ExUnit.Assertions + + alias OpenApiSpex.Cast.Error + alias OpenApiSpex.Reference + alias OpenApiSpex.Schema + + def assert_schema(value, schema) do + api_spec = Pleroma.Web.ApiSpec.spec() + + case OpenApiSpex.cast_value(value, schema, api_spec) do + {:ok, data} -> + data + + {:error, errors} -> + errors = + Enum.map(errors, fn error -> + message = Error.message(error) + path = Error.path_to_string(error) + "#{message} at #{path}" + end) + + flunk( + "Value does not conform to schema #{schema.title}: #{Enum.join(errors, "\n")}\n#{ + inspect(value) + }" + ) + end + end + + def resolve_schema(%Schema{} = schema), do: schema + + def resolve_schema(%Reference{} = ref) do + schemas = Pleroma.Web.ApiSpec.spec().components.schemas + Reference.resolve_schema(ref, schemas) + end + + def api_operations do + paths = Pleroma.Web.ApiSpec.spec().paths + + Enum.flat_map(paths, fn {_, path_item} -> + path_item + |> Map.take([:delete, :get, :head, :options, :patch, :post, :put, :trace]) + |> Map.values() + |> Enum.reject(&is_nil/1) + |> Enum.uniq() + end) + end +end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 064874201..781622476 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -51,6 +51,42 @@ defp oauth_access(scopes, opts \\ []) do %{user: user, token: token, conn: conn} end + defp json_response_and_validate_schema(conn, status \\ nil) do + content_type = + conn + |> Plug.Conn.get_resp_header("content-type") + |> List.first() + |> String.split(";") + |> List.first() + + status = status || conn.status + + %{private: %{open_api_spex: %{operation_id: op_id, operation_lookup: lookup, spec: spec}}} = + conn + + schema = lookup[op_id].responses[status].content[content_type].schema + json = json_response(conn, status) + + case OpenApiSpex.cast_value(json, schema, spec) do + {:ok, _data} -> + json + + {:error, errors} -> + errors = + Enum.map(errors, fn error -> + message = OpenApiSpex.Cast.Error.message(error) + path = OpenApiSpex.Cast.Error.path_to_string(error) + "#{message} at #{path}" + end) + + flunk( + "Response does not conform to schema of #{op_id} operation: #{ + Enum.join(errors, "\n") + }\n#{inspect(json)}" + ) + end + end + defp ensure_federating_or_authenticated(conn, url, user) do initial_setting = Config.get([:instance, :federating]) on_exit(fn -> Config.put([:instance, :federating], initial_setting) end) diff --git a/test/web/api_spec/app_operation_test.exs b/test/web/api_spec/app_operation_test.exs deleted file mode 100644 index 5b96abb44..000000000 --- a/test/web/api_spec/app_operation_test.exs +++ /dev/null @@ -1,45 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.ApiSpec.AppOperationTest do - use Pleroma.Web.ConnCase, async: true - - alias Pleroma.Web.ApiSpec - alias Pleroma.Web.ApiSpec.Schemas.AppCreateRequest - alias Pleroma.Web.ApiSpec.Schemas.AppCreateResponse - - import OpenApiSpex.TestAssertions - import Pleroma.Factory - - test "AppCreateRequest example matches schema" do - api_spec = ApiSpec.spec() - schema = AppCreateRequest.schema() - assert_schema(schema.example, "AppCreateRequest", api_spec) - end - - test "AppCreateResponse example matches schema" do - api_spec = ApiSpec.spec() - schema = AppCreateResponse.schema() - assert_schema(schema.example, "AppCreateResponse", api_spec) - end - - test "AppController produces a AppCreateResponse", %{conn: conn} do - api_spec = ApiSpec.spec() - app_attrs = build(:oauth_app) - - json = - conn - |> put_req_header("content-type", "application/json") - |> post( - "/api/v1/apps", - Jason.encode!(%{ - client_name: app_attrs.client_name, - redirect_uris: app_attrs.redirect_uris - }) - ) - |> json_response(200) - - assert_schema(json, "AppCreateResponse", api_spec) - end -end diff --git a/test/web/api_spec/schema_examples_test.exs b/test/web/api_spec/schema_examples_test.exs new file mode 100644 index 000000000..88b6f07cb --- /dev/null +++ b/test/web/api_spec/schema_examples_test.exs @@ -0,0 +1,43 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.SchemaExamplesTest do + use ExUnit.Case, async: true + import Pleroma.Tests.ApiSpecHelpers + + @content_type "application/json" + + for operation <- api_operations() do + describe operation.operationId <> " Request Body" do + if operation.requestBody do + @media_type operation.requestBody.content[@content_type] + @schema resolve_schema(@media_type.schema) + + if @media_type.example do + test "request body media type example matches schema" do + assert_schema(@media_type.example, @schema) + end + end + + if @schema.example do + test "request body schema example matches schema" do + assert_schema(@schema.example, @schema) + end + end + end + end + + for {status, response} <- operation.responses do + describe "#{operation.operationId} - #{status} Response" do + @schema resolve_schema(response.content[@content_type].schema) + + if @schema.example do + test "example matches schema" do + assert_schema(@schema.example, @schema) + end + end + end + end + end +end diff --git a/test/web/mastodon_api/controllers/app_controller_test.exs b/test/web/mastodon_api/controllers/app_controller_test.exs index e7b11d14e..a0b8b126c 100644 --- a/test/web/mastodon_api/controllers/app_controller_test.exs +++ b/test/web/mastodon_api/controllers/app_controller_test.exs @@ -27,7 +27,7 @@ test "apps/verify_credentials", %{conn: conn} do "vapid_key" => Push.vapid_config() |> Keyword.get(:public_key) } - assert expected == json_response(conn, 200) + assert expected == json_response_and_validate_schema(conn, 200) end test "creates an oauth app", %{conn: conn} do @@ -55,6 +55,6 @@ test "creates an oauth app", %{conn: conn} do "vapid_key" => Push.vapid_config() |> Keyword.get(:public_key) } - assert expected == json_response(conn, 200) + assert expected == json_response_and_validate_schema(conn, 200) end end diff --git a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs index 0b2ffa470..4222556a4 100644 --- a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs +++ b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs @@ -5,15 +5,13 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiControllerTest do use Pleroma.Web.ConnCase, async: true alias Pleroma.Web.ApiSpec - alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji - alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse import OpenApiSpex.TestAssertions test "with tags", %{conn: conn} do assert resp = conn |> get("/api/v1/custom_emojis") - |> json_response(200) + |> json_response_and_validate_schema(200) assert [emoji | _body] = resp assert Map.has_key?(emoji, "shortcode") @@ -23,19 +21,6 @@ test "with tags", %{conn: conn} do assert Map.has_key?(emoji, "category") assert Map.has_key?(emoji, "url") assert Map.has_key?(emoji, "visible_in_picker") - assert_schema(resp, "CustomEmojisResponse", ApiSpec.spec()) assert_schema(emoji, "CustomEmoji", ApiSpec.spec()) end - - test "CustomEmoji example matches schema" do - api_spec = ApiSpec.spec() - schema = CustomEmoji.schema() - assert_schema(schema.example, "CustomEmoji", api_spec) - end - - test "CustomEmojisResponse example matches schema" do - api_spec = ApiSpec.spec() - schema = CustomEmojisResponse.schema() - assert_schema(schema.example, "CustomEmojisResponse", api_spec) - end end diff --git a/test/web/mastodon_api/controllers/domain_block_controller_test.exs b/test/web/mastodon_api/controllers/domain_block_controller_test.exs index d66190c90..01a24afcf 100644 --- a/test/web/mastodon_api/controllers/domain_block_controller_test.exs +++ b/test/web/mastodon_api/controllers/domain_block_controller_test.exs @@ -6,11 +6,8 @@ defmodule Pleroma.Web.MastodonAPI.DomainBlockControllerTest do use Pleroma.Web.ConnCase alias Pleroma.User - alias Pleroma.Web.ApiSpec - alias Pleroma.Web.ApiSpec.Schemas.DomainBlocksResponse import Pleroma.Factory - import OpenApiSpex.TestAssertions test "blocking / unblocking a domain" do %{user: user, conn: conn} = oauth_access(["write:blocks"]) @@ -21,7 +18,7 @@ test "blocking / unblocking a domain" do |> put_req_header("content-type", "application/json") |> post("/api/v1/domain_blocks", %{"domain" => "dogwhistle.zone"}) - assert %{} = json_response(ret_conn, 200) + assert %{} == json_response_and_validate_schema(ret_conn, 200) user = User.get_cached_by_ap_id(user.ap_id) assert User.blocks?(user, other_user) @@ -30,7 +27,7 @@ test "blocking / unblocking a domain" do |> put_req_header("content-type", "application/json") |> delete("/api/v1/domain_blocks", %{"domain" => "dogwhistle.zone"}) - assert %{} = json_response(ret_conn, 200) + assert %{} == json_response_and_validate_schema(ret_conn, 200) user = User.get_cached_by_ap_id(user.ap_id) refute User.blocks?(user, other_user) end @@ -41,21 +38,10 @@ test "getting a list of domain blocks" do {:ok, user} = User.block_domain(user, "bad.site") {:ok, user} = User.block_domain(user, "even.worse.site") - conn = - conn - |> assign(:user, user) - |> get("/api/v1/domain_blocks") - - domain_blocks = json_response(conn, 200) - - assert "bad.site" in domain_blocks - assert "even.worse.site" in domain_blocks - assert_schema(domain_blocks, "DomainBlocksResponse", ApiSpec.spec()) - end - - test "DomainBlocksResponse example matches schema" do - api_spec = ApiSpec.spec() - schema = DomainBlocksResponse.schema() - assert_schema(schema.example, "DomainBlocksResponse", api_spec) + assert ["even.worse.site", "bad.site"] == + conn + |> assign(:user, user) + |> get("/api/v1/domain_blocks") + |> json_response_and_validate_schema(200) end end From bbf8554c975ea1ba9b5c809a7891ec0fb4a8e537 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 13:48:13 +0200 Subject: [PATCH 22/34] ActivitPub: Remove `like` function. We don't need another way to build likes. --- lib/pleroma/web/activity_pub/activity_pub.ex | 30 -------- .../activity_pub/activity_pub_controller.ex | 7 +- test/web/activity_pub/activity_pub_test.exs | 77 ++----------------- test/web/activity_pub/utils_test.exs | 5 +- 4 files changed, 15 insertions(+), 104 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 4a133498e..c67b3335d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -398,36 +398,6 @@ defp do_unreact_with_emoji(user, reaction_id, options) do end end - # TODO: This is weird, maybe we shouldn't check here if we can make the activity. - @spec like(User.t(), Object.t(), String.t() | nil, boolean()) :: - {:ok, Activity.t(), Object.t()} | {:error, any()} - def like(user, object, activity_id \\ nil, local \\ true) do - with {:ok, result} <- Repo.transaction(fn -> do_like(user, object, activity_id, local) end) do - result - end - end - - defp do_like( - %User{ap_id: ap_id} = user, - %Object{data: %{"id" => _}} = object, - activity_id, - local - ) do - with nil <- get_existing_like(ap_id, object), - like_data <- make_like_data(user, object, activity_id), - {:ok, activity} <- insert(like_data, local), - {:ok, object} <- add_like_to_object(activity, object), - :ok <- maybe_federate(activity) do - {:ok, activity, object} - else - %Activity{} = activity -> - {:ok, activity, object} - - {:error, error} -> - Repo.rollback(error) - end - end - @spec unlike(User.t(), Object.t(), String.t() | nil, boolean()) :: {:ok, Activity.t(), Activity.t(), Object.t()} | {:ok, Object.t()} | {:error, any()} def unlike(%User{} = actor, %Object{} = object, activity_id \\ nil, local \\ true) do diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 8b9eb4a2c..325a714b4 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -12,6 +12,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.Plugs.EnsureAuthenticatedPlug alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.ObjectView alias Pleroma.Web.ActivityPub.Relay @@ -421,7 +423,10 @@ defp handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do defp handle_user_activity(%User{} = user, %{"type" => "Like"} = params) do with %Object{} = object <- Object.normalize(params["object"]), - {:ok, activity, _object} <- ActivityPub.like(user, object) do + {_, {:ok, like_object, meta}} <- {:build_object, Builder.like(user, object)}, + {_, {:ok, %Activity{} = activity, _meta}} <- + {:common_pipeline, + Pipeline.common_pipeline(like_object, Keyword.put(meta, :local, true))} do {:ok, activity} else _ -> {:error, dgettext("errors", "Can't like object")} diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 6410df49b..53176917e 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -994,72 +994,6 @@ test "reverts emoji unreact on error" do end end - describe "like an object" do - test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do - 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(Federator.publish(like_activity)) - 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 "reverts like activity on error" do - note_activity = insert(:note_activity) - object = Object.normalize(note_activity) - user = insert(:user) - - with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do - assert {:error, :reverted} = ActivityPub.like(user, object) - end - - assert Repo.aggregate(Activity, :count, :id) == 1 - assert Repo.get(Object, object.id) == object - end - - test "adds a like activity to the db" do - note_activity = insert(:note_activity) - assert object = Object.normalize(note_activity) - - user = insert(:user) - user_two = insert(:user) - - {:ok, like_activity, object} = ActivityPub.like(user, object) - - assert like_activity.data["actor"] == user.ap_id - assert like_activity.data["type"] == "Like" - assert like_activity.data["object"] == object.data["id"] - assert like_activity.data["to"] == [User.ap_followers(user), note_activity.data["actor"]] - assert like_activity.data["context"] == object.data["context"] - assert object.data["like_count"] == 1 - assert object.data["likes"] == [user.ap_id] - - # Just return the original activity if the user already liked it. - {:ok, same_like_activity, object} = ActivityPub.like(user, object) - - assert like_activity == same_like_activity - assert object.data["likes"] == [user.ap_id] - assert object.data["like_count"] == 1 - - {:ok, _like_activity, object} = ActivityPub.like(user_two, object) - assert object.data["like_count"] == 2 - end - end - describe "unliking" do test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do Config.put([:instance, :federating], true) @@ -1071,7 +1005,8 @@ test "adds a like activity to the db" do {:ok, object} = ActivityPub.unlike(user, object) refute called(Federator.publish()) - {:ok, _like_activity, object} = ActivityPub.like(user, object) + {:ok, _like_activity} = CommonAPI.favorite(user, note_activity.id) + object = Object.get_by_id(object.id) assert object.data["like_count"] == 1 {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) @@ -1082,10 +1017,10 @@ test "adds a like activity to the db" do test "reverts unliking on error" do note_activity = insert(:note_activity) - object = Object.normalize(note_activity) user = insert(:user) - {:ok, like_activity, object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) + object = Object.normalize(note_activity) assert object.data["like_count"] == 1 with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do @@ -1106,7 +1041,9 @@ test "unliking a previously liked object" do {:ok, object} = ActivityPub.unlike(user, object) assert object.data["like_count"] == 0 - {:ok, like_activity, object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) + + object = Object.get_by_id(object.id) assert object.data["like_count"] == 1 {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) diff --git a/test/web/activity_pub/utils_test.exs b/test/web/activity_pub/utils_test.exs index e913a5148..b0bfed917 100644 --- a/test/web/activity_pub/utils_test.exs +++ b/test/web/activity_pub/utils_test.exs @@ -224,8 +224,7 @@ test "fetches only Create activities" do object = Object.normalize(activity) {:ok, [vote], object} = CommonAPI.vote(other_user, object, [0]) - vote_object = Object.normalize(vote) - {:ok, _activity, _object} = ActivityPub.like(user, vote_object) + {:ok, _activity} = CommonAPI.favorite(user, activity.id) [fetched_vote] = Utils.get_existing_votes(other_user.ap_id, object) assert fetched_vote.id == vote.id end @@ -346,7 +345,7 @@ test "fetches existing like" do user = insert(:user) refute Utils.get_existing_like(user.ap_id, object) - {:ok, like_activity, _object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) assert ^like_activity = Utils.get_existing_like(user.ap_id, object) end From 1df6af2a4c93257f94e2780e4317cfcf9bef7adb Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 13:59:48 +0200 Subject: [PATCH 23/34] Credo fixes. --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 325a714b4..d625530ec 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -13,9 +13,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder - alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.ObjectView + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Relay alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.UserView From cb12585098e0cc1e2e85d253812e1898e8034b7f Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 14:37:53 +0200 Subject: [PATCH 24/34] Announcements: Prevent race condition. --- lib/pleroma/web/activity_pub/activity_pub.ex | 1 + test/web/common_api/common_api_test.exs | 27 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index c67b3335d..4cce4f13c 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -438,6 +438,7 @@ def announce( defp do_announce(user, object, activity_id, local, public) do with true <- is_announceable?(object, user, public), + object <- Object.get_by_id(object.id), announce_data <- make_announce_data(user, object, activity_id, public), {:ok, activity} <- insert(announce_data, local), {:ok, object} <- add_announce_to_object(activity, object), diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index e87193c83..1758662b0 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -48,6 +48,33 @@ test "favoriting race condition" do assert object.data["like_count"] == 20 end + test "repeating race condition" do + user = insert(:user) + users_serial = insert_list(10, :user) + users = insert_list(10, :user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "."}) + + users_serial + |> Enum.map(fn user -> + CommonAPI.repeat(activity.id, user) + end) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["announcement_count"] == 10 + + users + |> Enum.map(fn user -> + Task.async(fn -> + CommonAPI.repeat(activity.id, user) + end) + end) + |> Enum.map(&Task.await/1) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["announcement_count"] == 20 + end + test "when replying to a conversation / participation, it will set the correct context id even if no explicit reply_to is given" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => ".", "visibility" => "direct"}) From 6e625a427cdc829714ad0365560d79aa4ee9c2e5 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 4 Dec 2019 09:49:17 +0300 Subject: [PATCH 25/34] reply filtering --- CHANGELOG.md | 1 + benchmarks/load_testing/fetcher.ex | 53 ++ docs/API/differences_in_mastoapi_responses.md | 2 +- lib/pleroma/user.ex | 14 + lib/pleroma/user/query.ex | 4 +- lib/pleroma/web/activity_pub/activity_pub.ex | 77 ++- lib/pleroma/web/common_api/activity_draft.ex | 16 +- .../controllers/timeline_controller.ex | 1 + test/web/activity_pub/activity_pub_test.exs | 486 ++++++++++++++++++ 9 files changed, 636 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 702c58180..affabcd95 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/). API Changes - Mastodon API: Support for `include_types` in `/api/v1/notifications`. - Mastodon API: Added `/api/v1/notifications/:id/dismiss` endpoint. +- Mastodon API: Add support for filtering replies in public and friends timelines - Admin API: endpoints for create/update/delete OAuth Apps. diff --git a/benchmarks/load_testing/fetcher.ex b/benchmarks/load_testing/fetcher.ex index 786929ace..3aa82b48a 100644 --- a/benchmarks/load_testing/fetcher.ex +++ b/benchmarks/load_testing/fetcher.ex @@ -495,4 +495,57 @@ defp render_long_thread(user) do formatters: formatters() ) end + + def query_replies(user) do + public_params = %{ + "type" => ["Create", "Announce"], + "local_only" => false, + "blocking_user" => user, + "muting_user" => user, + "count" => 20 + } + + Benchee.run(%{ + "Public timeline without reply filtering" => fn -> + ActivityPub.fetch_public_activities(public_params) + end, + "Public timeline with reply filtering - following" => fn -> + public_params + |> Map.put("reply_visibility", "following") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + end, + "Public timeline with reply filtering - self" => fn -> + public_params + |> Map.put("reply_visibility", "self") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + end + }) + + private_params = %{ + "type" => ["Create", "Announce"], + "blocking_user" => user, + "muting_user" => user, + "user" => user, + "count" => 20 + } + + recipients = [user.ap_id | User.following(user)] + + Benchee.run(%{ + "Home timeline without reply filtering" => fn -> + ActivityPub.fetch_activities(recipients, private_params) + end, + "Home timeline with reply filtering - following" => fn -> + private_params = Map.put(private_params, "reply_visibility", "following") + + ActivityPub.fetch_activities(recipients, private_params) + end, + "Home timeline with reply filtering - self" => fn -> + private_params = Map.put(private_params, "reply_visibility", "self") + ActivityPub.fetch_activities(recipients, private_params) + end + }) + end end diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index 1059155cf..c97fb8c56 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -14,7 +14,7 @@ Some apps operate under the assumption that no more than 4 attachments can be re Adding the parameter `with_muted=true` to the timeline queries will also return activities by muted (not by blocked!) users. Adding the parameter `exclude_visibilities` to the timeline queries will exclude the statuses with the given visibilities. The parameter accepts an array of visibility types (`public`, `unlisted`, `private`, `direct`), e.g., `exclude_visibilities[]=direct&exclude_visibilities[]=private`. - +Adding the parameter `reply_visibility` to the public and friends timelines quieries will filter replies. Possible values: without parameter (default) shows all replies, `following` - replies directed to you or users you follow, `self` - replies directed to you. ## Statuses - `visibility`: has an additional possible value `list` diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 477237756..b451202b2 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -832,6 +832,7 @@ def set_cache({:error, err}), do: {:error, err} def set_cache(%User{} = user) do Cachex.put(:user_cache, "ap_id:#{user.ap_id}", user) Cachex.put(:user_cache, "nickname:#{user.nickname}", user) + Cachex.put(:user_cache, "friends_ap_ids:#{user.nickname}", get_user_friends_ap_ids(user)) {:ok, user} end @@ -847,9 +848,22 @@ def update_and_set_cache(changeset) do end end + def get_user_friends_ap_ids(user) do + from(u in User.get_friends_query(user), select: u.ap_id) + |> Repo.all() + end + + @spec get_cached_user_friends_ap_ids(User.t()) :: [String.t()] + def get_cached_user_friends_ap_ids(user) do + Cachex.fetch!(:user_cache, "friends_ap_ids:#{user.ap_id}", fn _ -> + get_user_friends_ap_ids(user) + end) + end + def invalidate_cache(user) do Cachex.del(:user_cache, "ap_id:#{user.ap_id}") Cachex.del(:user_cache, "nickname:#{user.nickname}") + Cachex.del(:user_cache, "friends_ap_ids:#{user.ap_id}") end @spec get_cached_by_ap_id(String.t()) :: User.t() | nil diff --git a/lib/pleroma/user/query.ex b/lib/pleroma/user/query.ex index ec88088cf..ac77aab71 100644 --- a/lib/pleroma/user/query.ex +++ b/lib/pleroma/user/query.ex @@ -54,13 +54,13 @@ defmodule Pleroma.User.Query do select: term(), limit: pos_integer() } - | %{} + | map() @ilike_criteria [:nickname, :name, :query] @equal_criteria [:email] @contains_criteria [:ap_id, :nickname] - @spec build(criteria()) :: Query.t() + @spec build(Query.t(), criteria()) :: Query.t() def build(query \\ base_query(), criteria) do prepare_query(query, criteria) end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index c67b3335d..8b170b7f8 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -270,9 +270,9 @@ defp do_create(%{to: to, actor: actor, context: context, object: object} = param ), {:ok, activity} <- insert(create_data, local, fake), {:fake, false, activity} <- {:fake, fake, activity}, + {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, _ <- increase_replies_count_if_reply(create_data), _ <- increase_poll_votes_if_vote(create_data), - {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, {:ok, _actor} <- increase_note_count_if_public(actor, activity), :ok <- maybe_federate(activity) do {:ok, activity} @@ -700,12 +700,14 @@ def fetch_activities_for_context_query(context, opts) do do: [opts["user"].ap_id | User.following(opts["user"])] ++ public, else: public + opts = Map.put(opts, "user", opts["user"]) + from(activity in Activity) |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_set_thread_muted_field(opts) |> restrict_blocked(opts) - |> restrict_recipients(recipients, opts["user"]) + |> restrict_recipients(recipients, opts) |> where( [activity], fragment( @@ -740,7 +742,10 @@ def fetch_latest_activity_id_for_context(context, opts \\ %{}) do @spec fetch_public_activities(map(), Pagination.type()) :: [Activity.t()] def fetch_public_activities(opts \\ %{}, pagination \\ :keyset) do - opts = Map.drop(opts, ["user"]) + opts = + opts + |> Map.put("reply_user", opts["user"]) + |> Map.delete("user") [Constants.as_public()] |> fetch_activities_query(opts) @@ -976,13 +981,65 @@ defp restrict_tag(query, %{"tag" => tag}) when is_binary(tag) do defp restrict_tag(query, _), do: query - defp restrict_recipients(query, [], _user), do: query - - defp restrict_recipients(query, recipients, nil) do - from(activity in query, where: fragment("? && ?", ^recipients, activity.recipients)) + defp reply_recipients(user, "following") do + [user.ap_id | User.get_cached_user_friends_ap_ids(user)] end - defp restrict_recipients(query, recipients, user) do + defp reply_recipients(user, "self"), do: [user.ap_id] + + defp restrict_recipients(query, [], _opts), do: query + + defp restrict_recipients( + query, + recipients, + %{"user" => nil, "reply_user" => user, "reply_visibility" => visibility} + ) + when not is_nil(user) and visibility in ["following", "self"] do + reply_recipients = reply_recipients(user, visibility) + + from([activity, object] in query, + where: + fragment( + "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ? OR ? = ?)", + ^recipients, + activity.recipients, + object.data, + activity.recipients, + activity.actor, + ^reply_recipients, + activity.actor, + ^user.ap_id + ) + ) + end + + defp restrict_recipients(query, recipients, %{"user" => nil}) do + from(activity in query, + where: fragment("? && ?", ^recipients, activity.recipients) + ) + end + + defp restrict_recipients(query, recipients, %{"user" => user, "reply_visibility" => visibility}) + when visibility in ["following", "self"] do + reply_recipients = reply_recipients(user, visibility) + + from( + [activity, object] in query, + where: + fragment( + "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ?)", + ^recipients, + activity.recipients, + object.data, + activity.recipients, + activity.actor, + ^reply_recipients + ), + or_where: activity.actor == ^user.ap_id + ) + end + + defp restrict_recipients(query, recipients, %{"user" => user}) do from( activity in query, where: fragment("? && ?", ^recipients, activity.recipients), @@ -1254,13 +1311,15 @@ def fetch_activities_query(recipients, opts \\ %{}) do skip_thread_containment: Config.get([:instance, :skip_thread_containment]) } + opts = Map.put(opts, "user", opts["user"]) + Activity |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_preload_report_notes(opts) |> maybe_set_thread_muted_field(opts) |> maybe_order(opts) - |> restrict_recipients(recipients, opts["user"]) + |> restrict_recipients(recipients, opts) |> restrict_tag(opts) |> restrict_tag_reject(opts) |> restrict_tag_all(opts) diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex index c1cd15bb2..244cf2be5 100644 --- a/lib/pleroma/web/common_api/activity_draft.ex +++ b/lib/pleroma/web/common_api/activity_draft.ex @@ -84,14 +84,18 @@ defp attachments(%{params: params} = draft) do %__MODULE__{draft | attachments: attachments} end - defp in_reply_to(draft) do - case Map.get(draft.params, "in_reply_to_status_id") do - "" -> draft - nil -> draft - id -> %__MODULE__{draft | in_reply_to: Activity.get_by_id(id)} - end + defp in_reply_to(%{params: %{"in_reply_to_status_id" => ""}} = draft), do: draft + + defp in_reply_to(%{params: %{"in_reply_to_status_id" => id}} = draft) when is_binary(id) do + %__MODULE__{draft | in_reply_to: Activity.get_by_id(id)} end + defp in_reply_to(%{params: %{"in_reply_to_status_id" => %Activity{} = in_reply_to}} = draft) do + %__MODULE__{draft | in_reply_to: in_reply_to} + end + + defp in_reply_to(draft), do: draft + defp in_reply_to_conversation(draft) do in_reply_to_conversation = Participation.get(draft.params["in_reply_to_conversation_id"]) %__MODULE__{draft | in_reply_to_conversation: in_reply_to_conversation} diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index b3c58005e..a2ac9301e 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -100,6 +100,7 @@ def public(%{assigns: %{user: user}} = conn, params) do |> Map.put("local_only", local_only) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) + |> Map.put("user", user) |> ActivityPub.fetch_public_activities() conn diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 53176917e..8a1638a23 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1910,4 +1910,490 @@ test "old user must be in the new user's `also_known_as` list" do ActivityPub.move(old_user, new_user) end end + + test "doesn't retrieve replies activities with exclude_replies" do + user = insert(:user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "yeah"}) + + {:ok, _reply} = + CommonAPI.post(user, %{"status" => "yeah", "in_reply_to_status_id" => activity.id}) + + [result] = ActivityPub.fetch_public_activities(%{"exclude_replies" => "true"}) + + assert result.id == activity.id + + assert length(ActivityPub.fetch_public_activities()) == 2 + end + + describe "replies filtering with public messages" do + setup :public_messages + + test "public timeline", %{users: %{u1: user}} do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert length(activities_ids) == 16 + end + + test "public timeline with reply_visibility `following`", %{ + users: %{u1: user}, + u1: u1, + u2: u2, + u3: u3, + u4: u4, + activities: activities + } do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("reply_visibility", "following") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert length(activities_ids) == 14 + + visible_ids = + Map.values(u1) ++ Map.values(u2) ++ Map.values(u4) ++ Map.values(activities) ++ [u3[:r1]] + + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + + test "public timeline with reply_visibility `self`", %{ + users: %{u1: user}, + u1: u1, + u2: u2, + u3: u3, + u4: u4, + activities: activities + } do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("reply_visibility", "self") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert length(activities_ids) == 10 + visible_ids = Map.values(u1) ++ [u2[:r1], u3[:r1], u4[:r1]] ++ Map.values(activities) + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + + test "home timeline", %{ + users: %{u1: user}, + activities: activities, + u1: u1, + u2: u2, + u3: u3, + u4: u4 + } do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 13 + + visible_ids = + Map.values(u1) ++ + Map.values(u3) ++ + [ + activities[:a1], + activities[:a2], + activities[:a4], + u2[:r1], + u2[:r3], + u4[:r1], + u4[:r2] + ] + + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + + test "home timeline with reply_visibility `following`", %{ + users: %{u1: user}, + activities: activities, + u1: u1, + u2: u2, + u3: u3, + u4: u4 + } do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> Map.put("reply_visibility", "following") + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 11 + + visible_ids = + Map.values(u1) ++ + [ + activities[:a1], + activities[:a2], + activities[:a4], + u2[:r1], + u2[:r3], + u3[:r1], + u4[:r1], + u4[:r2] + ] + + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + + test "home timeline with reply_visibility `self`", %{ + users: %{u1: user}, + activities: activities, + u1: u1, + u2: u2, + u3: u3, + u4: u4 + } do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> Map.put("reply_visibility", "self") + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 9 + + visible_ids = + Map.values(u1) ++ + [ + activities[:a1], + activities[:a2], + activities[:a4], + u2[:r1], + u3[:r1], + u4[:r1] + ] + + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + end + + describe "replies filtering with private messages" do + setup :private_messages + + test "public timeline", %{users: %{u1: user}} do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert activities_ids == [] + end + + test "public timeline with default reply_visibility `following`", %{users: %{u1: user}} do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("reply_visibility", "following") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert activities_ids == [] + end + + test "public timeline with default reply_visibility `self`", %{users: %{u1: user}} do + activities_ids = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("local_only", false) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("reply_visibility", "self") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + |> Enum.map(& &1.id) + + assert activities_ids == [] + end + + test "home timeline", %{users: %{u1: user}} do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 12 + end + + test "home timeline with default reply_visibility `following`", %{users: %{u1: user}} do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> Map.put("reply_visibility", "following") + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 12 + end + + test "home timeline with default reply_visibility `self`", %{ + users: %{u1: user}, + activities: activities, + u1: u1, + u2: u2, + u3: u3, + u4: u4 + } do + params = + %{} + |> Map.put("type", ["Create", "Announce"]) + |> Map.put("blocking_user", user) + |> Map.put("muting_user", user) + |> Map.put("user", user) + |> Map.put("reply_visibility", "self") + + activities_ids = + ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) + |> Enum.map(& &1.id) + + assert length(activities_ids) == 10 + + visible_ids = + Map.values(u1) ++ Map.values(u4) ++ [u2[:r1], u3[:r1]] ++ Map.values(activities) + + assert Enum.all?(visible_ids, &(&1 in activities_ids)) + end + end + + defp public_messages(_) do + [u1, u2, u3, u4] = insert_list(4, :user) + {:ok, u1} = User.follow(u1, u2) + {:ok, u2} = User.follow(u2, u1) + {:ok, u1} = User.follow(u1, u4) + {:ok, u4} = User.follow(u4, u1) + + {:ok, u2} = User.follow(u2, u3) + {:ok, u3} = User.follow(u3, u2) + + {:ok, a1} = CommonAPI.post(u1, %{"status" => "Status"}) + + {:ok, r1_1} = + CommonAPI.post(u2, %{ + "status" => "@#{u1.nickname} reply from u2 to u1", + "in_reply_to_status_id" => a1.id + }) + + {:ok, r1_2} = + CommonAPI.post(u3, %{ + "status" => "@#{u1.nickname} reply from u3 to u1", + "in_reply_to_status_id" => a1.id + }) + + {:ok, r1_3} = + CommonAPI.post(u4, %{ + "status" => "@#{u1.nickname} reply from u4 to u1", + "in_reply_to_status_id" => a1.id + }) + + {:ok, a2} = CommonAPI.post(u2, %{"status" => "Status"}) + + {:ok, r2_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u2.nickname} reply from u1 to u2", + "in_reply_to_status_id" => a2.id + }) + + {:ok, r2_2} = + CommonAPI.post(u3, %{ + "status" => "@#{u2.nickname} reply from u3 to u2", + "in_reply_to_status_id" => a2.id + }) + + {:ok, r2_3} = + CommonAPI.post(u4, %{ + "status" => "@#{u2.nickname} reply from u4 to u2", + "in_reply_to_status_id" => a2.id + }) + + {:ok, a3} = CommonAPI.post(u3, %{"status" => "Status"}) + + {:ok, r3_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u3.nickname} reply from u1 to u3", + "in_reply_to_status_id" => a3.id + }) + + {:ok, r3_2} = + CommonAPI.post(u2, %{ + "status" => "@#{u3.nickname} reply from u2 to u3", + "in_reply_to_status_id" => a3.id + }) + + {:ok, r3_3} = + CommonAPI.post(u4, %{ + "status" => "@#{u3.nickname} reply from u4 to u3", + "in_reply_to_status_id" => a3.id + }) + + {:ok, a4} = CommonAPI.post(u4, %{"status" => "Status"}) + + {:ok, r4_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u4.nickname} reply from u1 to u4", + "in_reply_to_status_id" => a4.id + }) + + {:ok, r4_2} = + CommonAPI.post(u2, %{ + "status" => "@#{u4.nickname} reply from u2 to u4", + "in_reply_to_status_id" => a4.id + }) + + {:ok, r4_3} = + CommonAPI.post(u3, %{ + "status" => "@#{u4.nickname} reply from u3 to u4", + "in_reply_to_status_id" => a4.id + }) + + {:ok, + users: %{u1: u1, u2: u2, u3: u3, u4: u4}, + activities: %{a1: a1.id, a2: a2.id, a3: a3.id, a4: a4.id}, + u1: %{r1: r1_1.id, r2: r1_2.id, r3: r1_3.id}, + u2: %{r1: r2_1.id, r2: r2_2.id, r3: r2_3.id}, + u3: %{r1: r3_1.id, r2: r3_2.id, r3: r3_3.id}, + u4: %{r1: r4_1.id, r2: r4_2.id, r3: r4_3.id}} + end + + defp private_messages(_) do + [u1, u2, u3, u4] = insert_list(4, :user) + {:ok, u1} = User.follow(u1, u2) + {:ok, u2} = User.follow(u2, u1) + {:ok, u1} = User.follow(u1, u3) + {:ok, u3} = User.follow(u3, u1) + {:ok, u1} = User.follow(u1, u4) + {:ok, u4} = User.follow(u4, u1) + + {:ok, u2} = User.follow(u2, u3) + {:ok, u3} = User.follow(u3, u2) + + {:ok, a1} = CommonAPI.post(u1, %{"status" => "Status", "visibility" => "private"}) + + {:ok, r1_1} = + CommonAPI.post(u2, %{ + "status" => "@#{u1.nickname} reply from u2 to u1", + "in_reply_to_status_id" => a1.id, + "visibility" => "private" + }) + + {:ok, r1_2} = + CommonAPI.post(u3, %{ + "status" => "@#{u1.nickname} reply from u3 to u1", + "in_reply_to_status_id" => a1.id, + "visibility" => "private" + }) + + {:ok, r1_3} = + CommonAPI.post(u4, %{ + "status" => "@#{u1.nickname} reply from u4 to u1", + "in_reply_to_status_id" => a1.id, + "visibility" => "private" + }) + + {:ok, a2} = CommonAPI.post(u2, %{"status" => "Status", "visibility" => "private"}) + + {:ok, r2_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u2.nickname} reply from u1 to u2", + "in_reply_to_status_id" => a2.id, + "visibility" => "private" + }) + + {:ok, r2_2} = + CommonAPI.post(u3, %{ + "status" => "@#{u2.nickname} reply from u3 to u2", + "in_reply_to_status_id" => a2.id, + "visibility" => "private" + }) + + {:ok, a3} = CommonAPI.post(u3, %{"status" => "Status", "visibility" => "private"}) + + {:ok, r3_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u3.nickname} reply from u1 to u3", + "in_reply_to_status_id" => a3.id, + "visibility" => "private" + }) + + {:ok, r3_2} = + CommonAPI.post(u2, %{ + "status" => "@#{u3.nickname} reply from u2 to u3", + "in_reply_to_status_id" => a3.id, + "visibility" => "private" + }) + + {:ok, a4} = CommonAPI.post(u4, %{"status" => "Status", "visibility" => "private"}) + + {:ok, r4_1} = + CommonAPI.post(u1, %{ + "status" => "@#{u4.nickname} reply from u1 to u4", + "in_reply_to_status_id" => a4.id, + "visibility" => "private" + }) + + {:ok, + users: %{u1: u1, u2: u2, u3: u3, u4: u4}, + activities: %{a1: a1.id, a2: a2.id, a3: a3.id, a4: a4.id}, + u1: %{r1: r1_1.id, r2: r1_2.id, r3: r1_3.id}, + u2: %{r1: r2_1.id, r2: r2_2.id}, + u3: %{r1: r3_1.id, r2: r3_2.id}, + u4: %{r1: r4_1.id}} + end end From be34672d6768bdc9ece96669e07e940a98c9d933 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 4 Dec 2019 10:29:26 +0300 Subject: [PATCH 26/34] formatting --- docs/API/differences_in_mastoapi_responses.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index c97fb8c56..92086136f 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -15,6 +15,7 @@ Some apps operate under the assumption that no more than 4 attachments can be re Adding the parameter `with_muted=true` to the timeline queries will also return activities by muted (not by blocked!) users. Adding the parameter `exclude_visibilities` to the timeline queries will exclude the statuses with the given visibilities. The parameter accepts an array of visibility types (`public`, `unlisted`, `private`, `direct`), e.g., `exclude_visibilities[]=direct&exclude_visibilities[]=private`. Adding the parameter `reply_visibility` to the public and friends timelines quieries will filter replies. Possible values: without parameter (default) shows all replies, `following` - replies directed to you or users you follow, `self` - replies directed to you. + ## Statuses - `visibility`: has an additional possible value `list` From 1a75ef63b2f6fef96b9bf9d07b4963fb217d4017 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Fri, 24 Apr 2020 09:24:08 +0300 Subject: [PATCH 27/34] updating benchmarks --- benchmarks/load_testing/activities.ex | 2 +- benchmarks/load_testing/fetcher.ex | 85 +++++++++++++-------------- 2 files changed, 41 insertions(+), 46 deletions(-) diff --git a/benchmarks/load_testing/activities.ex b/benchmarks/load_testing/activities.ex index 23ee2b987..2b032943b 100644 --- a/benchmarks/load_testing/activities.ex +++ b/benchmarks/load_testing/activities.ex @@ -313,7 +313,7 @@ defp insert_activity("simple_thread", visibility, group, user, friends, non_frie tasks = get_reply_tasks(visibility, group) {:ok, activity} = - CommonAPI.post(user, %{"status" => "Simple status", "visibility" => "unlisted"}) + CommonAPI.post(user, %{"status" => "Simple status", "visibility" => visibility}) acc = {activity.id, ["@" <> actor.nickname, "reply to status"]} insert_replies(tasks, visibility, user, friends, non_friends, acc) diff --git a/benchmarks/load_testing/fetcher.ex b/benchmarks/load_testing/fetcher.ex index 3aa82b48a..6503deb41 100644 --- a/benchmarks/load_testing/fetcher.ex +++ b/benchmarks/load_testing/fetcher.ex @@ -41,6 +41,7 @@ defp fetch_timelines(user) do fetch_notifications(user) fetch_favourites(user) fetch_long_thread(user) + fetch_timelines_with_reply_filtering(user) end defp render_views(user) do @@ -496,56 +497,50 @@ defp render_long_thread(user) do ) end - def query_replies(user) do - public_params = %{ - "type" => ["Create", "Announce"], - "local_only" => false, - "blocking_user" => user, - "muting_user" => user, - "count" => 20 - } + defp fetch_timelines_with_reply_filtering(user) do + public_params = opts_for_public_timeline(user) - Benchee.run(%{ - "Public timeline without reply filtering" => fn -> - ActivityPub.fetch_public_activities(public_params) - end, - "Public timeline with reply filtering - following" => fn -> - public_params - |> Map.put("reply_visibility", "following") - |> Map.put("user", user) - |> ActivityPub.fetch_public_activities() - end, - "Public timeline with reply filtering - self" => fn -> - public_params - |> Map.put("reply_visibility", "self") - |> Map.put("user", user) - |> ActivityPub.fetch_public_activities() - end - }) + Benchee.run( + %{ + "Public timeline without reply filtering" => fn -> + ActivityPub.fetch_public_activities(public_params) + end, + "Public timeline with reply filtering - following" => fn -> + public_params + |> Map.put("reply_visibility", "following") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + end, + "Public timeline with reply filtering - self" => fn -> + public_params + |> Map.put("reply_visibility", "self") + |> Map.put("user", user) + |> ActivityPub.fetch_public_activities() + end + }, + formatters: formatters() + ) - private_params = %{ - "type" => ["Create", "Announce"], - "blocking_user" => user, - "muting_user" => user, - "user" => user, - "count" => 20 - } + private_params = opts_for_home_timeline(user) recipients = [user.ap_id | User.following(user)] - Benchee.run(%{ - "Home timeline without reply filtering" => fn -> - ActivityPub.fetch_activities(recipients, private_params) - end, - "Home timeline with reply filtering - following" => fn -> - private_params = Map.put(private_params, "reply_visibility", "following") + Benchee.run( + %{ + "Home timeline without reply filtering" => fn -> + ActivityPub.fetch_activities(recipients, private_params) + end, + "Home timeline with reply filtering - following" => fn -> + private_params = Map.put(private_params, "reply_visibility", "following") - ActivityPub.fetch_activities(recipients, private_params) - end, - "Home timeline with reply filtering - self" => fn -> - private_params = Map.put(private_params, "reply_visibility", "self") - ActivityPub.fetch_activities(recipients, private_params) - end - }) + ActivityPub.fetch_activities(recipients, private_params) + end, + "Home timeline with reply filtering - self" => fn -> + private_params = Map.put(private_params, "reply_visibility", "self") + ActivityPub.fetch_activities(recipients, private_params) + end + }, + formatters: formatters() + ) end end From 375ab05234a3590c161a2a5a4f715fbc61dafb34 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Fri, 24 Apr 2020 09:57:30 +0300 Subject: [PATCH 28/34] bench sync --- benchmarks/load_testing/activities.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/load_testing/activities.ex b/benchmarks/load_testing/activities.ex index 2b032943b..482e42fc1 100644 --- a/benchmarks/load_testing/activities.ex +++ b/benchmarks/load_testing/activities.ex @@ -279,7 +279,7 @@ defp insert_activity("like", visibility, group, user, friends, non_friends, opts actor = get_actor(group, user, friends, non_friends) with activity_id when not is_nil(activity_id) <- get_random_create_activity_id(), - {:ok, _activity, _object} <- CommonAPI.favorite(activity_id, actor) do + {:ok, _activity} <- CommonAPI.favorite(actor, activity_id) do :ok else {:error, _} -> From 8480f84615b696965d3c1ca34b5847af99fbdece Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 10:26:54 +0000 Subject: [PATCH 29/34] Update differences_in_mastoapi_responses.md --- docs/API/differences_in_mastoapi_responses.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index 92086136f..41ceda26b 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -14,7 +14,7 @@ Some apps operate under the assumption that no more than 4 attachments can be re Adding the parameter `with_muted=true` to the timeline queries will also return activities by muted (not by blocked!) users. Adding the parameter `exclude_visibilities` to the timeline queries will exclude the statuses with the given visibilities. The parameter accepts an array of visibility types (`public`, `unlisted`, `private`, `direct`), e.g., `exclude_visibilities[]=direct&exclude_visibilities[]=private`. -Adding the parameter `reply_visibility` to the public and friends timelines quieries will filter replies. Possible values: without parameter (default) shows all replies, `following` - replies directed to you or users you follow, `self` - replies directed to you. +Adding the parameter `reply_visibility` to the public and home timelines queries will filter replies. Possible values: without parameter (default) shows all replies, `following` - replies directed to you or users you follow, `self` - replies directed to you. ## Statuses From e2f3030c868ca087915294909dee304f7de1730f Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 10:27:51 +0000 Subject: [PATCH 30/34] Apply suggestion to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index affabcd95..ccc6a5bd4 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/). API Changes - Mastodon API: Support for `include_types` in `/api/v1/notifications`. - Mastodon API: Added `/api/v1/notifications/:id/dismiss` endpoint. -- Mastodon API: Add support for filtering replies in public and friends timelines +- Mastodon API: Add support for filtering replies in public and home timelines - Admin API: endpoints for create/update/delete OAuth Apps. From d89cd0a19733eec27b79b768df2e30a68bfc6d6b Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 18:25:26 +0200 Subject: [PATCH 31/34] Reply Filtering: Refactor. --- benchmarks/load_testing/fetcher.ex | 15 ++- benchmarks/mix/tasks/pleroma/load_testing.ex | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 114 +++++++----------- .../controllers/timeline_controller.ex | 3 +- test/web/activity_pub/activity_pub_test.exs | 13 +- 5 files changed, 69 insertions(+), 77 deletions(-) diff --git a/benchmarks/load_testing/fetcher.ex b/benchmarks/load_testing/fetcher.ex index 6503deb41..12c30f6f5 100644 --- a/benchmarks/load_testing/fetcher.ex +++ b/benchmarks/load_testing/fetcher.ex @@ -508,13 +508,13 @@ defp fetch_timelines_with_reply_filtering(user) do "Public timeline with reply filtering - following" => fn -> public_params |> Map.put("reply_visibility", "following") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() end, "Public timeline with reply filtering - self" => fn -> public_params |> Map.put("reply_visibility", "self") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() end }, @@ -531,12 +531,19 @@ defp fetch_timelines_with_reply_filtering(user) do ActivityPub.fetch_activities(recipients, private_params) end, "Home timeline with reply filtering - following" => fn -> - private_params = Map.put(private_params, "reply_visibility", "following") + private_params = + private_params + |> Map.put("reply_filtering_user", user) + |> Map.put("reply_visibility", "following") ActivityPub.fetch_activities(recipients, private_params) end, "Home timeline with reply filtering - self" => fn -> - private_params = Map.put(private_params, "reply_visibility", "self") + private_params = + private_params + |> Map.put("reply_filtering_user", user) + |> Map.put("reply_visibility", "self") + ActivityPub.fetch_activities(recipients, private_params) end }, diff --git a/benchmarks/mix/tasks/pleroma/load_testing.ex b/benchmarks/mix/tasks/pleroma/load_testing.ex index 72b225f09..388883240 100644 --- a/benchmarks/mix/tasks/pleroma/load_testing.ex +++ b/benchmarks/mix/tasks/pleroma/load_testing.ex @@ -44,6 +44,7 @@ defmodule Mix.Tasks.Pleroma.LoadTesting do ] def run(args) do + Logger.configure(level: :error) Mix.Pleroma.start_pleroma() clean_tables() {opts, _} = OptionParser.parse!(args, strict: @switches, aliases: @aliases) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 8b170b7f8..9ec31fb03 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -270,9 +270,9 @@ defp do_create(%{to: to, actor: actor, context: context, object: object} = param ), {:ok, activity} <- insert(create_data, local, fake), {:fake, false, activity} <- {:fake, fake, activity}, - {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, _ <- increase_replies_count_if_reply(create_data), _ <- increase_poll_votes_if_vote(create_data), + {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, {:ok, _actor} <- increase_note_count_if_public(actor, activity), :ok <- maybe_federate(activity) do {:ok, activity} @@ -700,14 +700,12 @@ def fetch_activities_for_context_query(context, opts) do do: [opts["user"].ap_id | User.following(opts["user"])] ++ public, else: public - opts = Map.put(opts, "user", opts["user"]) - from(activity in Activity) |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_set_thread_muted_field(opts) |> restrict_blocked(opts) - |> restrict_recipients(recipients, opts) + |> restrict_recipients(recipients, opts["user"]) |> where( [activity], fragment( @@ -742,10 +740,7 @@ def fetch_latest_activity_id_for_context(context, opts \\ %{}) do @spec fetch_public_activities(map(), Pagination.type()) :: [Activity.t()] def fetch_public_activities(opts \\ %{}, pagination \\ :keyset) do - opts = - opts - |> Map.put("reply_user", opts["user"]) - |> Map.delete("user") + opts = Map.drop(opts, ["user"]) [Constants.as_public()] |> fetch_activities_query(opts) @@ -981,65 +976,13 @@ defp restrict_tag(query, %{"tag" => tag}) when is_binary(tag) do defp restrict_tag(query, _), do: query - defp reply_recipients(user, "following") do - [user.ap_id | User.get_cached_user_friends_ap_ids(user)] + defp restrict_recipients(query, [], _user), do: query + + defp restrict_recipients(query, recipients, nil) do + from(activity in query, where: fragment("? && ?", ^recipients, activity.recipients)) end - defp reply_recipients(user, "self"), do: [user.ap_id] - - defp restrict_recipients(query, [], _opts), do: query - - defp restrict_recipients( - query, - recipients, - %{"user" => nil, "reply_user" => user, "reply_visibility" => visibility} - ) - when not is_nil(user) and visibility in ["following", "self"] do - reply_recipients = reply_recipients(user, visibility) - - from([activity, object] in query, - where: - fragment( - "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ? OR ? = ?)", - ^recipients, - activity.recipients, - object.data, - activity.recipients, - activity.actor, - ^reply_recipients, - activity.actor, - ^user.ap_id - ) - ) - end - - defp restrict_recipients(query, recipients, %{"user" => nil}) do - from(activity in query, - where: fragment("? && ?", ^recipients, activity.recipients) - ) - end - - defp restrict_recipients(query, recipients, %{"user" => user, "reply_visibility" => visibility}) - when visibility in ["following", "self"] do - reply_recipients = reply_recipients(user, visibility) - - from( - [activity, object] in query, - where: - fragment( - "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ?)", - ^recipients, - activity.recipients, - object.data, - activity.recipients, - activity.actor, - ^reply_recipients - ), - or_where: activity.actor == ^user.ap_id - ) - end - - defp restrict_recipients(query, recipients, %{"user" => user}) do + defp restrict_recipients(query, recipients, user) do from( activity in query, where: fragment("? && ?", ^recipients, activity.recipients), @@ -1104,6 +1047,41 @@ defp restrict_replies(query, %{"exclude_replies" => val}) when val == "true" or ) end + defp restrict_replies(query, %{ + "reply_filtering_user" => user, + "reply_visibility" => "self" + }) do + from( + [activity, object] in query, + where: + fragment( + "?->>'inReplyTo' is null OR ? = ANY(?)", + object.data, + ^user.ap_id, + activity.recipients + ) + ) + end + + defp restrict_replies(query, %{ + "reply_filtering_user" => user, + "reply_visibility" => "following" + }) do + from( + [activity, object] in query, + where: + fragment( + "?->>'inReplyTo' is null OR ? && array_remove(?, ?) OR ? = ?", + object.data, + ^[user.ap_id | User.get_cached_user_friends_ap_ids(user)], + activity.recipients, + activity.actor, + activity.actor, + ^user.ap_id + ) + ) + end + defp restrict_replies(query, _), do: query defp restrict_reblogs(query, %{"exclude_reblogs" => val}) when val == "true" or val == "1" do @@ -1311,15 +1289,14 @@ def fetch_activities_query(recipients, opts \\ %{}) do skip_thread_containment: Config.get([:instance, :skip_thread_containment]) } - opts = Map.put(opts, "user", opts["user"]) - Activity |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_preload_report_notes(opts) |> maybe_set_thread_muted_field(opts) |> maybe_order(opts) - |> restrict_recipients(recipients, opts) + |> restrict_recipients(recipients, opts["user"]) + |> restrict_replies(opts) |> restrict_tag(opts) |> restrict_tag_reject(opts) |> restrict_tag_all(opts) @@ -1334,7 +1311,6 @@ def fetch_activities_query(recipients, opts \\ %{}) do |> restrict_media(opts) |> restrict_visibility(opts) |> restrict_thread_visibility(opts, config) - |> restrict_replies(opts) |> restrict_reblogs(opts) |> restrict_pinned(opts) |> restrict_muted_reblogs(restrict_muted_reblogs_opts) diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index a2ac9301e..403d500e0 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -37,6 +37,7 @@ def home(%{assigns: %{user: user}} = conn, params) do |> Map.put("type", ["Create", "Announce"]) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) recipients = [user.ap_id | User.following(user)] @@ -100,7 +101,7 @@ def public(%{assigns: %{user: user}} = conn, params) do |> Map.put("local_only", local_only) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() conn diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 8a1638a23..edd7dfb22 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1936,7 +1936,7 @@ test "public timeline", %{users: %{u1: user}} do |> Map.put("local_only", false) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -1958,7 +1958,7 @@ test "public timeline with reply_visibility `following`", %{ |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "following") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -1985,7 +1985,7 @@ test "public timeline with reply_visibility `self`", %{ |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "self") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2008,6 +2008,7 @@ test "home timeline", %{ |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2046,6 +2047,7 @@ test "home timeline with reply_visibility `following`", %{ |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2084,6 +2086,7 @@ test "home timeline with reply_visibility `self`", %{ |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2131,6 +2134,7 @@ test "public timeline with default reply_visibility `following`", %{users: %{u1: |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2146,6 +2150,7 @@ test "public timeline with default reply_visibility `self`", %{users: %{u1: user |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2176,6 +2181,7 @@ test "home timeline with default reply_visibility `following`", %{users: %{u1: u |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2199,6 +2205,7 @@ test "home timeline with default reply_visibility `self`", %{ |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) From 0d05e1fe397d75d4381d9059bd1c049ab7030085 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 25 Apr 2020 18:24:10 +0300 Subject: [PATCH 32/34] [#1706] Prevented error on unresolved activity actors for timeline actions. --- lib/pleroma/web/admin_api/views/status_view.ex | 18 +++--------------- .../web/mastodon_api/views/status_view.ex | 13 ++++++++++--- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/lib/pleroma/web/admin_api/views/status_view.ex b/lib/pleroma/web/admin_api/views/status_view.ex index 360ddc22c..3637dee24 100644 --- a/lib/pleroma/web/admin_api/views/status_view.ex +++ b/lib/pleroma/web/admin_api/views/status_view.ex @@ -8,15 +8,16 @@ defmodule Pleroma.Web.AdminAPI.StatusView do require Pleroma.Constants alias Pleroma.User + alias Pleroma.Web.MastodonAPI.StatusView def render("index.json", opts) do safe_render_many(opts.activities, __MODULE__, "show.json", opts) end def render("show.json", %{activity: %{data: %{"object" => _object}} = activity} = opts) do - user = get_user(activity.data["actor"]) + user = StatusView.get_user(activity.data["actor"]) - Pleroma.Web.MastodonAPI.StatusView.render("show.json", opts) + StatusView.render("show.json", opts) |> Map.merge(%{account: merge_account_views(user)}) end @@ -26,17 +27,4 @@ defp merge_account_views(%User{} = user) do end defp merge_account_views(_), do: %{} - - defp get_user(ap_id) do - cond do - user = User.get_cached_by_ap_id(ap_id) -> - user - - user = User.get_by_guessed_nickname(ap_id) -> - user - - true -> - User.error_user(ap_id) - end - end end diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index b5850e1ae..b0c53acd9 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -45,7 +45,7 @@ defp get_replied_to_activities(activities) do end) end - defp get_user(ap_id) do + def get_user(ap_id, fake_record_fallback \\ true) do cond do user = User.get_cached_by_ap_id(ap_id) -> user @@ -53,8 +53,11 @@ defp get_user(ap_id) do user = User.get_by_guessed_nickname(ap_id) -> user - true -> + fake_record_fallback -> + # TODO: refactor (fake records is never a good idea) User.error_user(ap_id) + + true -> nil end end @@ -97,7 +100,11 @@ def render("index.json", opts) do UserRelationship.view_relationships_option(nil, []) true -> - actors = Enum.map(activities ++ parent_activities, &get_user(&1.data["actor"])) + # Note: unresolved users are filtered out + actors = + (activities ++ parent_activities) + |> Enum.map(&get_user(&1.data["actor"], false)) + |> Enum.filter(& &1) UserRelationship.view_relationships_option(reading_user, actors, source_mutes_only: opts[:skip_relationships] From e16437ff191f17b7ec59504d3c38e582ba76eedc Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 25 Apr 2020 18:42:08 +0300 Subject: [PATCH 33/34] [#1706] Formatting fix. --- lib/pleroma/web/mastodon_api/views/status_view.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/views/status_view.ex b/lib/pleroma/web/mastodon_api/views/status_view.ex index b0c53acd9..1d9082c09 100644 --- a/lib/pleroma/web/mastodon_api/views/status_view.ex +++ b/lib/pleroma/web/mastodon_api/views/status_view.ex @@ -57,7 +57,8 @@ def get_user(ap_id, fake_record_fallback \\ true) do # TODO: refactor (fake records is never a good idea) User.error_user(ap_id) - true -> nil + true -> + nil end end From 1bd9749a8f31e5f087b0d0ca75b13f4baf461997 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Sun, 26 Apr 2020 00:28:57 -0500 Subject: [PATCH 34/34] Let blob: pass CSP --- docs/configuration/hardening.md | 2 +- lib/pleroma/plugs/http_security_plug.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration/hardening.md b/docs/configuration/hardening.md index b54c28850..d3bfc4e4a 100644 --- a/docs/configuration/hardening.md +++ b/docs/configuration/hardening.md @@ -36,7 +36,7 @@ content-security-policy: default-src 'none'; base-uri 'self'; frame-ancestors 'none'; - img-src 'self' data: https:; + img-src 'self' data: blob: https:; media-src 'self' https:; style-src 'self' 'unsafe-inline'; font-src 'self'; diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 81e6b4f2a..6462797b6 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -75,7 +75,7 @@ defp csp_string do "default-src 'none'", "base-uri 'self'", "frame-ancestors 'none'", - "img-src 'self' data: https:", + "img-src 'self' data: blob: https:", "media-src 'self' https:", "style-src 'self' 'unsafe-inline'", "font-src 'self'",