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> wrote: > > >> On Jun 1, 2017, at 2:18 PM, Roman Grigoriadi <roman.grigori...@oracle.com> >> wrote: >> >>> >>> On 1 Jun 2017, at 20:08, Lance Andersen <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>> 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>> 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 <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 ™ 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>>> 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 <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>> >>>>>>>>>> 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 >>>>>>> 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> > > > <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>
