Validation fails for JSON arrays when errors exist across multiple contained objects

[everit-org]

Validation fails for JSON arrays when errors exist across multiple contained objects

[twbutler]

I followed the guidelines for pull requests, and added my schema and JSON subject files to an issue36/ folder as instructed. However, the tests seem to be having a problem reading the input JSON file, evidently they are always expecting a JSONObject rather than anticipating either a JSONObject or a JSONArray:

test[issue36](org.everit.json.schema.IssueTest)  Time elapsed: 0.026 sec  <<< ERROR!
java.lang.RuntimeException: failed to parse subject json file
        at org.everit.json.schema.IssueTest.validate(IssueTest.java:119)
        at org.everit.json.schema.IssueTest.lambda$test$4(IssueTest.java:105)
        at java.util.Optional.ifPresent(Optional.java:159)
        at org.everit.json.schema.IssueTest.test(IssueTest.java:105)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runners.Suite.runChild(Suite.java:128)
        at org.junit.runners.Suite.runChild(Suite.java:27)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
        at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
        at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
Caused by: org.json.JSONException: A JSONObject text must begin with '{' at 1 [character 2 line 1]
        at org.json.JSONTokener.syntaxError(JSONTokener.java:433)
        at org.json.JSONObject.<init>(JSONObject.java:195)
        at org.everit.json.schema.IssueTest.validate(IssueTest.java:112)
        ... 40 more

This is the same problem I had with the org.json library when I first started using it, which led to me writing my own code to figure out whether the input JSON was an object or an array before trying to read and validate it. It would be nice if the library had generic handling so that either an object or an array could be input, but that is another story. Bottom line: I think the IssueTest class will need to be modified before my JSON array input can be properly read. I suppose I will do just that...

[erosb]

Hello @twbutler ,

In its current state the IssueTest is only able to reproduce issues like "an valid input has been found invalid by the library" or "an invalid input has been found valid by the library".

On the other hand your issue is about reporting an inproper validation failure, and the IssueTest currently cannot assert if the proper ValidationException is thrown.

Most probably first I will have to perform some redesign of this integration test runner.

Thanks for reporting it anyway.

[twbutler]

@erosb,

Thanks for responding. Are you saying that since IssueTest would not support testing my particular issue, that I should not worry about submitting the pull request with my test data? The content I would be submitting in the files is the same as that given above in my first post...

Thanks...

[erosb]

Okay, I will just improve the testing infrastructure then I will go ahead with creating a proper reproduction with your inputs provided above.

[twbutler]

If it helps, I have already written a code segment in my cloned fork that allows IssueTest to read either JSON object or JSON array input without choking.

Line 112 - instead of:

      JSONObject subject = new JSONObject(new JSONTokener(new FileInputStream(file)));

Use the following:

      Object subject = null;
      JSONTokener jsonTok = new JSONTokener(new FileInputStream(file));

      // Determine if we have a single JSON message or an array of them
      Object jsonTest = jsonTok.nextValue();
      if (jsonTest instanceof JSONObject) {
        // The message contains a single JSON object
        subject = (JSONObject) jsonTest;
      }
      else if (jsonTest instanceof JSONArray) {
        // The message contains a JSON array
        subject = (JSONArray) jsonTest;
      }

With this change, I can run the tests and they all pass. As you said, the assertions in IssueTest would need rework in order to flag the incorrect output of the messages list in CausingExceptions for my issue 36.

[erosb]

As you said, the assertions in IssueTest would need rework in order to flag the incorrect output of the messages list in CausingExceptions

Yes, and this part will need more work. We will need some way to describe the expected exception, which will be probably some sort of JSON notation.

Anyway, this is not the first case when a bug related to causingExceptions occur, therefore I will definitely need to address this soon.

If you feel adventurous and have the time to contribute then I can give you some hints about what I have in mind to improve IssueTest to make your problem reproducible.

[twbutler]

Sure, I have a bit of time - What did you have in mind? Could it be as simple as including a file (called something like expectedCausingExceptions.txt) with the expected causing messages listed one on each line? So, in my case, the file would include:

#/0: 2 schema violations found
#/1: required key [price] not found

The test would look for the file, and if present, simply read each line into a List and then compare that List to the message content obtained from the getCausingExceptions() List...

[erosb]

Not exactly, but not much more complicated. Lets call this file expectedException.json and it should contain the tree (hierarchy) of the execptions and their causes.

Example:

{
  "message" : "#: 2 schema violations found",
  "causingExceptions" : [
     {
         "message": "#/0/name: expected type: String, found: JSONArray"
     },
     {
        "message" : "#/1: required key [price] not found",
        "causingExceptions" : []
     }
  ]
}

So the message key is required while the causingExceptions key is optional, and omitting the causingExceptions key should have the same meaning as if it would be an empty array (-> excepting no causing exceptions).

[twbutler]

I see. Your design sounds good to me. I will start coding up some changes to support the expectedException.json file as you have suggested...

[twbutler]

Well.. I am having an "ah-ha" moment... ^_^
I do not think that I understood how to properly investigate validation failures when I wrote my current code. I concluded from the "Investigating failures" portion of the documentation that if there was more than one validation failure, all one needed to do was call ValidationException#getCausingExceptions(), loop through the list it returned and call getMessage() on each item to collect all the validation messages. However, I am seeing now in my debugger that all the validation failures I was expecting are present, they are just deeper down, in sub-exception causingException lists. I am sorry that I did not realize this until now.
So, it seems that the proper way to get the complete list of validation failures is to call getCausingExceptions() on all ValidationException instances returned - not just the first one - and continue until no more ValidationException instances are returned.
I am going to refactor my application code now to do this. I have a strong hunch that we will be able to close this issue... Thanks for your patience...

[twbutler]

Yep, this issue was due to my incomplete understanding of the validation failure reporting mechanism. I now have better code that recursively processes all the sub-exceptions until all failures have been collected. Closing - and sorry for the noise. But hopefully, this can help others about to make the same mistake as I did...

[erosb]

No problem. Anyway if you have any local changes regarding IssueTest then could you please open a PR?

[twbutler]

I did not get very far. The next step I had was to write code to read the new expectedException.json file and compare its contents to the actual failures output by the validation library... Well, since I agreed to help, I will give some more time to this and send you a PR... stay tuned.

[erosb]

It doesn't have to be a polished implementation, I also don't mind a failing build, I just don't want your work to be lost.

[twbutler]

I think I am almost ready to issue the PR. However, first I wanted to ask about something else I found during this exercise that looks like a possible bug to me. It seems to me that the top level message contained in the first ValidationException thrown should report the correct total count of validation failures (e.g. "#: 3 schema violations found"). I just happened to notice a case in which I have 4 validation failures, but the top level message reports only 3. This is a minor issue in my thinking because all four failures are found and reported in the causingExceptions, it is just that the summary number is incorrect. But I thought I would see what you think. I plan to submit this case with my code that allows the expectedException.json file to be used to specify expected failures.

[erosb]

Hello,

I don't clearly see your point regarding the number of violations in the
exception message, but lets handle the itest improvements and the actual
bug two separate topics.

I'm looking forward to receive a PR from you in the upcoming days. Thank
you for working on it.

2016-06-20 20:51 GMT+02:00 twbutler [email protected]:

I think I am almost ready to issue the PR. However, first I wanted to ask
about something else I found during this exercise that looks like a
possible bug to me. It seems to me that the top level message contained in
the first ValidationException thrown should report the correct total count
of validation failures (e.g. "#: 3 schema violations found"). I just
happened to notice a case in which I have 4 validation failures, but the
top level message reports only 3. This is a minor issue in my thinking
because all four failures are found and reported in the causingExceptions,
it is just that the summary number is incorrect. But I thought I would see
what you think. I plan to submit this case with my code that allows the
expectedException.json file to be used to specify expected failures.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAQrBjxmBAIQuo00ZFS5_024hN7P-BPmks5qNuFGgaJpZM4I1rQb
.

Bence Erősme@github https://github.com/erosb

[shiragig]

Am getting JSON validation error when i run gulp certificate

[erosb]

You are at the wrong repository @shiragig . This is a java project, yours is a javascript error.