CFR: javax.xml.parsers: Using ServiceLoader to load JAXP parser factories (7169894: JAXP Plugability Layer: using service loader) (original) (raw)

Daniel Fuchs daniel.fuchs at oracle.com
Wed Dec 5 16:17:54 UTC 2012

Hi,

Please find below a revised version of the changes for the javax.xml.parsers package.

It hopefully incorporates all comments I have received so far.

<http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.01/>

best regards,

-- daniel

On 12/4/12 2:54 PM, Alan Bateman wrote:

On 03/12/2012 19:04, Daniel Fuchs wrote:

Hi,

This is a first webrev in a series that addresses a change intended for JDK 8: 7169894: JAXP Plugability Layer: using service loader I've been building up on Joe's work and Paul & Alan comments from an earlier discussion thread [1] This here addresses changes in the javax.xml.parsers package for the SAXParserFactory and DocumentBuilderFactory - and more specifically their no-argument newInstance() method. This change replaces the custom code that was reading the META-INF/services/ resources by a simple call to ServiceLoader. <http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.00/> Thank you very much for taking this one on. I think your approach to take javax.xml.parsers on its own is good as the previous review rounds got really stuck in the mire due to the number of factories, code duplication, and subtle specification and implementation differences. In the class descriptions it suggests that the default implementation may be "installed as a module" but we aren't there yet so I'm not sure about using the term "module". I think it should be okay to say "installed as an extension" as ServiceLoader uses this term. The class description also suggests that a SCE will be wrapped as a FactoryConfigurationException but in FactoryFinder I see that it actual wraps a RuntimeException. I think you can just use the no-arg constructor then then use initCause to set the cause to the SCE. Also the statement "If a misconfigured provider is encountered and SCE is thrown" can probably be changed to "If SCE is thrown" as it can be thrown for other reasons too. Minor nit is that the updates to DocumentBuilderFactory's class description requires a really wide screen compared to the rest of the javadoc. Minor implementation suggestion for findServiceProvider is that you can use for-each to iterate through the providers. Otherwise I think you've captured all the points of feedback from the original review. Finally one suggestion is to make keep notes on the "before & after" behavior, this is something that will be important for release and compatibility notes. -Alan.

More information about the core-libs-dev mailing list