commanded/eventstore

jsonb_serializer does not work because it tries to convet keys in json type data fields to atoms

norpan opened this issue · 5 comments

When serializing an event using EventStore.JsonbSerializer I get:

** (ArgumentError) argument error
    :erlang.binary_to_existing_atom("value", :utf8)
    (eventstore 1.1.0) lib/event_store/jsonb_serializer.ex:35: anonymous fn/2 in EventStore.JsonbSerializer.keys_to_atoms/1
    (stdlib 3.13) maps.erl:233: :maps.fold_1/3
    (eventstore 1.1.0) lib/event_store/jsonb_serializer.ex:34: EventStore.JsonbSerializer.keys_to_atoms/1
    (eventstore 1.1.0) lib/event_store/jsonb_serializer.ex:35: anonymous fn/2 in EventStore.JsonbSerializer.keys_to_atoms/1
    (stdlib 3.13) maps.erl:233: :maps.fold_1/3
    (eventstore 1.1.0) lib/event_store/jsonb_serializer.ex:34: EventStore.JsonbSerializer.keys_to_atoms/1
    (eventstore 1.1.0) lib/event_store/jsonb_serializer.ex:30: EventStore.JsonbSerializer.to_struct/2

This is because my event struct contains a json data field which has the key "value". Those keys should not be changed to atoms. Removing the recursive call in keys_to_atoms on line 35 of jsonb_serializer.ex fixes the issue, but I'm not sure if this is the correct solution.

@norpan You could try using the Commanded.EventStore.Adapters.EventStore.JsonbSerializer serializer in the following Commanded EventStore adapter issue. That implementation does not recursively convert key fields from strings to atoms. You will need to copy & paste the module into your Elixir application and confgure the EventStore to use the serializer.

Thanks for the quick reply! I just made the change mentioned above in the module and put it in my application. I reported it because others may have the same issue, and I'm not sure why you'd ever want to change string keys to atoms recursively in this case. You'd only want to do it if it's struct keys, right?

Also, it does look like that code you are referring to is converting to atoms, it just doesn't use the to_existing_atom which gives an argument error if the atom doesn't exist already.

Yes, you're right. The difference between the two is String.to_atom/1 vs String.to_existing_atom/1.

Ideally whether atoms are converted to strings and how should be made configurable (see issue #299). Currently the only way of controlling serialization is to implement the EventStore.Serializer behaviour yourself.

Ok, maybe it's not a bug then. I'll just close it and use my own serialize. Thanks!