[9] Review request JDK-8131334: SAAJ Plugability Layer: using java.util.ServiceLoader

[Miroslav Kos]

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
>>>>>
>>>>
>>>
>>
>