Crystal: JSON#mapping with custom converters and array types

[HCLarsen]

When trying to parse JSON with an array of time objects, and using a custom converter, I'm getting an error that #to_json doesn't output an array. I would have expected the code generated from JSON#mapping macro to know to apply the converter to each individual element in the array, but it seems otherwise. I'm not sure, is this the intended functionality, or is it a bug?

Sample code:

require "json"

class MyClass
  JSON.mapping(
    times: {type: Array(Time), converter: Time::EpochConverter}
  )
end

times = [] of Int64

times << Time.new.to_unix
times << Time.utc.to_unix

times.to_json

json = MyClass.from_json(times.to_json)
json.to_json

The specific error is:

no overload matches 'Time::EpochConverter.to_json' with types Array(Time), JSON::Builder
Overloads are:

• Time::EpochConverter.to_json(value : Time, json : JSON::Builder)
• Object#to_json(io : IO)
• Object#to_json()

[asterite]

Time::EpochConverter is for converting a String into Time and the other way around, not from Array into Time.

This is an interesting problem.

I think one way we can achieve this is by having JSON::ArrayConverter(Converter) which basically encodes/decodes an array using another converter as a generic type argument. Then you would use it like this:

require "json"

class MyClass
  JSON.mapping(
    times: {type: Array(Time), converter: JSON::ArrayConverter(Time::EpochConverter)}
  )
end

times = [] of Int64

times << Time.new.to_unix
times << Time.utc.to_unix

times.to_json

json = MyClass.from_json(times.to_json)
json.to_json

We should probably have JSON::HashConverter(Converter) too. And do the same with YAML.

What do you think?

I did a small test and it seems to work fine. The implementation I have so far is:

module JSON::ArrayConverter(Converter)
  def self.from_json(pull : JSON::PullParser)
    ary = Array(typeof(Converter.from_json(pull))).new
    pull.read_array do
      ary << Converter.from_json(pull)
    end
    ary
  end

  def self.to_json(values : Array, builder : JSON::Builder)
    builder.array do
      values.each do |value|
        Converter.to_json(value, builder)
      end
    end
  end
end

[HCLarsen]

I think it's a great solution, along with the HashConverter and adding both to YAML#mapping as well.

I just used your implementation in the project where I discovered this issue, and it appears to be working fine. Is it ok if I continue to use it until this gets added to the core library?

[asterite]

Of course!

[RX14]

@asterite do you mind PRing this?

[asterite]

It's a bit of work so I'll take my time, unless someone wants to work on this.

[rodrigopinto]

@asterite Have you started or would be ok if try to work on this?

[asterite]

@rodrigopinto Nope, please go ahead if you want 🙇