[9] Review request JDK-8131334: SAAJ Plugability Layer: using java.util.ServiceLoader
Miroslav Kos
miroslav.kos at oracle.com
Wed Sep 30 15:08:29 UTC 2015
On 30/09/15 16:20, Deva Sagar wrote:
> Hi Georgiy,
>
> I think both Miran and I are referring to the mention of
> SAAJMetaFactory in the bullet list about the public newInstance()
> method. The Javadoc of the SAAJMetaFactory class still describes it as
> an SPI and it is accessible from package-info. This is no different
> from the current public Javadocs.
>
> Thanks,
> Deva
>
>
>> On Sep 30, 2015, at 9:16 AM, Georgiy Rakov <georgiy.rakov at oracle.com
>> <mailto:georgiy.rakov at oracle.com>> wrote:
>>
>>
>>
>> On 29.09.2015 17: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.
>>>
>> Hello,
>>
>> according to my understanding the idea of SAAJMetaFactory is for SPI
>> developer to subclass it and thus to create a custom SAAJMetaFactory
>> implementation.
Yes, it's current state - the only description of lookup/instantiation
user SAAJMetaFactory is statement that it is service provider interaface
(stays unchanged):
https://docs.oracle.com/javase/8/docs/api/javax/xml/soap/SAAJMetaFactory.html
Service loader is the only specification here.
>> Afterwards user should configure SAAJ API in order it could use that
>> custom implementation.
User configuration controls discovery of implementation when used
newInstance method on other factories (MessageFactory, SOAPFactory,
SOAPConnectionFactory). If no implementation found, the fallback is user
(or default) SAAJMetaFactory.newXXXFactory()
>> But if SAAJMetaFactory mention from package-info.java is removed
>> there will be actually no spec defining how to configure SAAJ API for
>> using custom SAAJMetaFactory, or do I miss spec defining it?
No, there is still statement about service provider interface.
>> BTW: from this perspective referring to SAAJMetaFactory newInstance
>> method in spec is not relevant since it's SAAJ API implementation
>> internal machinery.
Yes, it is internal.
Actually I thing SAAJ API is pretty complicated already, so not adding
another spec for SAAJMetaFactory discovery is good idea ...
Thanks
Miran
>>
>> So I believe if SAAJMetaFactory mention from package-info.java is
>> removed the process of SAAJMetaFactory instantiation should be
>> described somehow in another place and without mentioning newInstance
>> method. Maybe it's worth rather modifying than removing this
>> mentioning in package-info.java?
>>
>> Thanks,
>> Georgiy.
>>
>>> 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>
>>>> 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