Community
Participate
Working Groups
Build Identifier: I20110526-1708 This bug is opened for tracking the new implementation of the fixes of bug 204132. The new code moves the validation of duplicate entity names and the validation of duplicate mapping file mapped classes from entity/type mapping to persistence unit since these validations are more cross persistence unit thing. The outcome of the validations is still the same: 1. An error is given when entities have the same names. Entity name is defaulted to the short name of a mapped class when no name attribute is specified; otherwise, the value of the name attribute. 2. A warning is given when specifying the same class multiple times in the mapping files regardless of the mapping types. Reproducible: Always
Created attachment 197619 [details] Proposed Patch
Created attachment 198051 [details] Proposed Patch Removed unnecessary duplicate validation when entity/class name is an empty string based on the previous proposed patch
Moving JPA specific bugs to new JPA component in bugzilla.
This patch has a small merge conflict with the current code. Please submit an updated patch so I can complete this review. Thanks!
Created attachment 199432 [details] Proposed Patch Removed conflict with current code
Some comments: AbstractPersistenceUnit - The methods buildEntities() and buildTypeMappings() need to be renamed. The "build" prefix should be reserved for methods that are going to be creating or are used in the creation of new objects. In this case, these methods are only retrieving existing objects, and are not being used to build up some other object. A prefix of "get" will probably be appropriate in this case. The names also need to be more descriptive. There are already methods for getEntities() and getTypeMappings(). The new methods you are creating do something different, so that needs to be expressed in the method name. Other than that, I think this patch is in pretty good shape.
Created attachment 199502 [details] Proposed Patch Integrated the review comments
Patch has been committed to head. The only other potential issue with the code was not exclusive to the patch code, so I am just going to address this issue post patch. The methods in AbstractPersistenceUnit that begin with 'map' are a bit questionable. I think the 'get' prefix may be more appropriate in this case but we can discuss it further.
