Quantcast

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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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
|  
Report Content as Inappropriate

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>
>   *
>
Loading...