Ensure user struct is casted properly before updating ets cache

lib/nostrum/cache/user_cache/ets.ex

@@ -11,6 +11,7 @@ defmodule Nostrum.Cache.UserCache.ETS do
 
   @table_name :nostrum_users
 
+  alias Nostrum.Snowflake
   alias Nostrum.Struct.User
   use Supervisor
 
@@ -51,24 +52,31 @@ defmodule Nostrum.Cache.UserCache.ETS do
   @doc "Update a user from upstream data."
   @spec update(map()) :: {User.t() | nil, User.t()}
   def update(info) do
-    converted = User.to_struct(info)
+    # We don't know if the user_id is an atom or a string here.
+    user_id =
+      (Map.get(info, :id) || Map.get(info, "id"))
+      |> Snowflake.cast!()
 
-    with {:ok, old_user} <- lookup(info.id),
-         new_user = Map.merge(old_user, info),
+    with {:ok, old_user} <- lookup(user_id),
+         new_user = User.to_struct(info, old_user),
          false <- old_user == new_user do
       :ets.insert(@table_name, {new_user.id, new_user})
       {old_user, new_user}
     else
       {:error, _} ->
         # User just came online, make sure to cache if possible
         # TODO: check for `:global_name` once fully rolled out?
-        if Enum.all?([:username, :discriminator], &Map.has_key?(info, &1)),
-          do: :ets.insert(@table_name, {info.id, info})
+
+        converted = User.to_struct(info)
+
+        if Enum.all?([:username, :discriminator], &is_map_key(info, &1)),
+          do: :ets.insert(@table_name, {converted.id, converted})
 
         {nil, converted}
 
       true ->
-        {nil, converted}
+        {:ok, old_user} = lookup(user_id)
+        {old_user, old_user}
     end
   end
 

lib/nostrum/cache/user_cache/mnesia.ex

@@ -13,6 +13,7 @@ if Code.ensure_loaded?(:mnesia) do
     @behaviour Nostrum.Cache.UserCache
 
     alias Nostrum.Cache.UserCache
+    alias Nostrum.Snowflake
     alias Nostrum.Struct.User
     use Supervisor
 
@@ -80,24 +81,26 @@ if Code.ensure_loaded?(:mnesia) do
     end
 
     @impl UserCache
-    @doc "Update the given member for the given guild in the cache."
+    @doc "Update a User if it exists in the cache."
     @spec update(map()) :: {User.t() | nil, User.t()}
     def update(payload) do
-      new_user = User.to_struct(payload)
+      # We don't know if the user_id is an atom or a string here.
+      user_id =
+        (Map.get(payload, :id) || Map.get(payload, "id"))
+        |> Snowflake.cast!()
 
-      old_user =
-        :mnesia.activity(:sync_transaction, fn ->
-          case :mnesia.read(@table_name, new_user.id, :write) do
-            [{_tag, _id, old_user} = entry] ->
-              :mnesia.write(put_elem(entry, 2, new_user))
-              old_user
+      :mnesia.activity(:sync_transaction, fn ->
+        case :mnesia.read(@table_name, user_id, :write) do
+          [{_tag, _id, old_user} = entry] ->
+            new_user = User.to_struct(payload, old_user)
 
-            [] ->
-              nil
-          end
-        end)
+            :mnesia.write(put_elem(entry, 2, new_user))
+            {old_user, new_user}
 
-      {old_user, new_user}
+          [] ->
+            {nil, User.to_struct(payload)}
+        end
+      end)
     end
 
     @impl UserCache

lib/nostrum/struct/user.ex

@@ -177,4 +177,17 @@ defmodule Nostrum.Struct.User do
 
     struct(__MODULE__, new)
   end
+
+  @doc false
+  @spec to_struct(map(), nil | __MODULE__.t()) :: __MODULE__.t()
+  def to_struct(map, nil), do: to_struct(map)
+
+  def to_struct(map, old_user) do
+    new =
+      map
+      |> Map.new(fn {k, v} -> {Util.maybe_to_atom(k), v} end)
+      |> Map.update(:id, nil, &Util.cast(&1, Snowflake))
+
+    struct(old_user, new)
+  end
 end

test/nostrum/cache/user_cache_meta_test.exs

@@ -89,7 +89,7 @@ defmodule Nostrum.Cache.UserCacheMetaTest do
 
       describe "update/1" do
         test "returns `{nil, after}` on uncached user" do
-          payload = %{id: 8_284_967_893_178_597_859_421}
+          payload = %{id: 8_284_967_893_178_597}
           expected = {nil, User.to_struct(payload)}
           assert ^expected = @cache.update(payload)
         end