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

1.0.60 Expects type To Be Array #461

Closed
tomdeering-wf opened this issue Oct 5, 2021 · 3 comments
Closed

1.0.60 Expects type To Be Array #461

tomdeering-wf opened this issue Oct 5, 2021 · 3 comments

Comments

@tomdeering-wf
Copy link

tomdeering-wf commented Oct 5, 2021

When dependabot brought us from using json-schema-validator 1.0.59 to 1.0.60 in an internal project, many tests started failing. The root cause seems to be that json-schema-validator 1.0.60 expects that "type" should be an array rather than a simple string, even though a simple string is allowed by the spec.

Here is a test case that reproduces the issue:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "properties": {
    "firstName": {
      "type": "string"
    },
    "lastName": {
      "type": "string"
    },
    "address": {
      "type": "object",
      "properties": {
        "street": {
          "type": "string"
        },
        "city": {
          "type": "string"
        }
      },
      "required": [
        "street",
        "city"
      ]
    }
  },
  "required": [
    "firstName",
    "lastName",
    "address"
  ]
}

Validated using:

final var config = new SchemaValidatorsConfig();
config.addKeywordWalkListener(ValidatorTypeCode.PROPERTIES.getValue(), new JsonSchemaWalkListener() {
      private final List<ParseResult.Property> path = Lists.newArrayList();

      @Override
      public WalkFlow onWalkStart(final WalkEvent walkEvent) {
        // This method is more confusing than it should be on first blush. The reason for this is
        // that technically properties can have the literal name "properties" and can also contain
        // periods which means we need complex logic as we can use simple string splitting or
        // jsonpath. We have to track the state of our recursive descent to strip out known prefixes
        // from our current path.

        // The first event is always the outer object properties.
        if (walkEvent.getAt().equalsIgnoreCase("$")) {
          path.add(new ParseResult.Property("properties", "$.properties"));
          return WalkFlow.CONTINUE;
        }

        final var previous = path.get(path.size() - 1);
        final var path = walkEvent.getAt();

        var name = StringUtils.removeStart(path, previous.getPath() + ".");
        final var property = new ParseResult.Property(name, walkEvent.getAt());

        properties.add(property);

        final var nextProperty = new ParseResult.Property(name, walkEvent.getAt() + ".properties");
        this.path.add(nextProperty);

        return WalkFlow.CONTINUE;
      }

      @Override
      public void onWalkEnd(final WalkEvent walkEvent,
                            final Set<ValidationMessage> validationMessages) {

        this.path.remove(path.size() - 1);
      }
    });
final var jsFactory = JsonSchemaFactory.getInstance(V7);
final var schema = jsFactory.getSchema(stream /*Of draft 7 schema definition*/, config);
final var result = schema.walk(value, true);

Expected: No validation errors
Actual (1.0.59): No validation errors
Actual (1.0.60): [$.type: string found, array expected, $.properties.firstName.type: string found, array expected, $.properties.lastName.type: string found, array expected, $.properties.address.type: string found, array expected, $.properties.address.properties.street.type: string found, array expected, $.properties.address.properties.city.type: string found, array expected]

The only change that could have broken us seems to be #452, but I'm not sure how

@tomdeering-wf tomdeering-wf changed the title 1.0.60 Breaks Nested Schema Types 1.0.60 Expects "type" To Be Array Oct 5, 2021
@stevehu
Copy link
Contributor

stevehu commented Oct 6, 2021

@tomdeering-wf Thanks for raising this issue. This is the reason we have short release cycles so that we can easily identify broken issues.

@bartoszm I am wondering if you could take a look at this issue?

@bartoszm
Copy link
Contributor

bartoszm commented Oct 6, 2021

@tomdeering-wf sure will look at it.

bartoszm added a commit to bartoszm/json-schema-validator that referenced this issue Oct 6, 2021
bartoszm added a commit to bartoszm/json-schema-validator that referenced this issue Oct 6, 2021
bartoszm added a commit to bartoszm/json-schema-validator that referenced this issue Oct 6, 2021
bartoszm added a commit to bartoszm/json-schema-validator that referenced this issue Oct 6, 2021
stevehu pushed a commit that referenced this issue Oct 8, 2021
* fix for #461 simple implementation (aligned with one-of)

* fix for #461 simple implementation

* simplify walker as test does not depend on its logic
@stevehu stevehu changed the title 1.0.60 Expects "type" To Be Array 1.0.60 Expects type To Be Array Oct 10, 2021
@stevehu
Copy link
Contributor

stevehu commented Oct 10, 2021

@tomdeering-wf, I have released 1.0.61 with the fix from @bartoszm. Could you please test it to confirm the issue is resolved in your case? Thanks a lot for raising the issue. Also, thank you @bartoszm for the quick response.

@stevehu stevehu closed this as completed Oct 10, 2021
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

3 participants