Support JSON::Serializable #253

Blacksmoke16 · 2018-07-15T15:16:56Z

This PR, fixes #253 brings about native support for JSON::Serializable. This is super exciting and probably my favorite PR so far. The benefits of this are immense, especially when working with JSON APIs.

class Foo < Granite::Base
  adapter mysql
  table_name foos

  field name : String
  field age : Int32
end

Single Object

foo = Foo.from_json(%({"name": "Granite", "age": 10}))

<Foo:0x55c77d901ea0
 @_current_callback=nil,
 @age=10,
 @created_at=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @name="Granite",
 @new_record=true,
 @updated_at=nil>

foo.to_json

{
  "name": "Granite",
  "age": 10
}

Array of Objects

foo = Array(Foo).from_json(%([{"name": "Granite1", "age": 10},{"name": "Granite2", "age": 50}]))

[#<Foo:0x55f66bdeaea0
  @_current_callback=nil,
  @age=10,
  @created_at=nil,
  @destroyed=false,
  @errors=[],
  @id=nil,
  @name="Granite1",
  @new_record=true,
  @updated_at=nil>,
 #<Foo:0x55f66bdeae10
  @_current_callback=nil,
  @age=50,
  @created_at=nil,
  @destroyed=false,
  @errors=[],
  @id=nil,
  @name="Granite2",
  @new_record=true,
  @updated_at=nil>]

foo.to_json

[
  {
    "name": "Granite1",
    "age": 10
  },
  {
    "name": "Granite2",
    "age": 50
  }
]

It works pretty much out of the box, but with one slight breaking change. By default, as you may have noticed above, when doing to_json it will not add nil values into the object, i.e. the id. If the user wants that, they would have to do:

@[JSON::Serializable::Options(emit_nulls: true)]
class Foo < Granite::Base
  adapter mysql
  table_name foos

  field name : String
  field age : Int32
end

foo = Foo.from_json(%({"name": "Granite", "age": 10}))

<Foo:0x55c77d901ea0
 @_current_callback=nil,
 @age=10,
 @created_at=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @name="Granite",
 @new_record=true,
 @updated_at=nil>

foo.to_json

{
  "name": "Granite",
  "age": 10,
  "id": null,
  "created_at": null,
  "updated_at": null
}

after_initialize

Another cool feature this brings is the after_initialize method. This method gets called after from_json is done parsing the given json. This allows you to set other fields that are not in the JSON directly or that require some more logic.

class Foo < Granite::Base
  adapter mysql
  table_name foos

  field name : String
  field age : Int32

  def after_initialize
      @age = 0
  end
end

foo = Foo.from_json(%({"name": "Granite1"}))

<Foo:0x55c77d901ea0
 @_current_callback=nil,
 @age=0,
 @created_at=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @name="Granite",
 @new_record=true,
 @updated_at=nil>

JSON::Serializable::Unmapped

 If the JSON::Serializable::Unmapped module is included, unknown properties in the JSON document will be stored in a Hash(String, JSON::Any). On serialization, any keys inside json_unmapped will be serialized appended to the current json object.

class Foo < Granite::Base
  include JSON::Serializable::Unmapped

  adapter mysql
  table_name foos

  field name : String
  field age : Int32
end

foo = Foo.from_json(%({"name": "Granite1", "age": 12, "foobar": true}))

#<Foo:0x55efba40ff00
 @_current_callback=nil,
 @age=12,
 @created_at=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @json_unmapped={"foobar" => true},
 @name="Granite1",
 @new_record=true,
 @updated_at=nil>

foo.to_json

{
  "name": "Granite",
  "age": 12,
  "foobar": true
}

on_to_json

Allows the user to tap into the to_json and do additional manipulations before returning it.

class Foo < Granite::Base
  adapter mysql
  table_name foos

  field name : String
  field age : Int32

  def on_to_json(json : JSON::Builder)
      json.field("years_young", @age)
  end
end

foo = Foo.from_json(%({"name": "Granite1", "age": 12}))

#<Foo:0x55eb12bd2f00
 @_current_callback=nil,
 @age=12,
 @created_at=nil,
 @destroyed=false,
 @errors=[],
 @id=nil,
 @name="Granite1",
 @new_record=true,
 @updated_at=nil>

foo.to_json

{
  "name": "Granite",
  "age": 12,
  "years_young": 12
}

Blacksmoke16 · 2018-07-15T16:06:39Z

Latest commit adds support for the @[JSON::Field()] property annotations

Usage is like field name : String, json_options: {ignore: true}

Is it worth adding specs for each of these options, or at this point are we just testing the std lib?

drujensen

@Blacksmoke16 This looks so much better. Thanks! 💯

Blacksmoke16 · 2018-07-16T13:30:55Z

👍 Yea no kidding, much better. Don't merge yet, think i want to include some more details in readme on at least the json_options hash on fields macro.

drujensen · 2018-07-16T13:31:26Z

@Blacksmoke16 Is there any side affects using the native version? I recall attempting this before but the constructor required supporting the JSON::Puller or something like that.

Blacksmoke16 · 2018-07-16T13:41:52Z

This is a bit difference than the JSON.mapping version. This was just released with Crystal 0.25.0.

Using this it isn't possible to give like Model.create(JSON::ANY), would have to do Model.from_json(json_string).

All the tests passed and all the Serializable extra stuff seems to work fine so unless something comes up i think we're good.

drujensen · 2018-07-16T13:47:29Z

aaah, I was wondering how this was possible without the constructor. I will checkout 0.25 release notes for more info. This is all excellent news! I prefer the .from_json so that is fine.

Blacksmoke16 · 2018-07-16T13:59:57Z

https://crystal-lang.org/api/0.25.1/JSON/Serializable.html

Is a good overview of it.

robacarp

I love how much this simplifies the code.

The only danger sign I see is the dependence on JSON::Serializable's use of after_initialize, which is apparently undocumented.

The annotations feel like a hack, but that's not your doing, it's just the way the stdlib was implemented.

I have a hunch json::serializable is going to get a substantial refactor before it really solidifies in crystal. We'll see I guess.

robacarp · 2018-07-16T16:42:15Z

src/granite/error.cr

-    else
-      "#{@field.to_s.capitalize} #{message}"
-    end
+    (@field == :base) ? @message : "#{@field.to_s.capitalize} #{message}"


Can we leave this as it was? I don't see the benefit to the more subtle syntax.

I can change it back if you want. I just have a thing for ternary, easier to read and cleaner imo.

I guess this is the sort of thing that a style guide helps communicate. My preference is to avoid ternaries wherever possible.

More than anything, this is an unrelated change to the rest of the pull. I don’t feel really strong about changing this, but I’d like to avoid someone else changing it back simply because they prefer it to be the other way. It just creates more potential for bugs to sneak in and opens the door for endless bike shedding.

Fair enough.

robacarp · 2018-07-16T16:42:53Z

src/granite/fields.cr

@@ -39,6 +39,9 @@ module Granite::Fields
    {% for name, options in FIELDS %}
      {% type = options[:type] %}
      {% suffixes = options[:raise_on_nil] ? ["?", ""] : ["", "!"] %}
+      {% if options[:json_options] %}
+         @[JSON::Field({{**options[:json_options]}})]
+      {% end %}


clever, I like it.

Blacksmoke16 · 2018-07-16T17:15:23Z

There is no dependence on after_initialize. It is just an undocumented feature that can be used.

It could be removed and the basic implementation would still be fine. It is just a method that gets called after the from_json method is finished.

Blacksmoke16 · 2018-07-17T01:10:58Z

Updated README on JSON support. Can see it at https://github.com/Blacksmoke16/granite/tree/JSON-Serializable-support#json-support.

Otherwise this is good to merge unless one of you finds an issue.

drujensen · 2018-07-17T19:05:21Z

@robacarp can we merge this?

robacarp

thanks @Blacksmoke16

Support JSON::Serializable

5a58c07

Blacksmoke16 added kind:enhancement pr:wip kind:breaking-change labels Jul 15, 2018

Field specific JSON serrialization options

92f48fb

Update readme

94c51a8

Blacksmoke16 added pr:needs-review and removed pr:wip labels Jul 16, 2018

Blacksmoke16 requested a review from a team July 16, 2018 12:02

drujensen approved these changes Jul 16, 2018

View reviewed changes

robacarp reviewed Jul 16, 2018

View reviewed changes

Add JSON section to README

55be8bc

This was referenced Jul 17, 2018

Support for fixtures #163

Open

Support YAML::Serializable #255

Merged

drujensen and others added 2 commits July 17, 2018 12:05

Merge branch 'master' into JSON-Serializable-support

7996770

Change error.to_s back to if/else

4d4617d

robacarp approved these changes Jul 17, 2018

View reviewed changes

robacarp merged commit 4c94445 into amberframework:master Jul 17, 2018

Blacksmoke16 mentioned this pull request Jul 19, 2018

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

Closed

Blacksmoke16 deleted the JSON-Serializable-support branch July 20, 2018 00:51

Blacksmoke16 mentioned this pull request Jul 23, 2018

Revert .is_a?(JSON::Any) #262

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support JSON::Serializable #253

Support JSON::Serializable #253

Blacksmoke16 commented Jul 15, 2018 •

edited

Loading

Blacksmoke16 commented Jul 15, 2018 •

edited

Loading

drujensen left a comment

Blacksmoke16 commented Jul 16, 2018

drujensen commented Jul 16, 2018 •

edited

Loading

Blacksmoke16 commented Jul 16, 2018

drujensen commented Jul 16, 2018

Blacksmoke16 commented Jul 16, 2018

robacarp left a comment

robacarp Jul 16, 2018

Blacksmoke16 Jul 16, 2018

robacarp Jul 17, 2018

Blacksmoke16 Jul 17, 2018

robacarp Jul 16, 2018

Blacksmoke16 commented Jul 16, 2018

Blacksmoke16 commented Jul 17, 2018

drujensen commented Jul 17, 2018

robacarp left a comment

Support JSON::Serializable #253

Support JSON::Serializable #253

Conversation

Blacksmoke16 commented Jul 15, 2018 • edited Loading

Single Object

Array of Objects

after_initialize

JSON::Serializable::Unmapped

on_to_json

Blacksmoke16 commented Jul 15, 2018 • edited Loading

drujensen left a comment

Choose a reason for hiding this comment

Blacksmoke16 commented Jul 16, 2018

drujensen commented Jul 16, 2018 • edited Loading

Blacksmoke16 commented Jul 16, 2018

drujensen commented Jul 16, 2018

Blacksmoke16 commented Jul 16, 2018

robacarp left a comment

Choose a reason for hiding this comment

robacarp Jul 16, 2018

Choose a reason for hiding this comment

Blacksmoke16 Jul 16, 2018

Choose a reason for hiding this comment

robacarp Jul 17, 2018

Choose a reason for hiding this comment

Blacksmoke16 Jul 17, 2018

Choose a reason for hiding this comment

robacarp Jul 16, 2018

Choose a reason for hiding this comment

Blacksmoke16 commented Jul 16, 2018

Blacksmoke16 commented Jul 17, 2018

drujensen commented Jul 17, 2018

robacarp left a comment

Choose a reason for hiding this comment

Blacksmoke16 commented Jul 15, 2018 •

edited

Loading

Blacksmoke16 commented Jul 15, 2018 •

edited

Loading

drujensen commented Jul 16, 2018 •

edited

Loading