Inconsistent handling of `required` list

[krdln]

Consider two schemas:

foo-schema:

{
  "$schema": "http://json-schema.org/schema#",
  "required": [
    "foo"
  ],
  "type": "object",
  "properties": {
    "foo": {
      "type": "integer"
    }
  }
}

and analoguous with bar field – bar-schema

{
  "$schema": "http://json-schema.org/schema#",
  "required": [
    "bar"
  ],
  "type": "object",
  "properties": {
    "bar": {
      "type": "integer"
    }
  }
}

When we attempt to merge them, all fields becomes optional. It seems that genson tries to optimize that case by omitting "required": [].

/tmp> genson -s foo-schema -s bar-schema | jq
{
  "$schema": "http://json-schema.org/schema#",
  "type": "object",
  "properties": {
    "foo": {
      "type": "integer"
    },
    "bar": {
      "type": "integer"
    }
  }
}

Let's save it as foobar-schema. If we try to merge it once again with foo-schema, we should expect no change (because merge(foo, bar, foo) should be equivalent to merge(foo, bar)), but for our surprise, we see that foo field is marked as required!

/tmp> genson -s foobar-schema -s foo-schema | jq
{
  "$schema": "http://json-schema.org/schema#",
  "required": [
    "foo"
  ],
  "type": "object",
  "properties": {
    "foo": {
      "type": "integer"
    },
    "bar": {
      "type": "integer"
    }
  }
}

I'm not sure whether lack of required field should be interpreted as "we don't know what's required" or "nothing is required" (I guess the latter though), but it'd be great for genson to handle it consistently :)

[wolverdude]

Sounds like a bug. I'm a bit busy and It doesn't sound urgent, so I'll look into it in a week or two.

[wolverdude]

Hmm... on closer look this is actually the intended behavior, but the only reason it's intended behavior is because I didn't fully think through the implications of the way I programmed it.

The schema generator looks for properties common to all objects it is fed and then populates the required field from that. This makes sense. However, I unthinkingly included that behavior when merging schemas. This is a clear violation of rule 2: any object that's valid under any given schema should validate under the new schema. If I'm adding required where it did not previously exist, that promise is broken.

I should be able to patch this pretty easily. Thanks for catching it!

[wolverdude]

Okay, never mind. I realize now that there is a very real reason for this behavior, and it has to do with seed schemas. And yeah, it'll get you into some non-intuitive situations.

The correct behavior under Rule 2 would be to nix any required properties as soon as it encounters a schema without a required field. However, this would mean that you would need to explicitly list all the required properties of any object you use in a seed schema. The compromise I came to was to only takes a schema's required properties into account if it explicitly lists them. Because the generator's output is lazy and leaves off the required key if there are no required properties, when you feed that generated schema back in, you lose the information that required had previously been empty, not missing.

I will definitely add docs about this to the README. I think it might also make sense to include an empty required key on the result whenever it has been computed to be empty instead of just being ignored completely.

@krdln, will that solve your problem? Would you prefer to add the empty required key to the output or keep the existing behavior?

[krdln]

I think it might also make sense to include an empty required key on the result whenever it has been computed to be empty instead of just being ignored completely.

That makes sense to me. 👍 I think it will be enough to solve the inconsistency in the example

Thanks for taking time to investigate the issue!

[wolverdude]

Version 1.1.0 is now live. Please let me know if you have any more issues.