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

Incorrect @Nullable JSR305 annotations generated when using 'required' array #906

Closed
jrehwaldt opened this issue Sep 7, 2018 · 0 comments
Milestone

Comments

@jrehwaldt
Copy link
Contributor

jrehwaldt commented Sep 7, 2018

Currently, the rule NotRequiredRule generates incorrect @Nullable annotations for fields that get correctly generated with @Nonnull. This happens only on nested objects, due to broken logic for checking the required array definition. When retrieving the schema content we get the root schema instead of the relevant sub schema and therefore the implementation checks against an incorrect hierarchy within the schema.

Schema

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "title": "SchemaTest body",
  "description": "Example for incorrect @Nullable annotations.",
  "type": "object",
  "properties": {
    "sub-array": {
      "type": "array",
      "items": {
        "type": "object",
        "required": [
          "nonnull-field"
        ],
        "properties": {
          "nonnull-field": {
            "type": "string"
          },
          "nullable-field": {
            "type": "string"
          }
        }
      }
    }
  }
}

Behaviour

(Look for HERE in SubArray)

package tests;

import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import javax.validation.Valid;


/**
 * SchemaTest body
 * <p>
 * Example for incorrect @Nullable annotations.
 * 
 */
public class SchemaTest {

    /**
     * 
     * (Can be null)
     * 
     */
    @Nullable
    @Valid
    private List<SubArray> subArray = new ArrayList<SubArray>();

    /**
     * No args constructor for use in serialization
     * 
     */
    public SchemaTest() {
    }

    /**
     * 
     * @param subArray
     */
    public SchemaTest(List<SubArray> subArray) {
        super();
        this.subArray = subArray;
    }

    public List<SubArray> getSubArray() {
        return subArray;
    }
}
package tests;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;

public class SubArray {

    /**
     * 
     * (Can be null)
     * (Required)
     * 
     */
    @Nullable // <-- HERE ############################################
    @NotNull
    @Nonnull
    private String nonnullField;
    /**
     * 
     * (Can be null)
     * 
     */
    @Nullable
    private String nullableField;

    /**
     * No args constructor for use in serialization
     * 
     */
    public SubArray() {
    }

    /**
     * 
     * @param nonnullField
     * @param nullableField
     */
    public SubArray(String nonnullField, String nullableField) {
        super();
        this.nonnullField = nonnullField;
        this.nullableField = nullableField;
    }

    /**
     * 
     * (Required)
     * 
     */
    public String getNonnullField() {
        return nonnullField;
    }

    public String getNullableField() {
        return nullableField;
    }
}

Expected

package tests;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;

public class SubArray {

    /**
     * 
     * (Required)
     * 
     */
    @NotNull
    @Nonnull
    private String nonnullField;
    /**
     * 
     * (Can be null)
     * 
     */
    @Nullable
    private String nullableField;

    /**
     * No args constructor for use in serialization
     * 
     */
    public SubArray() {
    }

    /**
     * 
     * @param nonnullField
     * @param nullableField
     */
    public SubArray(String nonnullField, String nullableField) {
        super();
        this.nonnullField = nonnullField;
        this.nullableField = nullableField;
    }

    /**
     * 
     * (Required)
     * 
     */
    public String getNonnullField() {
        return nonnullField;
    }

    public String getNullableField() {
        return nullableField;
    }
}

Possible solutions

  1. Pass down sub schemas instead of the root schema. I saw from the code the distinction parentContent and content is already there, but never used. I think this would be the correct solution.
  2. Apply NotRequiredRule for all fields that do not get RequiredRule called instead of this separate handling as it stands now. Haven't looked into it.
  3. Workaround: Override RuleFactory to return NoopRule for NotRequiredRule and extend ObjectRule to add @Nullable to all fields not annotated with @Nonnull after the original ObjectRule is done.
jrehwaldt added a commit to jrehwaldt/jsonschema2pojo that referenced this issue Sep 17, 2018
This depends on joelittlejohngh-917 in order to know if a rule is applied top-level or sub-level.
Alternatively, if joelittlejohngh-906 is fixed by providing sub-schemas for sub-levels
this could be used to check nesting.
jrehwaldt added a commit to jrehwaldt/jsonschema2pojo that referenced this issue Sep 18, 2018
jrehwaldt added a commit to jrehwaldt/jsonschema2pojo that referenced this issue Sep 18, 2018
…Rule already suggests).

The relevant bits are in `SchemaRule#L79`, where a child schema is obtained and passed down
the rule tree as the schema for the relevant section. This seems to be the expected behavior
by `NotRequiredRule`, among others.

Changes `Schema` to keep reference to its parent `Schema` instead of parent `JsonNode`.
This means `Schema(id, JsonNode content, JsonNode parentContent)` is replaced by
`Schema(id, JsonNode content, Schema parent)`.

`Schema#getParent` still returns _self_ as did `#getParentContent` previously.

Fixes joelittlejohngh-906 (JSR305 bug) and depends on joelittlejohngh-921 (integration tests for JSR305 bug)
joelittlejohn referenced this issue Sep 19, 2018
…h-906

Derive child schemas for objects and arrays
@joelittlejohn joelittlejohn changed the title Incorrect JSR305 annotations generated Incorrect @Nullable JSR305 annotations generated when using 'required' array Sep 19, 2018
@joelittlejohn joelittlejohn added this to the 1.0.0-beta1 milestone Sep 19, 2018
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

No branches or pull requests

2 participants