No way to deal with extra fields which we aren't handling

[SerialVelocity]

Hey,

If I have:

{
  "s": "a",
  "i": 1
}

and:

struct D {
  @mixin jsonizer
  @jsonize string s
}

I have no idea that I am not converting "i". Is it possible to make it so there's an option to disallow unknown fields?

[rcorre]

It could be implemented through a class/struct level attribute.

@jsonize(JsonizeIgnoreMissing.no)
struct D {
  @mixin jsonizer
  @jsonize string s
}

The above would enforce on any json keys that do not map to a member, and that enforce may eventually be replaced by a custom exception.

[SerialVelocity]

I'm not sure whether that is overloading @jsonize too much though. Is there a way to pass it in as a parameter to the mixin? That seems like the best place to put it to me.

[rcorre]

Agreed, I think a mixin parameter would make more sense.

[rcorre]

This is turning out to be non-trivial.
jsonizer does not try to examine all the entries in the json object, it simply tries to index the json object based on keys generated at compile time.
In other words, given struct S { int i; float f; }, JsonizeMe generates a deserialization method that tries to access json["i"] and json["f"], nothing more.
Identifying excess keys would require looping through all of the json entries.

[SerialVelocity]

Try doing it like this:

int fieldsFound = 0;
foreach(field; objectFields) {
  if(field in json) {
    this.field = json[field];
    ++fieldsFound;
  } else if(!field.isOptional()) {
      throw new JsonParsingException(getExtraAndMissingFields(json));
  }
}
if(!ignoreExtraFields && fieldsFound != json.length) {
    throw new JsonParsingException(getExtraAndMissingFields(json));
}

That's just pseudo-code for what I was thinking of.
Would that work? You would have to loop through the json object only in the error case then.

[rcorre]

That looks right. For some reason I assumed it would be more complicated dealing with optional fields but I guess not.

[SerialVelocity]

We probably want the default to be strict. The reason for this is, if someone forgets to not ignore extra keys, no error message will be shown. It will not do what they want silently. If the default was to be strict, they get an error and then go "oh yes, I wanted to ignore extra fields". Make sense?

[rcorre]

Thats a valid argument, but I could also see the case where, during testing, the user never encounters extra fields and doesn't bother to pass an argument to JsonizeMe. Later on a release starts failing because it is encountering extra fields even though the user never intended for this to be a failure in the first place. In my experience, I generally don't care if there are extra json fields, so JsonizeIgnoreExtraKeys would only be something I'd disable while testing to make sure I'm not missing something important in the json structure.

It would be possible to change the behavior based on debug/release flags, but I'm not sure if that would feel too unpredictable.

[SerialVelocity]

Yes, I tend to split it up into two parts:

• User is interfacing with an external API (my current use case): The user usually writes a wrapper which is parsing all of the data. The code then uses the wrapper and converts it into a usable library. For this, we always want to now when the API has changed and extra parameters are being passed back. If we don't, then when the API changes, random things can break and we would get no error messages (an example of this is when the API I'm using decided the split some data up into two fields while one kept the same name).
• User is consuming an API which is within his control (what I deal with at work): In this case, the user knows exactly what the API is that they are consuming. If they forget something, then they have usually messed up. The only case where they don't want new fields is when they are intentionally making the API backwards-compatible. When they do this, then usually you would make the consumer explicitly say that it doesn't care about extra fields (see the default for Jackson (Java Json Library))
• User is playing around with Json: They usually don't want to implement every field so in this case, they will get the error message straight away anyway.

For first two cases above, ETE tests should be used to test the full system. If no ETE tests are written, then the developer should at least be testing the endpoint that they are hitting manually before deploying to Prod.

Tell me if you don't agree :)

[rcorre]

I guess the question here is:
Which situation is more dangerous?

1. You ignore new fields that appear in the JSON and they actually represent a breaking API change
2. You fail on new fields, and your program refuses to parse data it should actually have no problem with.

My main use case up to this point was deserializing map data expored by Tiled. These json files were huge and contained a lot of data that I didn't care about; I just picked out the members I needed.

I was assuming that a breaking API change would usually cause failues due to missing/renamed members (which are non-optional by default).

API I'm using decided the split some data up into two fields while one kept the same name

Yeah, it would be bad not to catch something like that. I'm re-opening, as I'm leaning towards the more strict approach being the default.

[SerialVelocity]

Ok, looking forward to the change :)