comment inline
Michael Bouschen wrote:
>Hi Jielin,
>
>I agree with Tom's comments, you could do something like
> int index = getDatabaseQuery().getArguments().indexOf(pos);
> if (index == -1) {
> throw new IllegalArgumentException(...
> } else { ...
>
>About the tests added to entity-persistence-testing module: how about
>adding a new test class to the ejbqltesting package that includes input
>parameter tests: JUnitEJBQLParameterTestSuite. We could then add a
>couple of other parameter-related tests later. What do you think?
>
>
Sounds like a great idea to me.
-Tom
>Regards Michael
>
>
>
>>Hi Jie,
>>
>> One minor coment and one question.
>>
>>Comment: In the following lines of code:
>>--
>> if (!getDatabaseQuery().getArguments().contains(pos)) {
>> throw new
>>IllegalArgumentException(ExceptionLocalization.buildMessage("ejb30-wrong-argument-index",
>>new Object[]{position, getDatabaseQuery().getEJBQLString()}));
>> }
>> int index = getDatabaseQuery().getArguments().indexOf(pos);
>>--
>>
>> Is it possible to do the look-up for 'index' before the if and then use
>>the value of index to determine if we throw the exception? This will
>>avoid one lookup.
>>
>>Question:
>>
>>Can regression tests be added to the entity-persistence-testing module
>>for these bug fixes. I woudl suggest adding tests in either in one of
>>the EJBQL test suites defined in:
>>oracle.toplink.essentials.testing.tests.ejb.ejbqltesting or adding a
>>EJBQuery test suite to
>>oracle.toplink.essentials.testing.tests.cmp3.advanced
>>
>>Other than that, everything looks good.
>>
>>Thanks,
>>Tom
>>
>>jie leng wrote:
>>
>>
>>
>>>Hi, Tom/Michael,
>>>
>>>Here is the fix for glassfish issue 273 and 214.
>>>
>>>https://glassfish.dev.java.net/issues/show_bug.cgi?id=273
>>>
>>>and
>>>https://glassfish.dev.java.net/issues/show_bug.cgi?id=214
>>>
>>>Please review the change. The change is in EJBQueryImpl.java to handle
>>>validation check correctly for position query such as:
>>>
>>>1. String ejbql = "SELECT i FROM Item i WHERE i.name = ?1 AND
>>>i.itemId = ?3);
>>>
>>>Query query = em.createQuery(ejbql);
>>>
>>>query.setParameter(1, "Jie Leng");
>>>
>>>query.setParameter(2, "");
>>>
>>>query.setParameter(3, new Integer(1));
>>>
>>>It should throw:
>>>
>>>[java] Exception in thread "main" java.lang.IllegalArgumentException:
>>>You have attempted to set a parameter at position 2 which does not
>>>exist in this query string SELECT i FROM Item i WHERE i.name = ?1 AND
>>>i.itemId = ?3.
>>>
>>>2. String ejbql = "SELECT i FROM Item i WHERE i.name = ?1 AND i.itemId
>>>= ?3);
>>>
>>>Query query = em.createQuery(ejbql);
>>>
>>>query.setParameter(1, "Jie Leng");
>>>
>>>query.setParameter(3, new Integer(1));
>>>
>>>This query should be processed successfully even though ?2 is missing.
>>>
>>>
>>>Thanks.
>>>
>>>Jielin
>>>
>>>
>>>
>>>
>>>
>
>
>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com