Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 343851

Summary: Erroneous validation for non XmlElement mappings: Minimal support for all basic generic (and EclipseLink) JAXB attribute mapping types needed
Product: [WebTools] Dali JPA Tools Reporter: Paul Fullbright <paul.fullbright>
Component: JAXBAssignee: Paul Fullbright <paul.fullbright>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P2 CC: david_williams, neil.hauge
Version: 3.0Flags: david_williams: pmc_approved+
neil.hauge: pmc_approved? (raghunathan.srinivasan)
neil.hauge: pmc_approved? (naci.dai)
neil.hauge: pmc_approved? (deboer)
neil.hauge: pmc_approved? (neil.hauge)
neil.hauge: pmc_approved? (kaloyan)
neil.hauge: pmc_approved? (cbridgha)
neil.hauge: review+
Target Milestone: 3.0 RC1   
Hardware: PC   
OS: Windows Vista   
Whiteboard: PMC_approved
Attachments:
Description Flags
patch
none
updated patch
none
updated patch none

Description Paul Fullbright CLA 2011-04-26 10:20:48 EDT 
Since XmlElement is a default mapping that is applied if an attribute is mapped in no other way, there should be at least minimal support to indicate that an attribute is mapped in some other way than as an XmlElement.

For instance, currently the attribute:

@XmlElementRef(name = "element", namespace = "http://www.example.org/test", type = JAXBElement.class)
protected JAXBElement<String> globalElement;

is interpreted as being an XmlElement, and is given XmlElement validation.
Comment 1 Paul Fullbright CLA 2011-05-09 14:27:56 EDT 
Created attachment 195122 [details]
patch

This patch seems large, and it is.  But most of it is minimal boilerplate code for a small set of annotations and the mappings they identify.  Some basic generic support had to also be duplicated for the eclipselink plugins.  Tests were also added.

No existing API usage was altered (some constants and noextend interfaces were added).
Comment 2 Paul Fullbright CLA 2011-05-09 16:11:20 EDT 
Created attachment 195139 [details]
updated patch

Updated patch.  XmlElements mapping was not defaulting according to spec.
Comment 3 Neil Hauge CLA 2011-05-10 11:41:25 EDT 
Patch does seem large but upon inspection is indeed mostly just boilerplate resource model code.  Changes look good and my testing has revealed no issues.  I will send to PMC for review.
Comment 4 Paul Fullbright CLA 2011-05-10 12:02:18 EDT 
From Nan:

When applying @XmlElementRef or @XmlElementRefs to a property (getter or
setter) instead of a field of a class, the following CCE would be thrown:

----------
java.lang.ClassCastException:
org.eclipse.jpt.jaxb.core.internal.resource.java.source.SourceMethod cannot be
cast to org.eclipse.jpt.jaxb.core.resource.java.JavaResourceField
    at
org.eclipse.jpt.jaxb.core.internal.resource.java.XmlElementRefAnnotationDefinition.buildAnnotation(XmlElementRefAnnotationDefinition.java:48)
...
Comment 5 Paul Fullbright CLA 2011-05-10 12:08:19 EDT 
Created attachment 195239 [details]
updated patch

Fixed class cast issues
Comment 6 Neil Hauge CLA 2011-05-10 14:29:58 EDT 
    * 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 bug will result in erroneous validation errors on non-supported JAXB mappings.  Simply put, there will be red X's that the user won't be able to fix unless this bug is fixed.

    * Is there a work-around? If so, why do you believe the work-around is insufficient?

There is currently no workaround for this problem.

    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 

The fix has been tested by 2 Dali committers (including myself) and one contributor (Nan).  JUnit tests have been run as well.

    * Give a brief technical overview. Who has reviewed this fix? 

This patch simply adds the minimal boilerplate resource model code necessary to tolerate the existence of attribute mappings that we don't yet support.  By adding this code our model can be aware of these mappings and avoid defaulting to another mapping.  See comment #1 for additional info.  I have reviewed the fix.

    * What is the risk associated with this fix? 

Risk is low-moderate for this change.  Due to the severity of the bug I think the risk is tolerable to resolve this issue.
Comment 7 Paul Fullbright CLA 2011-05-11 11:02:58 EDT 
committed for rc1