Community
Participate
Working Groups
JAXB Generation is possible with the libraries provided by the JDK (see tools.jar), but Dali does not work with these JDK shipped libraries. The reason the shipped-with-JDK libraries do not simply work is that the package names are different than the non-JDK JAXB jars. All of the JDK shipped libraries have "internal" in the package name, where as the others (that we do support) do not. This problem was found doing general JAXB testing without using supplemental JAXB jars. General functional works fine, but class generation does not. That said, class gen is a huge part of JAXB functionality, so this needs to be fixed for 3.0. Dali should support the usage of both sets of libraries depending on what is available. If non-JDK libraries are available, they should be used by default, since the user has specifically added these to their environment. If those are not found, we should fall back to the JDK shipped libraries, which may or may not be available. It will depend on whether the tools.jar has been added to the JRE system libraries or the project classpath.
Blaise opined that the libraries in the jdk are internal so that a user could easily override the xjc with a different version, but he wasn't sure. I *think* we could probably accomplish this purely by checking for the non-internal class first, and then failing that for the internal one.
I have tested that class generation works well by simply using the internal classes.
Note: Currently supported xjc: com.sun.tools.xjc_*.jar's XJCFacade is in package: com.sun.tools.xjc Need to also support this xjc: JDK's tools.jar XJCFacade is in package: com.sun.tools.internal.xjc
Created attachment 195926 [details] Preliminary Patch 3
Comments on patch: a. Patch is correct in that if there isn't a JAXB faceted project we don't set a target since we don't know what the runtime environment is going to look like. We have to assume the environment will match whatever mechanism they are using for generation. There is no need to log an exception in this case as this is an expected use case. User can always set target if necessary in the wizard. b. We are missing validation for the case where we are using EclipseLink generation and the external XJC is not available. (I think you had said that might not be in this patch) Other than that the testing is looking good. I've tested the following cases: (pass indicates that generation worked or appropriate validation was received) 1. Generic JAXB with JDK XJC on classpath (pass) 2. Generic JAXB without JDK XJC on classpath (pass) 3. Generic JAXB without JDK XJC on classpath with JRE (no tools.jar) (pass) 3. Generic JAXB with external XJC on classpath (pass) 4. Generic JAXB with external XJC and JDK XJC on classpath (pass) 5. Generic JAXB with 1.5 JRE and no external XJC on classpath (pass) 6. Generic JAXB with 1.5 JRE and external XJC on classpath (pass) 7. EclipseLink JAXB with only JDK XJC (see item b above) 8. EclipeeLink JAXB with only external XJC on classpath (pass)
Created attachment 196052 [details] Proposed patch
1. Generic JAXB with JDK XJC on classpath (pass) 2. Generic JAXB without JDK XJC on classpath (pass) 3. Generic JAXB without JDK XJC on classpath with JRE (no tools.jar) (pass) 3. Generic JAXB with external XJC on classpath (pass) 4. Generic JAXB with external XJC and JDK XJC on classpath (pass) 5. Generic JAXB with 1.5 JRE and no external XJC on classpath (pass) 6. Generic JAXB with 1.5 JRE and external XJC on classpath (pass) 7. EclipseLink JAXB with only JDK XJC (pass) 8. EclipeeLink JAXB with only external XJC on classpath (pass) 9. EclipseLink JAXB from non faceted project (NPE)* *java.lang.NullPointerException at org.eclipse.jpt.jaxb.ui.internal.wizards.classesgen.ClassesGeneratorExtensionOptionsWizardPage.getProjectFacetVersion(ClassesGeneratorExtensionOptionsWizardPage.java:100) Need to return null if no faceted project exists in getProjectFacetVersion(). And I think my previous comments may have confused here. We should log the CoreException if we get one. Not sure if your testing showed something different, but I don't believe that you will get a CoreException if the project isn't faceted. That call will simply return null. After fixing this in my workspace, all of the non-faceted project tests worked fine. private IProjectFacetVersion getProjectFacetVersion() { IFacetedProject facetedProject = null; try { facetedProject = ProjectFacetsManager.create(this.getProject()); } catch(CoreException e) { JptJaxbUiPlugin.log(e); } return (facetedProject == null) ? null : facetedProject.getProjectFacetVersion(JaxbFacet.FACET); } Please post a new patch with this change. I have a merged patch at the moment so can't easily create a new patch for this bug.
Created attachment 196072 [details] Proposed patch 2 I cannot reproduce the case where ProjectFacetsManager.create(this.getProject()) returns null. But I added the additional null test.
(In reply to comment #8) > Created attachment 196072 [details] > Proposed patch 2 > > I cannot reproduce the case where > ProjectFacetsManager.create(this.getProject()) returns null. But I added the > additional null test. This requires testing with a plain Java project. Case #9 now passes.
Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. This is a major bug in that JAXB generation fails when the JDK's XJC is available. Since these classes were in a different package they were not being seen by our class generation code. Is there a work-around? If so, why do you believe the work-around is insufficient? Workaround is to put externally packaged XJC libraries on your project classpath, which is an ugly workaround since it would be bad practice to have these tools on your project classpath, when you only need them for generation. How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? This fix has been tested over a number of day by Tran, Paul, and myself. It has received a large amount of ad-hoc testing. Give a brief technical overview. Who has reviewed this fix? The patch adds a new constant for the JDK shipped XJC type, changes the validation so this is not seen as an error case, and sets the target version in the generation so that the generated JAXB classes are compatible with the JAXB facet version that has been configured. I have reviewed the fix. What is the risk associated with this fix? Low. Given the amount of testing I would say the risk is quite low at this point. The fix is also isolated to JAXB generation, so it would not interfere with other Dali functionality.
Approving so this can get into the build for Thursday. We can roll back if there are any PMC objections.
Committed for RC2
