Make avro serde accept clj maps with underscores, add option to skip mangling names.

[sathyavijayan]

Ive made a quick fix/change to the avro serde to make it accept non-idiomatic, but perfectly valid maps as input. For eg.

{"string_field": "hello", :enum_field "a_1"}

I also added an option to skip mangling names like in the abracad library. This will be useful when we need to use the original message.

[cddr]

I think this would be a useful change. If I had our time again, *mangle-names* == false would be the default but I think the ship has sailed on that.

@creese do you accept this in principle? If so, we might consider adding it as an option here

jackdaw/src/jackdaw/serdes/avro.clj

Line 607 in 6307689

key?]

so that end-users don't need to fiddle with dynamic vars.

[cddr]

It took me a while before I realized that the difference between the 1st and 2nd round-trip tests here was that the enum field was expressed as a string in the first, and a keyword in the second. Would you mind putting each one inside a testing block so that they can more easily be distinguished?

[sathyavijayan]

Thanks. I have restructured the tests and added documentation to make them more readable.

I used a dynamic var for *mangle-names* as it made more sense for me to have it as an 'application-level configuration'. It will lead to a consistent low level design within an application - either use idiomised values or not. (although, I agree the degree of ambiguity wouldn't be much as there will always be a default setting).

I noticed that the mangling is done only for the serde and not for json etc., So, as you mentioned, it would make sense to drop it altogether and push the responsibility to the apps if they need such a feature. The dynamic var will be a nice way to provide backward compatibility during transition if you decide to drop mangling in a future release.

Let me know your thoughts.

[sathyavijayan]

Hi @cddr, @creese any thoughts on this PR ? Let me know if you still like the dyn var to be moved to the topic-config map (last arg to the serde function) instead. The map is named topic-config which makes it a weird place for the mangle-names key - what do you think ?

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@4bbb6de). Click here to learn what that means.
The diff coverage is 87.5%.

@@            Coverage Diff            @@
##             master     #126   +/-   ##
=========================================
  Coverage          ?   80.86%           
=========================================
  Files             ?       39           
  Lines             ?     2315           
  Branches          ?      147           
=========================================
  Hits              ?     1872           
  Misses            ?      296           
  Partials          ?      147

Impacted Files	Coverage Δ
src/jackdaw/serdes/avro.clj	86.91% <87.5%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bbb6de...8b7a894. Read the comment docs.

[peey]

Anything blocking this from being merged?

[obohrer]

Can this be merged? This caused us issues as well as the data mangling was a surprise to us

[dakra]

+1 for merging this. Not be able to use maps with snake_case is a bit of a showstopper.