Repository: Kraigie/nostrum
Pull request #372 (2021-12-22 18:01:49)

Practice proper :gun2.0 safety
This was done to `Nostrum.Shard.Session` already when migrating from `:gun` `1.3` to `2.0`.


Patch:
diff --git a/lib/nostrum/application.ex b/lib/nostrum/application.ex
index 7d1dfec5..a186a665 100644
--- a/lib/nostrum/application.ex
+++ b/lib/nostrum/application.ex
@@ -28,6 +28,7 @@ defmodule Nostrum.Application do
   def setup_ets_tables do
     :ets.new(:gateway_url, [:set, :public, :named_table])
     :ets.new(:unavailable_guilds, [:set, :public, :named_table])
+    :ets.new(:users, [:set, :public, :named_table])
     :ets.new(:guild_shard_map, [:set, :public, :named_table])
     :ets.new(:channel_guild_map, [:set, :public, :named_table])
   end
diff --git a/lib/nostrum/cache/cache_supervisor.ex b/lib/nostrum/cache/cache_supervisor.ex
index 87ed896a..ecb279f4 100644
--- a/lib/nostrum/cache/cache_supervisor.ex
+++ b/lib/nostrum/cache/cache_supervisor.ex
@@ -22,7 +22,6 @@ defmodule Nostrum.Cache.CacheSupervisor do
       Nostrum.Cache.Me,
       # Uses the configured cache implementations.
       Nostrum.Cache.GuildCache,
-      Nostrum.Cache.UserCache,
       Nostrum.Cache.ChannelCache,
       Nostrum.Cache.PresenceCache
     ]
diff --git a/lib/nostrum/cache/guild_cache.ex b/lib/nostrum/cache/guild_cache.ex
index 8ca0abbd..b4ee4a76 100644
--- a/lib/nostrum/cache/guild_cache.ex
+++ b/lib/nostrum/cache/guild_cache.ex
@@ -53,7 +53,8 @@ defmodule Nostrum.Cache.GuildCache do
   alias Nostrum.Util
 
   @configured_cache :nostrum
-                    |> Application.compile_env([:caches, :guilds], @default_cache_implementation)
+                    |> Application.compile_env(:caches, %{})
+                    |> Map.get(:guilds, @default_cache_implementation)
 
   @typedoc "Specifies the reason for why a lookup operation has failed."
   @type reason ::
diff --git a/lib/nostrum/cache/user_cache.ex b/lib/nostrum/cache/user_cache.ex
index 059bcd79..630e66a7 100644
--- a/lib/nostrum/cache/user_cache.ex
+++ b/lib/nostrum/cache/user_cache.ex
@@ -2,36 +2,22 @@ defmodule Nostrum.Cache.UserCache do
   @default_cache_implementation Nostrum.Cache.UserCache.ETS
   @moduledoc """
   Cache behaviour & dispatcher for users.
-
-  You can call the functions provided by this module independent of which cache
-  is configured, and it will dispatch to the configured cache implementation.
-
-  By default, #{@default_cache_implementation} will be used for caching users.
-  You can override this in the `:caches` option of the `:nostrum` application
-  by setting the `:users` field to a different module implementing the behaviour
-  defined by this module.
-
-  See the documentation for the `Nostrum.Cache.GuildCache` module for more details.
-
   """
 
   alias Nostrum.Struct.User
   alias Nostrum.Util
   import Nostrum.Snowflake, only: [is_snowflake: 1]
 
-  @configured_cache :nostrum
-                    |> Application.compile_env([:caches, :users], @default_cache_implementation)
-
-  ## Supervisor callbacks
-
-  @doc false
-  defdelegate init(init_arg), to: @configured_cache
-
-  @doc false
-  defdelegate start_link(init_arg), to: @configured_cache
-
-  @doc false
-  defdelegate child_spec(opts), to: @configured_cache
+  if function_exported?(Application, :compile_env, 3) do
+    @configured_cache :nostrum
+                      |> Application.compile_env(:caches, %{})
+                      |> Map.get(:users, @default_cache_implementation)
+  else
+    # credo:disable-for-next-line Credo.Check.Warning.ApplicationConfigInModuleAttribute
+    @configured_cache :nostrum
+                      |> Application.get_env(:caches, %{})
+                      |> Map.get(:users, @default_cache_implementation)
+  end
 
   ## Behaviour specification
 
@@ -83,15 +69,40 @@ defmodule Nostrum.Cache.UserCache do
   @callback delete(snowflake :: User.id()) :: :noop | User.t()
 
   ## Dispatching
-  defdelegate get(id), to: @configured_cache
-  defdelegate create(payload), to: @configured_cache
-  defdelegate bulk_create(users), to: @configured_cache
-  defdelegate update(payload), to: @configured_cache
-  defdelegate delete(snowflake), to: @configured_cache
+
+  @doc "Retrieve a user using the selected cache implementation."
+  @spec get(User.id()) :: {:error, atom} | {:ok, User.t()}
+  def get(id) when is_snowflake(id) do
+    @configured_cache.get(id)
+  end
 
   @doc """
-  Same as `c:get/1`, but raises `Nostrum.Error.CacheError` in case of a failure.
+  Same as `get/1`, but raises `Nostrum.Error.CacheError` in case of a failure.
   """
   @spec get!(User.id()) :: no_return | User.t()
   def get!(id) when is_snowflake(id), do: id |> get |> Util.bangify_find(id, __MODULE__)
+
+  @doc "Create a user using the selected cache implementation."
+  @spec create(map()) :: User.t()
+  def create(payload) do
+    @configured_cache.create(payload)
+  end
+
+  @doc "Bulk create multiple users using the selected cache implementation."
+  @spec bulk_create(Enum.t()) :: :ok
+  def bulk_create(users) do
+    @configured_cache.bulk_create(users)
+  end
+
+  @doc "Update the given user using the selected cache implementation."
+  @spec update(map()) :: :noop | {User.t(), User.t()}
+  def update(payload) do
+    @configured_cache.update(payload)
+  end
+
+  @doc "Delete a user by ID using the selected cache implementation."
+  @spec delete(User.id()) :: :noop | User.t()
+  def delete(id) when is_snowflake(id) do
+    @configured_cache.delete(id)
+  end
 end
diff --git a/lib/nostrum/cache/user_cache/ets.ex b/lib/nostrum/cache/user_cache/ets.ex
index 5694ad2b..24d27e5c 100644
--- a/lib/nostrum/cache/user_cache/ets.ex
+++ b/lib/nostrum/cache/user_cache/ets.ex
@@ -1,5 +1,5 @@
 defmodule Nostrum.Cache.UserCache.ETS do
-  @table_name :nostrum_users
+  @table_name :users
   @moduledoc """
   An ETS-based cache for users.
 
@@ -12,18 +12,10 @@ defmodule Nostrum.Cache.UserCache.ETS do
   @behaviour Nostrum.Cache.UserCache
 
   alias Nostrum.Struct.User
-  use Supervisor
 
-  @doc "Start the supervisor."
-  def start_link(init_arg) do
-    Supervisor.start_link(__MODULE__, init_arg, name: __MODULE__)
-  end
-
-  @doc "Set up the ETS table."
-  @impl Supervisor
-  def init(_init_arg) do
+  @doc "Set up the cache for tests."
+  def setup do
     :ets.new(@table_name, [:set, :public, :named_table])
-    Supervisor.init([], strategy: :one_for_one)
   end
 
   @impl Nostrum.Cache.UserCache
diff --git a/lib/nostrum/voice/audio.ex b/lib/nostrum/voice/audio.ex
index b9bfe444..0b03ce8c 100644
--- a/lib/nostrum/voice/audio.ex
+++ b/lib/nostrum/voice/audio.ex
@@ -308,7 +308,7 @@ defmodule Nostrum.Voice.Audio do
   end
 
   def on_stall(%VoiceState{} = voice) do
-    if VoiceState.playing?(voice) and not is_nil(voice.ffmpeg_proc) do
+    unless is_nil(voice.ffmpeg_proc) do
       Proc.stop(voice.ffmpeg_proc)
     end
   end
diff --git a/lib/nostrum/voice/session.ex b/lib/nostrum/voice/session.ex
index e85e5a6d..bc9b48f9 100644
--- a/lib/nostrum/voice/session.ex
+++ b/lib/nostrum/voice/session.ex
@@ -110,7 +110,7 @@ defmodule Nostrum.Voice.Session do
   def handle_info({:gun_ws, _conn, _stream, {:close, errno, reason}}, state) do
     Logger.info("Voice websocket closed (errno #{errno}, reason #{inspect(reason)})")
 
-    # If we received a errno of 4006, session is no longer valid, so we must get a new session.
+    # If we received an errno of 4006, session is no longer valid, so we must get a new session.
     if errno == 4006 do
       %VoiceState{channel_id: chan, self_mute: mute, self_deaf: deaf} =
         Voice.get_voice(state.guild_id)
@@ -123,7 +123,7 @@ defmodule Nostrum.Voice.Session do
   end
 
   def handle_info(
-        {:gun_down, _conn, _proto, _reason, _killed_streams, _unprocessed_streams},
+        {:gun_down, _conn, _proto, _reason, _killed_streams},
         state
       ) do
     # Try to cancel the internal timer, but


Test patch:
diff --git a/test/nostrum/cache/user_cache_test.exs b/test/nostrum/cache/user_cache_test.exs
index e9c3f119..c7b800c8 100644
--- a/test/nostrum/cache/user_cache_test.exs
+++ b/test/nostrum/cache/user_cache_test.exs
@@ -34,7 +34,8 @@ defmodule Nostrum.Cache.UserCacheTest do
       }
 
       setup do
-        [pid: start_supervised!(@cache)]
+        @cache.setup()
+        :ok
       end
 
       describe "bulk_create/1" do