On 12/18/2012 10:00 AM, Daniel Fuchs wrote:
On 12/18/12 6:29 PM, Joe Wang wrote:
Hi Daniel,
The call: T provider = findServiceProvider(type) in static <T> T
find(Class<T> type, String factoryId, ClassLoader cl, String
fallbackClassName) ignored factoryId, and assumed it's the same as
type.getName(). I looked back I had the same bug in my original patch.
I don't think that's a bug in the new code - but rather possibly a
bug in the old code ;-).
The old code did not have the problem since it's reading
"META-INF/services/" + factoryId directly.
There's no way you can pass a property name to the ServiceLoader.
True. That's why it looked a bit awkward since 'factoryId' is not passed
in as did before, as in findJarServiceProvider(factoryId).
The JAR Specification says:
"A service provider identifies itself by placing a
provider-configuration file in the resource directory
META-INF/services. The file's name should consist of the
fully-qualified name of the abstract service class."
So I think the current code is correct in ignoring factoryId - because
according to the spec the file name should be the same as the
abstract class name.
So the StAX API implied that factoryId could be anything, but that would
violate the JAR specification. In other words, it would only work as
intended if the factoryId is specified as a System Property, or in
stax.properties or jaxp.properties.
I would think we should return an error if factoryId != type.getName()
before the call "findServiceProvider(type)".
-Joe
-- daniel
-Joe
On 12/18/2012 8:39 AM, Daniel Fuchs wrote:
Hi,
Thanks for the review.
I updated the webrev to keep track of your suggested change.
<http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.stream/webrev.01/>
-- daniel
On 12/18/12 4:00 PM, Alan Bateman wrote:
I looked through this installment and aside from an aside from an
alignment issue at lines 101-102 in XMLEventFactory.java then it looks
good to me.
Also thank you again for being so careful as you work through each of
these areas.
-Alan