Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate out of the box JSON::Serializable support #250

Closed
Blacksmoke16 opened this issue Jul 4, 2018 · 4 comments
Closed

Investigate out of the box JSON::Serializable support #250

Blacksmoke16 opened this issue Jul 4, 2018 · 4 comments

Comments

@Blacksmoke16
Copy link
Contributor

Blacksmoke16 commented Jul 4, 2018

I would like to start a discussion on how doable support for Crystal JSON::Serializable would be for Granite.

If this works out how I think it will this would be a major improvement on the ability for models to work much better with JSON APIs. It would allow:

  1. Better .from_json support
  2. Possibly allow for the JSON::Field annotations to better control serialization/deserialization.
  3. Tap into the after_initialize event. I..e you could run validations after consuming a JSON object to check if the provided values are allowed with the provided validate_* validators. Or populate meta fields based on JSON values + a DB query for example.

I'm not too familiar with how/what would have to be done to support this (or if its even doable). So i wanted to see what thoughts you guys have.

@robacarp
Copy link
Member

robacarp commented Jul 9, 2018

@Blacksmoke16 I'm all for ideas! The current json integration is minimal at best and various ideas have been floated in the past, both here and on Amber.

@Blacksmoke16
Copy link
Contributor Author

Support for this would be amazing.

I been looking into it here and there but im not too sure what is going on still.

It seems from_json is trying to create a Symbol type that is not able to be instantiated.

instantiating '(Symbol | Nil):Class#new(JSON::PullParser)'
in /usr/share/crystal/src/json/from_json.cr:196: expanding macro

  {% begin %}
  ^

in macro 'macro_140137072880928' /usr/share/crystal/src/json/from_json.cr:196, line 18:

   1. 
   2.     # Here we store types that are not primitive types
   3.     
   4. 
   5.     
   6.       
   7.         
   8.       
   9.     
  10.       
  11.         return pull.read_null if pull.kind == :null
  12.       
  13.     
  14. 
  15.     # If after traversing all the types we are left with just one
  16.     # non-primitive type, we can parse it directly (no need to use `read_raw`)
  17.     
> 18.       return Symbol.new(pull)
  19.     
  20.   

undefined method 'new' for Symbol:Class

@Blacksmoke16
Copy link
Contributor Author

Blacksmoke16 commented Jul 15, 2018

Update:

This is due to the usage of @_current_callback using the type Symbol?, and our @errors instance var not having an overload for JSON::PullParser

Looking into it more

in /usr/share/crystal/src/json/from_json.cr:111: no overload matches 'Granite::Error.new' with type JSON::PullParser
Overloads are:
 - Granite::Error.new(field : String | Symbol | JSON::Any, message : String | ::Nil = "")

    yield T.new(pull)
            ^~~

But if you add an initialize method for PullParser it yells at you saying instance variable '@granite_errors' of Granite::Base must be Array(Granite::Error), not (Array(Granite::Error) | Array(JSON::Error))

Update 2: Not really sure on how to best fix the errors thing since adding that type to errors array goes down the rabbit hole more. It then yells as you there is no overload for JSON::Error.new with JSON::PullParser

@robacarp any ideas?

Update 3: Was able to fix it by making the error array of type Granite::Error vs just Error

Update 4: Was able to have a better fix by adding some @[JSON::Field(ignore: true)] to the errors array so from_json doesnt try to parse data into it, or display the array when doing to_json

@Blacksmoke16
Copy link
Contributor Author

Fixed with #253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants