Add the ability to reset object properties to `nil` from JSON by DulMephistos · Pull Request #403 · tristanhimmelman/ObjectMapper

DulMephistos · 2016-03-03T08:37:23Z

The current version has a problem that makes it impossible for us to in our app in production, so we did all necessary modifications and now I'm requesting this pull request here and it may help other as well.

We needed the ability to reset some fields to nil when updating existing objects with new JSON data. We do that by setting the desired field in the JSON string with "null" value and we expect that ObjectMapper will set that field to nil when mapping to the object since the JSON contains the field in its body.

Example:

Imagine a `Player` class like this:

class Player {
   var firstName: String?
   var lastName: String?
   var team: String?
}

Initialized:

let json = "{\"firstName\": \"Bob\",\"lastName\": \"Sass\",\"team\": \"Red\"}"
let player = Mapper<Player>().map(json)

Update name and remove team

let json = "{\"firstName\": \"Ryan\",\"lastName\": \"Mitchel\",\"team\": null}"
Mapper().map(json, toObject: player)

This was previously impossible because the Mapper would not update the field if the value was nil

…verting from JSON if optional and the json node contains "null"

tristanhimmelman · 2016-03-22T01:18:35Z

Hi @pixel4, thanks for your work. This looks good and I think I will merge. I'm curious though, why was it necessary to add the isKeyPresent check?

DulMephistos · 2016-03-22T16:00:01Z

Hi @tristanhimmelman, I'm glad you like it, thanks. We use this library all the time and this change is key for us.

You need the flag isKeyPresent in order to tell if the value was present in the JSON or if it was missing. When the value is present and it's nil you can't tell by just setting the value to nil because it's ambiguous.
So when're about to map the properties and you see a nil value, without this flag it's impossible to differentiate between both scenarios.

tristanhimmelman · 2016-03-22T16:51:28Z


-		// check if a value exists for the current key
-		if nested == false {
-			currentValue = JSONDictionary[key]


This check is important for performance reasons. We previously didn't include it and the valueFor function became a performance bottleneck because it was being called so often without any need.

Can you please revert this

absolutely.. I'll revert it and check my tests tonight.

tristanhimmelman · 2016-03-22T16:53:37Z

One other thing that would be great to have before I merge this is a simple unit test that checks this functionality so that we ensure it is not broken down line with other updates. Would you be able to add one?

DulMephistos · 2016-03-22T19:46:00Z

yes.. I can add unit tests as well, np

…-json

DulMephistos · 2016-03-23T06:21:52Z

Hey @tristanhimmelman,

I just reverted the change you asked and also added unit tests for this new functionality.

I think we should also update the minor version of it since we added some considerable changes to its behavior on some edge cases, but I did not put the version change on this pull request because I figure you'd want to do this on a separate commit.

tristanhimmelman · 2016-03-23T16:26:47Z

@pixel4 thanks for the updates. Looks great!

I agree about updating the minor version. I will take care of that when I release.

DulMephistos · 2016-03-23T19:39:17Z

Thanks @tristanhimmelman

Add the ability to reset the property on the object to nil when con…

f22340e

…verting from JSON if optional and the json node contains "null"

tristanhimmelman reviewed Mar 22, 2016
View reviewed changes

Fabio Teles added 3 commits March 22, 2016 23:16

Revert nested check for performance reasons

849c2da

Add unit tests for nullable JSON mapping

fac3538

Merge remote-tracking branch 'upstream/master' into add-nullable-from…

70720a7

…-json

tristanhimmelman merged commit f72ad15 into tristanhimmelman:master Mar 23, 2016

Navigation Menu

Add the ability to reset object properties to `nil` from JSON - #403

Add the ability to reset object properties to `nil` from JSON#403
tristanhimmelman merged 4 commits into
tristanhimmelman:masterfrom
DulMephistos:add-nullable-from-json

DulMephistos commented Mar 3, 2016

Uh oh!

tristanhimmelman commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 22, 2016

Uh oh!

tristanhimmelman Mar 22, 2016

Uh oh!

DulMephistos Mar 22, 2016

Uh oh!

tristanhimmelman commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 23, 2016

Uh oh!

tristanhimmelman commented Mar 23, 2016

Uh oh!

DulMephistos commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DulMephistos commented Mar 3, 2016

Imagine a Player class like this:

Initialized:

Update name and remove team

Uh oh!

tristanhimmelman commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 22, 2016

Uh oh!

tristanhimmelman Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

DulMephistos Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

tristanhimmelman commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 22, 2016

Uh oh!

DulMephistos commented Mar 23, 2016

Uh oh!

tristanhimmelman commented Mar 23, 2016

Uh oh!

DulMephistos commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Imagine a `Player` class like this: