bbalser / json_serde

Apache License 2.0
11 stars 4 forks source link

Maps with atom keys not handled correctly #8

Open mercurio opened 3 years ago

mercurio commented 3 years ago

A map with atoms for keys is not serialized/deserialized correctly:

iex(4)> x = %{foo: 42}
%{foo: 42}
iex(5)> y = JsonSerde.deserialize!(JsonSerde.serialize!(x))
%{"foo" => 42}
iex(6)> x.foo
42
iex(7)> y.foo
** (KeyError) key :foo not found in: %{"foo" => 42}

Atom keys in custom structs are handled correctly:

iex(7)> defmodule Foo do
...(7)> defstruct [:foo]
...(7)> end
{:module, Foo, ...
iex(8)> xx = %Foo{foo: 42}
%Foo{foo: 42}
iex(9)> yy = JsonSerde.deserialize!(JsonSerde.serialize!(xx))
%Foo{foo: 42}
iex(10)> xx.foo
42
iex(11)> yy.foo
42

Also, numerical keys are not handled correctly:

iex(4)> mat = %{0 => "a", 1 => "b"}
%{0 => "a", 1 => "b"}
iex(5)> mat2 = JsonSerde.deserialize!(JsonSerde.serialize!(mat))
%{"0" => "a", "1" => "b"}
iex(6)> mat[0]
"a"
iex(7)> mat2[0]
nil

It appears that only string keys are supported. Is there a way to get this to work?

tmaszk commented 4 months ago

I had a need for maps with atom keys as well. I created a fork and started a PR in this repo.

With the changes in my fork, here's an example of serializing/deserializing with atom keys:

iex> map = %{a: 1, b:  2, c: "three"}
%{a: 1, b: 2, c: "three"}

iex> {:ok, serialized} = JsonSerde.serialize(map)
{:ok, "{\"a__is_atom__\":1,\"b__is_atom__\":2,\"c__is_atom__\":\"three\"}"}

iex> JsonSerde.deserialize(serialized)
{:ok, %{a: 1, b: 2, c: "three"}}

On note, the new functionality is optional and is off by default. To enable the new functionality, add this line to config.exs

config :json_serde, :encode_atom_keys, true

There are cleaner ways of accomplishing this, but I was trying to make it easy/optional to adopt the change.

bbalser commented 4 months ago

I am so sorry I missed this issue/PR. Will take a look at it tomorrow.