Saxon

This was previously reported in https://saxonica.plan.io/issues/3490

Hello Micheal, thank you for your response.

I can confirm that running validation from command line produces validation error as expected.

After further investigation I found that real source of our problem is that version 9.9.1.7 doesn't notify error listner (set up by method setErrorListener on SchemaValidator instance) as opposed to version 9.7.0.21 where error method was invoked with following exception

org.xml.sax.SAXParseException; lineNumber: 4; columnNumber: 33; Non-unique value found for constraint uniqueKey: ("A", "B", "C", "D")
....
Caused by: ValidationException: Non-unique value found for constraint uniqueKey: ("A", "B", "C", "D")
....

Do you have any suggestions?

OK, I'll try and see if I can reproduce it with a SchemaValidator. 9.9 introduced a new mechanism for reporting invalidity, but setErrorListener() should still work.

Sorry, can't reproduce the problem. The following JUnit test works as expected (on both 9.9 and 10.2):

public void testErrorListenerBug4744() {
        final List<TransformerException> errorList = new ArrayList<>();
        FileOutputStream out = null;
        try {
            out = new FileOutputStream(ConfigTest.DATA_DIR + "sandpit/validation.out");
            Processor proc = new Processor(true);
            SchemaManager sm = proc.getSchemaManager();
            StreamSource source_xsd = new StreamSource(new File("...../UniqueKeyBug.xsd"));
            StreamSource source_xml = new StreamSource(new File("...../UniqueKeyBug.xml"));
            ErrorListener listener = new ErrorListener() {
                @Override
                public void error(TransformerException exception) {
                    errorList.add(exception);
                }

                @Override
                public void fatalError(TransformerException exception) throws TransformerException {
                    throw exception;
                }

                @Override
                public void warning(TransformerException exception) {
                    errorList.add(exception);
                }
            };

            SchemaValidator sv = sm.newSchemaValidator();
            sv.setErrorListener(listener);
            sm.load(source_xsd);

            Serializer serializer = proc.newSerializer();
            serializer.setOutputProperty(Serializer.Property.INDENT, "yes");
            serializer.setOutputProperty(Serializer.Property.OMIT_XML_DECLARATION, "yes");
            serializer.setOutputStream(out);
            sv.setDestination(serializer);

            sv.validate(source_xml);
            fail("Invalidity not detected") ;
            out.close();

        } catch (SaxonApiException sapie) {
            try {
                out.close();
            } catch (IOException ee) {
                fail();
            }
            assertEquals(1, errorList.size());
        } catch (Exception e) {
            fail(e.getMessage());
        }
    }

Perhaps you're expecting it to be notified to the fatalError() callback?

We are not using ErrorListener directly, instead we use our implementation of ErrorHandler wrapped in SchemaFactoryImpl.ErrorListenerWrappingErrorHandler, so you are right that ErrorListener is notified about validation error, but this particular implementation doesn't proppagate this error to our ErrorHandler.

For some reason incoming ValidationException is marked as hasBeenReported=true (in our case this behaviour is the same regardless of library version), but in new library version there is additional check in ErrorListenerWrappingErrorHandler.error method

if (exception instanceof XPathException) {
    if (((XPathException)exception).hasBeenReported()) {
        return;
    }

 

    ((XPathException)exception).setHasBeenReported(true);
}

preventing ErrorHandler invocation.

Hi Micheal, thats the last bug which prevents us from upgrading from 9.7 to 9.9. Do you have a plan to fix this? If so, when?

Do you have a plan to fix this issue? If yes then when?

It will be fixed in the next maintenance release. Current status is that we found a fix but it caused regressions, and this revealed that we needed more tests in this area.

Thanks. We are looking forward to it ;)

Sorry this one has taken so long. Various attempts at fixing what looks like a very simple problem got tied up in complexities involving other bugs and configuration issues.

The basic problem (as you identify) is that for some errors -- specifically, errors in integrity constraints -- the flag "hasBeenNotified" is set on an error before notifying the error, and some error listeners look at this flag and interpret it as a signal to ignore the error.

The basic problem is still there in 10.x, but I haven't been able to find a combination of circumstances where it matters. However, moving the setting of the flag until after the signalling of the error seems to make sense, and clears the problem on the 9.9 branch.

I've looked again at the apparent regressions caused by the change, and have decided they were spurious. The failure that worried me was in a test for bug #4507. This test appears in the unit tests for 9.9 but appears not to have been migrated to 10.x (probably because the data file is 500Mb, which is a bit heavy for a permanent regression test). After careful analysis I have decided that the expected results were incorrect and the code is actually behaving correctly.

So I think we can deem this fixed; the only problem now is the logistics of delivering a fix. Ordinarily, we would no longer be producing new maintenance releases on the 9.9 branch if there's any chance of a workaround (because the code is now very stable, and any changes risk destabilisation). We're about to ship 10.5, and I'd encourage you to move forward onto the 10.x branch. Alternatively, I think there's a very simple workaround for 9.9: instead of using SchemaFactoryImpl.ErrorListenerWrappingErrorHandler as your ErrorListener, make a copy (or subclass) that ignores the "hasBeenReported()" flag:

public void error(TransformerException exception) throws TransformerException {
            try {
>                if (exception instanceof XPathException) {
>                    if (((XPathException) exception).hasBeenReported()) {
>                        return;
>                    }
>                    ((XPathException) exception).setHasBeenReported(true);
>              }
                handler.error(toSAXException(exception));
            } catch (SAXException e) {
                throw toTransformerException(e);
            }
        }

On the 10.x branch we have deprecated SchemaValidator.setErrorListener() because the JAXP ErrorListener (and the ErrorHandler, for that matter) require creation of an exception object for each validation error reported, which is very expensive. However, it remains available. The ErrorListenerWrappingErrorHandler is gone, but you can easily provide your own, or you can move forward to the new InvalidityHandlerWrappingErrorHandler which replaces it.

Is there a reason why InvalidityHandlerWrappingErrorHandler is not a static class? (At least in version 10.3)

No, no reason. In fact I've already made this change for the next maintenance release.