Hi Evgeny,
The failure was a result of the other part of that change:
..-105,6 +105,7 @@ public class SQLTemplate extends Abstrac
public SQLTemplate(DataMap rootMap, String defaultTemplate) {
setDefaultTemplate(defaultTemplate);
setRoot(rootMap);
+ setFetchingDataRows(true); // ObjEntity not passed, so it's
DataRow query
}
So, SQLTemplate property defaults... I actually agree with you that
the proposed defaults per r935207 make more sense when viewed in
isolation. But I'd still like to keep the old ones for consistency
reasons:
1. All SQLTemplate constructors create a query that fetches objects,
so users will have to manually change to DataRows if they need to,
without having to think about the query creation history.
2. When there are 2 properties in an object "abc" and "xyz", calling
setAbc should not change the value of xyz implicitly. Different
behavior may be confusing to many users.
So back to the original issue CAY-1328... Adding
"q2.setFetchingDataRows(true)" to the test fixes it. So the Jira IMO
is about better error reporting, not changing the defaults. E.g.
consider this:
SQLTemplate q2 = new SQLTemplate(testDataMap, "SELECT * FROM ARTIST");
q2.setFetchingDataRows(false); // the user might override the new
default by mistake,
// causing that same NPE as the Jira mentions
If instead of an NPE Cayenne would throw CayenneRuntimeException
saying "SQLTemplate is not mapped to ObjEntity and no SQLResult is
set, so 'fetchingDataRows' property must be set to 'false'", I think
we'll solve the problem in the least confusing way for the end user.
One possible place to perform this validation is in the
Query.route(..) method.
Does this make sense?
Andrus
On Apr 18, 2010, at 7:31 PM, Evgeny Ryabitskiy wrote:
> This was add to fix another JUnit test (that was failing).
> JUnit test was expecting that this flag is false while use
> setFatchingDataRows.
> So I add it here, to didn't change previous behavior
> Maybe I didn't understand the meaning of SQLResult....
>
> Evgeny.
>
>
> 2010/4/18 Andrus Adamchik <andru..bjectstyle.org>:
>>
>> On Apr 18, 2010, at 12:46 AM, evgen..pache.org wrote:
>>
>>> public void setResult(SQLResult resultSet) {
>>> + setFetchingDataRows(false); // turn off mapping to
>>> DataRows, use
>>> explicit
>>> this.result = resultSet;
>>> }
>>
>> Implicit flipping of the DataRows flag outside constructor seems
>> counterintuitive. Also SQLResult is not equal to an object fetch.
>> So I think
>> this is wrong.
>>
>> Andrus
>
This archive was generated by hypermail 2.0.0 : Tue Apr 27 2010 - 07:13:48 EDT