Hi guys,
Happy new year to you all! And apologies for this long email :-)
I think we still haven't converged on this issue in
javax.xml.stream - so let me make a recap.
The issue is for the newInstance/newFactory methods
that take a factoryId parameter, in the factories for
the javax.xml.stream package:
[
<http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.stream/webrev.01/>
FactoryFinder.java: line 267-268 right hand side. ]
recap of the issue:
-------------------
In case the provided factoryId did not correspond to any
System/JAXP/StAX property, the old code used to look
for a service in META-INF/services using the factoryId as the
name of the file for looking up the implementation class.
In case factoryId was not the same as the base service
class (or a subclass of it) it still would have worked
although it went in contradiction to the Jar Specification
mentioned in the javadoc, since the Jar Specification clearly
states that the file name should be the fully qualified name
of the base service class.
Since we're going to replace the code that looked up for
a service in META-INF with a call to ServiceLoader, we can
no longer fully preserve that old behavior, because ServiceLoader
only takes a base service class (and no arbitrary file name).
what choices do we have?
------------------------
The question is therefore how we want to change this.
I think we have 4 (exclusive) possibilities.
In the case where a factoryId is provided, but no
System/JAXP/StAX property by that name has been found,
we could choose to either:
1. Always call ServiceLoader.load(type) where type is the
service base class.
2. Never call ServiceLoader.load(type)
3. Only call ServiceLoader.load(type) when the provided
factoryId is equal to type.getName()
4. If factoryId is equal to type.getName(), call
ServiceLoader.load(type), otherwise,
attempt to load factoryId as a class - and if that
class is a subclass of 'type' then call
ServiceLoader.load for that subclass.
pros & cons:
------------
Here is a list of pros & cons for each of these options:
1. is the naive implementation I originally proposed.
Pros:
- P1.1: simple
- P1.2: no change in behavior if factoryId == type.getName()
Cons:
- C1.1: may find no provider when the old code used to
find one, (case where factoryId="foo", there's a
provider for "foo" and there's no provider for type.getName())
- C1.2: may find a provider when the old code used to
find none (case where factoryId="foo", there's no provider
for foo, but there's a provider for type.getName())
- C1.3: may find a different provider than what the old
code use to find (case where factoryId="foo", there's a
provider for "foo" and there's also a provider for
type.getName())
2. is more drastic: if you specify a property - then the property
has to be defined somewhere for newInstance/newFactory to
succeed.
Pros:
- P2.1: simple
Cons:
- C2.1: there's a change of behavior even when
factoryId == type.getName() and no property by that
name is defined.
- C2.2: in all cases where the old code used to find a
provider by looking at META-INF/services, the new code
will find nothing.
Although it's the most simple - it's probably the most risky
in terms of compatibility.
3. Same as 1. except that C1.2 and C1.3 are removed.
4. Same as 3. except that it's more complex (so P1.1 is
removed) and that C1.1 will not occur in the case
where factoryId is the name of a subclass of 'type'
In conclusion:
--------------
Since the original spec only says that factoryId is
"same as property" - it's very difficult to figure out
which of these 4 options is the closer to the behavior
intended by the spec.
I personally think that the case where factoryId="foo",
and no property "foo" is defined, but a provider exists for
"foo" and an implementation is returned, is a bug - since
it's in contradiction with the Jar Specification mentioned
in the spec which clearly states that "foo" should
have been a service class name.
So although 4. is the implementation that would offer the
greater compatibility with existing code, I am sorely
tempted to recommend that we do 3. and clarify
the spec on this point.
(3: Only call ServiceLoader.load(type) when the provided
factoryId is equal to type.getName())
Best regards,
-- daniel
On 12/19/12 10:45 AM, Daniel Fuchs wrote:
On 12/19/12 12:10 AM, Joe Wang wrote:
It's different. If 'foo.bar' is specified but not found, it indicates
a configuration error. If the factory falls back to an impl by the
default factory id, it would serve to hide the error.
Yes - I fully agree with that.
Note that newInstance/newFactory with a factoryId parameter do not
fall back to the default implementation.
Ahh! Yes I missed that. When called from newInstance with a factoryId
parameter the fallbackClassname parameter
is null...
So should we still call ServiceLoader when fallbackClassname is null and
factoryId is type.getName()? It would be more
backward compatible - since previously it was looking in the jars and
found (valid) providers registered with that name.
On the other hand we could alter the spec to say that if no property
with the factoryId name is found - then
no fallback loading is perform (either on ServiceLoader or
system-default implementation) and an error is thrown.
-- daniel
