[9] Review request JDK-8131334: SAAJ Plugability Layer: using java.util.ServiceLoader
Miroslav Kos
miroslav.kos at oracle.com
Wed Sep 30 12:28:30 UTC 2015
On 29/09/15 16:43, Deva Sagar wrote:
> Hi Miran,
>
> Sorry for the delay in responding to this - I was busy with WLS
> release close down, and now I have moved to a different job in the
> cloud organization outside of WLS and web services.
>
> Regarding your question in #3 - I think Georgiy made a good
> observation about SAAJMetaFactory - making a public newInstance method
> as you suggest would make it agree with the package-info, but does not
> correspond to the API specification. The package-info in Java SE 8 API
> docs says "SAAJMetaFactory is a service provider interface. There are
> no public methods on this class.” If we put in a public newInstance()
> method, it does not agree with this statement in API Javadoc. It might
> be better in my opinion to remove SAAJMetaFactory mention from
> package-info.java unless there is a good reason to keep it.
Agree, let's keep that simple. The updated specdiff is here:
http://cr.openjdk.java.net/~mkos/8131334/specdiff.05/
ApiNote removed, SAAJMetaFactory kept package private.
Thanks
Miran
>
> The rest of your changes look ok to me. Also cc’ing Chen for any
> additional comments
>
> Deva
>
>
>
>> On Sep 29, 2015, at 9:45 AM, Miroslav Kos <miroslav.kos at oracle.com
>> <mailto:miroslav.kos at oracle.com>> wrote:
>>
>> On 25/09/15 20:24, Georgiy Rakov wrote:
>>> Hello Miroslav,
>>>
>>> sorry for delay, was busy due to deadlines.
>>>
>>> I think it's just spec I am to review. Some comments for the moment
>>> please see below:
>>>
>>> 1. package-info.java:
>>>
>>> * <li>Checks if a system property with the same name as the factory class is set (/_*i.e.*_/
>>> * {@code javax.xml.soap.SOAPFactory}).
>>>
>>> I guess "e.g." is meant, that is "for example"?
>> Sure, thanks.
>>>
>>> 2. SAAJMetaFactory.java:
>>>
>>> Following is specified normatively:
>>> 62 * <p>Property {@code javax.xml.soap.MetaFactory} used in previous
>>> 63 * specifications (up to 1.3) is still supported, but it is strongly recommended to migrate to new property
>>> 64 * {@code javax.xml.soap.SAAJMetaFactory}.
>>> while in package-info.java: the same idea is specified as API note
>>> that is non-normatively:
>>> * @apiNote
>>> * <p>For {@link javax.xml.soap.SAAJMetaFactory}, property {@code javax.xml.soap.MetaFactory} used in previous
>>> * specifications (up to 1.3) is still supported, but it is strongly recommended to migrate to new property
>>> * {@code javax.xml.soap.SAAJMetaFactory}.
>>> */
>>> I believe this idea should be specified everywhere the same way that
>>> is either normatively or non-normatively. If it's to be defined
>>> normatively more details should be specified I believe.
>> Made non-normative.
>>>
>>> 3. SAAJMetaFactory.getInstance method is package private. So:
>>> - This method is not supposed to be used by users, is it?
>>> - The javadoc of this method isn't actually a public specification.
>>> Thus Oracle claims nothing by it and external Java SE implementers
>>> are not required to follow this JavaDoc, that is this JavaDoc is
>>> intended to be used internally only, isn't it?
>>> I understand that this was before, but It looks very strange. Is
>>> this really not a mistake? BTW: this method and spec are not visible
>>> in API JavaDoc
>>> <https://docs.oracle.com/javase/8/docs/api/javax/xml/soap/SAAJMetaFactory.html>.
>>>
>>> It looks like a mistake moreover because public specification
>>> defined in package-info.java being reviewed refers to it:
>>> * There are several factories defined in the SAAJ API to discover and load specific implementation:
>>> *
>>> * <ul>
>>> * <li>{@link javax.xml.soap.SOAPFactory}
>>> * <li>{@link javax.xml.soap.MessageFactory}
>>> * <li>{@link javax.xml.soap.SOAPConnectionFactory}
>>> /_** <li>{@link javax.xml.soap.SAAJMetaFactory}*_/
>>> * </ul>
>>> *
>>> * These factories define {@code/_*newInstance()*_/} method which uses a common lookup procedure to determine
>>> * the implementation class:
>> It's good catch - it looks like error. An easy fix would be change
>> the method to newInstance and make it public - Deva, would you agree?
>> http://cr.openjdk.java.net/~mkos/8131334/specdiff.04/index.html
>>
>> Anyway those are changes to API - should I withdraw
>> http://ccc.us.oracle.com/8131334 and create new one?
>>
>> Thanks
>> Miran
>>
>>
>>
>>> Thank you,
>>> Goergiy.
>>>
>>> On 25.09.2015 16:34, Miroslav Kos wrote:
>>>> Ping ...
>>>>
>>>> On 11/09/15 17:57, Miroslav Kos wrote:
>>>>> Hi again,
>>>>> would somebody find a time to review?
>>>>>
>>>>> Thanks
>>>>> Miran
>>>>>
>>>>> On 20/08/15 16:17, Miroslav Kos wrote:
>>>>>> Hi everybody,
>>>>>>
>>>>>> I am sending changes for review for
>>>>>> JDK-8131334: SAAJ Plugability Layer: using java.util.ServiceLoader
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8131334
>>>>>> webrev: http://cr.openjdk.java.net/~mkos/8131334/jaxws.01/
>>>>>>
>>>>>> It's about migrating to standard java.util.ServiceLoader. This
>>>>>> part of service discovery was implemented previously "own" way.
>>>>>> There are some changes in javadoc and implementation has been
>>>>>> refactored in order to use same code as in JAX-WS and JAX-B.
>>>>>>
>>>>>> Testing - I run JAX-WS unit tests (JAX-WS standalone repo), JCK9
>>>>>> + new tests specificaly developed for testing service discovery
>>>>>> in SAAJ.
>>>>>>
>>>>>> Thanks
>>>>>> Miran
>>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list