On 11/01/2013 19:14, Daniel Fuchs wrote:
Although it seems cleaner to use a SchemaFactoryConfigurationError,
we could wrap the ServiceConfigurationError in
an IllegalArgumentException - which is what would have eventually
been thrown in the old code.
If that seems like a safer strategy I could update the changes in
this sense.
I went through the existing code and it appears that configuration
issues (missing class listed in
META-INF/services/javax.xml.validation.SchemaFactory for example) are
just ignored. This causes it to continue and probably fall back to the
default implementation, and IAE if the default implementation does not
support the schema language. On the other hand, if the provider's
constructor throws a runtime exception or error then it just gets
propagated. So overall the existing implementation is inconsistent and
anything we do (add the proposed SchemaFactoryConfigurationError or
throw IAE with an appropriate cause) is going to be a change in
behavior. As SchemaFactoryConfigurationError is much cleaner then I'm
wondering if we should just "bite the bullet" and get it over with.
One small thing that I noticed is that the provider's
isSchemaLanguageSupported method will be invoked twice when running with
assertions is enabled. This is observable, and might cause a provider to
initialize itself twice so I wonder if it would be better to remove part
of this assert.
For the non-SL cases then if newXMLSchemaFactoryNoServiceLoader isn't
static or public then the factory is ignored. I could imagine that being
hard to debug so so I wonder about treating it as a configuration error.
Otherwise I think these changes look good.
-Alan.
