From da378633ac9317fe7b2283f8e977f6cff90c6cd0 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 27 Nov 2018 17:54:54 +0100 Subject: [PATCH] Moar coverage Signed-off-by: Thomas Citharel --- lib/mobilizon/actors/actor.ex | 27 ++-- lib/mobilizon/actors/actors.ex | 2 +- .../actors/service/reset_password.ex | 5 +- lib/mobilizon/events/events.ex | 2 +- .../controllers/activity_pub_controller.ex | 16 +- lib/mobilizon_web/resolvers/user.ex | 5 +- .../views/activity_pub/actor_view.ex | 27 ++-- lib/service/activity_pub/activity_pub.ex | 5 +- test/mobilizon/actors/actors_test.exs | 10 +- .../activity_pub_controller_test.exs | 149 +++++++++++++----- .../controllers/page_controller_test.exs | 7 + .../resolvers/user_resolver_test.exs | 111 +++++++++++++ test/support/factory.ex | 5 + 13 files changed, 287 insertions(+), 84 deletions(-) diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index 18e0a25eb..6e8402cc1 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/lib/mobilizon/actors/actor.ex @@ -222,13 +222,17 @@ defmodule Mobilizon.Actors.Actor do If actor A and C both follow actor B, actor B's followers are A and C """ - def get_followers(%Actor{id: actor_id} = _actor) do + def get_followers(%Actor{id: actor_id} = _actor, page \\ 1, limit \\ 10) do + start = (page - 1) * limit + Repo.all( from( a in Actor, join: f in Follower, on: a.id == f.actor_id, - where: f.target_actor_id == ^actor_id + where: f.target_actor_id == ^actor_id, + limit: ^limit, + offset: ^start ) ) end @@ -238,13 +242,17 @@ defmodule Mobilizon.Actors.Actor do If actor A follows actor B and C, actor A's followings are B and B """ - def get_followings(%Actor{id: actor_id} = _actor) do + def get_followings(%Actor{id: actor_id} = _actor, page \\ 1, limit \\ 10) do + start = (page - 1) * limit + Repo.all( from( a in Actor, join: f in Follower, on: a.id == f.target_actor_id, - where: f.actor_id == ^actor_id + where: f.actor_id == ^actor_id, + limit: ^limit, + offset: ^start ) ) end @@ -271,10 +279,8 @@ defmodule Mobilizon.Actors.Actor do ) end - @spec follow(struct(), struct(), boolean()) :: Follower.t() | {:error, String.t()} - def follow(%Actor{} = follower, %Actor{} = followed, approved \\ true) do - + def follow(%Actor{} = followed, %Actor{} = follower, approved \\ true) do with {:suspended, false} <- {:suspended, followed.suspended}, # Check if followed has blocked follower {:already_following, false} <- {:already_following, following?(follower, followed)} do @@ -298,9 +304,12 @@ defmodule Mobilizon.Actors.Actor do end @spec following?(struct(), struct()) :: boolean() - def following?(%Actor{id: follower_actor_id} = _follower_actor, %Actor{followers: followers} = _followed) do + def following?( + %Actor{id: follower_actor_id} = _follower_actor, + %Actor{followers: followers} = _followed + ) do followers - |> Enum.map(&(&1.actor_id)) + |> Enum.map(& &1.actor_id) |> Enum.member?(follower_actor_id) end end diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index 5d3c16626..762c79b0e 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -367,7 +367,7 @@ defmodule Mobilizon.Actors do def get_local_actor_by_name_with_everything(name) do actor = Repo.one(from(a in Actor, where: a.preferred_username == ^name and is_nil(a.domain))) - Repo.preload(actor, :organized_events) + Repo.preload(actor, [:organized_events, :followers, :followings]) end @spec get_actor_by_name_with_everything(String.t(), atom() | nil) :: Actor.t() diff --git a/lib/mobilizon/actors/service/reset_password.ex b/lib/mobilizon/actors/service/reset_password.ex index 192b327d3..ba5e1e0d1 100644 --- a/lib/mobilizon/actors/service/reset_password.ex +++ b/lib/mobilizon/actors/service/reset_password.ex @@ -23,7 +23,10 @@ defmodule Mobilizon.Actors.Service.ResetPassword do ) do {:ok, user} else - _err -> + {:error, %Ecto.Changeset{errors: [password: {"registration.error.password_too_short", _}]}} -> + {:error, :password_too_short} + + err -> {:error, :invalid_token} end end diff --git a/lib/mobilizon/events/events.ex b/lib/mobilizon/events/events.ex index 91ce07bc6..5bf220078 100644 --- a/lib/mobilizon/events/events.ex +++ b/lib/mobilizon/events/events.ex @@ -889,7 +889,7 @@ defmodule Mobilizon.Events do def get_comment_full_from_uuid(uuid) do with %Comment{} = comment <- Repo.get_by!(Comment, uuid: uuid) do - Repo.preload(comment, [:actor, :attributed_to]) + Repo.preload(comment, [:actor, :attributed_to, :in_reply_to_comment]) end end diff --git a/lib/mobilizon_web/controllers/activity_pub_controller.ex b/lib/mobilizon_web/controllers/activity_pub_controller.ex index cb5f865bd..a7e9a650d 100644 --- a/lib/mobilizon_web/controllers/activity_pub_controller.ex +++ b/lib/mobilizon_web/controllers/activity_pub_controller.ex @@ -69,9 +69,8 @@ defmodule MobilizonWeb.ActivityPubController do end def following(conn, %{"name" => name, "page" => page}) do - with %Actor{} = actor <- Actors.get_local_actor_by_name(name) do - {page, _} = Integer.parse(page) - + with {page, ""} = Integer.parse(page), + %Actor{} = actor <- Actors.get_local_actor_by_name_with_everything(name) do conn |> put_resp_header("content-type", "application/activity+json") |> json(ActorView.render("following.json", %{actor: actor, page: page})) @@ -79,7 +78,7 @@ defmodule MobilizonWeb.ActivityPubController do end def following(conn, %{"name" => name}) do - with %Actor{} = actor <- Actors.get_local_actor_by_name(name) do + with %Actor{} = actor <- Actors.get_local_actor_by_name_with_everything(name) do conn |> put_resp_header("content-type", "application/activity+json") |> json(ActorView.render("following.json", %{actor: actor})) @@ -87,9 +86,8 @@ defmodule MobilizonWeb.ActivityPubController do end def followers(conn, %{"name" => name, "page" => page}) do - with %Actor{} = actor <- Actors.get_local_actor_by_name(name) do - {page, _} = Integer.parse(page) - + with {page, ""} = Integer.parse(page), + %Actor{} = actor <- Actors.get_local_actor_by_name_with_everything(name) do conn |> put_resp_header("content-type", "application/activity+json") |> json(ActorView.render("followers.json", %{actor: actor, page: page})) @@ -97,7 +95,7 @@ defmodule MobilizonWeb.ActivityPubController do end def followers(conn, %{"name" => name}) do - with %Actor{} = actor <- Actors.get_local_actor_by_name(name) do + with %Actor{} = actor <- Actors.get_local_actor_by_name_with_everything(name) do conn |> put_resp_header("content-type", "application/activity+json") |> json(ActorView.render("followers.json", %{actor: actor})) @@ -157,6 +155,6 @@ defmodule MobilizonWeb.ActivityPubController do def errors(conn, _e) do conn |> put_status(500) - |> json("error") + |> json("Unknown Error") end end diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index 7a9db6399..b9c0d2356 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/lib/mobilizon_web/resolvers/user.ex @@ -87,12 +87,13 @@ defmodule MobilizonWeb.Resolvers.User do """ def send_reset_password(_parent, %{email: email, locale: locale}, _resolution) do with {:ok, user} <- Actors.get_user_by_email(email, false), - {:ok, email} <- + {:ok, %Bamboo.Email{} = _email_html} <- Mobilizon.Actors.Service.ResetPassword.send_password_reset_email(user, locale) do {:ok, email} else {:error, :user_not_found} -> - {:error, "No user to validate with this email was found"} + # TODO : implement rate limits for this endpoint + {:error, "No user with this email was found"} {:error, :email_too_soon} -> {:error, "You requested again a confirmation email too soon"} diff --git a/lib/mobilizon_web/views/activity_pub/actor_view.ex b/lib/mobilizon_web/views/activity_pub/actor_view.ex index 598034978..d66e13bd8 100644 --- a/lib/mobilizon_web/views/activity_pub/actor_view.ex +++ b/lib/mobilizon_web/views/activity_pub/actor_view.ex @@ -47,7 +47,7 @@ defmodule MobilizonWeb.ActivityPub.ActorView do def render("following.json", %{actor: actor, page: page}) do actor - |> Actor.get_followings() + |> Actor.get_followings(page) |> collection(actor.following_url, page) |> Map.merge(Utils.make_json_ld_header()) end @@ -66,7 +66,7 @@ defmodule MobilizonWeb.ActivityPub.ActorView do def render("followers.json", %{actor: actor, page: page}) do actor - |> Actor.get_followers() + |> Actor.get_followers(page) |> collection(actor.followers_url, page) |> Map.merge(Utils.make_json_ld_header()) end @@ -77,7 +77,8 @@ defmodule MobilizonWeb.ActivityPub.ActorView do %{ "id" => actor.followers_url, "type" => "OrderedCollection", - "totalItems" => length(followers), + # TODO put me back + # "totalItems" => length(followers), "first" => collection(followers, actor.followers_url, 1) } |> Map.merge(Utils.make_json_ld_header()) @@ -148,22 +149,22 @@ defmodule MobilizonWeb.ActivityPub.ActorView do |> Map.merge(Utils.make_json_ld_header()) end - def collection(collection, iri, page, total \\ nil) do - offset = (page - 1) * 10 - items = Enum.slice(collection, offset, 10) - items = Enum.map(items, fn account -> account.url end) - total = total || length(collection) + def collection(collection, iri, page, _total \\ nil) do + items = Enum.map(collection, fn account -> account.url end) - map = %{ + # TODO : Add me back + # total = total || length(collection) + + %{ "id" => "#{iri}?page=#{page}", "type" => "OrderedCollectionPage", "partOf" => iri, - "totalItems" => total, + # "totalItems" => total, "orderedItems" => items } - if offset < total do - Map.put(map, "next", "#{iri}?page=#{page + 1}") - end + # if offset < total do + # Map.put(map, "next", "#{iri}?page=#{page + 1}") + # end end end diff --git a/lib/service/activity_pub/activity_pub.ex b/lib/service/activity_pub/activity_pub.ex index 4556e00e2..7c07452d8 100644 --- a/lib/service/activity_pub/activity_pub.ex +++ b/lib/service/activity_pub/activity_pub.ex @@ -154,7 +154,7 @@ defmodule Mobilizon.Service.ActivityPub do end def follow(%Actor{} = follower, %Actor{} = followed, _activity_id \\ nil, local \\ true) do - with {:ok, follow} <- Actor.follow(follower, followed, true), + with {:ok, follow} <- Actor.follow(followed, follower, true), data <- make_follow_data(follower, followed, follow.id), {:ok, activity} <- insert(data, local), :ok <- maybe_federate(activity) do @@ -251,8 +251,7 @@ defmodule Mobilizon.Service.ActivityPub do followers = if actor.followers_url in activity.recipients do - {:ok, followers} = Actor.get_followers(actor) - followers |> Enum.filter(fn follower -> is_nil(follower.domain) end) + Actor.get_followers(actor) |> Enum.filter(fn follower -> is_nil(follower.domain) end) else [] end diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs index 2773cd76c..223a862ba 100644 --- a/test/mobilizon/actors/actors_test.exs +++ b/test/mobilizon/actors/actors_test.exs @@ -585,23 +585,23 @@ defmodule Mobilizon.ActorsTest do actor = Actors.get_actor_with_everything!(actor.id) target_actor = Actors.get_actor_with_everything!(target_actor.id) - {:ok, follower} = Actor.follow(actor, target_actor) + {:ok, follower} = Actor.follow(target_actor, actor) assert follower.actor.id == actor.id # Referesh followers/followings actor = Actors.get_actor_with_everything!(actor.id) target_actor = Actors.get_actor_with_everything!(target_actor.id) - assert target_actor.followers |> Enum.map(&(&1.actor_id)) == [actor.id] - assert actor.followings |> Enum.map(&(&1.target_actor_id)) == [target_actor.id] + assert target_actor.followers |> Enum.map(& &1.actor_id) == [actor.id] + assert actor.followings |> Enum.map(& &1.target_actor_id) == [target_actor.id] # Test if actor is already following target actor - {:error, msg} = Actor.follow(actor, target_actor) + {:error, msg} = Actor.follow(target_actor, actor) assert msg =~ "already following" # Test if target actor is suspended target_actor = %{target_actor | suspended: true} - {:error, msg} = Actor.follow(actor, target_actor) + {:error, msg} = Actor.follow(target_actor, actor) assert msg =~ "suspended" end end diff --git a/test/mobilizon_web/controllers/activity_pub_controller_test.exs b/test/mobilizon_web/controllers/activity_pub_controller_test.exs index 19036c521..f9450a04a 100644 --- a/test/mobilizon_web/controllers/activity_pub_controller_test.exs +++ b/test/mobilizon_web/controllers/activity_pub_controller_test.exs @@ -3,6 +3,7 @@ defmodule MobilizonWeb.ActivityPubControllerTest do import Mobilizon.Factory alias MobilizonWeb.ActivityPub.{ActorView, ObjectView} alias Mobilizon.Actors + alias Mobilizon.Actors.Actor alias Mobilizon.Service.ActivityPub alias Mobilizon.Service.ActivityPub.Utils use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney @@ -47,6 +48,32 @@ defmodule MobilizonWeb.ActivityPubControllerTest do end end + describe "/comments/:uuid" do + test "it returns a json representation of the comment", %{conn: conn} do + comment = insert(:comment) + + conn = + conn + |> put_req_header("accept", "application/activity+json") + |> get("/comments/#{comment.uuid}") + + assert json_response(conn, 200) == + ObjectView.render("comment.json", %{comment: comment |> Utils.make_comment_data()}) + end + + # TODO ! + # test "it returns 404 for non-public comments", %{conn: conn} do + # event = insert(:event, public: false) + + # conn = + # conn + # |> put_req_header("accept", "application/activity+json") + # |> get("/events/#{event.uuid}") + + # assert json_response(conn, 404) + # end + end + describe "/@:preferred_username/inbox" do test "it inserts an incoming event into the database", %{conn: conn} do use_cassette "activity_pub_controller/mastodon-post-activity_actor_call" do @@ -91,46 +118,88 @@ defmodule MobilizonWeb.ActivityPubControllerTest do end end - # describe "/actors/:nickname/followers" do - # test "it returns the followers in a collection", %{conn: conn} do - # user = insert(:user) - # user_two = insert(:user) - # User.follow(user, user_two) - # - # result = - # conn - # |> get("/users/#{user_two.nickname}/followers") - # |> json_response(200) - # - # assert result["first"]["orderedItems"] == [user.ap_id] - # end - # - # test "it works for more than 10 users", %{conn: conn} do - # user = insert(:user) - # - # Enum.each(1..15, fn _ -> - # other_user = insert(:user) - # User.follow(other_user, user) - # end) - # - # result = - # conn - # |> get("/users/#{user.nickname}/followers") - # |> json_response(200) - # - # assert length(result["first"]["orderedItems"]) == 10 - # assert result["first"]["totalItems"] == 15 - # assert result["totalItems"] == 15 - # - # result = - # conn - # |> get("/users/#{user.nickname}/followers?page=2") - # |> json_response(200) - # - # assert length(result["orderedItems"]) == 5 - # assert result["totalItems"] == 15 - # end - # end + describe "/@actor/followers" do + test "it returns the followers in a collection", %{conn: conn} do + actor = insert(:actor) + actor2 = insert(:actor) + Actor.follow(actor, actor2) + + result = + conn + |> get("/@#{actor.preferred_username}/followers") + |> json_response(200) + + assert result["first"]["orderedItems"] == [actor2.url] + end + + test "it works for more than 10 actors", %{conn: conn} do + actor = insert(:actor) + + Enum.each(1..15, fn _ -> + other_actor = insert(:actor) + Actor.follow(actor, other_actor) + end) + + result = + conn + |> get("/@#{actor.preferred_username}/followers") + |> json_response(200) + + assert length(result["first"]["orderedItems"]) == 10 + # assert result["first"]["totalItems"] == 15 + # assert result["totalItems"] == 15 + + result = + conn + |> get("/@#{actor.preferred_username}/followers?page=2") + |> json_response(200) + + assert length(result["orderedItems"]) == 5 + # assert result["totalItems"] == 15 + end + end + + describe "/@actor/following" do + test "it returns the followings in a collection", %{conn: conn} do + actor = insert(:actor) + actor2 = insert(:actor) + Actor.follow(actor, actor2) + + result = + conn + |> get("/@#{actor2.preferred_username}/following") + |> json_response(200) + + assert result["first"]["orderedItems"] == [actor.url] + end + + test "it works for more than 10 actors", %{conn: conn} do + actor = insert(:actor) + + Enum.each(1..15, fn _ -> + other_actor = insert(:actor) + Actor.follow(other_actor, actor) + end) + + result = + conn + |> get("/@#{actor.preferred_username}/following") + |> json_response(200) + + assert length(result["first"]["orderedItems"]) == 10 + # assert result["first"]["totalItems"] == 15 + # assert result["totalItems"] == 15 + + result = + conn + |> get("/@#{actor.preferred_username}/following?page=2") + |> json_response(200) + + assert length(result["orderedItems"]) == 5 + # assert result["totalItems"] == 15 + end + end + # # describe "/@:preferred_username/following" do # test "it returns the following in a collection", %{conn: conn} do diff --git a/test/mobilizon_web/controllers/page_controller_test.exs b/test/mobilizon_web/controllers/page_controller_test.exs index f90af3b1e..37516df23 100644 --- a/test/mobilizon_web/controllers/page_controller_test.exs +++ b/test/mobilizon_web/controllers/page_controller_test.exs @@ -1,8 +1,15 @@ defmodule MobilizonWeb.PageControllerTest do use MobilizonWeb.ConnCase + import Mobilizon.Factory test "GET /", %{conn: conn} do conn = get(conn, "/") assert html_response(conn, 200) end + + test "GET /@actor", %{conn: conn} do + actor = insert(:actor) + conn = get(conn, "/@#{actor.preferred_username}") + assert html_response(conn, 200) + end end diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index a9adfef3a..df71dc91b 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/test/mobilizon_web/resolvers/user_resolver_test.exs @@ -1,6 +1,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do use MobilizonWeb.ConnCase alias Mobilizon.Actors + alias Mobilizon.Actors.{User, Actor} alias MobilizonWeb.AbsintheHelpers import Mobilizon.Factory use Bamboo.Test @@ -234,4 +235,114 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do assert hd(json_response(res, 200)["errors"])["message"] == "No user to validate with this email was found" end + + test "test send_reset_password/3 with valid email", context do + user = insert(:user) + + mutation = """ + mutation { + sendResetPassword( + email: "#{user.email}" + ) + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["sendResetPassword"] == user.email + end + + test "test send_reset_password/3 with invalid email", context do + mutation = """ + mutation { + sendResetPassword( + email: "oh no" + ) + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == "No user with this email was found" + end + + test "test reset_password/3 with valid email", context do + %User{} = user = insert(:user) + %Actor{} = insert(:actor, user: user) + {:ok, _email_sent} = Mobilizon.Actors.Service.ResetPassword.send_password_reset_email(user) + %User{reset_password_token: reset_password_token} = Mobilizon.Actors.get_user!(user.id) + + mutation = """ + mutation { + resetPassword( + token: "#{reset_password_token}", + password: "new password" + ) { + user { + id + } + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["resetPassword"]["user"]["id"] == to_string(user.id) + end + + test "test reset_password/3 with a password too short", context do + %User{} = user = insert(:user) + {:ok, _email_sent} = Mobilizon.Actors.Service.ResetPassword.send_password_reset_email(user) + %User{reset_password_token: reset_password_token} = Mobilizon.Actors.get_user!(user.id) + + mutation = """ + mutation { + resetPassword( + token: "#{reset_password_token}", + password: "new" + ) { + user { + id + } + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == "password_too_short" + end + + test "test reset_password/3 with an invalid token", context do + %User{} = user = insert(:user) + {:ok, _email_sent} = Mobilizon.Actors.Service.ResetPassword.send_password_reset_email(user) + %User{} = Mobilizon.Actors.get_user!(user.id) + + mutation = """ + mutation { + resetPassword( + token: "not good", + password: "new" + ) { + user { + id + } + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == "invalid_token" + end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 54b072e88..2ac1f4153 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -23,9 +23,13 @@ defmodule Mobilizon.Factory do %Mobilizon.Actors.Actor{ preferred_username: preferred_username, domain: nil, + followers: [], + followings: [], keys: pem, type: :Person, url: MobilizonWeb.Endpoint.url() <> "/@#{preferred_username}", + followers_url: MobilizonWeb.Endpoint.url() <> "/@#{preferred_username}/followers", + following_url: MobilizonWeb.Endpoint.url() <> "/@#{preferred_username}/following", user: nil } end @@ -81,6 +85,7 @@ defmodule Mobilizon.Factory do actor: build(:actor), event: build(:event), uuid: uuid, + in_reply_to_comment: nil, url: "#{MobilizonWeb.Endpoint.url()}/comments/#{uuid}" } end