RFR: 8176508 Update JAX-WS RI integration to latest version

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFR: 8176508 Update JAX-WS RI integration to latest version

Roman Grigoriadi
Hi,

Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.

JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/

Summary of changes:

jaxws/src/java.activation/share/classes/javax/activation/*
These are from Bill, fixing JDK-8049379

jaxws/src/java.xml.bind/share/classes/javax/xml/bind/*
JDK-8169496 - JAXB annotated classes needs to be open for JAXB impl
module. Javadoc updates + propagation of openness if JAXB impl is in
other module than java.xml.bind. This needs an update to JAXB JCK tests.

jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/**
Contains a fix for reported bug after removal dependencies to JDK
internal classes from SAAJ + formattig updates according to review
comments on last sync.

jaxws/src/java.xml.ws/share/classes/**/*Messages.java
jaxws/src/jdk.xml.ws/share/classes/**/*Messages.java
These are generated files, which were modified manually in JDK in order
to solve JDK-8153944. Generation of these file was updated (removed
Lambda), these changes should respect JDK-8153944 fix.

Patch also contains several small bugfixes, not tracked in JBS.

Best regards,
Roman
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Alan Bateman


On 12/03/2017 14:39, Roman Grigoriadi wrote:
> Hi,
>
> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
> Webrev:
> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/
>
I skimmed the changes and have a few comments (I'm sure Lance or someone
else will do a more detailed review).

In JAXBContext then "must be open to the java.xml.bind module" should be
"must be open to at least the java.xml.bind module" so as to cover the
case that the package is opened unconditionally or to java.xml.bind and
other modules. In addition, include "at least" makes it consistent with
other wording that we have agreed for other areas.

In MailcapCommandMap then the following doesn't seem right for the class
description:

   59  * (Where <i>java.home</i> is the value of the "java.home" System
property
   60  * and <i>conf</i> is the directory named "conf" if it exists,
   61  * otherwise the directory named "lib"; the "conf" directory was
   62  * introduced in JDK 1.9.)

It might be simpler to just have javadoc specify that it attepts to
locate the `mailcap` file in the Java run-time image and then add an
@implNote with the details as to where it looks for specific runtime
releases.

I see the new source file ModuleUtil is using java.util.StringTokenizer.
It's use in new code has been discouraged for many years and maybe this
could start out using String.split rather than the legacy class.

The change to version.properties reminds me to ask if there is anything
in the jaxws repo to indicate the version of the JAX-* components? It's
often difficult to determine what bits are in the JDK vs. the upstream
project.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Bill Shannon-2
Alan Bateman wrote on 03/12/17 08:14 AM:

> In MailcapCommandMap then the following doesn't seem right for the class
> description:
>
>   59  * (Where <i>java.home</i> is the value of the "java.home" System property
>   60  * and <i>conf</i> is the directory named "conf" if it exists,
>   61  * otherwise the directory named "lib"; the "conf" directory was
>   62  * introduced in JDK 1.9.)
>
> It might be simpler to just have javadoc specify that it attepts to locate the
> `mailcap` file in the Java run-time image and then add an @implNote with the
> details as to where it looks for specific runtime releases.

We talked about this before.

I decided not to use @implNote because it makes it easier to keep the source
in sync with the standalone version that's still built with an older JDK
(and javadoc).

It probably would've been better to word this differently so that it's clear
the location of the file is an implementation detail that depends on the the
JDK implementation, but at this point I think we've reached the point of
diminishing returns and it's time to just get this done.

Reply | Threaded
Open this post in threaded view
|

RE: RFR: 8176508 Update JAX-WS RI integration to latest version

Iris Clark-3
Hi, Bill.

>>   59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>   60  * and <i>conf</i> is the directory named "conf" if it exists,
>>   61  * otherwise the directory named "lib"; the "conf" directory was
>>   62  * introduced in JDK 1.9.)

In line 62, can we change "JDK 1.9" to "JDK 9" (drop the "1.") to match the
current version number?

Thanks,
Iris
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen
In reply to this post by Roman Grigoriadi
Hi all,

Additional minor comments on top of what others already provided

MimetypesFileTypeMap.java

        - Are the Parens around lines 54-57 really needed?
        - defaultType and confDir, shouldn’t these be all caps like PROG?
        - For the doPrivileged method, not sure the  minimum JDK version you want to be compatible with, but you could use a lambda or specify the specific permission
ModuleUtil.java
        -  The BufferedReader I would take advantage of try with resources so you do not have to specify finally


> On Mar 12, 2017, at 10:39 AM, Roman Grigoriadi <[hidden email]> wrote:
>
> Hi,
>
> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/
>
> Summary of changes:
>
> jaxws/src/java.activation/share/classes/javax/activation/*
> These are from Bill, fixing JDK-8049379
>
> jaxws/src/java.xml.bind/share/classes/javax/xml/bind/*
> JDK-8169496 - JAXB annotated classes needs to be open for JAXB impl module. Javadoc updates + propagation of openness if JAXB impl is in other module than java.xml.bind. This needs an update to JAXB JCK tests.
>
> jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/**
> Contains a fix for reported bug after removal dependencies to JDK internal classes from SAAJ + formattig updates according to review comments on last sync.
>
> jaxws/src/java.xml.ws/share/classes/**/*Messages.java
> jaxws/src/jdk.xml.ws/share/classes/**/*Messages.java
> These are generated files, which were modified manually in JDK in order to solve JDK-8153944. Generation of these file was updated (removed Lambda), these changes should respect JDK-8153944 fix.
>
> Patch also contains several small bugfixes, not tracked in JBS.
>
> Best regards,
> Roman

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Bill Shannon-2
In reply to this post by Iris Clark-3
Looks like I have a chance to tweak the comments for the JAF changes.
Any final comments before I apply these changes to all 4 copies of
this code?

$ diff -u MailcapCommandMap.java.orig MailcapCommandMap.java
--- MailcapCommandMap.java.orig 2017-03-15 15:10:43.731176917 -0700
+++ MailcapCommandMap.java      2017-03-15 15:15:23.009441462 -0700
@@ -50,16 +50,18 @@
  * <ol>
  * <li> Programatically added entries to the MailcapCommandMap instance.
  * <li> The file {@code .mailcap} in the user's home directory.
- * <li> The file <i>java.home</i>{@code /}<i>conf</i>{@code /mailcap}.
+ * <li> The file {@code mailcap} in the Java runtime.
  * <li> The file or resources named {@code META-INF/mailcap}.
  * <li> The file or resource named {@code META-INF/mailcap.default}
  * (usually found only in the {@code activation.jar} file).
  * </ol>
  * <p>
- * (Where <i>java.home</i> is the value of the "java.home" System property
- * and <i>conf</i> is the directory named "conf" if it exists,
- * otherwise the directory named "lib"; the "conf" directory was
- * introduced in JDK 1.9.)
+ * (The current implementation looks for the {@code mailcap} file
+ * in the Java runtime in the directory <i>java.home</i>{@code /conf}
+ * if it exists, and otherwise in the directory
+ * <i>java.home</i>{@code /lib}, where <i>java.home</i> is the value
+ * of the "java.home" System property.  Note that the "conf" directory was
+ * introduced in JDK 9.)
  * <p>
  * <b>Mailcap file format:</b><p>
  *
$ diff -u MimetypesFileTypeMap.java.orig MimetypesFileTypeMap.java
--- MimetypesFileTypeMap.java.orig      2017-03-15 15:10:54.166714725 -0700
+++ MimetypesFileTypeMap.java   2017-03-15 15:16:11.104610417 -0700
@@ -45,16 +45,18 @@
  * <ol>
  * <li> Programmatically added entries to the MimetypesFileTypeMap instance.
  * <li> The file {@code .mime.types} in the user's home directory.
- * <li> The file <i>java.home</i>{@code /}<i>conf</i>{@code /mime.types}.
+ * <li> The file {@code mime.types} in the Java runtime.
  * <li> The file or resources named {@code META-INF/mime.types}.
  * <li> The file or resource named {@code META-INF/mimetypes.default}
  * (usually found only in the {@code activation.jar} file).
  * </ol>
  * <p>
- * (Where <i>java.home</i> is the value of the "java.home" System property
- * and <i>conf</i> is the directory named "conf" if it exists,
- * otherwise the directory named "lib"; the "conf" directory was
- * introduced in JDK 1.9.)
+ * (The current implementation looks for the {@code mime.types} file
+ * in the Java runtime in the directory <i>java.home</i>{@code /conf}
+ * if it exists, and otherwise in the directory
+ * <i>java.home</i>{@code /lib}, where <i>java.home</i> is the value
+ * of the "java.home" System property.  Note that the "conf" directory was
+ * introduced in JDK 9.)
  * <p>
  * <b>MIME types file format:</b>
  *

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Bill Shannon-2
I got no response to this so I'm assuming everyone is happy now!  :-)

Bill Shannon wrote on 03/15/2017 03:20 PM:

> Looks like I have a chance to tweak the comments for the JAF changes.
> Any final comments before I apply these changes to all 4 copies of
> this code?
>
> $ diff -u MailcapCommandMap.java.orig MailcapCommandMap.java
> --- MailcapCommandMap.java.orig 2017-03-15 15:10:43.731176917 -0700
> +++ MailcapCommandMap.java      2017-03-15 15:15:23.009441462 -0700
> @@ -50,16 +50,18 @@
>   * <ol>
>   * <li> Programatically added entries to the MailcapCommandMap instance.
>   * <li> The file {@code .mailcap} in the user's home directory.
> - * <li> The file <i>java.home</i>{@code /}<i>conf</i>{@code /mailcap}.
> + * <li> The file {@code mailcap} in the Java runtime.
>   * <li> The file or resources named {@code META-INF/mailcap}.
>   * <li> The file or resource named {@code META-INF/mailcap.default}
>   * (usually found only in the {@code activation.jar} file).
>   * </ol>
>   * <p>
> - * (Where <i>java.home</i> is the value of the "java.home" System property
> - * and <i>conf</i> is the directory named "conf" if it exists,
> - * otherwise the directory named "lib"; the "conf" directory was
> - * introduced in JDK 1.9.)
> + * (The current implementation looks for the {@code mailcap} file
> + * in the Java runtime in the directory <i>java.home</i>{@code /conf}
> + * if it exists, and otherwise in the directory
> + * <i>java.home</i>{@code /lib}, where <i>java.home</i> is the value
> + * of the "java.home" System property.  Note that the "conf" directory was
> + * introduced in JDK 9.)
>   * <p>
>   * <b>Mailcap file format:</b><p>
>   *
> $ diff -u MimetypesFileTypeMap.java.orig MimetypesFileTypeMap.java
> --- MimetypesFileTypeMap.java.orig      2017-03-15 15:10:54.166714725 -0700
> +++ MimetypesFileTypeMap.java   2017-03-15 15:16:11.104610417 -0700
> @@ -45,16 +45,18 @@
>   * <ol>
>   * <li> Programmatically added entries to the MimetypesFileTypeMap instance.
>   * <li> The file {@code .mime.types} in the user's home directory.
> - * <li> The file <i>java.home</i>{@code /}<i>conf</i>{@code /mime.types}.
> + * <li> The file {@code mime.types} in the Java runtime.
>   * <li> The file or resources named {@code META-INF/mime.types}.
>   * <li> The file or resource named {@code META-INF/mimetypes.default}
>   * (usually found only in the {@code activation.jar} file).
>   * </ol>
>   * <p>
> - * (Where <i>java.home</i> is the value of the "java.home" System property
> - * and <i>conf</i> is the directory named "conf" if it exists,
> - * otherwise the directory named "lib"; the "conf" directory was
> - * introduced in JDK 1.9.)
> + * (The current implementation looks for the {@code mime.types} file
> + * in the Java runtime in the directory <i>java.home</i>{@code /conf}
> + * if it exists, and otherwise in the directory
> + * <i>java.home</i>{@code /lib}, where <i>java.home</i> is the value
> + * of the "java.home" System property.  Note that the "conf" directory was
> + * introduced in JDK 9.)
>   * <p>
>   * <b>MIME types file format:</b>
>   *
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Roman Grigoriadi
In reply to this post by Alan Bateman
Hi,

you can find new web rev here:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>

Previous review comments are addressed.

> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.


Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
There is:
# for JDK integration - include version in source zip
jaxb.jdk.version=2.3.0-b170412.1723

We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.

Roman


> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email]> wrote:
>
>
>
> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>> Hi,
>>
>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/
>>
> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>
> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>
> In MailcapCommandMap then the following doesn't seem right for the class description:
>
>  59  * (Where <i>java.home</i> is the value of the "java.home" System property
>  60  * and <i>conf</i> is the directory named "conf" if it exists,
>  61  * otherwise the directory named "lib"; the "conf" directory was
>  62  * introduced in JDK 1.9.)
>
> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>
> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>
> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Bill Shannon-2
Shouldn't the version information be in the manifest for the jar file containing
the classes, accessed via java.lang.Package?

Roman Grigoriadi wrote on 05/ 3/17 09:49 AM:

> Hi,
>
> you can find new web rev here:
> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/
> <http://cr.openjdk.java.net/%7Eaefimov/jaxws-integrations/8176508/01/>
>
> Previous review comments are addressed.
>
>> The change to version.properties reminds me to ask if there is anything in
>> the jaxws repo to indicate the version of the JAX-* components? It's often
>> difficult to determine what bits are in the JDK vs. the upstream project.
>
> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we
> are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
> There is:
> # for JDK integration - include version in source zip
> jaxb.jdk.version=2.3.0-b170412.1723
>
> We can add another version.properties file with versions of all JAX-*
> components. We may also change version from 2.3.0-SNAPSHOT to something unique
> like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>
> Roman
>
>
>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>
>>
>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>> Hi,
>>>
>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/
>>> <http://cr.openjdk.java.net/%7Eaefimov/jaxws-integrations/8176508/00/>
>>>
>> I skimmed the changes and have a few comments (I'm sure Lance or someone else
>> will do a more detailed review).
>>
>> In JAXBContext then "must be open to the java.xml.bind module" should be
>> "must be open to at least the java.xml.bind module" so as to cover the case
>> that the package is opened unconditionally or to java.xml.bind and other
>> modules. In addition, include "at least" makes it consistent with other
>> wording that we have agreed for other areas.
>>
>> In MailcapCommandMap then the following doesn't seem right for the class
>> description:
>>
>>  59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>  60  * and <i>conf</i> is the directory named "conf" if it exists,
>>  61  * otherwise the directory named "lib"; the "conf" directory was
>>  62  * introduced in JDK 1.9.)
>>
>> It might be simpler to just have javadoc specify that it attepts to locate
>> the `mailcap` file in the Java run-time image and then add an @implNote with
>> the details as to where it looks for specific runtime releases.
>>
>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's
>> use in new code has been discouraged for many years and maybe this could
>> start out using String.split rather than the legacy class.
>>
>> The change to version.properties reminds me to ask if there is anything in
>> the jaxws repo to indicate the version of the JAX-* components? It's often
>> difficult to determine what bits are in the JDK vs. the upstream project.
>>
>> -Alan
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen
In reply to this post by Roman Grigoriadi
Hi Roman,

I made a pass through the webrev and have the following feedback:


src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up

src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted

src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?

src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM

src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off

src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments

src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted

src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted

src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace

src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017

src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017

src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already



> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email]> wrote:
>
> Hi,
>
> you can find new web rev here:
> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>
>
> Previous review comments are addressed.
>
>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>
>
> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
> There is:
> # for JDK integration - include version in source zip
> jaxb.jdk.version=2.3.0-b170412.1723
>
> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>
> Roman
>
>
>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email]> wrote:
>>
>>
>>
>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>> Hi,
>>>
>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508
>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/
>>>
>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>
>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>
>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>
>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>> 61  * otherwise the directory named "lib"; the "conf" directory was
>> 62  * introduced in JDK 1.9.)
>>
>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>
>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>
>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>
>> -Alan
>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Roman Grigoriadi
Hi,

New webrev can be found here:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>

Previous review comments have been addressed.

New changes since last webrev upload:

Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.

JAXB-API:
 - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
 - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
 - better JAXBException message when package of jaxb classes is not open to java.xml.bind

JAXB-RI:
 - fixed escaping newlines when using bundled jaxws transport.

SAAJ-RI
 - fixed TCK test failures

JAXWS-RI
 - fixed parsing wsdl in secure mode

We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>

Please review.

Thanks,
Roman

> On 8 May 2017, at 22:38, Lance Andersen <[hidden email]> wrote:
>
> Hi Roman,
>
> I made a pass through the webrev and have the following feedback:
>
>
> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>
> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>
> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>
> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>
> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>
> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>
> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>
> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>
> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>
> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>
> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>
>
>
>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Hi,
>>
>> you can find new web rev here:
>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>
>>
>> Previous review comments are addressed.
>>
>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>
>>
>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>> There is:
>> # for JDK integration - include version in source zip
>> jaxb.jdk.version=2.3.0-b170412.1723
>>
>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>
>> Roman
>>
>>
>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>>
>>>
>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>> Hi,
>>>>
>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>
>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>
>>>>
>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>
>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>
>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>
>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>> 62  * introduced in JDK 1.9.)
>>>
>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>
>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>
>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>
>>> -Alan
>>
>
> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> [hidden email] <mailto:[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen
Hi Roman,

The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)

> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email]> wrote:
>
> Hi,
>
> New webrev can be found here:
> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>
>
> Previous review comments have been addressed.
>
> New changes since last webrev upload:
>
> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>
> JAXB-API:
>  - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>  - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>  - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>
> JAXB-RI:
>  - fixed escaping newlines when using bundled jaxws transport.
>
> SAAJ-RI
>  - fixed TCK test failures
>
> JAXWS-RI
>  - fixed parsing wsdl in secure mode
>
> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>
>
> Please review.
>
> Thanks,
> Roman
>
>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Hi Roman,
>>
>> I made a pass through the webrev and have the following feedback:
>>
>>
>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>
>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>
>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>
>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>
>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>
>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>
>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>
>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>
>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>
>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>
>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>
>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>
>>
>>
>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> Hi,
>>>
>>> you can find new web rev here:
>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>
>>>
>>> Previous review comments are addressed.
>>>
>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>
>>>
>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>> There is:
>>> # for JDK integration - include version in source zip
>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>
>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>
>>> Roman
>>>
>>>
>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>>
>>>>
>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>> Hi,
>>>>>
>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>
>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>
>>>>>
>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>
>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>
>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>
>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>> 62  * introduced in JDK 1.9.)
>>>>
>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>
>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>
>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>
>>>> -Alan
>>>
>>
>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> [hidden email] <mailto:[hidden email]>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Aleks Efimov
I'm sorry Lance! I'm uploading new version of the webrev right now to
the same location :-) It should be on-line soon =)

Best,
Aleksei

On 06/01/2017 06:34 PM, Lance Andersen wrote:

> Hi Roman,
>
> The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)
>
>> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email]> wrote:
>>
>> Hi,
>>
>> New webrev can be found here:
>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>
>>
>> Previous review comments have been addressed.
>>
>> New changes since last webrev upload:
>>
>> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>>
>> JAXB-API:
>>   - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>>   - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>>   - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>>
>> JAXB-RI:
>>   - fixed escaping newlines when using bundled jaxws transport.
>>
>> SAAJ-RI
>>   - fixed TCK test failures
>>
>> JAXWS-RI
>>   - fixed parsing wsdl in secure mode
>>
>> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>
>>
>> Please review.
>>
>> Thanks,
>> Roman
>>
>>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> Hi Roman,
>>>
>>> I made a pass through the webrev and have the following feedback:
>>>
>>>
>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>>
>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>>
>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>>
>>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>>
>>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>>
>>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>>
>>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>>
>>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>>
>>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>>
>>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>>
>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>>
>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>>
>>>
>>>
>>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> you can find new web rev here:
>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>
>>>>
>>>> Previous review comments are addressed.
>>>>
>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>
>>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>>> There is:
>>>> # for JDK integration - include version in source zip
>>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>>
>>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>>
>>>> Roman
>>>>
>>>>
>>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>
>>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>
>>>>>>
>>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>>
>>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>>
>>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>>
>>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>>> 62  * introduced in JDK 1.9.)
>>>>>
>>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>>
>>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>>
>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>
>>>>> -Alan
>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>   <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>   <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> [hidden email] <mailto:[hidden email]>
>   <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>   <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>   <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> [hidden email] <mailto:[hidden email]>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen
Hi Aleks,

Its all good.  I don’t suppose there is a webrev of the diffs from 01 to 02 (or an easy way to see what changed)?

Best
Lance

> On Jun 1, 2017, at 2:06 PM, Aleks Efimov <[hidden email]> wrote:
>
> I'm sorry Lance! I'm uploading new version of the webrev right now to the same location :-) It should be on-line soon =)
>
> Best,
> Aleksei
>
> On 06/01/2017 06:34 PM, Lance Andersen wrote:
>> Hi Roman,
>>
>> The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)
>>
>>> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> New webrev can be found here:
>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>
>>>
>>> Previous review comments have been addressed.
>>>
>>> New changes since last webrev upload:
>>>
>>> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>>>
>>> JAXB-API:
>>>  - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>>>  - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>>>  - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>>>
>>> JAXB-RI:
>>>  - fixed escaping newlines when using bundled jaxws transport.
>>>
>>> SAAJ-RI
>>>  - fixed TCK test failures
>>>
>>> JAXWS-RI
>>>  - fixed parsing wsdl in secure mode
>>>
>>> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>
>>>
>>> Please review.
>>>
>>> Thanks,
>>> Roman
>>>
>>>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>> Hi Roman,
>>>>
>>>> I made a pass through the webrev and have the following feedback:
>>>>
>>>>
>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>>>
>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>>>
>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>>>
>>>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>>>
>>>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>>>
>>>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>>>
>>>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>>>
>>>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>>>
>>>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>>>
>>>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>>>
>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>>>
>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>>>
>>>>
>>>>
>>>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> you can find new web rev here:
>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>
>>>>>
>>>>> Previous review comments are addressed.
>>>>>
>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>
>>>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>>>> There is:
>>>>> # for JDK integration - include version in source zip
>>>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>>>
>>>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>>>
>>>>> Roman
>>>>>
>>>>>
>>>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>
>>>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>
>>>>>>>
>>>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>>>
>>>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>>>
>>>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>>>
>>>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>>>> 62  * introduced in JDK 1.9.)
>>>>>>
>>>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>>>
>>>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>>>
>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>
>>>>>> -Alan
>>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering
>>>> 1 Network Drive
>>>> Burlington, MA 01803
>>>> [hidden email] <mailto:[hidden email]>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> [hidden email] <mailto:[hidden email]>
>>
>>
>>
>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Roman Grigoriadi

> On 1 Jun 2017, at 20:08, Lance Andersen <[hidden email]> wrote:
>
> Hi Aleks,
>
> Its all good.  I don’t suppose there is a webrev of the diffs from 01 to 02 (or an easy way to see what changed)?

Unfortunately no list of changed files, just a description of changes in my last message.
In addition to those fix for JDK-8176741 is included in this upload and some new messages (modeler*.properties, wscompile*.properties)

Maybe I can produce some diff of changed files from git history of the standalone (probably still many files) if that would be of any help. We may try to sync in a smaller parts next time.

Roman


>
> Best
> Lance
>> On Jun 1, 2017, at 2:06 PM, Aleks Efimov <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> I'm sorry Lance! I'm uploading new version of the webrev right now to the same location :-) It should be on-line soon =)
>>
>> Best,
>> Aleksei
>>
>> On 06/01/2017 06:34 PM, Lance Andersen wrote:
>>> Hi Roman,
>>>
>>> The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)
>>>
>>>> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> New webrev can be found here:
>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>>
>>>>
>>>> Previous review comments have been addressed.
>>>>
>>>> New changes since last webrev upload:
>>>>
>>>> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>>>>
>>>> JAXB-API:
>>>>  - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>>>>  - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>>>>  - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>>>>
>>>> JAXB-RI:
>>>>  - fixed escaping newlines when using bundled jaxws transport.
>>>>
>>>> SAAJ-RI
>>>>  - fixed TCK test failures
>>>>
>>>> JAXWS-RI
>>>>  - fixed parsing wsdl in secure mode
>>>>
>>>> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397> <https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>>
>>>>
>>>> Please review.
>>>>
>>>> Thanks,
>>>> Roman
>>>>
>>>>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>
>>>>> Hi Roman,
>>>>>
>>>>> I made a pass through the webrev and have the following feedback:
>>>>>
>>>>>
>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>>>>
>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>>>>
>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>>>>
>>>>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>>>>
>>>>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>>>>
>>>>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>>>>
>>>>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>>>>
>>>>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>>>>
>>>>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>>>>
>>>>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>>>>
>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>>>>
>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>>>>
>>>>>
>>>>>
>>>>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> you can find new web rev here:
>>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>>
>>>>>>
>>>>>> Previous review comments are addressed.
>>>>>>
>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>
>>>>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>>>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>>>>> There is:
>>>>>> # for JDK integration - include version in source zip
>>>>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>>>>
>>>>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>>>>
>>>>>> Roman
>>>>>>
>>>>>>
>>>>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508> <https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>>
>>>>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>>
>>>>>>>>
>>>>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>>>>
>>>>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>>>>
>>>>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>>>>
>>>>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>>>>> 62  * introduced in JDK 1.9.)
>>>>>>>
>>>>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>>>>
>>>>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>>>>
>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>>
>>>>>>> -Alan
>>>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>>> Oracle Java Engineering
>>>>> 1 Network Drive
>>>>> Burlington, MA 01803
>>>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>
>>>
>>>
>>
>
> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> [hidden email] <mailto:[hidden email]>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen

> On Jun 1, 2017, at 2:18 PM, Roman Grigoriadi <[hidden email]> wrote:
>
>>
>> On 1 Jun 2017, at 20:08, Lance Andersen <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Hi Aleks,
>>
>> Its all good.  I don’t suppose there is a webrev of the diffs from 01 to 02 (or an easy way to see what changed)?
>
> Unfortunately no list of changed files, just a description of changes in my last message.

OK, thank you.,
> In addition to those fix for JDK-8176741 is included in this upload and some new messages (modeler*.properties, wscompile*.properties)
>
> Maybe I can produce some diff of changed files from git history of the standalone (probably still many files) if that would be of any help. We may try to sync in a smaller parts next time.

I would not worry about it.  I was hoping to just look at just the changes from the previous, but will be easier for me to just review the entire webrev again.

>
> Roman
>
>
>>
>> Best
>> Lance
>>> On Jun 1, 2017, at 2:06 PM, Aleks Efimov <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> I'm sorry Lance! I'm uploading new version of the webrev right now to the same location :-) It should be on-line soon =)
>>>
>>> Best,
>>> Aleksei
>>>
>>> On 06/01/2017 06:34 PM, Lance Andersen wrote:
>>>> Hi Roman,
>>>>
>>>> The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)
>>>>
>>>>> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> New webrev can be found here:
>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>>
>>>>>
>>>>> Previous review comments have been addressed.
>>>>>
>>>>> New changes since last webrev upload:
>>>>>
>>>>> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>>>>>
>>>>> JAXB-API:
>>>>>  - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>>>>>  - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>>>>>  - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>>>>>
>>>>> JAXB-RI:
>>>>>  - fixed escaping newlines when using bundled jaxws transport.
>>>>>
>>>>> SAAJ-RI
>>>>>  - fixed TCK test failures
>>>>>
>>>>> JAXWS-RI
>>>>>  - fixed parsing wsdl in secure mode
>>>>>
>>>>> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397> <https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>>
>>>>>
>>>>> Please review.
>>>>>
>>>>> Thanks,
>>>>> Roman
>>>>>
>>>>>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>
>>>>>> Hi Roman,
>>>>>>
>>>>>> I made a pass through the webrev and have the following feedback:
>>>>>>
>>>>>>
>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>>>>>
>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>>>>>
>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>>>>>
>>>>>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>>>>>
>>>>>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>>>>>
>>>>>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>>>>>
>>>>>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>>>>>
>>>>>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>>>>>
>>>>>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>>>>>
>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>>>>>
>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> you can find new web rev here:
>>>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>>
>>>>>>>
>>>>>>> Previous review comments are addressed.
>>>>>>>
>>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>>
>>>>>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>>>>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>>>>>> There is:
>>>>>>> # for JDK integration - include version in source zip
>>>>>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>>>>>
>>>>>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>>>>>
>>>>>>> Roman
>>>>>>>
>>>>>>>
>>>>>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>>>>>
>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508> <https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>>
>>>>>>>>>
>>>>>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>>>>>
>>>>>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>>>>>
>>>>>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>>>>>
>>>>>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>>>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>>>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>>>>>> 62  * introduced in JDK 1.9.)
>>>>>>>>
>>>>>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>>>>>
>>>>>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>>>>>
>>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>>>
>>>>>>>> -Alan
>>>>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>>>> Oracle Java Engineering
>>>>>> 1 Network Drive
>>>>>> Burlington, MA 01803
>>>>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering
>>>> 1 Network Drive
>>>> Burlington, MA 01803
>>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>
>>>>
>>>>
>>>
>>
>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> [hidden email] <mailto:[hidden email]>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Mandy Chung
In reply to this post by Roman Grigoriadi

> On May 31, 2017, at 5:06 AM, Roman Grigoriadi <[hidden email]> wrote:
>
> Hi,
>
> New webrev can be found here:
> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>

jaxp/src/java.xml/share/classes/module-info.java
  I’m happy to see this qualified exports removed.

Can you update jdk/test/jdk/modules/etc/JdkQualifiedExportTest.java to
remove "java.xml/com.sun.xml.internal.stream.writers” from KNOWN_EXCEPTION.

java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamReaderFactory.java
java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamWriterFactory.java
jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/MrJarUtil.java
   MrJarUtil::getNoPoolProperty is not MR specific.  Are you trying to keep
   the different default value when building for older release?

It would be clearer if you want to define a constant for the default value that is subject to the runtime version.

XMLStreamReaderFactory and XMLStreamWriterFactory would get the property value with the MR-specific default.

That’re the files I reviewed.

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Roman Grigoriadi

> On 1 Jun 2017, at 21:25, Mandy Chung <[hidden email]> wrote:
>
>
>> On May 31, 2017, at 5:06 AM, Roman Grigoriadi <[hidden email]> wrote:
>>
>> Hi,
>>
>> New webrev can be found here:
>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>
>
> jaxp/src/java.xml/share/classes/module-info.java
>  I’m happy to see this qualified exports removed.
>
> Can you update jdk/test/jdk/modules/etc/JdkQualifiedExportTest.java to
> remove "java.xml/com.sun.xml.internal.stream.writers” from KNOWN_EXCEPTION.
>
> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamReaderFactory.java
> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamWriterFactory.java
> jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/MrJarUtil.java
>   MrJarUtil::getNoPoolProperty is not MR specific.  Are you trying to keep
>   the different default value when building for older release?

In standalone MrJarUtil exists and is packaged twice, but there is no need / way to sync JDK8 and below version of this file.
Here is second non-synced version of this file for 8 and below runtime:
https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java <https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java>
>
> It would be clearer if you want to define a constant for the default value that is subject to the runtime version.

>
> XMLStreamReaderFactory and XMLStreamWriterFactory would get the property value with the MR-specific default.
>

Than MrJarUtil would need to have only such constant for default value and metho getNoPoolProperty could be moved elsewhere.

> That’re the files I reviewed.
>
> Mandy

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Mandy Chung

> On Jun 1, 2017, at 10:29 PM, Roman Grigoriadi <[hidden email]> wrote:
>
>
>> On 1 Jun 2017, at 21:25, Mandy Chung <[hidden email] <mailto:[hidden email]>> wrote:
>>
>>
>>> On May 31, 2017, at 5:06 AM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> Hi,
>>>
>>> New webrev can be found here:
>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>>
>>
>> jaxp/src/java.xml/share/classes/module-info.java
>>  I’m happy to see this qualified exports removed.
>>
>> Can you update jdk/test/jdk/modules/etc/JdkQualifiedExportTest.java to
>> remove "java.xml/com.sun.xml.internal.stream.writers” from KNOWN_EXCEPTION.
>>
>> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamReaderFactory.java
>> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamWriterFactory.java
>> jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/MrJarUtil.java
>>   MrJarUtil::getNoPoolProperty is not MR specific.  Are you trying to keep
>>   the different default value when building for older release?
>
> In standalone MrJarUtil exists and is packaged twice, but there is no need / way to sync JDK8 and below version of this file.
> Here is second non-synced version of this file for 8 and below runtime:
> https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java <https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java>
>>
>> It would be clearer if you want to define a constant for the default value that is subject to the runtime version.
>
>>
>> XMLStreamReaderFactory and XMLStreamWriterFactory would get the property value with the MR-specific default.
>>
>
> Than MrJarUtil would need to have only such constant for default value and metho getNoPoolProperty could be moved elsewhere.



Since this is the only method right now, would it be better to check if Runtime.Version class is present, then use “true” as the default for the “noPool” property; otherwise “false”.    MrJarUtil seems a misleading name and maybe just move this method XMLStreamXXXFactory.  It’s a suggestion.  I won’t object if you want to keep this as is in JDK 9 and clean up in JDK 10 due to the timing.

Mandy

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Lance Andersen
In reply to this post by Lance Andersen
Hi Roman,

Overall looks OK.

A couple of minor comments

-  JAXBContext for methods such as createValidator() I would suggest adding the @Deprecated annotation
-  jdk.xml.bind and jdk.xml.ws have been updated via  http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/0d797e800441 <http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/0d797e800441> so you probably and omit

Best
Lance

> On Jun 1, 2017, at 3:03 PM, Lance Andersen <[hidden email]> wrote:
>
>
>> On Jun 1, 2017, at 2:18 PM, Roman Grigoriadi <[hidden email]> wrote:
>>
>>>
>>> On 1 Jun 2017, at 20:08, Lance Andersen <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> Hi Aleks,
>>>
>>> Its all good.  I don’t suppose there is a webrev of the diffs from 01 to 02 (or an easy way to see what changed)?
>>
>> Unfortunately no list of changed files, just a description of changes in my last message.
>
> OK, thank you.,
>> In addition to those fix for JDK-8176741 is included in this upload and some new messages (modeler*.properties, wscompile*.properties)
>>
>> Maybe I can produce some diff of changed files from git history of the standalone (probably still many files) if that would be of any help. We may try to sync in a smaller parts next time.
>
> I would not worry about it.  I was hoping to just look at just the changes from the previous, but will be easier for me to just review the entire webrev again.
>
>>
>> Roman
>>
>>
>>>
>>> Best
>>> Lance
>>>> On Jun 1, 2017, at 2:06 PM, Aleks Efimov <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>> I'm sorry Lance! I'm uploading new version of the webrev right now to the same location :-) It should be on-line soon =)
>>>>
>>>> Best,
>>>> Aleksei
>>>>
>>>> On 06/01/2017 06:34 PM, Lance Andersen wrote:
>>>>> Hi Roman,
>>>>>
>>>>> The webrev seems to have gotten moved as I was reviewing it and now it is gone :-)
>>>>>
>>>>>> On May 31, 2017, at 8:06 AM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> New webrev can be found here:
>>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>>
>>>>>>
>>>>>> Previous review comments have been addressed.
>>>>>>
>>>>>> New changes since last webrev upload:
>>>>>>
>>>>>> Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with descriptions of files in java.xml.ws, which calls JAXBContext#newInstance.
>>>>>>
>>>>>> JAXB-API:
>>>>>> - JAXBContext.java - deprecated implementation discovery with jaxb.properties resource.
>>>>>> - ContextFinder when called with String contextPath now tries to resolve jaxb.properties with Class#getResource if ClassLoader#getResource fails due to insufficient openness of jaxb.properties resource package.
>>>>>> - better JAXBException message when package of jaxb classes is not open to java.xml.bind
>>>>>>
>>>>>> JAXB-RI:
>>>>>> - fixed escaping newlines when using bundled jaxws transport.
>>>>>>
>>>>>> SAAJ-RI
>>>>>> - fixed TCK test failures
>>>>>>
>>>>>> JAXWS-RI
>>>>>> - fixed parsing wsdl in secure mode
>>>>>>
>>>>>> We have one JCK runtime test failure, which should be probably fixed in tests, I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397> <https://bugs.openjdk.java.net/browse/JCK-7308397 <https://bugs.openjdk.java.net/browse/JCK-7308397>>
>>>>>>
>>>>>> Please review.
>>>>>>
>>>>>> Thanks,
>>>>>> Roman
>>>>>>
>>>>>>> On 8 May 2017, at 22:38, Lance Andersen <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>
>>>>>>> Hi Roman,
>>>>>>>
>>>>>>> I made a pass through the webrev and have the following feedback:
>>>>>>>
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files  - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted.  Also what happens to the svn info in this file with the move to github?
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use &trade; for TM
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make the comments starting at 139 be consistent with the other comments
>>>>>>>
>>>>>>> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted
>>>>>>>
>>>>>>> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in the workspace
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java -  the copyright was reverted
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace
>>>>>>>
>>>>>>> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017
>>>>>>>
>>>>>>> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017
>>>>>>>
>>>>>>> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> you can find new web rev here:
>>>>>>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/>>>
>>>>>>>>
>>>>>>>> Previous review comments are addressed.
>>>>>>>>
>>>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>>>
>>>>>>>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example
>>>>>>>> in jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>>>>>>>> There is:
>>>>>>>> # for JDK integration - include version in source zip
>>>>>>>> jaxb.jdk.version=2.3.0-b170412.1723
>>>>>>>>
>>>>>>>> We can add another version.properties file with versions of all JAX-* components. We may also change version from 2.3.0-SNAPSHOT to something unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo.
>>>>>>>>
>>>>>>>> Roman
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 12 Mar 2017, at 16:14, Alan Bateman <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 12/03/2017 14:39, Roman Grigoriadi wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
>>>>>>>>>>
>>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508> <https://bugs.openjdk.java.net/browse/JDK-8176508 <https://bugs.openjdk.java.net/browse/JDK-8176508>>
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/>>
>>>>>>>>>>
>>>>>>>>> I skimmed the changes and have a few comments (I'm sure Lance or someone else will do a more detailed review).
>>>>>>>>>
>>>>>>>>> In JAXBContext then "must be open to the java.xml.bind module" should be "must be open to at least the java.xml.bind module" so as to cover the case that the package is opened unconditionally or to java.xml.bind and other modules. In addition, include "at least" makes it consistent with other wording that we have agreed for other areas.
>>>>>>>>>
>>>>>>>>> In MailcapCommandMap then the following doesn't seem right for the class description:
>>>>>>>>>
>>>>>>>>> 59  * (Where <i>java.home</i> is the value of the "java.home" System property
>>>>>>>>> 60  * and <i>conf</i> is the directory named "conf" if it exists,
>>>>>>>>> 61  * otherwise the directory named "lib"; the "conf" directory was
>>>>>>>>> 62  * introduced in JDK 1.9.)
>>>>>>>>>
>>>>>>>>> It might be simpler to just have javadoc specify that it attepts to locate the `mailcap` file in the Java run-time image and then add an @implNote with the details as to where it looks for specific runtime releases.
>>>>>>>>>
>>>>>>>>> I see the new source file ModuleUtil is using java.util.StringTokenizer. It's use in new code has been discouraged for many years and maybe this could start out using String.split rather than the legacy class.
>>>>>>>>>
>>>>>>>>> The change to version.properties reminds me to ask if there is anything in the jaxws repo to indicate the version of the JAX-* components? It's often difficult to determine what bits are in the JDK vs. the upstream project.
>>>>>>>>>
>>>>>>>>> -Alan
>>>>>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>>>>> Oracle Java Engineering
>>>>>>> 1 Network Drive
>>>>>>> Burlington, MA 01803
>>>>>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>
>>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif <http://oracle.com/us/design/oracle-email-sig-198324.gif>>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>>> Oracle Java Engineering
>>>>> 1 Network Drive
>>>>> Burlington, MA 01803
>>>>> [hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>> <oracle_sig_logo.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> [hidden email] <mailto:[hidden email]>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> [hidden email] <mailto:[hidden email]>
>
>
>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



12