Fix refactoring opportunities

This commit is contained in:
miffigriffi 2019-09-22 13:41:24 +02:00 committed by Thomas Citharel
parent 4d4fdaaffe
commit 20dfce5c83
No known key found for this signature in database
GPG key ID: A061B9DDE0CA0773
18 changed files with 122 additions and 90 deletions

View file

@ -73,7 +73,7 @@ defmodule MobilizonWeb.API.Reports do
defp get_report_comments(%Actor{id: actor_id}, comment_ids) do
{:get_report_comments,
Events.list_comments_by_actor_and_ids(actor_id, comment_ids) |> Enum.map(& &1.url)}
actor_id |> Events.list_comments_by_actor_and_ids(comment_ids) |> Enum.map(& &1.url)}
end
defp get_report_comments(_, _), do: {:get_report_comments, nil}

View file

@ -47,7 +47,7 @@ defmodule MobilizonWeb.MIME do
new_filename =
if length(parts) > 1 do
Enum.drop(parts, -1) |> Enum.join(".")
parts |> Enum.drop(-1) |> Enum.join(".")
else
Enum.join(parts)
end
@ -60,7 +60,9 @@ defmodule MobilizonWeb.MIME do
new_filename <> "." <> ext
true ->
Enum.join([new_filename, String.split(content_type, "/") |> List.last()], ".")
extension = content_type |> String.split("/") |> List.last()
Enum.join([new_filename, extension], ".")
end
end

View file

@ -60,7 +60,8 @@ defmodule MobilizonWeb.Plugs.UploadedMedia do
defp get_media(conn, {:static_dir, directory}, _, opts) do
static_opts =
Map.get(opts, :static_plug_opts)
opts
|> Map.get(:static_plug_opts)
|> Map.put(:at, [@path])
|> Map.put(:from, directory)

View file

@ -12,7 +12,7 @@ defmodule MobilizonWeb.Resolvers.Address do
"""
@spec search(map(), map(), map()) :: {:ok, list(Address.t())}
def search(_parent, %{query: query, page: _page, limit: _limit}, %{context: %{ip: ip}}) do
country = Geolix.lookup(ip) |> Map.get(:country, nil)
country = ip |> Geolix.lookup() |> Map.get(:country, nil)
local_addresses = Task.async(fn -> Addresses.search_addresses(query, country: country) end)
@ -40,7 +40,7 @@ defmodule MobilizonWeb.Resolvers.Address do
"""
@spec reverse_geocode(map(), map(), map()) :: {:ok, list(Address.t())}
def reverse_geocode(_parent, %{longitude: longitude, latitude: latitude}, %{context: %{ip: ip}}) do
country = Geolix.lookup(ip) |> Map.get(:country, nil)
country = ip |> Geolix.lookup() |> Map.get(:country, nil)
local_addresses =
Task.async(fn -> Addresses.reverse_geocode(longitude, latitude, country: country) end)

View file

@ -10,13 +10,16 @@ defmodule MobilizonWeb.Resolvers.Admin do
alias Mobilizon.Events.Event
alias Mobilizon.Service.Statistics
def list_action_logs(_parent, %{page: page, limit: limit}, %{
context: %{current_user: %User{role: role}}
})
def list_action_logs(
_parent,
%{page: page, limit: limit},
%{context: %{current_user: %User{role: role}}}
)
when is_moderator(role) do
with action_logs <- Mobilizon.Admin.list_action_logs(page, limit) do
action_logs =
Enum.map(action_logs, fn %ActionLog{
action_logs
|> Enum.map(fn %ActionLog{
target_type: target_type,
action: action,
actor: actor,
@ -25,11 +28,7 @@ defmodule MobilizonWeb.Resolvers.Admin do
} = action_log ->
with data when is_map(data) <-
transform_action_log(String.to_existing_atom(target_type), action, action_log) do
Map.merge(data, %{
actor: actor,
id: id,
inserted_at: inserted_at
})
Map.merge(data, %{actor: actor, id: id, inserted_at: inserted_at})
end
end)
|> Enum.filter(& &1)

View file

@ -31,7 +31,8 @@ defmodule MobilizonWeb.Resolvers.Group do
def list_groups(_parent, %{page: page, limit: limit}, _resolution) do
{
:ok,
Actors.list_groups(page, limit)
page
|> Actors.list_groups(limit)
|> Enum.map(fn actor -> Person.proxify_pictures(actor) end)
}
end

View file

@ -4,12 +4,11 @@
# Upstream: https://git.pleroma.social/pleroma/pleroma/blob/develop/lib/pleroma/reverse_proxy.ex
defmodule MobilizonWeb.ReverseProxy do
@keep_req_headers ~w(accept user-agent accept-encoding cache-control if-modified-since) ++
~w(if-unmodified-since if-none-match if-range range)
@keep_req_headers ~w(accept user-agent accept-encoding cache-control
if-modified-since if-unmodified-since if-none-match if-range range)
@resp_cache_headers ~w(etag date last-modified cache-control)
@keep_resp_headers @resp_cache_headers ++
~w(content-type content-disposition content-encoding content-range) ++
~w(accept-ranges vary)
@keep_resp_headers @resp_cache_headers ++ ~w(content-type content-disposition
content-encoding content-range accept-ranges vary)
@default_cache_control_header "public, max-age=1209600"
@valid_resp_codes [200, 206, 304]
@max_read_duration :timer.seconds(30)
@ -21,9 +20,11 @@ defmodule MobilizonWeb.ReverseProxy do
MobilizonWeb.ReverseProxy.call(conn, url, options)
It is not meant to be added into a plug pipeline, but to be called from another plug or controller.
It is not meant to be added into a plug pipeline, but to be called from another
plug or controller.
Supports `#{inspect(@methods)}` HTTP methods, and only allows `#{inspect(@valid_resp_codes)}` status codes.
Supports `#{inspect(@methods)}` HTTP methods, and only allows
`#{inspect(@valid_resp_codes)}` status codes.
Responses are chunked to the client while downloading from the upstream.
@ -32,34 +33,52 @@ defmodule MobilizonWeb.ReverseProxy do
* request: `#{inspect(@keep_req_headers)}`
* response: `#{inspect(@keep_resp_headers)}`
If no caching headers (`#{inspect(@resp_cache_headers)}`) are returned by upstream, `cache-control` will be
set to `#{inspect(@default_cache_control_header)}`.
If no caching headers (`#{inspect(@resp_cache_headers)}`) are returned by
upstream, `cache-control` will be set to `#{inspect(@default_cache_control_header)}`.
Options:
* `redirect_on_failure` (default `false`). Redirects the client to the real remote URL if there's any HTTP
errors. Any error during body processing will not be redirected as the response is chunked. This may expose
remote URL, clients IPs, .
* `redirect_on_failure` (default `false`). Redirects the client to the real
remote URL if there's any HTTP errors. Any error during body processing will
not be redirected as the response is chunked. This may expose remote URL,
clients IPs, .
* `max_body_length` (default `#{inspect(@max_body_length)}`): limits the content length to be approximately the
specified length. It is validated with the `content-length` header and also verified when proxying.
* `max_body_length` (default `#{inspect(@max_body_length)}`): limits the
content length to be approximately the specified length. It is validated with
the `content-length` header and also verified when proxying.
* `max_read_duration` (default `#{inspect(@max_read_duration)}` ms): the total time the connection is allowed to
read from the remote upstream.
* `max_read_duration` (default `#{inspect(@max_read_duration)}` ms): the total
time the connection is allowed to read from the remote upstream.
* `inline_content_types`:
* `true` will not alter `content-disposition` (up to the upstream),
* `false` will add `content-disposition: attachment` to any request,
* a list of whitelisted content types
* `keep_user_agent` will forward the client's user-agent to the upstream. This may be useful if the upstream is
doing content transformation (encoding, ) depending on the request.
* `keep_user_agent` will forward the client's user-agent to the upstream.
This may be useful if the upstream is doing content transformation
(encoding, ) depending on the request.
* `req_headers`, `resp_headers` additional headers.
* `http`: options for [hackney](https://github.com/benoitc/hackney).
"""
import Plug.Conn
require Logger
@type option ::
{:keep_user_agent, boolean}
| {:max_read_duration, :timer.time() | :infinity}
| {:max_body_length, non_neg_integer | :infinity}
| {:http, []}
| {:req_headers, [{String.t(), String.t()}]}
| {:resp_headers, [{String.t(), String.t()}]}
| {:inline_content_types, boolean | [String.t()]}
| {:redirect_on_failure, boolean}
@hackney Application.get_env(:mobilizon, :hackney, :hackney)
@httpoison Application.get_env(:mobilizon, :httpoison, HTTPoison)
@ -78,20 +97,7 @@ defmodule MobilizonWeb.ReverseProxy do
"video/quicktime"
]
require Logger
import Plug.Conn
@type option() ::
{:keep_user_agent, boolean}
| {:max_read_duration, :timer.time() | :infinity}
| {:max_body_length, non_neg_integer() | :infinity}
| {:http, []}
| {:req_headers, [{String.t(), String.t()}]}
| {:resp_headers, [{String.t(), String.t()}]}
| {:inline_content_types, boolean() | [String.t()]}
| {:redirect_on_failure, boolean()}
@spec call(Plug.Conn.t(), url :: String.t(), [option()]) :: Plug.Conn.t()
@spec call(Plug.Conn.t(), url :: String.t(), [option]) :: Plug.Conn.t()
def call(_conn, _url, _opts \\ [])
def call(conn = %{method: method}, url, opts) when method in @methods do
@ -114,7 +120,8 @@ defmodule MobilizonWeb.ReverseProxy do
response(conn, client, url, code, headers, opts)
else
{:ok, code, headers} ->
head_response(conn, url, code, headers, opts)
conn
|> head_response(url, code, headers, opts)
|> halt()
{:error, {:invalid_http_response, code}} ->

View file

@ -11,9 +11,10 @@ defmodule MobilizonWeb.Upload.Filter.Dedupe do
alias MobilizonWeb.Upload
def filter(%Upload{name: name} = upload) do
extension = String.split(name, ".") |> List.last()
extension = name |> String.split(".") |> List.last()
shasum = :crypto.hash(:sha256, File.read!(upload.tempfile)) |> Base.encode16(case: :lower)
filename = shasum <> "." <> extension
{:ok, %Upload{upload | id: shasum, path: filename}}
end
end

View file

@ -24,7 +24,8 @@ defmodule Mobilizon.Service.ActivityPub.Transmogrifier do
if is_binary(Enum.at(actor, 0)) do
Enum.at(actor, 0)
else
Enum.find(actor, fn %{"type" => type} -> type in ["Person", "Service", "Application"] end)
actor
|> Enum.find(fn %{"type" => type} -> type in ["Person", "Service", "Application"] end)
|> Map.get("id")
end
end

View file

@ -348,7 +348,7 @@ defmodule Mobilizon.Service.ActivityPub.Utils do
if is_nil(metadata.options) do
res
else
options = struct(Mobilizon.Events.EventOptions, metadata.options) |> Map.from_struct()
options = Events.EventOptions |> struct(metadata.options) |> Map.from_struct()
Enum.reduce(options, res, fn {key, value}, acc ->
(value && Map.put(acc, camelize(key), value)) ||

View file

@ -57,7 +57,11 @@ defmodule Mobilizon.Service.Export.Feed do
@spec build_actor_feed(Actor.t(), list(), boolean()) :: String.t()
defp build_actor_feed(%Actor{} = actor, events, public \\ true) do
display_name = Actor.display_name(actor)
self_url = Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom") |> URI.decode()
self_url =
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "atom")
|> URI.decode()
title =
if public,
@ -66,8 +70,8 @@ defmodule Mobilizon.Service.Export.Feed do
# Title uses default instance language
feed =
Feed.new(
self_url,
self_url
|> Feed.new(
DateTime.utc_now(),
Gettext.gettext(MobilizonWeb.Gettext, title, actor: display_name)
)
@ -102,7 +106,8 @@ defmodule Mobilizon.Service.Export.Feed do
description = event.description || ""
entry =
Entry.new(event.url, event.publish_at || event.inserted_at, event.title)
event.url
|> Entry.new(event.publish_at || event.inserted_at, event.title)
|> Entry.link(event.url, rel: "alternate", type: "text/html")
|> Entry.content({:cdata, description}, type: "html")
|> Entry.published(event.publish_at || event.inserted_at)
@ -156,14 +161,11 @@ defmodule Mobilizon.Service.Export.Feed do
# Build an atom feed from actor and it's public events
@spec build_user_feed(list(), User.t(), String.t()) :: String.t()
defp build_user_feed(events, %User{email: email}, token) do
self_url = Routes.feed_url(Endpoint, :going, token, "atom") |> URI.decode()
self_url = Endpoint |> Routes.feed_url(:going, token, "atom") |> URI.decode()
# Title uses default instance language
Feed.new(
self_url,
DateTime.utc_now(),
gettext("Feed for %{email} on Mobilizon", email: email)
)
self_url
|> Feed.new(DateTime.utc_now(), gettext("Feed for %{email} on Mobilizon", email: email))
|> Feed.link(self_url, rel: "self")
|> Feed.generator("Mobilizon", uri: "https://joinmobilizon.org", version: version())
|> Feed.entries(Enum.map(events, &get_entry/1))

View file

@ -93,7 +93,8 @@ defmodule Mobilizon.Service.Formatter do
end
def html_escape(text, "text/plain") do
Regex.split(@link_regex, text, include_captures: true)
@link_regex
|> Regex.split(text, include_captures: true)
|> Enum.map_every(2, fn chunk ->
{:safe, part} = Phoenix.HTML.html_escape(chunk)
part

View file

@ -68,7 +68,7 @@ defmodule Mobilizon.Service.Geospatial.Addok do
region: Map.get(properties, "state"),
description: Map.get(properties, "name") || street_address(properties),
floor: Map.get(properties, "floor"),
geom: Map.get(geometry, "coordinates") |> Provider.coordinates(),
geom: geometry |> Map.get("coordinates") |> Provider.coordinates(),
postal_code: Map.get(properties, "postcode"),
street: properties |> street_address()
}

View file

@ -70,7 +70,7 @@ defmodule Mobilizon.Service.Geospatial.Photon do
region: Map.get(properties, "state"),
description: Map.get(properties, "name") || street_address(properties),
floor: Map.get(properties, "floor"),
geom: Map.get(geometry, "coordinates") |> Provider.coordinates(),
geom: geometry |> Map.get("coordinates") |> Provider.coordinates(),
postal_code: Map.get(properties, "postcode"),
street: properties |> street_address()
}

View file

@ -28,7 +28,7 @@ defmodule Mobilizon.Service.ActivityPub.ActivityPubTest do
Signature.sign(actor, %{
host: "example.com",
"content-length": 15,
digest: Jason.encode!(%{id: "my_id"}) |> Signature.build_digest(),
digest: %{id: "my_id"} |> Jason.encode!() |> Signature.build_digest(),
"(request-target)": Signature.generate_request_target("POST", "/inbox"),
date: Signature.generate_date_header()
})

View file

@ -784,7 +784,9 @@ defmodule Mobilizon.Service.ActivityPub.TransmogrifierTest do
assert :error == Transmogrifier.handle_incoming(reject_data)
# Organiser is not present since we use factories directly
assert Events.list_participants_for_event(event.id) |> Enum.map(& &1.id) ==
assert event.id
|> Events.list_participants_for_event()
|> Enum.map(& &1.id) ==
[]
end
@ -812,9 +814,10 @@ defmodule Mobilizon.Service.ActivityPub.TransmogrifierTest do
assert activity.data["actor"] == participant_url
# The only participant left is the organizer
assert Events.list_participants_for_event(event.id) |> Enum.map(& &1.id) == [
organizer_participation.id
]
assert event.id
|> Events.list_participants_for_event()
|> Enum.map(& &1.id) ==
[organizer_participation.id]
end
test "it refuses Leave activities when actor is the only organizer" do

View file

@ -16,7 +16,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "atom")
|> URI.decode()
)
@ -48,7 +49,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "atom")
|> URI.decode()
)
@ -63,7 +65,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn
|> put_req_header("accept", "application/atom+xml")
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "atom")
|> URI.decode()
)
@ -93,7 +96,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "ics")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "ics")
|> URI.decode()
)
@ -121,7 +125,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "ics")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "ics")
|> URI.decode()
)
@ -136,7 +141,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn
|> put_req_header("accept", "text/calendar")
|> get(
Routes.feed_url(Endpoint, :actor, actor.preferred_username, "ics")
Endpoint
|> Routes.feed_url(:actor, actor.preferred_username, "ics")
|> URI.decode()
)
@ -163,7 +169,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :event, event1.uuid, "ics")
Endpoint
|> Routes.feed_url(:event, event1.uuid, "ics")
|> URI.decode()
)
@ -194,7 +201,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :going, feed_token.token, "atom")
Endpoint
|> Routes.feed_url(:going, feed_token.token, "atom")
|> URI.decode()
)
@ -228,7 +236,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn
|> put_req_header("accept", "application/atom+xml")
|> get(
Routes.feed_url(Endpoint, :going, feed_token.token, "atom")
Endpoint
|> Routes.feed_url(:going, feed_token.token, "atom")
|> URI.decode()
)
@ -247,7 +256,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :going, "not existing", "atom")
Endpoint
|> Routes.feed_url(:going, "not existing", "atom")
|> URI.decode()
)
@ -272,7 +282,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn
|> put_req_header("accept", "text/calendar")
|> get(
Routes.feed_url(Endpoint, :going, feed_token.token, "ics")
Endpoint
|> Routes.feed_url(:going, feed_token.token, "ics")
|> URI.decode()
)
@ -302,7 +313,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn
|> put_req_header("accept", "text/calendar")
|> get(
Routes.feed_url(Endpoint, :going, feed_token.token, "ics")
Endpoint
|> Routes.feed_url(:going, feed_token.token, "ics")
|> URI.decode()
)
@ -318,7 +330,8 @@ defmodule MobilizonWeb.FeedControllerTest do
conn =
conn
|> get(
Routes.feed_url(Endpoint, :going, "not existing", "ics")
Endpoint
|> Routes.feed_url(:going, "not existing", "ics")
|> URI.decode()
)

View file

@ -33,7 +33,8 @@ defmodule MobilizonWeb.Resolvers.TagResolverTest do
tags = json_response(res, 200)["data"]["tags"]
assert tags |> length == 3
assert Enum.filter(tags, fn tag -> tag["slug"] == tag1.slug end)
assert tags
|> Enum.filter(fn tag -> tag["slug"] == tag1.slug end)
|> hd
|> Map.get("related")
|> Enum.map(fn tag -> tag["slug"] end)