In unmarshaller, make errorLimitCounter customizable. #1154
Conversation
facundovs
changed the title
|
@bravehorsie or someone with access, any chance to review this? Fixes #1139 |
|
@bravehorsie are there any news about it? |
|
Took a lot of searching to find this, but this problem causes some of the code I work on to stop throwing errors when it should be. We test for proper errors being handled. Would be great if this PR could get reviewed and merged. |
|
Any news about this merge? |
|
@bravehorsie I see you are a committer on this project. Can this PR get some attention? This continues to be a problem with JAXB that has not been resolved. |
There was a problem hiding this comment.
Besides comments in code please also fix copyright years in updated files and add copyright headers to files that were created.
After fixing comments above, the change looks good, thank you @facundovs !
If you want this PR to be accepted you need to sign http://www.oracle.com/technetwork/community/oca-486395.html
| handleEvent(new ValidationEventImpl(ValidationEvent.WARNING, Messages.ERRORS_LIMIT_EXCEEDED.format(), | ||
| getLocator().getLocation(), null), true); | ||
| } | ||
| return errorsCounter >= 0; | ||
| return false; |
There was a problem hiding this comment.
This won't work, if we always return false on shouldErrorBeReported(), error will never appear in the log.
Should be errorsCounter >= 0
| return true; | ||
|
|
||
| if (errorsCounter >= 0) { | ||
| --errorsCounter; | ||
| if (errorsCounter == 0) // it's possible to miss this because of concurrency. If required add synchronization here | ||
| if (errorsCounter == -1) // it's possible to miss this because of concurrency. If required add synchronization here |
There was a problem hiding this comment.
Due to comment on concurrency should be errorsCounter < 0 instead of ==
| } | ||
|
|
||
| public void testCustomErrorReportLimit() { | ||
| int customErrorReportLimit = 5; |
There was a problem hiding this comment.
custom error limit is never passed to context here, so test will fail after fixing shouldBeErrorReported()
please add unmarshaller.setProperty(UnmarshallerProperties.ERROR_REPORT_LIMIT, customErrorReportLimit); below int customErrorReportLimit = 5;
| unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes())); | ||
| } | ||
| catch (JAXBException e) { | ||
| assertThat(e.getMessage(), is(EXPECTED_ERROR_MESSAGE)); |
There was a problem hiding this comment.
Due to return false; error in shouldBeErrorReported() this assertion is never called in any of test methods.
|
@facundovs please can you fix reported issues? |
No description provided.