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

Fri, 16 Jun 2017 09:20:07 -0700 

Hi,
Please, help to review the latest changes that addresses the following comments: 1. JAXBContext for methods such as createValidator() I would suggest adding the @Deprecated annotation 

    @Deprecated added to JAXBContext::createValidator


2.   jdk.xml.bind and jdk.xml.ws have been updated via 
http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/0d797e800441 so you 
probably and omit



    Changes in jdk.xml.bind and jdk.xml.ws module declaration files are 
not needed because they were addressed by JDK-8181702. Thank you, Lance!


3. MimetypesFileTypeMap.java:
    - Are the Parens around lines 54-57 really needed?


        I think that the parens are not needed too. Removed them + also 
removed parens from MailcapCommandMap javadoc.
        Bill, please, confirm if you think otherwise, I can return them 
back.


    - defaultType and confDir, shouldn’t these be all caps like PROG?


        Converted to upper case. Bill, please, let us know if it you 
think that we need to leave it in lower case.




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



    Removed "java.xml/com.sun.xml.internal.stream.writers” from 
JdkQualifiedExportTest test.



New webrev can be found here: 
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/03
Specdiff: 
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/specdiff_03/overview-summary.html



With Best Regards,
Aleksei

On 06/13/2017 08:53 PM, Dmitry Kornilov wrote:

Aleks, can you please address Lance’s comments and commit the patch to JDK?

—Dmitry



On 13 Jun 2017, at 20:28, Mandy Chung <mandy.ch...@oracle.com> wrote:

My comment can be addressed separately as a follow-up issue.  I’m okay with 
that.

I suggest to get Aleks’ help to get this change in JDK 9 asap.

Mandy


On Jun 13, 2017, at 11:11 AM, Lance Andersen <lance.ander...@oracle.com> wrote:

I think that is fine.  You would also want to make sure that Mandy’s feedback 
was addressed.

Best
Lance



On Jun 13, 2017, at 2:08 PM, Dmitry Kornilov <dmitry.korni...@oracle.com> wrote:

Roman is on vacation this week and coming back next Tuesday, May 20. If we are 
in a hurry with this sync (and we are) I can ask Aleks Efimov to make these 
changes and commit it to JDK.

—Dmitry



On 13 Jun 2017, at 01:24, Lance Andersen <lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>> wrote:

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 <lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>> wrote:



On Jun 1, 2017, at 2:18 PM, Roman Grigoriadi <roman.grigori...@oracle.com 
<mailto:roman.grigori...@oracle.com>> wrote:


On 1 Jun 2017, at 20:08, Lance Andersen <lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>>> 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 <aleksej.efi...@oracle.com 
<mailto:aleksej.efi...@oracle.com> <mailto:aleksej.efi...@oracle.com 
<mailto:aleksej.efi...@oracle.com>>> 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 <roman.grigori...@oracle.com 
<mailto:roman.grigori...@oracle.com> <mailto:roman.grigori...@oracle.com 
<mailto:roman.grigori...@oracle.com>>> 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/>> 
<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>> 
<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 <lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> 
<mailto:lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>>>> 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 <roman.grigori...@oracle.com <mailto:roman.grigori...@oracle.com> 
<mailto:roman.grigori...@oracle.com <mailto:roman.grigori...@oracle.com>> <mailto:roman.grigori...@oracle.com 
<mailto:roman.grigori...@oracle.com> <mailto:roman.grigori...@oracle.com 
<mailto:roman.grigori...@oracle.com>>>> 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/>>> 
<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 <alan.bate...@oracle.com <mailto:alan.bate...@oracle.com> 
<mailto:alan.bate...@oracle.com <mailto:alan.bate...@oracle.com>> <mailto:alan.bate...@oracle.com 
<mailto:alan.bate...@oracle.com> <mailto:alan.bate...@oracle.com <mailto:alan.bate...@oracle.com>>>> 
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>> 
<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/>> 
<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>>> 
<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
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com> <mailto:lance.ander...@oracle.com 
<mailto:lance.ander...@oracle.com>>>

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




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

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




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




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





























					Reply via email to