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

Bug 331610

Summary:

[Validation] Need validation for queries

Product:

[WebTools] Dali JPA Tools

Reporter:

Nan Li <nan.n.li>

Component:

General

Assignee:

Nan Li <nan.n.li>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

neil.hauge

Version:

unspecified

Flags:

neil.hauge: review+

Target Milestone:

3.0 M5

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

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

Description Nan Li

2010-12-01 16:03:43 EST

Build Identifier: 20100917-0705

1. The name and query attributes must be specified. Null and empty string value of the name and query attributes should be reported.
2. Name conflict should be reported when adding a new query.
3. The Add Query dialog should validate that the given name is not a duplicate.
4. The query names given in the nested @NamedQueries and @NamedNativeQueries annotations should also be verified.

Note:
1. Validation for named store procedure query is listed in bug 240060.
2. Validation #2 should be done against all three types of queries.

Reproducible: Always

Steps to Reproduce:
Directly annotating the java entity class with the @NamedQuery/@NamedQueries and @NamedNativeQuery/@NamedNativeQueries or editing the mapping file with <named-query> and <named-native-query> elements or defining through UI

Comment 1 Nan Li

2010-12-21 17:11:09 EST

Created attachment 185676 [details]
Patch

Comment 2 Nan Li

2010-12-21 17:14:33 EST

The changes are tested and attached for review.

Comment 3 Neil Hauge

2010-12-21 17:46:12 EST

Please note that until the fix has been committed, we want to leave the bug in an open state.

Comment 4 Neil Hauge

2010-12-28 17:33:47 EST

Patch comments:

- Regarding item 3.  The Add Dialog name validation needs to consider whether or not the duplicate is really an override, in the case where it is being defined in the ORM XML.  This would not be an error and therefore should be allowed.  A warning could be used in this case to notify the user that the query will override an existing query.

Comment 5 Nan Li

2011-01-10 16:29:38 EST

Created attachment 186435 [details]
Patch

This patch includes the previous fixes and the fixes for the patch review comments.

Comment 6 Nan Li

2011-01-11 14:22:08 EST

Additional comments to the patch:

1. Some warning messages are confusing. Possible changes could be

eclipselink_ui_details.properties:
EclipseLinkConverterStateObject_nameExists = The given name already exists. This converter may override the existing one(s) with the same name.

jpt_ui_details.properties:
AddQueryDialog_nameExists=The given name exists. This query may be overridden by or override the one(s) with the same name.

jpt_ui_details_orm.properties:
GeneratorStateObject_nameExists=The given name already exists. This generator may override the existing one(s) with the same name.

2. We could make a small change like below to the method clearMessage() of ValidatingDialog class to return a message instead of null when the creation is ready to go.

	protected final void clearMessage(){
		setMessage("It is ready to create!");
	}

Comment 7 Neil Hauge

2011-01-11 23:19:52 EST

(In reply to comment #6)

- I like the wording suggestions proposed in item 1.  For clearMessage() we should set the message back to the default message displayed when the dialog is first displayed.

- There is a missing file in the patch: org.eclipse.jpt.core.internal.validation.JpaValidationMessages

- In JavaEclipseLinkConverter there is a reference to JpaValidationMessages.ID_MAPPING_UNRESOLVED_CONVERTER_NAME.  Since this is a Converter reference this message should be defined in the EclipseLinkJpaValidationMessages.  Not sure if there are any other instances like this.

I'd like to chat with you about some of the validation logic as well so I'll get in touch with you tomorrow about that.

Comment 8 Nan Li

2011-01-13 14:24:03 EST

(In reply to comment #7)
 
> - I like the wording suggestions proposed in item 1.  For clearMessage() we
> should set the message back to the default message displayed when the dialog is
> first displayed.
This is done and will be in the new proposed patch of this bug along with the changes to the warning messages.

> - There is a missing file in the patch:
> org.eclipse.jpt.core.internal.validation.JpaValidationMessages
This file was in the proposed patch of bug 332673 to avoid committing it multiple times, but will be included in the new proposed patch of this bug and remove it from the new proposed patch of bug 332673.

> - In JavaEclipseLinkConverter there is a reference to
> JpaValidationMessages.ID_MAPPING_UNRESOLVED_CONVERTER_NAME.  Since this is a
> Converter reference this message should be defined in the
> EclipseLinkJpaValidationMessages.  Not sure if there are any other instances
> like this.
The ID_MAPPING_UNRESOLVED_CONVERTER_NAME was defined in the wrong file, but the content of it is in the right file, EclipseLinkJpaValidationMessages. The reference is changed to EclipseLinkJpaValidationMessages.ID_MAPPING_UNRESOLVED_CONVERTER_NAME and this is the only case that has such issue.

Comment 9 Nan Li

2011-01-13 14:57:41 EST

Created attachment 186772 [details]
Patch

Compare to the previous patch, this one includes

1. Fixes for the text range of converters
2. Updates for the warning message
3. Updates for integrating the comments

Comment 10 Nan Li

2011-01-20 12:55:35 EST

Created attachment 187219 [details]
Patch

-Merged the new changes of the code from others
-Updated the code according to the new changes

Comment 11 Nan Li

2011-01-21 15:22:29 EST

Created attachment 187323 [details]
Patch

-Merged new changes to OrmEclipseLinkConverterContainerImpl.java

Comment 12 Neil Hauge

2011-01-21 17:52:24 EST

Patch committed to head.