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

Bug 348773

Summary:

[Validation] Validation for duplicate entity names and mapping file mapped classes

Product:

[WebTools] Dali JPA Tools

Reporter:

Nan Li <nan.n.li>

Component:

JPA

Assignee:

Nan Li <nan.n.li>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

neil.hauge

Version:

unspecified

Flags:

neil.hauge: review+

Target Milestone:

3.1 M1

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Proposed Patch	none
Proposed Patch	none
Proposed Patch	none
Proposed Patch	neil.hauge: iplog+

Description Nan Li

2011-06-08 12:03:03 EDT

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

Comment 1 Nan Li

2011-06-08 12:04:20 EDT

Created attachment 197619 [details]
Proposed Patch

Comment 2 Nan Li

2011-06-15 16:07:25 EDT

Created attachment 198051 [details]
Proposed Patch

Removed unnecessary duplicate validation when entity/class name is an empty string based on the previous proposed patch

Comment 3 Neil Hauge

2011-07-01 16:23:58 EDT

Moving JPA specific bugs to new JPA component in bugzilla.

Comment 4 Neil Hauge

2011-07-11 12:15:19 EDT

This patch has a small merge conflict with the current code.  Please submit an updated patch so I can complete this review.  Thanks!

Comment 5 Nan Li

2011-07-11 13:19:27 EDT

Created attachment 199432 [details]
Proposed Patch

Removed conflict with current code

Comment 6 Neil Hauge

2011-07-11 17:54:49 EDT

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.

Comment 7 Nan Li

2011-07-12 10:26:09 EDT

Created attachment 199502 [details]
Proposed Patch

Integrated the review comments

Comment 8 Neil Hauge

2011-07-12 12:43:48 EDT

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.