| Summary: | [Validation] Version needs validation for acceptable attribute type. | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] Dali JPA Tools | Reporter: | Nan Li <nan.n.li> | ||||||||||
| Component: | General | Assignee: | Nan Li <nan.n.li> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | jolene.moffitt, karenfbutzke, neil.hauge | ||||||||||
| Version: | unspecified | Flags: | neil.hauge:
review+
|
||||||||||
| Target Milestone: | 3.0 M6 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows 7 | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Nan Li
Apparently this state is a result of modeling our model after the orm.xml, which allows for this configuration. This seems like either a problem in the spec, or an intentional gray area left for implementations to do something differently in XML configuration. I will inquire as to the reason for this and report back.
<xsd:complexType name="version">
<xsd:annotation>
<xsd:documentation>
@Target({METHOD, FIELD}) @Retention(RUNTIME)
public @interface Version {}
</xsd:documentation>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="column" type="orm:column" minOccurs="0"/>
<xsd:element name="temporal" type="orm:temporal" minOccurs="0"/>
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required"/>
<xsd:attribute name="
After further discussion, we have determined that validation should be used to handle this conflict, and the UI should remain as-is to allow the user to more easily alter the metadata in error states. It is also likely that EclipseLink could support this combination in the future. The validation should focus on Temporal and Version independently, and validate each based on their constraints. Created attachment 189099 [details]
patch
Two validation were added for the version mapping:
1. Validation of the attribute type if version mapping is specified
2. Validation of the attribute type if temporal mapping is specified
Updating title as Temporal validation is covered in bug 336006. This bug should focus on Version mapping validation. The Version mapping validation should focus on the existence of a valid attribute type. However, given the discussion with Mike, this validation should be a warning and not an error in the Generic platform, since this is a portability issue. We should have a different message for the EclipseLink platform that is specific to their current behavior, which is not to allow any other attribute types. I think an error message would be appropriate for EclispeLink, since it is specifically disallowed. So, we should validate any version mapping to determine if they are using a portable attribute type. If not, in Generic there should be an warning with a message like: "The persistent field or property for a Version mapping must be of type int, Integer, short, Short, long, Long, or Timestamp to ensure portability to other JPA providers" For EclipseLink, we would have an error message like: "The persistent field or property for a Version mapping must be of type int, Integer, short, Short, long, Long, or Timestamp" Created attachment 189203 [details]
Patch
Integrated the review comments and now a warning will be given on generic JPA platform and an error on EclipseLink platform for the incompatible attribute types
(In reply to comment #5) > Created attachment 189203 [details] > Patch > > Integrated the review comments and now a warning will be given on generic JPA > platform and an error on EclipseLink platform for the incompatible attribute > types I think this patch is missing an updated JpaValidationMessages file. Also, have you tested this to ensure that it works with the primitives that are valid. Looking at the list of supported types and the way they are checked, I would think that this would result in invalid problems for the valid primitive types such as int. (In reply to comment #6) > (In reply to comment #5) > > Created attachment 189203 [details] [details] > > Patch > > > > Integrated the review comments and now a warning will be given on generic JPA > > platform and an error on EclipseLink platform for the incompatible attribute > > types > > I think this patch is missing an updated JpaValidationMessages file. Also, > have you tested this to ensure that it works with the primitives that are > valid. Looking at the list of supported types and the way they are checked, I > would think that this would result in invalid problems for the valid primitive > types such as int. You are right. I used another method (getMetamodelTypeName()) to get the persistent attribute type and then changed to the current way (getPersistentAttribute().getTypeName()) thinking maybe it's more appropriate. I remember I tested it, but maybe that's done before the change was made. Anyway, the primitives are included in the type list now and tested. BTW, when do we use getMetamodelTypeName()? It seems to work with the Canonical Metamodel and return the same value for type int and Integer. Created attachment 189423 [details]
patch
Included the primitives in the list of the supported types
This is really picky, but I think the validate methods should be validateAttributeType() instead of the plural validateAttributeTypes(). Could be confusing, unless I am missing something. Other than that, this patch should be ready to commit. Thanks. Created attachment 189701 [details]
patch
changed the method's name as suggested
Patch committed to head. Verified in Build I-3.3.0-20110414085808 Verified validation for acceptable attribute type. See the link to view test steps for verification. http://wiki.eclipse.org/Dali_3.0_M6 However if you add the entity to the orm.xml or eclipselink-orm.xml the warning/error will disappear. This is being fixed in Bug 336296 |