Details
Bug
Closed
Major
Description
Having the default case boolean values actually return something leads to trouble if the data isn't correct. The problem I ran into was where I had a boolean value in the database that got stored as '' instead of '0' for false. This would lead to Castor loading the object fine, but when I go to save it I get generally wierd error messages such as ArrayIndexOutOfBounds (as happened on this extended object case). Other times I get the ObjectModified exception, but with no fields listed.
Something should be done, exception thrown or what have you, when the boolean values do not match up to anything. The current default of False should not be returned as it leads the user to believe everything is the way it should be, which is not the case.
Attached a simple test case that will lead to ArrayOutOfBounds exception. Has its own ant file and sql scripts to run.
Activity
While working at this issue I found that we introduced a problem by commiting CASTOR-1173. The problem apears if you call db.getDatabase() multipl times on the same JDOManager instance as transaction manager will be set to null at second and following calls. If included a quick fix with the patch that i don't want to commit with this patch but want to resolve when working at CASTOR-1179.
In addition I found a problem in SQLEngine that caused the ArrayIndexOutOfBoundException to be thrown. The reason was that we checked field.length instead of _field.length (take care on the underline) in the for loop to find the fields that caused the update to fail.
After all that I got to the real problem which has been how we handle conversion of char or string values in database to boolean properties in persistent objects. Pre patch we always returned a boolean value even if the database contained a value which does not conform to the rules defined. This behaviour caused that we could load records with wrong values without problems but got a ObjectModifiedException when we tried to update these values as our update statment includes checking of all old values in the where clause of SQL statement.
With the patch this behavier changes to strict checking if the loaded char/string value matches one of the boolean characters defined by conversion rule. If the values does not match a ClassCastException explaing the fail rason will be thrown.
Having said that this change is quite critical as it changes castors default load behavior for boolean values which may cause some users to see exceptions haven't seen with previous releases. Thinking about the pro's and con's of the patch I agree with Nick that it would be better to see that there is a problem at load instead of commit time.
Before commiting this I like to here some other opinons first. But be aware that time to next release is quite short 
Forgot to mention, but the code base I was working with for this was 0.9.7, but it acted the same way in 0.9.6 and expect it to do the same thing in CVS as well.