Implement infrastructur to execute JDOQL queries as single queries or API queries

• Key: JDO-156
• Type: Improvement
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Michael Watzek
• Reporter: Michael Watzek
• Component/s: tck2

Description

Each concrete query test class contains code creating and executing JDO queries using the API methods on javax.jdo.Query. Testing the JDO2 feature "single string queries", we need a facility to write a JDOQL query once and execute that as an API query and a single string query.

For this purpose, we propose to introduce a new class capable to hold all JDO query elements, such as the canddidate class, the filter, etc. Instances of that class may be used as factories for JDO query instances.

Moreover, we propose to introduce common methods in class Query_Test compiling and executing query element holder instances as API queries and single string queries.

Comments

[Michael Watzek]

The attached patch introduces a new class QueryElementHolder. This class may be used to create a JDO query instance using a single string or using the API methods on javax.jdo.Query. Moreover, the patch introduces compile and execute methods on class Query_Test, taking an instance of QueryElementHolder as parameter.

[Michael Watzek]

The second attachment replaces the first. Please review this one instead of the first one.

[Michael Watzek]

The second patch contains a file (companyForQueryTests.xml) which is part of patch JDO-168. Please ignore the file in the second patch.

[Michael Bouschen]

The patch looks good, just a few comments:

QueryElementHolder:
- I propose to change the constructor arguments specifying the range query element. One constructor could take two long arguments. This will call the Query API method setRange(long, long). The other constructor takes two String arguments. This will call the Query API method setRange(String) and pass a string created from the two string arguments with a comma in between. We cannot use a single range string, because the JDOQL syntax is different for setRange argument and the range specification in the single string case.
- The link tag on line 31 misses the closing curly bracket (}).
- I think you need to fully qualify the Query class in the link tag: {@link javax.jdo.Query}.
- How about adding a javadoc to toString saying that this methods returns the single string JDOQL representation?
- The javadoc of method getAPIQuery could say that it calls the Query API methods to specify query elements such as setFilter etc.
- How about if method getAPIQuery stores the extent instance in a local variable and then calls pm.newQuery(extent)?

QueryTest:
- Line 387 has the same comment as line 119. How about moving method getCompanyModelInstances in the first section of company model helper methods?
- It is confusing that the private method compile declared on line 487 does not create a single string Query using method getSingleStringQuery provided by QueryElementHolder. The reason is that the implementation checks argument queryElementHolder being null to decide whether to use the Query API or create a single string query. How about adding a boolean flag called useQueryAPI for this? If the queryElementHolder is specified then the new flag determines whether to call getAPIQuery or setSingleStringQuery on the holder. If there is no holder the method takes the argument singleStringQuery to call Query.newQuery directly.
- Method execute: would it make sense to add a new method checkUniqueQueryResult to check the expected result of a query with unique=true? I think we need a TCK test case making sure that a query with unique=true does not return a list. I think we can give a better error message if we add the extra method checkUniqueQueryResult.

[Michael Watzek]

The third patch contains all the comments.

[Michael Bouschen]

Two minors:

- QueryElementHolder: there is a private method rangeToString to create the range element for a single string query. I propose to add a similar method for a query created using the API methods.
- QueryTest: the private methods compile and execute both have flags to determine whether itis a API or singleStringQuery: useAPIQuery and asStringString. How about uiosng the same flag (asStringString) in both cases?

From my point of view the patch is ready for check in.

[Michael Watzek]

Third patch has been checked in.