crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.34k stars 1.62k forks source link

Support for key transform in `JSON::Serializable::Options` #10793

Open kimburgess opened 3 years ago

kimburgess commented 3 years ago

Feature Request

When mapping from an external JSON model to an internal type it is possible to use the JSON::Field annotation's key property to define any differences in naming. This works well for single keys that require explicit overrides, but becomes quite verbose when the key format itself differs.

As an example, an external service may present

{
   "anExampleKey": 42,
   "anotherItem": "foo"
}

Which may be desirable to capture as:

struct Example
  include JSON::Serializable

  @[JSON::Field(key: "anExampleKey")]
  property an_example_key : Int32

  @[JSON::Field(key: "anotherItem")]
  property another_item : String
end

This could be specified more succinctly by defining a single key transform for the object.

Adding a property—key_converter—that enables a converter (similar to the JSON::Field converter property) appears to be a clean solution for this. This would point to a type that defines from_json_key(String) : String and to_json_key(String) : String

@[JSON::Serializable::Options(key_converter: JSON::Serializable::LowerCamelCaseCoverter)]
struct Example
  include JSON::Serializable
  property an_example_key : Int32
  property another_item : String
end

Proposal is to add support for this option, along with the following set of converters (each mapping to/from snake_case_form as per the default crystal naming convention):

Converter Example
LowerCamelCaseConverter an_example"anExample"
UpperCamelCaseConverter an_example"AnExample"
CapitalizedSnakeCaseConverter an_example"AN_EXAMPLE"
asterite commented 3 years ago

This is a really good idea!

asterite commented 3 years ago

I do wonder if this wouldn't be easier with just a symbol instead of a class. What would the method to convert the key look like? It seems like it would need to be a macro? But we don't have custom macro methods... Just thinking out loud.

Blacksmoke16 commented 3 years ago

But we don't have custom macro methods

yet :wink:. cough #8835 cough.

I do wonder if this wouldn't be easier with just a symbol instead of a class

FWIW, Athena's serializer supports this via a symbol as well: either :camelcase, :underscore, or :identical, with the default being :identical which essentially means "use whatever the ivar's name is". Granted this doesn't allow for super custom stuff, but it deff handles all the common ones, which should be sufficient for most use cases.

See https://athenaframework.org/Serializer/Annotations/Name/#Athena::Serializer::Annotations::Name--naming-strategies.

kimburgess commented 3 years ago

The transform should be able to happen at runtime, as this is when all the key matching runs too: https://github.com/crystal-lang/crystal/blob/a190f2423aca6922979b933645e10c9bc2ddf7ab/src/json/serialization.cr#L192-L195 Doing this also allows a little more flexibility in non std-lib usage of this as it can be extended arbitrarily to match all the ~terrifying~ interesting ways different services present models in the real world.

kimburgess commented 3 years ago

Further thoughts on behaviour: if a key is explicitly defined on the field, this should take precedence over a key transform specified on the containing type. This will require a minor shuffle of JSON::Serializable to support that, but looks pretty doable.

straight-shoota commented 3 years ago

As an alternative solution, we could use a method hook to transform the key value. By default, the method would just return the input value, but it can be overridden to apply any kind of transformations.

struct Example
  include JSON::Serializable
  property an_example_key : Int32
  property another_item : String

  protected def convert_json_key(value)
    value.camelcase
  end
end
Sija commented 3 years ago

@straight-shoota But then if you want to share this functionality you still end up with some kind of converter modules which you include.

straight-shoota commented 3 years ago

Yes, that's similar. But the benefit of this solution is that it's easier to implement a customized mapping. Simple upper or lower camelcase may work well in many use cases, but I'm pretty sure that quite often there's need for more control, because maybe just one or two field names have some inconsistencies not covered by the generic transformation. Then you just need to implement this in the overridden hook method, there's no need to reference it anywehere.

naqvis commented 3 years ago

Since module already has Class annotation JSON::Serializable::Options, we can follow the style of JSON::Field via which one can configure a key converter and provide a type which implements an interface with methods for conversion from and to.

Interface could be something like

def self.from_json_key(val)
def self.to_json_key(val)

This will have the added benefits of:

  1. Explicitness
  2. Follow the existing API semantics
  3. Have hooks to perform any simple and/or complex transformation/conversion
  4. stdlib may or may not provide any implementation for custom key transformations (as they are use-case specific)
straight-shoota commented 3 years ago

I think two-way conversion would be unnecessary and could easily lead to inconsistencies when the transformations do not match up exactly.

kostya commented 3 years ago

for me this JSON::Serializable::Options looks quite weird, maybe remove it? I think @straight-shoota example is good, just like other extensions add it as module:

struct Example
  include JSON::Serializable
  include JSON::Serializable::CamelCaseKey

  property an_example_key : Int32
  property another_item : String
end

module JSON::Serializable::CamelCaseKey
  protected def convert_json_key(value)
    value.camelcase
  end
end
naqvis commented 3 years ago

I think two-way conversion would be unnecessary and could easily lead to inconsistencies when the transformations do not match up exactly.

but then one-way conversion is definitely going to lead to inconsistent output of to_json, as field name is already converted/transformed during the conversion and there is no memoization mechanism in place which can keep track of original value.

naqvis commented 3 years ago

for me this JSON::Serializable::Options looks quite weird, maybe remove it?

Annotation syntax is hard to get used to and annotation usage in API is quite limited, so at first glance it gives such weird feelings to its users, but that doesn't make this language feature obsolete.

straight-shoota commented 3 years ago

One-way conversion goes from ivar name to JSON field name. The ivar name is explicit in code, so that's a given and there's no need to convert to that. This conversion directly enables to_json and it also works for from_json with comparison based on the JSON field name.

naqvis commented 3 years ago

This conversion directly enables to_json and it also works for from_json with comparison based on the JSON field name.

This is where i'm trying to understand how one way conversion is going to enable bi-directional ivar name <-> JSON field name mapping. Does that mean same conversion hook will be invoked twice one at the time of parsing JSON (JSON Field -> ivar association) and 2nd at the time of building json (ivar -> JSON field name)?

straight-shoota commented 3 years ago

In #to_json it is applied as json.field(convert_json_key({{value[:key]}})) { ... }, and in .from_json it goes like this:

case key = pull.read_object_key
{% for name, value in properties %}
when convert_json_key({{value[:key]}})
  # ...
{% end %}
end