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

Check schema required array before is required flag #964

Merged
merged 3 commits into from Mar 27, 2019

Conversation

newmen
Copy link
Contributor

@newmen newmen commented Mar 26, 2019

Hi,

The generated code is incorrect when a required field is an object, and it has required internal fields. It because of wrong algorithm checks is required flag of the field previously than tests a top defined required fields list. I've changed the checking order.

I mean the case where is the top schema follows:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": ["object", "null"],
  "required": [
    "payment_type",
    "change_alignment",
    "billing_alignment"
  ],
  "properties": {
    "payment_type": {
      "type": "object",
      "$ref": "payment_type.json"
    },
    "change_alignment": {
      "type": "object",
      "$ref": "change_alignment.json"
    },
    "billing_alignment": {
      "type": "object",
      "$ref": "billing_alignment.json"
    }
  }
}

and billing_alignment.json is:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "required": [
    "date_type"
  ],
  "properties": {
    "date_type": {
      "type": "string",
      "enum": [
        "activation",
        "anniversary"
      ]
    },
    "day": {
      "type": "integer",
      "minimum": 1,
      "maximum": 28
    }
  }
}

For this example, the wrong plugin with useOptionalForGetters=true generates Optional<BillingAlignment> type of getBillingAlignment method instead of just BillingAlignment.
I've fixed it and now the type is BillingAlignment.

}

private boolean isDeclaredAs(String type, String nodeName, JsonNode node, Schema schema) {
return hasEnumerated(schema, type, nodeName) || hasFlag(node, type);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change the checking order here if you feel the increasing performance here.
We can do that because hasFlag is returning boolean independently and if it returns false then we check the top required array. In the previous implementation, we did not check the top required array if the test of flag returned `false.

@joelittlejohn joelittlejohn merged commit 8207053 into joelittlejohn:master Mar 27, 2019
@newmen newmen deleted the fix-required-or-optional branch March 28, 2019 04:01
@newmen
Copy link
Contributor Author

newmen commented Apr 1, 2019

@joelittlejohn, when do you want to release 1.0.1?

@joelittlejohn joelittlejohn added this to the 1.0.1 milestone Apr 1, 2019
@joelittlejohn
Copy link
Owner

joelittlejohn commented Apr 2, 2019 via email

@newmen
Copy link
Contributor Author

newmen commented Apr 2, 2019

Awesome!

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

Successfully merging this pull request may close these issues.

None yet

2 participants