Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

[WIP] Dialyzer fixes #868

Open
wants to merge 10 commits into
base: v1.2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ jobs:
paths:
- ~/var/dialyzer
when: always
## We're silenting dialyzer failure until we've fixed all dialyzer offense.
## TODO: enable this once Dialyzer is fixed.
# - notify_slack_failure
- notify_slack_failure

test:
executor: builder_pg
Expand Down Expand Up @@ -243,7 +241,7 @@ workflows:
requires: [build]
filters: *all_branches
- publish:
requires: [lint, test]
requires: [lint, test, dialyze]
filters: *all_branches

# Deploy to staging in case of master branch.
Expand Down
9 changes: 8 additions & 1 deletion .dialyzer_ignore.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,12 @@
{"apps/ewallet_db/lib/ewallet_db/account.ex", :no_return},

# TODO: Remove extra attributes e.g. `Map.put(wallet, :account_id, account.id)` from `%Wallet{}`
{"apps/ewallet/lib/ewallet/gates/transaction_consumption_consumer_gate.ex", :call}
{"apps/ewallet/lib/ewallet/gates/transaction_consumption_consumer_gate.ex", :call},

# Warnings from `Chaperon.Scenario.Logging.log_info/2` macro that we couldn't touch
{"apps/load_tester/lib/load_tester/scenarios/index.ex", :unmatched_return},

# Warnings from `Sentry.Phoenix.Endpoint` that we couldn't touch
{"apps/admin_api/lib/admin_api/endpoint.ex", :unmatched_return},
{"apps/ewallet_api/lib/ewallet_api/endpoint.ex", :unmatched_return},
]
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ check-format:
check-credo:
$(ENV_TEST) mix credo 2>&1

check-dialyzer:
$(ENV_TEST) mix dialyzer --halt-exit-status >&1
check-dialyzer: build-test
$(ENV_TEST) mix dialyzer --halt-exit-status 2>&1

.PHONY: format check-format check-credo

Expand Down
37 changes: 17 additions & 20 deletions apps/activity_logger/lib/activity_logger/activity_repo.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ defmodule ActivityLogger.ActivityRepo do
defmacro __using__(opts) do
quote bind_quoted: [opts: opts] do
@repo opts[:repo]
@type resp ::
{:ok, any()}
| {:error, any()}
| {:error, :no_originator_given}
| {:error, Multi.name(), any(), %{optional(Multi.name()) => any()}}

import Ecto.{Changeset, Query}
alias ActivityLogger.ActivityLog
Expand All @@ -29,44 +34,36 @@ defmodule ActivityLogger.ActivityRepo do
@repo
end

@spec insert_record_with_activity_log(%Changeset{}, Keyword.t(), Multi.t()) ::
{:ok, any()}
| {:error, any()}
| {:error, :no_originator_given}
| {:error, Multi.name(), any(), %{optional(Multi.name()) => any()}}
@spec insert_record_with_activity_log(%Changeset{}) :: resp()
@spec insert_record_with_activity_log(%Changeset{}, Keyword.t()) :: resp()
@spec insert_record_with_activity_log(%Changeset{}, Keyword.t(), Multi.t()) :: resp()
def insert_record_with_activity_log(changeset, opts \\ [], multi \\ Multi.new()) do
:insert
|> perform(changeset, opts, multi)
|> handle_perform_result(:insert, changeset)
end

@spec update_record_with_activity_log(%Changeset{}, Keyword.t(), Multi.t()) ::
{:ok, any()}
| {:error, any()}
| {:error, :no_originator_given}
| {:error, Multi.name(), any(), %{optional(Multi.name()) => any()}}
@spec update_record_with_activity_log(%Changeset{}) :: resp()
@spec update_record_with_activity_log(%Changeset{}, Keyword.t()) :: resp()
@spec update_record_with_activity_log(%Changeset{}, Keyword.t(), Multi.t()) :: resp()
def update_record_with_activity_log(changeset, opts \\ [], multi \\ Multi.new()) do
:update
|> perform(changeset, opts, multi)
|> handle_perform_result(:update, changeset)
end

@spec delete_record_with_activity_log(map(), Keyword.t(), Multi.t()) ::
{:ok, any()}
| {:error, any()}
| {:error, :no_originator_given}
| {:error, Multi.name(), any(), %{optional(Multi.name()) => any()}}
@spec delete_record_with_activity_log(map()) :: resp()
@spec delete_record_with_activity_log(map(), Keyword.t()) :: resp()
@spec delete_record_with_activity_log(map(), Keyword.t(), Multi.t()) :: resp()
def delete_record_with_activity_log(changeset, opts \\ [], multi \\ Multi.new()) do
:delete
|> perform(changeset, opts, multi)
|> handle_perform_result(:delete, changeset)
end

@spec perform(atom(), %Changeset{}, Keyword.t(), Multi.t()) ::
{:ok, any()}
| {:error, any()}
| {:error, :no_originator_given}
| {:error, Multi.name(), any(), %{optional(Multi.name()) => any()}}
@spec perform(atom(), %Changeset{}) :: resp()
@spec perform(atom(), %Changeset{}, Keyword.t()) :: resp()
@spec perform(atom(), %Changeset{}, Keyword.t(), Multi.t()) :: resp()
def perform(action, changeset, opts \\ [], multi \\ Multi.new()) do
Multi
|> apply(action, [Multi.new(), :record, changeset, opts])
Expand Down
13 changes: 7 additions & 6 deletions apps/activity_logger/lib/activity_logger/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ defmodule ActivityLogger.Application do
import Supervisor.Spec
DeferredConfig.populate(:activity_logger)

:telemetry.attach(
"appsignal-ecto",
[:activity_logger, :repo, :query],
&Ecto.handle_event/4,
nil
)
_ =
:telemetry.attach(
"appsignal-ecto",
[:activity_logger, :repo, :query],
&Ecto.handle_event/4,
nil
)

children = [
supervisor(ActivityLogger.Repo, []),
Expand Down
14 changes: 9 additions & 5 deletions apps/activity_logger/test/support/test_user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,20 @@ defmodule ActivityLogger.TestUser do
|> Repo.insert_record_with_activity_log([])
end

@spec insert_with_document(map()) :: {:ok, %TestUser{}} | {:error, Ecto.Changeset.t()}
def insert_with_document(attrs) do
%TestUser{}
|> changeset(attrs)
|> Repo.insert_record_with_activity_log(
[],
Multi.run(Multi.new(), :document, fn %{record: record} ->
TestDocument.insert(%{
title: record.username,
originator: record
})
Multi.run(Multi.new(), :document, fn %{record: record}, _ ->
{:ok, _} =
TestDocument.insert(%{
title: record.username,
originator: record
})

:ok
end)
)
end
Expand Down
2 changes: 1 addition & 1 deletion apps/admin_api/lib/admin_api/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ defmodule AdminAPI.Application do
DeferredConfig.populate(:admin_api)

settings = Application.get_env(:admin_api, :settings)
EWalletConfig.Config.register_and_load(:admin_api, settings)
_ = EWalletConfig.Config.register_and_load(:admin_api, settings)

Config.configure_cors_plug()
EWallet.configure_socket_endpoints([AdminAPI.V1.Endpoint])
Expand Down
2 changes: 2 additions & 0 deletions apps/admin_api/lib/admin_api/endpoint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ defmodule AdminAPI.Endpoint do
use Appsignal.Phoenix
use Sentry.Phoenix.Endpoint

# Code reloading can be explicitly enabled under the
# :code_reloader configuration of your endpoint.
if code_reloading? do
plug(Phoenix.CodeReloader)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ defmodule AdminAPI.V1.AdminUserAuthenticator do
"""
def expire_token(conn, originator) do
token_string = conn.private[:auth_auth_token]
AuthToken.expire(token_string, :admin_api, originator)
_ = AuthToken.expire(token_string, :admin_api, originator)
handle_fail_auth(conn, :auth_token_expired)
end

Expand Down
15 changes: 8 additions & 7 deletions apps/admin_api/lib/admin_api/v1/controllers/user_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,14 @@ defmodule AdminAPI.V1.UserController do
originator <- Originator.extract(conn.assigns),
attrs <- Map.put(attrs, "originator", originator),
{:ok, user} <- User.insert(attrs) do
case Account.get(attrs["account_id"]) do
nil ->
:noop

account ->
{:ok, _account_user} = AccountUser.link(account.uuid, user.uuid, originator)
end
_ =
case Account.get(attrs["account_id"]) do
nil ->
:noop

account ->
{:ok, _account_user} = AccountUser.link(account.uuid, user.uuid, originator)
end

respond_single(user, conn)
else
Expand Down
4 changes: 2 additions & 2 deletions apps/admin_api/test/support/channel_case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ defmodule AdminAPI.ChannelCase do

setup tags do
# Restarts `EWalletConfig.Config` so it does not hang on to a DB connection for too long.
Supervisor.terminate_child(EWalletConfig.Supervisor, EWalletConfig.Config)
Supervisor.restart_child(EWalletConfig.Supervisor, EWalletConfig.Config)
_ = Supervisor.terminate_child(EWalletConfig.Supervisor, EWalletConfig.Config)
_ = Supervisor.restart_child(EWalletConfig.Supervisor, EWalletConfig.Config)

:ok = Sandbox.checkout(EWalletDB.Repo)
:ok = Sandbox.checkout(LocalLedgerDB.Repo)
Expand Down
10 changes: 4 additions & 6 deletions apps/admin_api/test/support/conn_case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ defmodule AdminAPI.ConnCase do

setup tags do
# Restarts `EWalletConfig.Config` so it does not hang on to a DB connection for too long.
Supervisor.terminate_child(EWalletConfig.Supervisor, EWalletConfig.Config)
Supervisor.restart_child(EWalletConfig.Supervisor, EWalletConfig.Config)
_ = Supervisor.terminate_child(EWalletConfig.Supervisor, EWalletConfig.Config)
_ = Supervisor.restart_child(EWalletConfig.Supervisor, EWalletConfig.Config)

:ok = Sandbox.checkout(EWalletDB.Repo)
:ok = Sandbox.checkout(LocalLedgerDB.Repo)
Expand Down Expand Up @@ -273,9 +273,7 @@ defmodule AdminAPI.ConnCase do
account = Account.get_master_account()
master_wallet = Account.get_primary_wallet(account)

if mint do
mint!(token, amount * 100)
end
_ = if mint, do: mint!(token, amount * 100)

transfer!(
master_wallet.address,
Expand Down Expand Up @@ -394,7 +392,7 @@ defmodule AdminAPI.ConnCase do
num_remaining = num_required - Repo.aggregate(schema, :count, count_field)
factory_name = get_factory(schema)

insert_list(num_remaining, factory_name, attrs)
_ = insert_list(num_remaining, factory_name, attrs)
Repo.all(schema)
end

Expand Down
Loading