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....@oracle.com <mailto:miroslav....@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






Reply via email to