chore: resolve sidekiq json-unsafe warning

[QQism]

Sidekiq 6.4 has started to log warnings
(sidekiq/sidekiq#5071) if the job arguments are
not strictly JSON-safe i.e. serializing to JSON and deserializing from
JSON yield consistent outputs. To what it means, we cannot use a hash
with symbol keys since doing the JSON rountrip will change them all to
string keys.

This fix will transform the hash argument with symbol keys to string
keys so Sidekiq won't complain it (and break it starting version 7.0).

Before

After

[porbas]

Thank you for your contribution @QQism!
I worked on it recently in other place and came up with slightly different solution f8a2e76

The only not JSON compatlibe are symbol keys there, so let's simply stringify them, without JSON dump/parse roundtrip.
What do you think about this approach?

[QQism]

Thank you for your contribution @QQism! I worked on it recently in other place and came up with slightly different solution f8a2e76

The only not JSON compatlibe are symbol keys there, so let's simply stringify them, without JSON dump/parse roundtrip. What do you think about this approach?

Hi @porbas, thanks for the reply. My concern at first is a hash could contain nested hashes (with symbolized keys), so we would need something like Rails's deep_stringify_keys.

Personally I think the JSON roundtrip is the simplest approach here since

• (1) the method is already a Ruby built-in thing and we don't need to write a complicated recursive method for the nested hash (assuming we don't want to introduce Rails dependencies)
• and (2) we can be sure that our arguments sending to Sidekiq (after the JSON roundtrip) are actually JSON-safe, which is why Sidekiq added the warning in the first place.

[porbas]

My concern at first is a hash could contain nested hashes

There is no such possibility. We're dealing with SerializedRecord here and calling to_h on it. We have only one level of symbol keys and all JSON compatible values - given that used serializer dumps to Strings or other JSON compatible types. It is true for YAML and for JSON serializers.

https://github.com/RailsEventStore/rails_event_store/blob/master/ruby_event_store/lib/ruby_event_store/record.rb#L54-L64
https://github.com/RailsEventStore/rails_event_store/blob/master/ruby_event_store/lib/ruby_event_store/serialized_record.rb#L42-L51

[QQism]

We have only one level of symbol keys and all JSON compatible values - given that used serializer dumps to Strings or other JSON compatible types

I didn't know about it. Then, I guess transform_keys would be sufficient here. Happy to make the change using your solution.

[porbas]

Thank you. Merged.

I've changed transform_keys! to transform_keys which is sufficient here.
I've also added test case to have 100% mutant test coverage.

You can always run mutation tests by running make mutate

[pedrocarmona]

Hi @porbas, I just noticed this change for the gem contrib/ruby_event_store-sidekiq_scheduler is not yet released to rubygems.

Meanwhile, I did:

gem 'ruby_event_store-sidekiq_scheduler', github: 'RailsEventStore/rails_event_store', glob: 'contrib/ruby_event_store-sidekiq_scheduler/*.gemspec'

However, in my app, I still see some events that still raise a unsafe serialization alert from sidekiq, but perhaps its app specific, and I need to check them closer.

[porbas]

Hi @pedrocarmona, I've released v 0.1.1 of ruby_event_store-sidekiq_scheduler. Thanks!

Feel free to create another PR/issue in case you found further issues.