Merge pull request #20 from renatomassaro/improve-structs-support-in-…

…map-type

Improve structs support in Map type

lib/feeb/db/type/map.ex

@@ -53,6 +53,6 @@ defmodule Feeb.DB.Type.Map do
     nil
   end
 
-  defp load_structs(map, %{load_structs: false}), do: map
-  defp load_structs(map, _), do: Utils.Map.load_structs(map)
+  defp load_structs(map, %{load_structs: true}), do: Utils.Map.load_structs(map)
+  defp load_structs(map, _), do: map
 end

lib/feeb/db/utils/map.ex

@@ -58,36 +58,17 @@ defmodule Utils.Map do
 
   def load_structs(map) when is_map(map) do
     Enum.reduce(map, %{}, fn
-      {k, v}, acc when is_map(v) ->
-        new_v =
-          if Map.has_key?(v, :__struct__) or Map.has_key?(v, "__struct__") do
-            convert_to_struct(v[:__struct__] || v["__struct__"], v)
+      {k, v}, acc ->
+        {new_k, new_v} =
+          if k == :__struct__ or k == "__struct__" do
+            {:__struct__, String.to_existing_atom(v)}
           else
-            load_structs(v)
+            {k, load_structs(v)}
           end
 
-        Map.put(acc, k, new_v)
-
-      {k, v}, acc ->
-        Map.put(acc, k, load_structs(v))
+        Map.put(acc, new_k, new_v)
     end)
-    |> maybe_convert_top_level_struct()
   end
 
   def load_structs(v), do: v
-
-  defp maybe_convert_top_level_struct(%{__struct__: struct_mod} = entries),
-    do: convert_to_struct(struct_mod, entries)
-
-  defp maybe_convert_top_level_struct(%{"__struct__" => struct_mod} = entries),
-    do: convert_to_struct(struct_mod, entries)
-
-  defp maybe_convert_top_level_struct(map),
-    do: map
-
-  defp convert_to_struct(raw_struct_mod, entries) when is_binary(raw_struct_mod),
-    do: convert_to_struct(String.to_existing_atom(raw_struct_mod), safe_atomify_keys(entries))
-
-  defp convert_to_struct(struct_mod, entries) when is_atom(struct_mod),
-    do: struct(struct_mod, entries)
 end

priv/test/migrations/test/241020150400_all_types.sql

@@ -20,6 +20,7 @@ CREATE TABLE all_types (
   datetime_utc_precision_microsecond TEXT,
   datetime_utc_precision_default TEXT,
   map TEXT,
+  map_load_structs TEXT,
   map_nullable TEXT,
   map_keys_atom TEXT,
   map_keys_safe_atom TEXT,

test/db/schema_test.exs

@@ -49,6 +49,7 @@ defmodule DB.SchemaTest do
                :datetime_utc_precision_microsecond,
                :datetime_utc_precision_default,
                :map,
+               :map_load_structs,
                :map_nullable,
                :map_keys_atom,
                :map_keys_safe_atom,

test/db/type/map_test.exs

@@ -39,23 +39,39 @@ defmodule Feeb.DB.Type.MapTest do
       map = %{uri: uri, version: version}
       stringified_map = Utils.Map.stringify_keys(map)
 
-      params = AllTypes.creation_params(%{map: map, map_keys_string: map})
+      # By default, structs are *not* loaded (since loading may cause performance issues and isn't
+      # always desirable). When a struct is retrieved in a `load_structs: false` map, we'll simply
+      # return the :__struct__ key as is
+      map_with_structs_unloaded =
+        map
+        |> put_in([:uri, Access.key!(:__struct__)], "Elixir.URI")
+        |> put_in([:version, Access.key!(:__struct__)], "Elixir.Version")
+
+      params =
+        AllTypes.creation_params(%{
+          map: map,
+          map_load_structs: map,
+          map_keys_string: map
+        })
 
       # Structs are parsed correctly
       all_types = AllTypes.new(params)
       assert all_types.map == map
+      assert all_types.map_load_structs == map
       assert all_types.map_keys_string == stringified_map
 
       DB.begin(@context, shard_id, :write)
       assert {:ok, db_all_types} = DB.insert(all_types)
-      assert db_all_types.map == map
+      assert db_all_types.map == map_with_structs_unloaded
+      assert db_all_types.map_load_structs == map
       assert db_all_types.map_keys_string == stringified_map
 
       # Structs are stored next to the map, under the __struct__ key, except in the stringified map
-      assert [[raw_map, raw_map_keys_string]] =
-               DB.raw!("select map, map_keys_string from all_types")
+      assert [[raw_map, raw_map_load_structs, raw_map_keys_string]] =
+               DB.raw!("select map, map_load_structs, map_keys_string from all_types")
 
       assert raw_map =~ "\"__struct__\":\"Elixir.URI\""
+      assert raw_map_load_structs =~ "\"__struct__\":\"Elixir.URI\""
       refute raw_map_keys_string =~ "\"__struct__\":\"Elixir.URI\""
     end
 
@@ -65,6 +81,7 @@ defmodule Feeb.DB.Type.MapTest do
       params =
         AllTypes.creation_params(%{
           map: version,
+          map_load_structs: version,
           map_keys_string: version,
           map_keys_safe_atom: version
         })
@@ -78,17 +95,57 @@ defmodule Feeb.DB.Type.MapTest do
       # entirely and instead we end up with the raw map with stringified keys
       all_types = AllTypes.new(params)
       assert all_types.map == version
+      assert all_types.map_load_structs == version
       assert all_types.map_keys_safe_atom == version
       assert all_types.map_keys_string == stringified_map
 
       # We can store/load the struct in the database (when keys is atom or safe_atom)
       DB.begin(@context, shard_id, :write)
       assert {:ok, db_all_types} = DB.insert(all_types)
-      assert db_all_types.map == version
-      assert db_all_types.map_keys_safe_atom == version
+      assert db_all_types.map == Map.put(version, :__struct__, "Elixir.Version")
+      assert db_all_types.map_load_structs == version
+      assert db_all_types.map_keys_safe_atom == Map.put(version, :__struct__, "Elixir.Version")
       assert db_all_types.map_keys_string == stringified_map
     end
 
+    test "stores and loads struct (struct inside struct)", %{shard_id: shard_id} do
+      # `v2` has `v1` within it (nested struct)
+      v1 = %Version{major: 1, minor: 0, patch: 0, build: nil, pre: []}
+      v2 = %Version{major: 2, minor: 0, patch: 0, build: v1, pre: []}
+
+      stringified_v1 = Utils.Map.stringify_keys(v1)
+      stringified_v2 = Utils.Map.stringify_keys(v2)
+
+      params =
+        AllTypes.creation_params(%{
+          map: v2,
+          map_load_structs: v2,
+          map_keys_string: v2
+        })
+
+      all_types = AllTypes.new(params)
+
+      # `map` will keep the struct as-is because it hasn't been serialized to/from JSON yet
+      assert all_types.map == v2
+      assert all_types.map_load_structs == v2
+      assert all_types.map_load_structs.build == v1
+      assert all_types.map_keys_string == stringified_v2
+      assert all_types.map_keys_string["build"] == stringified_v1
+
+      # We can store/load the struct in the database (when keys are atomified)
+      DB.begin(@context, shard_id, :write)
+      assert {:ok, db_all_types} = DB.insert(all_types)
+
+      # After loaded, `map` will return the "naive" map whereas `%{load_structs: true}` will iterate
+      # over each value in the map and convert any structs it finds along the way
+      assert db_all_types.map.__struct__ == "Elixir.Version"
+      assert db_all_types.map.build == Map.put(v1, :__struct__, "Elixir.Version")
+      assert db_all_types.map_load_structs == v2
+      assert db_all_types.map_load_structs.build == v1
+      assert db_all_types.map_keys_string == stringified_v2
+      assert db_all_types.map_keys_string["build"] == stringified_v1
+    end
+
     test "supports nullable", %{shard_id: shard_id} do
       params = AllTypes.creation_params(%{map_nullable: nil})
 

test/support/db/schemas/all_types.ex

@@ -23,6 +23,7 @@ defmodule Sample.AllTypes do
     {:datetime_utc_precision_microsecond, {:datetime_utc, precision: :microsecond, nullable: true}},
     {:datetime_utc_precision_default, {:datetime_utc, nullable: true}},
     {:map, :map},
+    {:map_load_structs, {:map, load_structs: true, nullable: true}},
     {:map_nullable, {:map, nullable: true}},
     {:map_keys_atom, {:map, keys: :atom, nullable: true}},
     {:map_keys_safe_atom, {:map, keys: :safe_atom, nullable: true}},